Skip to content
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

Plugin service accounts #8526

Conversation

stephen-crawford
Copy link
Contributor

@stephen-crawford stephen-crawford commented Jul 7, 2023

Description

This PR is part of addressing opensearch-project/security#2941.

It introduces the Application interface as well as a ServiceAccountManager interface which is implemented by IdentityPlugins. With this new structure, when a plugin is installed, it is assigned a new ServiceAccount principal which can be used to reference the plugin. The service account principal is then assigned a set of permissions (to be implemented at a later date per opensearch-project/security#2943) which it can use to operate without impersonating the undefined user context.

Here is a diagram explaining the design idea:

image

Zooming in on the installation process:

image

And finally on the request flow:

image

Related Issues

This PR is part of addressing opensearch-project/security#2941.

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

stephen-crawford and others added 7 commits July 5, 2023 14:44
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Stephen Crawford <steecraw@amazon.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Stephen Crawford <steecraw@amazon.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Stephen Crawford <steecraw@amazon.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@stephen-crawford
Copy link
Contributor Author

@peternied this is the service accounts for Plugins PR you and I discussed if you end up taking a look while I am oncall.

Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR feels incomplete, while it is associating service accounts with extensions, they aren't changing any behavior, I know this is a building block towards OBO/ServiceAccount tokens, but its hard to see the design through the scaffolding. e.g. the diagrams which are great reference OBO token generation which isn't included in this PR.

Feels like this should wait until after the Scopes ApplicationManager lands, as there are differences in this version from the other PR.

Finally I think your IDE is going reordering imports, please revert all those changes. If you'd like to change the imports please do that as a seperate and tool enforced PR

@stephen-crawford
Copy link
Contributor Author

stephen-crawford commented Jul 27, 2023

Hi @peternied, I agree that this PR is incomplete. I am mostly looking for feedback on the design at this time. I am hoping to get input on the mechanisms suggested in the diagrams before implementing the entire set of changes it will require. I also suspect that this will be the version of things that we ultimately settle on.

The changes in the Scopes PR are not adequately supportive of the final use cases. My apologies for not being more clear when I mentioned looking for feedback on my outstanding PRs. This one is very much in progress. Thank you for taking the time to review it.

I have also tried repeatedly to disable the reorganization of imports by IntelliJ. It refuses to work... I will try again.

Edit: Import issue is common with IntelliJ: https://youtrack.jetbrains.com/issue/IDEABKL-6456?_gl=1*1rt6gn4*_ga*MTc2MTU0MTQuMTY5MDQ2NTExNg..*_ga_9J976DJZ68*MTY5MDQ2NTExNi4xLjAuMTY5MDQ2NTExNi4wLjAuMA..&_ga=2.175616608.399880902.1690465116-17615414.1690465116

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Stephen Crawford <steecraw@amazon.com>
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

…sing change and need to update tests for passing

Signed-off-by: Stephen Crawford <steecraw@amazon.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com>
@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2023

Gradle Check (Jenkins) Run Completed with:

@opensearch-trigger-bot
Copy link
Contributor

Compatibility status:



> Task :checkCompatibility
Incompatible components: [https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/security-analytics.git]
Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

BUILD SUCCESSFUL in 26m 20s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants