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

Remove transitive dependencies from encryption-sdk/build.gradle #9779

Merged
merged 1 commit into from
Sep 5, 2023

Conversation

reta
Copy link
Collaborator

@reta reta commented Sep 5, 2023

Description

Remove org.bouncycastle:bcprov-jdk15to18: not needed and breaks the plugins

Related Issues

Closes opensearch-project/security#3309

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.

@reta
Copy link
Collaborator Author

reta commented Sep 5, 2023

@peternied @cwperks @willyborankin please check it out

@peternied peternied added the backport 2.x Backport to 2.x branch label Sep 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

Gradle Check (Jenkins) Run Completed with:

…lugins

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

Compatibility status:

Checks if related components are compatible with change 54cfc0f

Incompatible components

Incompatible components: [https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/anomaly-detection.git]

Skipped components

Compatible components

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

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

Compatibility status:

Checks if related components are compatible with change afae0aa

Incompatible components

Incompatible components: [https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/neural-search.git]

Skipped components

Compatible components

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/asynchronous-search.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/k-nn.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.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

@peternied peternied changed the title Remove org.bouncycastle:bcprov-jdk15to18: not needed and breaks the plugins Remove transitive dependencies from encryption-sdk/build.gradle Sep 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

Compatibility status:

Checks if related components are compatible with change c8cb1c5

Incompatible components

Incompatible components: [https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/neural-search.git]

Skipped components

Compatible components

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/job-scheduler.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/k-nn.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/performance-analyzer.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      2 org.opensearch.index.shard.RemoteStoreRefreshListenerTests.classMethod
      1 org.opensearch.smoketest.SmokeTestMultiNodeClientYamlTestSuiteIT.test {yaml=pit/10_basic/Delete all}
      1 org.opensearch.index.shard.RemoteStoreRefreshListenerTests.testRefreshAfterCommit
      1 org.opensearch.cluster.allocation.ClusterRerouteIT.testDelayWithALargeAmountOfShards
      1 org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness

@reta reta merged commit 9d6c43a into opensearch-project:main Sep 5, 2023
11 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 5, 2023
…lugins (#9779)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
(cherry picked from commit 9d6c43a)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
reta pushed a commit that referenced this pull request Sep 6, 2023
…lugins (#9779) (#9781)

(cherry picked from commit 9d6c43a)

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>
@vikasvb90
Copy link
Contributor

@reta We need to revert this change. BC is required by AWS encryption SDK which is currently not being used as we are waiting for approval from crypto BR team for custom changes. We have currently added just a NoOpCryptoHandler in place of it.

@reta
Copy link
Collaborator Author

reta commented Sep 6, 2023

@reta We need to revert this change. BC is required by AWS encryption SDK which is currently not being used as we are waiting for approval from crypto BR team for custom changes. We have currently added just a NoOpCryptoHandler in place of it.

@vikasvb90 We cannot revert this change, the BC should be added to the component which uses it: I suspect this a plugin, right? Adding hard dependency on BC breaks the whole plugin ecosystem.

@reta
Copy link
Collaborator Author

reta commented Sep 6, 2023

@vikasvb90 additionally, please check the dependencies https://mvnrepository.com/artifact/com.amazonaws/aws-encryption-sdk-java/2.4.1: it potentially needs bcprov-ext but not bcprov

@vikasvb90
Copy link
Contributor

@reta No, it is not the plugin which uses it but the lib itself. Let me check the bcprov dependency but we anyways need other dependencies and bcprov-ext dependency. Also, I am not sure if adding bcprov-ext can avoid changes required for security plugin.

@reta
Copy link
Collaborator Author

reta commented Sep 6, 2023

@reta No, it is not the plugin which uses it but the lib itself. Let me check the bcprov dependency but we anyways need other dependencies and bcprov-ext dependency.

If lib uses it in any reasonable manner, it should either:

  • fail to compile
  • fail at runtime (I would imply tests here)

Nothing from the above is happening. Please provide more details where it is needed because everything indicates the otherwise.

@reta
Copy link
Collaborator Author

reta commented Sep 6, 2023

but we anyways need other dependencies and

@vikasvb90 which other dependencies? the only transitive ones are common-lang3 and it has been kept intact. The library must not bring the commons-logging or slf4j-api (it could indicated it needs it) - this is the responsibility of end consumer to provide, either plugin / server / client / extension.

@vikasvb90
Copy link
Contributor

You are right that it is not being used right now. As I mentioned earlier, that this had changes done in encryption SDK to support partial encryption/decryption which is yet to undergo a review from SDK team itself and that's why it is just a NoOpCryptoHandler. Let me get back on how we can enforce its usage in lib meanwhile but inclusion of the dependency is something we need.

kaushalmahi12 pushed a commit to kaushalmahi12/OpenSearch that referenced this pull request Sep 12, 2023
…lugins (opensearch-project#9779)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>
brusic pushed a commit to brusic/OpenSearch that referenced this pull request Sep 25, 2023
…lugins (opensearch-project#9779)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Signed-off-by: Ivan Brusic <ivan.brusic@flocksafety.com>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…lugins (opensearch-project#9779)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
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 skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Security Plugin cannot startup due to AccessControlException: access denied
9 participants