Skip to content

securityadmin: Replace TransportClient by RestHighLevelClient#1638

Merged
cliu123 merged 6 commits intoopensearch-project:mainfrom
rs-eliatra:transportclient-removal
Mar 2, 2022
Merged

securityadmin: Replace TransportClient by RestHighLevelClient#1638
cliu123 merged 6 commits intoopensearch-project:mainfrom
rs-eliatra:transportclient-removal

Conversation

@rs-eliatra
Copy link
Copy Markdown
Contributor

@rs-eliatra rs-eliatra commented Feb 22, 2022 •
edited
Loading

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 force-pushed the transportclient-removal branch from bd8c297 to 0feced5 Compare February 23, 2022 17:47
@rs-eliatra rs-eliatra changed the title Transportclient removal securityadmin: Replace TransportClient by RestHighLevelClient Feb 23, 2022
cliu123
cliu123 reviewed Feb 23, 2022
View reviewed changes
@rs-eliatra rs-eliatra force-pushed the transportclient-removal branch from 0feced5 to ab9cb0a Compare February 23, 2022 23:30
@rs-eliatra
securityadmin: Replace TransportClient by RestHighLevelClient
de4bc1d 
Signed-off-by: rs-eliatra <rafal.stobiecki@eliatra.com>
@rs-eliatra
Fix 'BindException' in some tests
04d5dcb 
Implement getting free port in tests: SinkProviderTLSTest and WebhookAuditLogTest

Signed-off-by: rs-eliatra <rafal.stobiecki@eliatra.com>
@rs-eliatra rs-eliatra force-pushed the transportclient-removal branch from ab9cb0a to 04d5dcb Compare February 23, 2022 23:34
cliu123
cliu123 reviewed Feb 24, 2022
View reviewed changes
@cliu123 cliu123 requested a review from vrozov February 24, 2022 05:18
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 24, 2022 •
edited by codecov bot
Loading

Codecov Report

❌ Patch coverage is 50.64516% with 153 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.58%. Comparing base (fc3842c) to head (8081472).
⚠️ Report is 1534 commits behind head on main.

Files with missing lines Patch % Lines
...a/org/opensearch/security/tools/SecurityAdmin.java 39.31% 102 Missing and 40 partials ⚠️
...opensearch/security/rest/SecurityWhoAmIAction.java 78.37% 7 Missing and 1 partial ⚠️
.../action/configupdate/ConfigUpdateNodeResponse.java 83.33% 0 Missing and 1 partial ⚠️
...opensearch/security/filter/SecurityRestFilter.java 66.66% 0 Missing and 1 partial ⚠️
...arch/security/rest/SecurityConfigUpdateAction.java 95.23% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1638      +/-   ##
============================================
- Coverage     64.61%   64.58%   -0.04%     
- Complexity     3217     3240      +23     
============================================
  Files           247      249       +2     
  Lines         17352    17479     +127     
  Branches       3082     3096      +14     
============================================
+ Hits          11212    11288      +76     
- Misses         4594     4634      +40     
- Partials       1546     1557      +11
Files with missing lines Coverage Δ
.../opensearch/security/OpenSearchSecurityPlugin.java 81.00% <100.00%> (+0.07%) ⬆️
...rity/action/configupdate/ConfigUpdateResponse.java 100.00% <100.00%> (ø)
...g/opensearch/security/support/WildcardMatcher.java 61.03% <ø> (ø)
.../action/configupdate/ConfigUpdateNodeResponse.java 86.36% <83.33%> (-1.14%) ⬇️
...opensearch/security/filter/SecurityRestFilter.java 76.66% <66.66%> (+0.80%) ⬆️
...arch/security/rest/SecurityConfigUpdateAction.java 95.23% <95.23%> (ø)
...opensearch/security/rest/SecurityWhoAmIAction.java 78.37% <78.37%> (ø)
...a/org/opensearch/security/tools/SecurityAdmin.java 45.91% <39.31%> (-1.81%) ⬇️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rs-eliatra
review remarks: rename tests
191bfe9 
Signed-off-by: rs-eliatra <rafal.stobiecki@eliatra.com>
@rs-eliatra rs-eliatra force-pushed the transportclient-removal branch from 12758fe to 191bfe9 Compare February 24, 2022 17:28
@rs-eliatra
refactor & fixes: SSLContext configuration
426c6bf 
Signed-off-by: rs-eliatra <rafal.stobiecki@eliatra.com>
@cliu123
Copy link
Copy Markdown
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
View reviewed changes
@rs-eliatra
review remarks
95322de 
Signed-off-by: rs-eliatra <rafal.stobiecki@eliatra.com>
peternied
peternied previously approved these changes Mar 2, 2022
View reviewed changes
cliu123
cliu123 reviewed Mar 2, 2022
View reviewed changes
@rs-eliatra
review remarks
8081472 
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
Copy Markdown
Member

cliu123 commented Mar 2, 2022 •
edited
Loading

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
Copy Markdown
Member

peternied 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

@cliu123
Copy link
Copy Markdown
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 @DarshitChanpura
securityadmin: Replace TransportClient by RestHighLevelClient (opense…
a53bad7 
…arch-project#1638)

Signed-off-by: rs-eliatra <rafal.stobiecki@eliatra.com>
@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
[Backport] WebhookAuditLogTest find free port from opensearch-project…
cb0bd6a 
…#1638

Signed-off-by: Peter Nied <petern@amazon.com>
@peternied peternied mentioned this pull request Dec 2, 2022
Merged
3 tasks
peternied added a commit that referenced this pull request Dec 6, 2022
@peternied @stephen-crawford
Windows build and test support for 1.3 (#2291)
8e1adfa 
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>
@cwperks cwperks mentioned this pull request Mar 10, 2023
Open
wuychn pushed a commit to ochprince/security that referenced this pull request Mar 16, 2023
@rs-eliatra
securityadmin: Replace TransportClient by RestHighLevelClient (opense…
9ce8883 
…arch-project#1638)

Signed-off-by: rs-eliatra <rafal.stobiecki@eliatra.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nibix nibix nibix left review comments

@cliu123 cliu123 cliu123 approved these changes

@vrozov vrozov Awaiting requested review from vrozov

+1 more reviewer

@peternied peternied peternied approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

v2.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@rs-eliatra @codecov-commenter @cliu123 @peternied @nibix