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

[FEATURE] Improve built-in secure transports support #4179

Merged
merged 2 commits into from
Mar 28, 2024

Conversation

reta
Copy link
Collaborator

@reta reta commented Mar 27, 2024

Description

Incorporate further improvements from opensearch-project/OpenSearch#12907

Issues Resolved

Part of opensearch-project/OpenSearch#12903

Is this a backport? If so, please add backport PR # and/or commits #

Testing

[Please provide details of testing done: unit testing, integration testing and manual testing]

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.

@reta
Copy link
Collaborator Author

reta commented Mar 27, 2024

Depends on opensearch-project/OpenSearch#12907

Copy link
Contributor

@stephen-crawford stephen-crawford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple comments but clean code as always. Thanks @reta

@reta
Copy link
Collaborator Author

reta commented Mar 28, 2024

The BWC / plugins check fail, we need distributions being published

Copy link

codecov bot commented Mar 28, 2024

Codecov Report

Attention: Patch coverage is 86.27451% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 66.19%. Comparing base (a870b40) to head (65c6e3c).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4179      +/-   ##
==========================================
- Coverage   66.21%   66.19%   -0.02%     
==========================================
  Files         302      301       -1     
  Lines       21701    21709       +8     
  Branches     3501     3505       +4     
==========================================
+ Hits        14369    14371       +2     
- Misses       5579     5581       +2     
- Partials     1753     1757       +4     
Files Coverage Δ
.../opensearch/security/OpenSearchSecurityPlugin.java 84.32% <100.00%> (-0.30%) ⬇️
...opensearch/security/filter/SecurityRestFilter.java 70.37% <100.00%> (ø)
...earch/security/http/NonSslHttpServerTransport.java 100.00% <ø> (ø)
.../ssl/http/netty/Netty4ConditionalDecompressor.java 100.00% <100.00%> (ø)
...sl/http/netty/Netty4HttpRequestHeaderVerifier.java 87.50% <100.00%> (+1.78%) ⬆️
...arch/security/ssl/OpenSearchSecuritySSLPlugin.java 86.30% <66.66%> (+0.05%) ⬆️
.../security/ssl/OpenSearchSecureSettingsFactory.java 79.31% <75.00%> (-20.69%) ⬇️

... and 1 file with indirect coverage changes

@reta
Copy link
Collaborator Author

reta commented Mar 28, 2024

@scrawfor99 may I ask you please to reapprove (new tests were pushed), thank you!

@reta
Copy link
Collaborator Author

reta commented Mar 28, 2024

The BWC tests are failing because the change is breaking (on experimental API) and for a clean build, we would need #4187 first

@reta reta merged commit f375765 into opensearch-project:main Mar 28, 2024
82 checks passed
dlin2028 pushed a commit to dlin2028/security that referenced this pull request May 1, 2024
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