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

securityadmin: Replace TransportClient by RestHighLevelClient #1638

Merged
merged 6 commits into from
Mar 2, 2022

Conversation

rs-eliatra
Copy link
Contributor

@rs-eliatra rs-eliatra commented Feb 22, 2022

Description

[Describe what this change achieves]

Issues Resolved

#1578

Testing

Testing done:

  • unit testing
  • manual tests

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.

@rs-eliatra rs-eliatra requested a review from a team February 22, 2022 20:51
@rs-eliatra rs-eliatra changed the title Transportclient removal securityadmin: Replace TransportClient by RestHighLevelClient Feb 23, 2022
Signed-off-by: rs-eliatra <rafal.stobiecki@eliatra.com>
Implement getting free port in tests: SinkProviderTLSTest and WebhookAuditLogTest

Signed-off-by: rs-eliatra <rafal.stobiecki@eliatra.com>
@cliu123 cliu123 requested a review from vrozov February 24, 2022 05:18
@codecov-commenter
Copy link

codecov-commenter commented Feb 24, 2022

Codecov Report

Merging #1638 (8081472) into main (fc3842c) will decrease coverage by 0.55%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1638      +/-   ##
============================================
- Coverage     64.61%   64.05%   -0.56%     
+ Complexity     3217     3212       -5     
============================================
  Files           247      249       +2     
  Lines         17352    17478     +126     
  Branches       3082     3087       +5     
============================================
- Hits          11212    11196      -16     
- Misses         4594     4723     +129     
- Partials       1546     1559      +13     
Impacted Files Coverage Δ
...g/opensearch/security/support/WildcardMatcher.java 61.03% <ø> (ø)
...a/org/opensearch/security/tools/SecurityAdmin.java 45.55% <38.46%> (-2.18%) ⬇️
...opensearch/security/filter/SecurityRestFilter.java 76.66% <66.66%> (+0.80%) ⬆️
...opensearch/security/rest/SecurityWhoAmIAction.java 78.37% <78.37%> (ø)
.../action/configupdate/ConfigUpdateNodeResponse.java 86.36% <83.33%> (-1.14%) ⬇️
...arch/security/rest/SecurityConfigUpdateAction.java 95.23% <95.23%> (ø)
.../opensearch/security/OpenSearchSecurityPlugin.java 81.00% <100.00%> (+0.07%) ⬆️
...rity/action/configupdate/ConfigUpdateResponse.java 100.00% <100.00%> (ø)
...mazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java 9.43% <0.00%> (-75.48%) ⬇️
...ava/com/amazon/dlic/auth/ldap2/MakeJava9Happy.java 39.13% <0.00%> (-4.35%) ⬇️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc3842c...8081472. Read the comment docs.

Signed-off-by: rs-eliatra <rafal.stobiecki@eliatra.com>
Signed-off-by: rs-eliatra <rafal.stobiecki@eliatra.com>
@cliu123
Copy link
Member

cliu123 commented Mar 1, 2022

@rs-eliatra @nibix 1.3 branch has been created to hold all changes for 1.3 branch. This PR will be included in 2.0 branch, so main is the right target branch.

cliu123
cliu123 previously approved these changes Mar 2, 2022
Signed-off-by: rs-eliatra <rafal.stobiecki@eliatra.com>
peternied
peternied previously approved these changes Mar 2, 2022
Signed-off-by: rs-eliatra <rafal.stobiecki@eliatra.com>
@peternied peternied added backport 1.3 backport to 1.3 branch v1.3.0 labels Mar 2, 2022
@cliu123 cliu123 removed the backport 1.3 backport to 1.3 branch label Mar 2, 2022
@cliu123
Copy link
Member

cliu123 commented Mar 2, 2022

Removing the label backport 1.3 because TransportClient is removed from OpenSearch 2.0.

@cliu123 cliu123 added v2.0.0 and removed v1.3.0 labels Mar 2, 2022
@peternied
Copy link
Member

@cliu123 Is there a reason we do not want this change in 1.3? Seems like it will more than make the pencils down date

@cliu123
Copy link
Member

cliu123 commented Mar 2, 2022

@cliu123 Is there a reason we do not want this change in 1.3? Seems like it will more than make the pencils down date

This change is not necessary for 1.3.0 since TransportClient still exists in 1.3.0. This PR is a part of removing TransportClient effort. There will be more PRs to be done, for example #1660. 1.3.0 code freeze is being targeted for next week, I think it will be safer to only merge things that have been completely settled down to 1.3.0 : ). But removing TransportClient is still not done yet.

@cliu123 cliu123 merged commit 81a43c2 into opensearch-project:main Mar 2, 2022
DarshitChanpura pushed a commit to DarshitChanpura/security that referenced this pull request Mar 7, 2022
@rs-eliatra rs-eliatra deleted the transportclient-removal branch April 13, 2022 08:56
peternied added a commit to peternied/security that referenced this pull request Dec 2, 2022
peternied added a commit that referenced this pull request Dec 6, 2022
Windows build and test support for 1.3

- Use most recent format of CI workflows from main
- Backport #2206
- Supply workarounds for JDK8 incompatible APIs for Encoding / Pattern matching (Thanks @cwperks!)
- Backport only freeport logic from #1638
- Backport #1758
- All updates to TestAuditlogImpl.java from main
  - #2180
  - #1935 
  - #1920
  - #1914
  - #1829 
  - And Targeted test updates for ComplianceAuditlogTest and BasicAuditlogTest to keep up with TestAuditlogImpl.java updates

Signed-off-by: Peter Nied <petern@amazon.com>
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com>
Co-authored-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com>
wuychn pushed a commit to ochprince/security that referenced this pull request Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants