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

[Backport 2.x] React to changes in ActionListener in core #3182

Merged

Conversation

RyanL1997
Copy link
Collaborator

@RyanL1997 RyanL1997 commented Aug 15, 2023

Description

Manual backport for #3153 of React to changes in ActionListener in core
Also includes a fix for a call to XContentHelper.toXContent in response to opensearch-project/OpenSearch#9156

Issues Resolved

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: Ryan Liang <jiallian@amazon.com>
@RyanL1997 RyanL1997 force-pushed the manual-backport-actionlistener-fix branch from 904aeab to 55ba25f Compare August 15, 2023 18:44
Signed-off-by: Ryan Liang <jiallian@amazon.com>
@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Merging #3182 (02ba199) into 2.x (4a14af4) will decrease coverage by 0.04%.
Report is 1 commits behind head on 2.x.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##                2.x    #3182      +/-   ##
============================================
- Coverage     62.22%   62.19%   -0.04%     
+ Complexity     3312     3310       -2     
============================================
  Files           265      265              
  Lines         19502    19502              
  Branches       3328     3328              
============================================
- Hits          12136    12130       -6     
- Misses         5741     5744       +3     
- Partials       1625     1628       +3     
Files Changed Coverage Δ
.../opensearch/security/OpenSearchSecurityPlugin.java 84.26% <ø> (ø)
.../security/action/whoami/TransportWhoAmIAction.java 28.57% <ø> (ø)
...nsearch/security/action/whoami/WhoAmIResponse.java 0.00% <ø> (ø)
...ty/configuration/ConfigurationLoaderSecurity7.java 65.67% <ø> (ø)
...ity/configuration/DlsFilterLevelActionHandler.java 57.20% <ø> (ø)
...rch/security/configuration/DlsFlsRequestValve.java 0.00% <ø> (ø)
...search/security/configuration/DlsFlsValveImpl.java 59.23% <ø> (ø)
...arch/security/dlic/rest/api/AbstractApiAction.java 75.95% <ø> (ø)
...ch/security/dlic/rest/api/FlushCacheApiAction.java 62.50% <ø> (ø)
...earch/security/dlic/rest/api/MigrateApiAction.java 3.66% <ø> (ø)
... and 2 more

... and 2 files with indirect coverage changes

@cwperks
Copy link
Member

cwperks commented Aug 15, 2023

We may need to manually delete caches for the failing windows checks

@peternied peternied changed the title [Manual Backport 2.x] React to changes in ActionListener in core [Backport 2.x] React to changes in ActionListener in core Aug 15, 2023
@peternied
Copy link
Member

Looks like this was required because we've got those Clean up REST API changes in main that haven't been pulled into 2.x [1] - and likely won't until the set of changes is complete.

@peternied
Copy link
Member

@cwperks It looks like the codeql build task is broken because of a cached version of the snapshot - should we disable the gradle build cache on our github actions? Is this something we've been doing manually

@cwperks
Copy link
Member

cwperks commented Aug 15, 2023

@peternied any idea how much disabling caching would slow down the CI? I am in favor of disabling it because it has been making our builds unpredictable.

@peternied
Copy link
Member

@cwperks I'll take a slower correct over a fast and incorrect any day. I'll create a PR

@cwperks
Copy link
Member

cwperks commented Aug 15, 2023

Why is codeql running ./gradlew compileIntegrationTestJava on 2.x?

Task :compileIntegrationTestJava FAILED

https://github.com/opensearch-project/security/actions/runs/5870600207/job/15918932614?pr=3182

Either all integration test changes need to be backport to 2.x from main or this check should not run on 2.x. I would be in support of disabling the check on this branch and only keeping integration tests in main since they are not run as part of CI. We should remove anything integration test code from 2.x.

@peternied
Copy link
Member

Why is codeql running ./gradlew compileIntegrationTestJava on 2.x?

@cwperks Good find, can you create an issue and make sure this is all unified between 2.x and main?

…listener-fix

Signed-off-by: Peter Nied <petern@amazon.com>
@peternied
Copy link
Member

peternied commented Aug 15, 2023

@RyanL1997 FYI - I just merged in the change to disable the gradle build cache, we should expect all builds to pass prior to merging.

Note; We don't require codeql. so lets follow up separately

@cwperks cwperks merged commit ee81de7 into opensearch-project:2.x Aug 15, 2023
50 of 52 checks passed
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.

4 participants