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

Main repository azure managed identity support #12559

Conversation

chengwushi-netapp
Copy link
Contributor

@chengwushi-netapp chengwushi-netapp commented Mar 8, 2024

Description

This PR added supported for managed identity in the repository-azure plugin.

Related Issues

Resolves #12423

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)
  • Public documentation issue/PR created

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.

Copy link
Contributor

github-actions bot commented Mar 8, 2024

❌ Gradle check result for 9d0d565: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

github-actions bot commented Mar 8, 2024

❌ Gradle check result for a330e99: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

github-actions bot commented Mar 8, 2024

Compatibility status:

Checks if related components are compatible with change 3cb3c7e

Incompatible components

Skipped components

Compatible components

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

@chengwushi-netapp
Copy link
Contributor Author

This PR is expected to fail the Task :plugins:repository-azure:thirdPartyAudit because I am uncertain about the best approach to resolve this failure.

From my understanding, the thirdPartyAudit requires all dependencies to be explicitly declared. For instance, I have the following dependency tree from using com.azure:azure-identity:1.11.2.

com.azure:azure-identity:1.11.2
----net.java.dev.jna:jna-platform:5.14.0
----com.microsoft.azure:msal4j-persistence-extension:1.2.0
----com.microsoft.azure:msal4j:1.14.2
--------com.nimbusds:oauth2-oidc-sdk:11.10
------------com.nimbusds:nimbus-jose-jwt:9.37.3
------------com.nimbusds:content-type:2.3
------------com.nimbusds:lang-tag:1.7
----net.minidev:json-smart:2.5.0
--------net.minidev:accessors-smart:2.5.0
------------org.ow2.asm:asm:9.6

Note: For simplicity, I have not listed all the nested dependencies, as it would make the tree too large for demonstration purposes.

I believe to pass the Task :plugins:repository-azure:thirdPartyAudit check, I would need to declare all these nested dependencies, not just a subset of them.

Therefore, I have the following questions, and I would greatly appreciate it if anyone could provide answers:

  1. Do i need to include all the nested dependencies?
    1.1 If the answer is yes, wouldn't the list of dependencies become too large? What happens if one of them contains vulnerabilities?
    1.2 If the answer is no, how do I determine which dependencies I can ignore?
  2. Is there a way to automate the addition of nested dependencies to the build file? I am currently adding them manually by referencing mavenCentral.
  3. Are there any best practices or recommended approaches when dealing with complex dependency trees when adding new dependencies in OpenSearch?
  4. I am aware of the ignoreMissingClasses in the thirdPartyAudit check. How do we determine if we can ignore a missing class found in the thirdPartyAudit check?
  5. When adding a new dependency, I was under the impression that Gradle would automatically fetch its nested dependencies. However, in my experience while testing this PR on an Azure Virtual Machine with an attached managed identity, I found that I had to manually install three nested dependencies, even though I had already included com.azure:azure-identity:1.11.2. The dependencies I had to manually add were com.microsoft.azure:msal4j:1.14.2, com.nimbusds:oauth2-oidc-sdk:11.10, and net.minidev:json-smart:2.5.0. Could anyone clarify why these dependencies weren't automatically fetched by Gradle?

chengwushi-netapp and others added 17 commits May 14, 2024 14:56
…buildSrc/version.properties

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
…tead of new uri, and added a few comments for clarity

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
…led time

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
… (since it is not applicable to managed identity configuration)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
…hudownNow() and Thread.currentThread().interrupt()

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
@chengwushi-netapp chengwushi-netapp force-pushed the main-repository-azure-managed-identity-support branch from 3f4e213 to 280dd0a Compare May 14, 2024 04:57
Copy link
Contributor

✅ Gradle check result for 280dd0a: SUCCESS

@chengwushi-netapp
Copy link
Contributor Author

chengwushi-netapp commented May 14, 2024

