Removing flaky test case testAutoInit

[peternied]

Description

Considering what is tested - the auto log events that are creating when a test cluster starts up. This isn't a useful test case, removing.

Issues Resolved

• Resolves [Flaky Test] org.opensearch.security.auditlog.compliance.RestApiComplianceAuditlogTest.testAutoInit is flaky #3730

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.

[codecov]

Codecov Report

Merging #3732 (36f24a5) into main (ee66199) will increase coverage by 1.38%.
Report is 3 commits behind head on main.
The diff coverage is 82.22%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3732      +/-   ##
==========================================
+ Coverage   64.88%   66.27%   +1.38%     
==========================================
  Files         292      292              
  Lines       20776    20806      +30     
  Branches     3409     3411       +2     
==========================================
+ Hits        13481    13789     +308     
+ Misses       5606     5317     -289     
- Partials     1689     1700      +11     

Files	Coverage Δ
.../org/opensearch/security/auth/BackendRegistry.java	77.92% <100.00%> (+15.38%)	⬆️
...g/opensearch/security/filter/SecurityResponse.java	96.29% <100.00%> (+3.43%)	⬆️
...org/opensearch/security/httpclient/HttpClient.java	86.13% <ø> (ø)
...curity/securityconf/impl/AllowlistingSettings.java	75.00% <100.00%> (ø)
...curity/securityconf/impl/WhitelistingSettings.java	75.00% <100.00%> (ø)
...dlic/auth/http/saml/AuthTokenProcessorHandler.java	50.28% <50.00%> (ø)
...opensearch/security/filter/SecurityRestFilter.java	83.84% <50.00%> (+14.61%)	⬆️
...ic/auth/http/kerberos/HTTPSpnegoAuthenticator.java	0.00% <0.00%> (ø)

... and 37 files with indirect coverage changes

[cwperks]

I'm not convinced yet that this is a test that should be removed.

I took a glance at this test suite and it looks like it needs to be refactored to use TestAuditLogImpl.doThenWaitForMessages

As it is, this test is useless because its not idempotent. The audit log is not cleared before this test runs so it relies on being at a certain location in the test suite to run properly.

[peternied]

@cwperks if we had an issue to create a test that verifies the audit actions during the test cluster setup process - I think it would need to be authored differently to properly measure what happens. Behavior such as calling the cluster health API once instead of twice, the test would need to be written to interpret those changes. I think we'd need to rebuild the test to accommodate - more than just use the better concurrency safe capture method.

In terms of risk of not having this test is pretty low on my list. I'd rather spend time building better support for the new credentials generator or tackling some incoming bugs we have then try to rehab this test case.

[peternied]

There will be changes that destabilize or block contributions. The impact of these changes will be localized on the repository or even the entire OpenSearch project. We should bias towards keeping contributions unblocked by immediately reverting impacting changes, these reverts will be done by a maintainer. After the change has been reverted, an issue will be openned to re-merge the change and callout the elements of the contribution that need extra examination such as additional tests or even pull request workflows.

Exceptional, instead of immediately reverting, if a contributor knows how and will resolve the issue in an hour or less we should fix-forward to reduce overhead.

From the maintainers guidance

@cwperks Our builds are constantly failing due to this test. And I think it will take more than an hour to fix - with that in mind do you think we could merge as is? When the rewritten version is ready we add it back in, but until that happens we will have builds that are more stable.

Not blocking this PR to let other PRs affected by the test flow through.

[cwperks]

@peternied I've lifted the Requested Changes, but my preference would be to merge the test fix for this instead of removal. This whole suite of tests needs to be revisited. Our audit log tests could be a lot stronger than what they are today.

[opensearch-trigger-bot]

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/security/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/security/backport-2.x
# Create a new branch
git switch --create backport/backport-3732-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 ca8aafe3ff75b1dbc39f4ed857d5c385e0025bcf
# Push it to GitHub
git push --set-upstream origin backport/backport-3732-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/security/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-3732-to-2.x.

[cwperks]

@peternied Are you planning to open a backport for this PR?