-
Notifications
You must be signed in to change notification settings - Fork 0
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
Run IT tests with security plugin #335
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a big fan of integration test around security scenarios, added a couple of comments/considerations.
@@ -57,6 +60,20 @@ ext { | |||
projectSubstitutions = [:] | |||
licenseFile = rootProject.file('LICENSE.TXT') | |||
noticeFile = rootProject.file('NOTICE') | |||
|
|||
getSecurityPluginDownloadLink = { -> | |||
var repo = "https://aws.oss.sonatype.org/content/repositories/snapshots/org/opensearch/plugin/" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Security Plugin maintainers (myself included) have had trouble with build breaks and fixing them in a timely manner - it would be unfortunate for your integration tests to be flaky due to downstream issues. This isn't an issue with release builds with the trade-off of longer time between updates.
integ-test/build.gradle
Outdated
setting 'plugins.security.ssl.transport.enforce_hostname_verification', 'false' | ||
// https is disabled, because `OpenSearchCluster` is hardcoded to validate cluster health by http | ||
// refer how IT framework implemented in security plugin and reuse/copy to activate https | ||
setting 'plugins.security.ssl.http.enabled', 'false' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some features of the security plugin - such as being able to make changes to the security configuration dynamically that will not be possible with this setting disabled. Depends if your tests will ever need to exercise that kind of functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This issue was raised because Release testing runs IT tests with the security plugin. If we can catch errors early, we can avoid blocking release.
integ-test/build.gradle
Outdated
setting 'plugins.security.ssl.transport.enforce_hostname_verification', 'false' | ||
// https is disabled, because `OpenSearchCluster` is hardcoded to validate cluster health by http | ||
// refer how IT framework implemented in security plugin and reuse/copy to activate https | ||
setting 'plugins.security.ssl.http.enabled', 'false' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This issue was raised because Release testing runs IT tests with the security plugin. If we can catch errors early, we can avoid blocking release.
integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java
Show resolved
Hide resolved
58c5c1f
to
90f2873
Compare
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
9277659
to
e25e7af
Compare
Signed-off-by: Yury-Fridlyand <[email protected]>
integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java
Show resolved
Hide resolved
integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java
Show resolved
Hide resolved
integ-test/src/test/java/org/opensearch/sql/legacy/OpenSearchSQLRestTestCase.java
Show resolved
Hide resolved
Signed-off-by: Yury-Fridlyand <[email protected]>
Codecov Report
@@ Coverage Diff @@
## integ-IT-with-security #335 +/- ##
=========================================================
Coverage 97.51% 97.51%
Complexity 4657 4657
=========================================================
Files 408 408
Lines 11933 11933
Branches 829 829
=========================================================
Hits 11637 11637
Misses 289 289
Partials 7 7
Flags with carried forward coverage won't be shown. Click here to find out more. 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
* Run IT tests with security plugin (#335) * Add extra IT flow. Signed-off-by: Yury-Fridlyand <[email protected]> * Remove unneeded files. Signed-off-by: Yury-Fridlyand <[email protected]> * Typo fix. Signed-off-by: Yury-Fridlyand <[email protected]> * Fix GHA matrix syntax. Signed-off-by: Yury-Fridlyand <[email protected]> * Fix GHA matrix syntax. Signed-off-by: Yury-Fridlyand <[email protected]> * Code clean up. Signed-off-by: Yury-Fridlyand <[email protected]> * Optimize downloading. Signed-off-by: Yury-Fridlyand <[email protected]> * Apply suggestions from code review Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> * Update integ-test/build.gradle Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> * Typo fix. Signed-off-by: Yury-Fridlyand <[email protected]> * Rework implementation. Signed-off-by: Yury-Fridlyand <[email protected]> * Address PR review. Signed-off-by: Yury-Fridlyand <[email protected]> * Address PR feedback + some fixes. Signed-off-by: Yury-Fridlyand <[email protected]> --------- Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> * Minor fix. Signed-off-by: Yury-Fridlyand <[email protected]> * Address PR feedback. Signed-off-by: Yury-Fridlyand <[email protected]> * Typo fix. Signed-off-by: Yury-Fridlyand <[email protected]> --------- Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]>
* Run IT tests with security plugin (#335) * Add extra IT flow. Signed-off-by: Yury-Fridlyand <[email protected]> * Remove unneeded files. Signed-off-by: Yury-Fridlyand <[email protected]> * Typo fix. Signed-off-by: Yury-Fridlyand <[email protected]> * Fix GHA matrix syntax. Signed-off-by: Yury-Fridlyand <[email protected]> * Fix GHA matrix syntax. Signed-off-by: Yury-Fridlyand <[email protected]> * Code clean up. Signed-off-by: Yury-Fridlyand <[email protected]> * Optimize downloading. Signed-off-by: Yury-Fridlyand <[email protected]> * Apply suggestions from code review Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> * Update integ-test/build.gradle Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> * Typo fix. Signed-off-by: Yury-Fridlyand <[email protected]> * Rework implementation. Signed-off-by: Yury-Fridlyand <[email protected]> * Address PR review. Signed-off-by: Yury-Fridlyand <[email protected]> * Address PR feedback + some fixes. Signed-off-by: Yury-Fridlyand <[email protected]> --------- Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> * Minor fix. Signed-off-by: Yury-Fridlyand <[email protected]> * Address PR feedback. Signed-off-by: Yury-Fridlyand <[email protected]> * Typo fix. Signed-off-by: Yury-Fridlyand <[email protected]> --------- Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> (cherry picked from commit 7e3a718) Signed-off-by: Yury-Fridlyand <[email protected]>
Description
This PR adds a new IT gradle task:
integTestWithSecurity
. It starts a cluster with security plugin installed (it takes latest snapshot), configures cluster, http client for tests and runs one test which required to be run with security plugin.A new GHA is added which runs this test task.
This PR temporary includes files from opensearch-project#1943.
Issues Resolved
opensearch-project#1713
Check List
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.