@andrross thank you for your review, i have responded to all your comments. However, this PR would need approvals from @andrross and @reta again, because I have rebased the branch to pass the tests. Cheers, chengwu

@reta reta merged commit a03db0d into opensearch-project:main May 14, 2024
28 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 14, 2024
* Added support for Azure Managed Identity in repository-azure

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* Refactor tokenCredentialType as an enum when constructing AzureStorageSetting

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* fixed indentation

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* fixed syntax

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* removed unused imports

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* applied changes after running :plugins:repository-azure:spotlessApply

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added transitive dependencies

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* changed getStorageBlobEndpoint to private and using asm version from buildSrc/version.properties

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* run spotlessApply

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* update shas for asm 9.7

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* use version.jna for jna-platform

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* change string 'core.windows.net' to be a constant, use uri.create instead of new uri, and added a few comments for clarity

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added one more comment line

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactor TokenCredentialType to not have NOT_APPLICABLE

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactored code based on recommended changes from Andriy Redko

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added a jvm security policy for reactor-core jar

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* fixed failing forbidden api fix

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* removed the jvm security policy for reactor-core which was added

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactored code, such that storage endpoint is not evaluated at compiled time

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactored token credential types checks

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* changed clientlogger in azurestoragesetting to 'AzureStorageService'

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added a nullable argument to getStorageEndpoint

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* Fix IdentityClient security permissions, get rid of connection string (since it is not applicable to managed identity configuration)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Responded to feedback from Andrew Ross, fixed typo, spelling, added shudownNow() and Thread.currentThread().interrupt()

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

---------

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Co-authored-by: Andriy Redko <andriy.redko@aiven.io>
(cherry picked from commit a03db0d)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
reta added a commit that referenced this pull request May 14, 2024
* Added support for Azure Managed Identity in repository-azure

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* Refactor tokenCredentialType as an enum when constructing AzureStorageSetting

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* fixed indentation

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* fixed syntax

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* removed unused imports

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* applied changes after running :plugins:repository-azure:spotlessApply

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added transitive dependencies

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* changed getStorageBlobEndpoint to private and using asm version from buildSrc/version.properties

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* run spotlessApply

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* update shas for asm 9.7

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* use version.jna for jna-platform

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* change string 'core.windows.net' to be a constant, use uri.create instead of new uri, and added a few comments for clarity

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added one more comment line

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactor TokenCredentialType to not have NOT_APPLICABLE

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactored code based on recommended changes from Andriy Redko

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added a jvm security policy for reactor-core jar

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* fixed failing forbidden api fix

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* removed the jvm security policy for reactor-core which was added

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactored code, such that storage endpoint is not evaluated at compiled time

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactored token credential types checks

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* changed clientlogger in azurestoragesetting to 'AzureStorageService'

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added a nullable argument to getStorageEndpoint

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* Fix IdentityClient security permissions, get rid of connection string (since it is not applicable to managed identity configuration)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Responded to feedback from Andrew Ross, fixed typo, spelling, added shudownNow() and Thread.currentThread().interrupt()

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

---------

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Co-authored-by: Andriy Redko <andriy.redko@aiven.io>
(cherry picked from commit a03db0d)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
reta added a commit that referenced this pull request May 14, 2024
* Added support for Azure Managed Identity in repository-azure



* Refactor tokenCredentialType as an enum when constructing AzureStorageSetting



* fixed indentation



* fixed syntax



* removed unused imports



* applied changes after running :plugins:repository-azure:spotlessApply



* added transitive dependencies



* changed getStorageBlobEndpoint to private and using asm version from buildSrc/version.properties



* run spotlessApply



* update shas for asm 9.7



* use version.jna for jna-platform



* change string 'core.windows.net' to be a constant, use uri.create instead of new uri, and added a few comments for clarity



* added one more comment line



* refactor TokenCredentialType to not have NOT_APPLICABLE



* refactored code based on recommended changes from Andriy Redko



* added a jvm security policy for reactor-core jar



* fixed failing forbidden api fix



* removed the jvm security policy for reactor-core which was added



* refactored code, such that storage endpoint is not evaluated at compiled time



* refactored token credential types checks



* changed clientlogger in azurestoragesetting to 'AzureStorageService'



* added a nullable argument to getStorageEndpoint



* Fix IdentityClient security permissions, get rid of connection string (since it is not applicable to managed identity configuration)



* Responded to feedback from Andrew Ross, fixed typo, spelling, added shudownNow() and Thread.currentThread().interrupt()



---------




(cherry picked from commit a03db0d)

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Andriy Redko <andriy.redko@aiven.io>
deshsidd pushed a commit to deshsidd/OpenSearch that referenced this pull request May 17, 2024
)

* Added support for Azure Managed Identity in repository-azure

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* Refactor tokenCredentialType as an enum when constructing AzureStorageSetting

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* fixed indentation

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* fixed syntax

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* removed unused imports

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* applied changes after running :plugins:repository-azure:spotlessApply

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added transitive dependencies

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* changed getStorageBlobEndpoint to private and using asm version from buildSrc/version.properties

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* run spotlessApply

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* update shas for asm 9.7

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* use version.jna for jna-platform

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* change string 'core.windows.net' to be a constant, use uri.create instead of new uri, and added a few comments for clarity

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added one more comment line

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactor TokenCredentialType to not have NOT_APPLICABLE

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactored code based on recommended changes from Andriy Redko

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added a jvm security policy for reactor-core jar

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* fixed failing forbidden api fix

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* removed the jvm security policy for reactor-core which was added

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactored code, such that storage endpoint is not evaluated at compiled time

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactored token credential types checks

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* changed clientlogger in azurestoragesetting to 'AzureStorageService'

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added a nullable argument to getStorageEndpoint

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* Fix IdentityClient security permissions, get rid of connection string (since it is not applicable to managed identity configuration)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Responded to feedback from Andrew Ross, fixed typo, spelling, added shudownNow() and Thread.currentThread().interrupt()

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

---------

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Co-authored-by: Andriy Redko <andriy.redko@aiven.io>
parv0201 pushed a commit to parv0201/OpenSearch that referenced this pull request Jun 10, 2024
)

* Added support for Azure Managed Identity in repository-azure

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* Refactor tokenCredentialType as an enum when constructing AzureStorageSetting

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* fixed indentation

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* fixed syntax

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* removed unused imports

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* applied changes after running :plugins:repository-azure:spotlessApply

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added transitive dependencies

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* changed getStorageBlobEndpoint to private and using asm version from buildSrc/version.properties

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* run spotlessApply

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* update shas for asm 9.7

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* use version.jna for jna-platform

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* change string 'core.windows.net' to be a constant, use uri.create instead of new uri, and added a few comments for clarity

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added one more comment line

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactor TokenCredentialType to not have NOT_APPLICABLE

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactored code based on recommended changes from Andriy Redko

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added a jvm security policy for reactor-core jar

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* fixed failing forbidden api fix

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* removed the jvm security policy for reactor-core which was added

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactored code, such that storage endpoint is not evaluated at compiled time

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* refactored token credential types checks

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* changed clientlogger in azurestoragesetting to 'AzureStorageService'

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* added a nullable argument to getStorageEndpoint

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

* Fix IdentityClient security permissions, get rid of connection string (since it is not applicable to managed identity configuration)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Responded to feedback from Andrew Ross, fixed typo, spelling, added shudownNow() and Thread.currentThread().interrupt()

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>

---------

Signed-off-by: Chengwu Shi <chengwu.shi@netapp.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Co-authored-by: Andriy Redko <andriy.redko@aiven.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch enhancement Enhancement or improvement to existing feature or request Plugins v2.15.0 Issues and PRs related to version 2.15.0 v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Plugins] enable support for Azure Managed Identity in the repository-azure plugin
6 participants