-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Extensions] Add Transport API hook for fetching Service Account information from security plugin #6900
Comments
Hi, I think #2609 leads to the wrong link (it's some bug from the main Opensearch repo). I guess it should be opensearch-project/security#2609 |
Oops, nice catch. I updated the links and assigned it to you. Let me know if you have any questions or run into any issues. |
Hi @scrawfor99 created PR: #7307 |
Hi @scrawfor99 |
Hi @MaciejMierzwa, thank you! I will take a look at this today :) |
Recapping offline conversation:
|
Hi, pushed my changes for the security side ServiceAccountManager implementation. Could you please take a look at it @scrawfor99 or @scrawfor99. Here's PR: opensearch-project/security#2775 |
Hi @MaciejMierzwa, thank you for following up. Sorry for not being clear before. Basically, the idea is going to be to use an interface in core that is then implemented in Security Plugin. To test this there will be a simplified implementation of the interface in core and a simplified version of the interface itself in the Security plugin. This will let us do isolated testing within each code body. I have made progress towards making these changes in a PR and would welcome your thoughts as I move things forward. I will review what you have been working on here as well and after I add in the interface to the code base we can move some of what you have been working onto into the appropriate locations. Part of the reason the details have not been entirely clear are that we have made a couple shifts in design since the initial outlining of the plans and some of these shifts were not made until after you did a significant amount of work. |
This issue is the Core-side of 2609
In order to implement Service Accounts (#2597), it needs to be possible to fetch service account details. To do this, OpenSearch core should be able to call the Security Plugin to provide a Service Account Authorization Header back to the Extensions Manager in core. This will let core create requests on behalf of extensions.
Inside core, a hook needs to be added that allows the API introduced in #2609 to be called when a new extension is registered with the Extension Manager. An outline of what this may look like can be found here.
The implementation process requires the addition of a
ServiceAccountManager
class insideserver/src/main/java/org/opensearch/identity/
. TheServiceAccountManager
should be connected to anIdentityService
which is built when(FeatureFlags.isEnabled(FeatureFlags.EXTENSIONS)
inNode.java
. This will turn on the IdentityService whenever the cluster launches and extensions are enabled.The
IdentityService
should construct an instance of theServiceAccountManager
and then use theServiceAccountManager
to interact with the Security Plugin. The abstraction is used so that if any code is later moved from the Security Plugin into core, the entire system does not require a rework.Inside the
ServiceAccountManager
class, there should be a hook to the Security Plugin API for getting a service account based on anextensionUniqueId
.An outline of possible changes can be found here. This may be helpful for referencing.
This issues completion will include the addition of classes
ServiceAccountManager
,IdentityService
, and possibly others. The complete PR should demonstrate the ability for core to call the Security Plugin's Service Account get API and retrieve a response. As a result, #2609 must be completed before this issue can be completed. For now, it is appropriate to receive a basic string from the Security Plugin. The completed PR should also include tests that verify that the response received by core is in a valid format and should also build on node start.The text was updated successfully, but these errors were encountered: