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

Accepts https as a property to set securityEnabled flag #1100

Conversation

DarshitChanpura
Copy link
Member

@DarshitChanpura DarshitChanpura commented Feb 8, 2024

Build file '/local/home/dchanp/codebase/index-management/build.gradle' line: 441

* What went wrong:
Execution failed for task ':integTest'.
> `cluster{::integTest}` failed to wait for cluster health yellow after 40 SECONDS
    Unexpected end of file from server

Upon investigation I discovered that the integration tests triggered by opensearch-build do not pass security as an inline argument, instead they pass https (code-line). This cause the index-management tests in RC builds run with-security to fail with Not a valid SSL/TLS record: NotSslRecordException which translates to mismatch between http vs https. The protocol is set here conditionally to https via securityEnabled flag via

String protocol = securityEnabled ? "https" : "http"

  • The culprit:
    def securityEnabled = System.getProperty("security", "false") == "true" || System.getProperty("https", "false") == "true"

This PR adds an additional check to toggle securityEnabled flag by checking if https was passed.

  • The fix:
def securityEnabled = System.getProperty("security", "false") == "true" || System.getProperty("https", "false") == "true"

CheckList:

  • 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: Darshit Chanpura <dchanp@amazon.com>
@DarshitChanpura
Copy link
Member Author

@bowenlan-amzn @r1walz mind reviewing this one?

Copy link

codecov bot commented Feb 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4d8ef69) 75.37% compared to head (bc10724) 75.36%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1100      +/-   ##
============================================
- Coverage     75.37%   75.36%   -0.01%     
- Complexity     2813     2814       +1     
============================================
  Files           367      367              
  Lines         17038    17038              
  Branches       2370     2370              
============================================
- Hits          12842    12841       -1     
- Misses         2893     2894       +1     
  Partials       1303     1303              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

build.gradle Show resolved Hide resolved
@bowenlan-amzn bowenlan-amzn merged commit 5ff29d1 into opensearch-project:main Feb 8, 2024
36 of 37 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 8, 2024
(cherry picked from commit 5ff29d1)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 8, 2024
(cherry picked from commit 5ff29d1)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport 2.12 v2.12.0 Issues targeting release v2.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants