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 <[email protected]>
Signed-off-by: Chengwu Shi <[email protected]>
Signed-off-by: Chengwu Shi <[email protected]>
…tead of new uri, and added a few comments for clarity

Signed-off-by: Chengwu Shi <[email protected]>
Signed-off-by: Chengwu Shi <[email protected]>
… (since it is not applicable to managed identity configuration)

Signed-off-by: Andriy Redko <[email protected]>
…hudownNow() and Thread.currentThread().interrupt()

Signed-off-by: Chengwu Shi <[email protected]>
@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 <[email protected]>

* Refactor tokenCredentialType as an enum when constructing AzureStorageSetting

Signed-off-by: Chengwu Shi <[email protected]>

* fixed indentation

Signed-off-by: Chengwu Shi <[email protected]>

* fixed syntax

Signed-off-by: Chengwu Shi <[email protected]>

* removed unused imports

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

* added transitive dependencies

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

* run spotlessApply

Signed-off-by: Chengwu Shi <[email protected]>

* update shas for asm 9.7

Signed-off-by: Chengwu Shi <[email protected]>

* use version.jna for jna-platform

Signed-off-by: Chengwu Shi <[email protected]>

* 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 <[email protected]>

* added one more comment line

Signed-off-by: Chengwu Shi <[email protected]>

* refactor TokenCredentialType to not have NOT_APPLICABLE

Signed-off-by: Chengwu Shi <[email protected]>

* refactored code based on recommended changes from Andriy Redko

Signed-off-by: Chengwu Shi <[email protected]>

* added a jvm security policy for reactor-core jar

Signed-off-by: Chengwu Shi <[email protected]>

* fixed failing forbidden api fix

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

* refactored token credential types checks

Signed-off-by: Chengwu Shi <[email protected]>

* changed clientlogger in azurestoragesetting to 'AzureStorageService'

Signed-off-by: Chengwu Shi <[email protected]>

* added a nullable argument to getStorageEndpoint

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Andriy Redko <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

---------

Signed-off-by: Chengwu Shi <[email protected]>
Signed-off-by: Andriy Redko <[email protected]>
Co-authored-by: Andriy Redko <[email protected]>
(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 <[email protected]>

* Refactor tokenCredentialType as an enum when constructing AzureStorageSetting

Signed-off-by: Chengwu Shi <[email protected]>

* fixed indentation

Signed-off-by: Chengwu Shi <[email protected]>

* fixed syntax

Signed-off-by: Chengwu Shi <[email protected]>

* removed unused imports

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

* added transitive dependencies

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

* run spotlessApply

Signed-off-by: Chengwu Shi <[email protected]>

* update shas for asm 9.7

Signed-off-by: Chengwu Shi <[email protected]>

* use version.jna for jna-platform

Signed-off-by: Chengwu Shi <[email protected]>

* 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 <[email protected]>

* added one more comment line

Signed-off-by: Chengwu Shi <[email protected]>

* refactor TokenCredentialType to not have NOT_APPLICABLE

Signed-off-by: Chengwu Shi <[email protected]>

* refactored code based on recommended changes from Andriy Redko

Signed-off-by: Chengwu Shi <[email protected]>

* added a jvm security policy for reactor-core jar

Signed-off-by: Chengwu Shi <[email protected]>

* fixed failing forbidden api fix

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

* refactored token credential types checks

Signed-off-by: Chengwu Shi <[email protected]>

* changed clientlogger in azurestoragesetting to 'AzureStorageService'

Signed-off-by: Chengwu Shi <[email protected]>

* added a nullable argument to getStorageEndpoint

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Andriy Redko <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

---------

Signed-off-by: Chengwu Shi <[email protected]>
Signed-off-by: Andriy Redko <[email protected]>
Co-authored-by: Andriy Redko <[email protected]>
(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 <[email protected]>
Signed-off-by: Andriy Redko <[email protected]>
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 <[email protected]>
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 <[email protected]>

* Refactor tokenCredentialType as an enum when constructing AzureStorageSetting

Signed-off-by: Chengwu Shi <[email protected]>

* fixed indentation

Signed-off-by: Chengwu Shi <[email protected]>

* fixed syntax

Signed-off-by: Chengwu Shi <[email protected]>

* removed unused imports

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

* added transitive dependencies

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

* run spotlessApply

Signed-off-by: Chengwu Shi <[email protected]>

* update shas for asm 9.7

Signed-off-by: Chengwu Shi <[email protected]>

* use version.jna for jna-platform

Signed-off-by: Chengwu Shi <[email protected]>

* 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 <[email protected]>

* added one more comment line

Signed-off-by: Chengwu Shi <[email protected]>

* refactor TokenCredentialType to not have NOT_APPLICABLE

Signed-off-by: Chengwu Shi <[email protected]>

* refactored code based on recommended changes from Andriy Redko

Signed-off-by: Chengwu Shi <[email protected]>

* added a jvm security policy for reactor-core jar

Signed-off-by: Chengwu Shi <[email protected]>

* fixed failing forbidden api fix

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

* refactored token credential types checks

Signed-off-by: Chengwu Shi <[email protected]>

* changed clientlogger in azurestoragesetting to 'AzureStorageService'

Signed-off-by: Chengwu Shi <[email protected]>

* added a nullable argument to getStorageEndpoint

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Andriy Redko <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

---------

Signed-off-by: Chengwu Shi <[email protected]>
Signed-off-by: Andriy Redko <[email protected]>
Co-authored-by: Andriy Redko <[email protected]>
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 <[email protected]>

* Refactor tokenCredentialType as an enum when constructing AzureStorageSetting

Signed-off-by: Chengwu Shi <[email protected]>

* fixed indentation

Signed-off-by: Chengwu Shi <[email protected]>

* fixed syntax

Signed-off-by: Chengwu Shi <[email protected]>

* removed unused imports

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

* added transitive dependencies

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

* run spotlessApply

Signed-off-by: Chengwu Shi <[email protected]>

* update shas for asm 9.7

Signed-off-by: Chengwu Shi <[email protected]>

* use version.jna for jna-platform

Signed-off-by: Chengwu Shi <[email protected]>

* 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 <[email protected]>

* added one more comment line

Signed-off-by: Chengwu Shi <[email protected]>

* refactor TokenCredentialType to not have NOT_APPLICABLE

Signed-off-by: Chengwu Shi <[email protected]>

* refactored code based on recommended changes from Andriy Redko

Signed-off-by: Chengwu Shi <[email protected]>

* added a jvm security policy for reactor-core jar

Signed-off-by: Chengwu Shi <[email protected]>

* fixed failing forbidden api fix

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

* refactored token credential types checks

Signed-off-by: Chengwu Shi <[email protected]>

* changed clientlogger in azurestoragesetting to 'AzureStorageService'

Signed-off-by: Chengwu Shi <[email protected]>

* added a nullable argument to getStorageEndpoint

Signed-off-by: Chengwu Shi <[email protected]>

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

Signed-off-by: Andriy Redko <[email protected]>

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

Signed-off-by: Chengwu Shi <[email protected]>

---------

Signed-off-by: Chengwu Shi <[email protected]>
Signed-off-by: Andriy Redko <[email protected]>
Co-authored-by: Andriy Redko <[email protected]>
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