-
Notifications
You must be signed in to change notification settings - Fork 62
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
Enhanced CI workflow to include security plugin build and replication security tests #296
Enhanced CI workflow to include security plugin build and replication security tests #296
Conversation
… security tests Signed-off-by: Sai Kumar <[email protected]>
LGTM |
- name: Build and run Replication tests | ||
run: | | ||
ls -al src/test/resources/security/plugin | ||
./gradlew clean release -Dbuild.snapshot=true -Dopensearch.version=1.3.0-SNAPSHOT -PnumNodes=1 -Psecurity=true |
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.
Since we can't combine the coverage, I think we can upload coverage only for tests with security enabled and ignore for -Psecurity=false
. Don't have to do as part of this PR though. Thoughts?
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.
Makes sense. Let's address coverage once all the changes are in.
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.
LGTM
working-directory: ./security | ||
run: | | ||
mvn clean package -Padvanced -DskipTests | ||
cp target/releases/opensearch-security-*-SNAPSHOT.zip ../src/test/resources/security/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.
Do we have an issue open for this? I understand why you are doing it, but you should be using an artifact produced by CI instead of rebuilding security so that someone coming to this project can just check out the code and build it without any kind of prerequisites.
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.
we are tracking the move to maven distributed version of plugin builds for replication under this issue: #274 which is currently dependent on opensearch-project/opensearch-build#716
For the local, security plugin build is not mandatory - Developers can still independently checkout replication repo, build and run locally.
Signed-off-by: Sai Kumar [email protected]
Description
Enhanced CI workflow to include security plugin build and replication security tests
Issues Resolved
#295
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.