Use version of org.apache.commons:commons-lang3 defined in core #3306

cwperks · 2023-09-05T14:18:49Z

Description

Use version of org.apache.commons:commons-lang3 defined in core

https://github.com/opensearch-project/OpenSearch/blob/main/buildSrc/version.properties#L42

Fixes issue seen on spotless upgrade PR against 2.x: #3298

Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)

Maintenance

Check List

New functionality includes testing
New functionality has been documented
Commits are signed per the DCO using --signoff

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.

Signed-off-by: Craig Perkins <cwperx@amazon.com>

cwperks · 2023-09-05T14:23:08Z

FYI we are getting this dependency transitively through rest-highlevel-client now. It's coming through the new opensearch-encryption-sdk: https://github.com/opensearch-project/OpenSearch/blob/main/libs/encryption-sdk/build.gradle

org.apache.commons:commons-lang3:3.13.0
\--- org.opensearch:opensearch-encryption-sdk:2.10.0-SNAPSHOT
     \--- org.opensearch:opensearch:2.10.0-SNAPSHOT
          \--- org.opensearch.client:opensearch-rest-high-level-client:2.10.0-SNAPSHOT
               \--- runtimeClasspath

org.apache.commons:commons-lang3:3.12.0 -> 3.13.0
\--- runtimeClasspath

cwperks · 2023-09-05T14:24:35Z

Seeing this error on plugin install:

java.lang.IllegalStateException: failed to load plugin class [org.opensearch.security.OpenSearchSecurityPlugin]
Likely root cause: java.lang.InternalError: cannot create instance of org.bouncycastle.jcajce.provider.digest.GOST3411$Mappings : java.security.AccessControlException: access denied ("java.security.SecurityPermission" "putProviderProperty.BC")
	at org.bouncycastle.jce.provider.BouncyCastleProvider.loadServiceClass(Unknown Source)
	at org.bouncycastle.jce.provider.BouncyCastleProvider.loadAlgorithms(Unknown Source)
	at org.bouncycastle.jce.provider.BouncyCastleProvider.setup(Unknown Source)
	at org.bouncycastle.jce.provider.BouncyCastleProvider.access$000(Unknown Source)
	at org.bouncycastle.jce.provider.BouncyCastleProvider$1.run(Unknown Source)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at org.bouncycastle.jce.provider.BouncyCastleProvider.<init>(Unknown Source)
	at org.opensearch.security.OpenSearchSecurityPlugin$2.run(OpenSearchSecurityPlugin.java:338)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at org.opensearch.security.OpenSearchSecurityPlugin.<init>(OpenSearchSecurityPlugin.java:334)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
	at org.opensearch.plugins.PluginsService.loadPlugin(PluginsService.java:782)
	at org.opensearch.plugins.PluginsService.loadBundle(PluginsService.java:731)
	at org.opensearch.plugins.PluginsService.loadBundles(PluginsService.java:533)
	at org.opensearch.plugins.PluginsService.<init>(PluginsService.java:195)
	at org.opensearch.node.Node.<init>(Node.java:469)
	at org.opensearch.node.Node.<init>(Node.java:396)
	at org.opensearch.bootstrap.Bootstrap$5.<init>(Bootstrap.java:242)
	at org.opensearch.bootstrap.Bootstrap.setup(Bootstrap.java:242)
	at org.opensearch.bootstrap.Bootstrap.init(Bootstrap.java:404)
	at org.opensearch.bootstrap.OpenSearch.init(OpenSearch.java:180)
	at org.opensearch.bootstrap.OpenSearch.execute(OpenSearch.java:171)
	at org.opensearch.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:104)
	at org.opensearch.cli.Command.mainWithoutErrorHandling(Command.java:138)
	at org.opensearch.cli.Command.main(Command.java:101)
	at org.opensearch.bootstrap.OpenSearch.main(OpenSearch.java:137)
	at org.opensearch.bootstrap.OpenSearch.main(OpenSearch.java:103)

Its odd because this grant is already defined in plugin-security.policy file.

codecov · 2023-09-05T14:26:02Z

Codecov Report

Merging #3306 (17c0e4c) into main (3af850e) will decrease coverage by 0.04%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #3306      +/-   ##
============================================
- Coverage     63.27%   63.23%   -0.04%     
+ Complexity     3450     3448       -2     
============================================
  Files           263      263              
  Lines         20040    20040              
  Branches       3344     3344              
============================================
- Hits          12680    12673       -7     
- Misses         5732     5740       +8     
+ Partials       1628     1627       -1

see 6 files with indirect coverage changes

willyborankin · 2023-09-05T15:00:28Z

@cwperks looks like implementation "org.bouncycastle:bcprov-jdk15to18:${versions.bouncycastle}" is in the SDK now.

willyborankin · 2023-09-05T15:02:47Z

@reta looks like cryto-sdk brought BC as deps without permissions as result Sec plugin does not work and I think other plugins as well. OS now tries to check permissions via SDK but not via plugin :-)

peternied · 2023-09-05T15:09:25Z

Associated change [1] & backport [2]

peternied · 2023-09-05T15:14:18Z

I am going to push this change in, we need to figure out what is wrong with the SecurityManager, but let's handle that separately from this version alignment that needs to happen across main & 2.x

opensearch-trigger-bot · 2023-09-05T15:14:48Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/security/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/security/backport-2.x
# Create a new branch
git switch --create backport/backport-3306-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 fd3a143be713a31c278386a21bd2236542101b7d
# Push it to GitHub
git push --set-upstream origin backport/backport-3306-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/security/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-3306-to-2.x.

