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] Update imports for files refactored in core PR #8157 (#3003) #3036

Merged

Conversation

DarshitChanpura
Copy link
Member

@DarshitChanpura DarshitChanpura commented Jul 19, 2023

Description

Backports 37aacdc from #3003

Also adds integration tests to 2.x line.

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.

Sorry, something went wrong.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…ect#3003)

* Update imports for files refactored in core PR #8157

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

* Update references to old packages in test files

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

* Get remaining bad imports in integrationTest

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

* Update log4j in bwc build.gradle

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

* Use versions.log4j

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

* Also reference guava version

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

* Update integtest.sh

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

* Update tests that expect certain amount of headers in a response

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

* Empty commit

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

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
(cherry picked from commit 37aacdc)
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
@codecov
Copy link

codecov bot commented Jul 19, 2023

Codecov Report

Merging #3036 (0d0021e) into 2.x (e2a3a38) will decrease coverage by 0.05%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##                2.x    #3036      +/-   ##
============================================
- Coverage     62.25%   62.21%   -0.05%     
+ Complexity     3311     3308       -3     
============================================
  Files           264      264              
  Lines         19468    19468              
  Branches       3326     3326              
============================================
- Hits          12120    12112       -8     
- Misses         5721     5729       +8     
  Partials       1627     1627              
Impacted Files Coverage Δ
...ic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java 58.42% <ø> (ø)
...mazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java 84.48% <ø> (ø)
...ic/auth/http/kerberos/HTTPSpnegoAuthenticator.java 0.00% <ø> (ø)
...dlic/auth/http/saml/AuthTokenProcessorHandler.java 47.05% <ø> (ø)
...zon/dlic/auth/http/saml/HTTPSamlAuthenticator.java 69.02% <ø> (ø)
.../opensearch/security/OpenSearchSecurityPlugin.java 84.31% <ø> (-0.32%) ⬇️
.../action/configupdate/ConfigUpdateNodeResponse.java 77.27% <ø> (ø)
...urity/action/configupdate/ConfigUpdateRequest.java 94.11% <ø> (ø)
...rity/action/configupdate/ConfigUpdateResponse.java 64.28% <ø> (ø)
...tion/configupdate/TransportConfigUpdateAction.java 100.00% <ø> (ø)
... and 76 more

... and 1 file with indirect coverage changes

@DarshitChanpura
Copy link
Member Author

Plugin install seems to be failing due to stale core artifact. Will wait for new artifact to be available before re-running these tasks.

The error below should not be thrown as that is the correct path for Writeable.Reader:

java.lang.NoClassDefFoundError: org/opensearch/core/common/io/stream/Writeable$Reader

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
@DarshitChanpura
Copy link
Member Author

Backwards-compatibility also seems to be failing only for windows due to stale artifact issue too.

scripts/integtest.sh Outdated Show resolved Hide resolved
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
@DarshitChanpura
Copy link
Member Author

Backwards compatibility tests are broken because the CI task checks out 2.x and current HEAD at 2.x doesn;t contain the import changes. (This PR brings those changes)

@DarshitChanpura
Copy link
Member Author

We either need to force-merge this to fix bwc tests or temporarily change the reference to current branch to DarshitChanpura/backport/backport-3003-to-2.x/ and fix that once this PR is merged into 2.x

@RyanL1997
Copy link
Collaborator

I think this is the similar issue happened on main, and if this is the case I think Craig's procedure can be also applied to here. But yeah, instead of referencing to the current 2.x, reference to current fix's branch will definitely resolve the issue.

@cwperks
Copy link
Member

cwperks commented Jul 20, 2023

bwc tests need to be revisited on the 2.x branch. I see a change in main that isn't in 2.x that could help the issue: #2253

cwperks
cwperks previously approved these changes Jul 20, 2023
@cwperks
Copy link
Member

cwperks commented Jul 20, 2023

I think the bwc tests in 2.x are doing too much. It should be testing from the 1.3 -> branch of the PR against 2.x

@cwperks
Copy link
Member

cwperks commented Jul 20, 2023

@DarshitChanpura I believe you can update these lines: https://github.com/opensearch-project/security/blob/2.x/.github/workflows/ci.yml#L92-L93

The new values would be 2.9 and current_branch respectively

Edit: create-bwc-build/action.yaml also needs the corresponding current_branch section:

- name: Checkout Branch from Fork
  if: ${{ inputs.plugin-branch == 'current_branch' }}
  uses: actions/checkout@v2
  with:
    path: ${{ inputs.plugin-branch }}

- uses: actions/checkout@v3
  if: ${{ inputs.plugin-branch != 'current_branch' }}
  with:
    repository: opensearch-project/security
    ref: ${{ inputs.plugin-branch }}
    path: ${{ inputs.plugin-branch }}

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
@cwperks cwperks merged commit a0ac52b into opensearch-project:2.x Jul 21, 2023
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.

None yet

3 participants