Fixing security tests

[joshpalis]

Description

Enabled security integration tests using -Dhttps flag. Now security enabled integration tests will run with the following commands

• ./gradlew integTest -Dhttps=true -Duser=admin -Dpassword=admin
• ./gradlew integTest -Dsecurity.enabled=true

Issues Resolved

Resolves #469

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.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a812e51) 71.91% compared to head (9fdde23) 71.91%.
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##               main     #477   +/-   ##
=========================================
  Coverage     71.91%   71.91%           
  Complexity      621      621           
=========================================
  Files            78       78           
  Lines          3133     3133           
  Branches        234      234           
=========================================
  Hits           2253     2253           
  Misses          776      776           
  Partials        104      104           

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

[owaiskazi19]

Thanks for addressing this @joshpalis.
QQ: How does security tests work for other plugins with security.enabled flag?

[dbwiddis]

In the issue summary, add "Fixes" or "Resolves" prior to the issue number you're fixing. That will auto-link the issue to close it when the PR is merged.

[dbwiddis]

LGTM! Is there a way to re-trigger the build that autocut the issue to verify the fix?

[joshpalis]

QQ: How does security tests work for other plugins with security.enabled flag?

Other plugins such as KNN only pull in the security plugin when the security.enabled flag is set. However they do not filter tests based on this flag. In our case, when running with security enabled, we only include the secure integration tests. The issue was that the distribution build job only passes the https flag, the security.enabled flag is something that we added for our plugin, so our non-security enabled tests were not filtered out. These changes ensure that non-security tests are filtered out based on the https flag instead of the security.enabled flag

[owaiskazi19]

Other plugins such as KNN only pull in the security plugin when the security.enabled flag is set. However they do not filter tests based on this flag. In our case, when running with security enabled, we only include the secure integration tests. The issue was that the distribution build job only passes the https flag, the security.enabled flag is something that we added for our plugin, so our non-security enabled tests were not filtered out. These changes ensure that non-security tests are filtered out based on the https flag instead of the security.enabled flag

The reason KNN and other plugins work with security-enabled flag is because they have the same integ tests running for security plugin enabled and disabled case. Whereas we have different tests for both. Instead, we should have the same test cases by only changing the url of the cluster to http or https.
Also, the https flag is set here from the build repo.
I am fine merging this for 2.12 but for the next release we can incorporate #478.

[owaiskazi19]

LGTM! Is there a way to re-trigger the build that autocut the issue to verify the fix?

Builds run once a day. Ongoing build https://build.ci.opensearch.org/view/Build/job/distribution-build-opensearch/9294/

[owaiskazi19]

In the issue summary, add "Fixes" or "Resolves" prior to the issue number you're fixing. That will auto-link the issue to close it when the PR is merged.

For autocut issue we should avoid using fixes or closes because if the build passes, autocut bot itself closes the issue.

[dbwiddis]

For autocut issue we should avoid using fixes or closes because if the build passes, autocut bot itself closes the issue.

Or we can close it and autocut will reopen it when it fails again...