cwperks · 2023-09-05T15:17:40Z

Opened backport: #3307

…d in core (opensearch-project#3306) Backport of fd3a143 from opensearch-project#3306 Signed-off-by: Craig Perkins <cwperx@amazon.com> Signed-off-by: Peter Nied <peternied@hotmail.com>

willyborankin · 2023-09-05T15:18:57Z

I am going to push this change in, we need to figure out what is wrong with the SecurityManager, but let's handle that separately from this version alignment that needs to happen across main & 2.x

@peternied I suspect we need use it as static instance and do not register as a default security provider. Given the fact that we do not have good tests for permissions. It will take sometime to pass BC instance through all code base we have.

reta · 2023-09-05T15:19:15Z

@reta looks like cryto-sdk brought BC as deps without permissions as result Sec plugin does not work and I think other plugins as well. OS now tries to check permissions via SDK but not via plugin :-)

Yeah, I think we have a problem now ....

api project(":libs:opensearch-encryption-sdk

I don't really understand why it is needed there, but things are messed up now

peternied · 2023-09-05T15:19:45Z

Creating manually backport [1]

[1] [Backport 2.x] Use version of org.apache.commons:commons-lang3 defined in core (#3306) #3308

peternied · 2023-09-05T15:30:16Z

I've created an issue to track this problem [1] impacting both main and the 2.10 line

[1] Security Plugin cannot startup due to AccessControlException: access denied #3309

…d in core (#3307) Backport of #3306 to 2.x Signed-off-by: Craig Perkins <cwperx@amazon.com>

…search-project#3306) Use version of org.apache.commons:commons-lang3 defined in core Signed-off-by: Craig Perkins <cwperx@amazon.com>

Use version of org.apache.commons:commons-lang3 defined in core

b62318f

Signed-off-by: Craig Perkins <cwperx@amazon.com>

cwperks requested review from cliu123, DarshitChanpura, davidlago, peternied, RyanL1997, stephen-crawford, reta and willyborankin as code owners September 5, 2023 14:18

cwperks added the backport 2.x backport to 2.x branch label Sep 5, 2023

cwperks mentioned this pull request Sep 5, 2023

[Backport 2.x] dependabot: bump com.diffplug.spotless from 6.20.0 to 6.21.0 #3298

Merged

Fix typo

17c0e4c

Signed-off-by: Craig Perkins <cwperx@amazon.com>

cwperks mentioned this pull request Sep 5, 2023

Fix commons-lang version #3301

Closed

3 tasks

reta approved these changes Sep 5, 2023

View reviewed changes

peternied merged commit fd3a143 into opensearch-project:main Sep 5, 2023
48 of 57 checks passed

opensearch-trigger-bot bot added the backport-failed label Sep 5, 2023

cwperks mentioned this pull request Sep 5, 2023

[Backport 2.x] Use version of org.apache.commons:commons-lang3 defined in core #3307

Merged

peternied mentioned this pull request Sep 5, 2023

[Backport 2.x] Use version of org.apache.commons:commons-lang3 defined in core (#3306) #3308

Closed

3 tasks

peternied removed the backport-failed label Sep 5, 2023

peternied pushed a commit that referenced this pull request Sep 5, 2023

[Backport 2.x] Use version of org.apache.commons:commons-lang3 define…

4de9c9e

…d in core (#3307) Backport of #3306 to 2.x Signed-off-by: Craig Perkins <cwperx@amazon.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use version of org.apache.commons:commons-lang3 defined in core #3306

Use version of org.apache.commons:commons-lang3 defined in core #3306

cwperks commented Sep 5, 2023

cwperks commented Sep 5, 2023 •

edited

Loading

cwperks commented Sep 5, 2023 •

edited

Loading

codecov bot commented Sep 5, 2023 •

edited

Loading

willyborankin commented Sep 5, 2023

willyborankin commented Sep 5, 2023 •

edited

Loading

peternied commented Sep 5, 2023

peternied commented Sep 5, 2023

opensearch-trigger-bot bot commented Sep 5, 2023

cwperks commented Sep 5, 2023

willyborankin commented Sep 5, 2023

reta commented Sep 5, 2023

peternied commented Sep 5, 2023

peternied commented Sep 5, 2023

Use version of org.apache.commons:commons-lang3 defined in core #3306

Use version of org.apache.commons:commons-lang3 defined in core #3306

Conversation

cwperks commented Sep 5, 2023

Description

Check List

cwperks commented Sep 5, 2023 • edited Loading

cwperks commented Sep 5, 2023 • edited Loading

codecov bot commented Sep 5, 2023 • edited Loading

Codecov Report

willyborankin commented Sep 5, 2023

willyborankin commented Sep 5, 2023 • edited Loading

peternied commented Sep 5, 2023

peternied commented Sep 5, 2023

opensearch-trigger-bot bot commented Sep 5, 2023

cwperks commented Sep 5, 2023

willyborankin commented Sep 5, 2023

reta commented Sep 5, 2023

peternied commented Sep 5, 2023

peternied commented Sep 5, 2023

cwperks commented Sep 5, 2023 •

edited

Loading

cwperks commented Sep 5, 2023 •

edited

Loading

codecov bot commented Sep 5, 2023 •

edited

Loading

willyborankin commented Sep 5, 2023 •

edited

Loading