Fix audit log writing errors for rollover-enabled alias indices #5900
+228
−8
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,202 @@ | ||
| /* | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * The OpenSearch Contributors require contributions made to | ||
| * this file be licensed under the Apache-2.0 license or a | ||
| * compatible open source license. | ||
| * | ||
| * Modifications Copyright OpenSearch Contributors. See | ||
| * GitHub history for details. | ||
| */ | ||
| package org.opensearch.security.auditlog.sink; | ||
|
|
||
| import java.util.Map; | ||
|
|
||
| import org.junit.BeforeClass; | ||
| import org.junit.ClassRule; | ||
| import org.junit.Test; | ||
|
|
||
| import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest; | ||
| import org.opensearch.action.admin.indices.alias.get.GetAliasesRequest; | ||
| import org.opensearch.action.admin.indices.alias.get.GetAliasesResponse; | ||
| import org.opensearch.action.admin.indices.create.CreateIndexRequest; | ||
| import org.opensearch.action.admin.indices.exists.indices.IndicesExistsRequest; | ||
| import org.opensearch.action.search.SearchRequest; | ||
| import org.opensearch.action.search.SearchResponse; | ||
| import org.opensearch.index.query.QueryBuilders; | ||
| import org.opensearch.search.builder.SearchSourceBuilder; | ||
| import org.opensearch.test.framework.AuditConfiguration; | ||
| import org.opensearch.test.framework.AuditFilters; | ||
| import org.opensearch.test.framework.cluster.ClusterManager; | ||
| import org.opensearch.test.framework.cluster.LocalCluster; | ||
| import org.opensearch.test.framework.cluster.TestRestClient; | ||
| import org.opensearch.transport.client.Client; | ||
|
|
||
| import static org.hamcrest.CoreMatchers.equalTo; | ||
| import static org.hamcrest.CoreMatchers.not; | ||
| import static org.hamcrest.MatcherAssert.assertThat; | ||
| import static org.hamcrest.Matchers.greaterThan; | ||
| import static org.hamcrest.Matchers.is; | ||
| import static org.hamcrest.collection.IsMapContaining.hasKey; | ||
| import static org.awaitility.Awaitility.await; | ||
|
|
||
| /** | ||
| * Integration tests for {@link InternalOpenSearchSink} with write-alias–based configuration. | ||
| * | ||
| * <p>These tests validate the audit sink behavior when a preconfigured write alias | ||
| * is used instead of a concrete index name. In this setup, the audit sink must | ||
| * detect that the target name is an alias and avoid attempting index creation.</p> | ||
| * | ||
| * <h5>Tested Code Path:</h5> | ||
| * <p>These tests focus on the {@code metadata.hasAlias(indexName)} branch in | ||
| * {@code createIndexIfAbsent()}, ensuring that alias targets are correctly | ||
| * recognized and accepted.</p> | ||
| * | ||
| * <p>This behavior is required to support Index Lifecycle Management (ILM) | ||
| * patterns where a write alias points to a rolling series of indices.</p> | ||
| * | ||
| * @see InternalOpenSearchSinkIntegrationTest for regular index-based tests | ||
| */ | ||
| public class InternalOpenSearchSinkIntegrationTestAuditAlias { | ||
|
|
||
| private static final String AUDIT_ALIAS = "security-audit-write-alias"; | ||
| private static final String BACKING_INDEX = "security-audit-backend-000001"; | ||
|
|
||
| @ClassRule | ||
| public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) | ||
| .nodeSettings(Map.of("plugins.security.audit.config.index", AUDIT_ALIAS)) | ||
| .internalAudit(new AuditConfiguration(true).filters(new AuditFilters().enabledRest(true).enabledTransport(false))) | ||
| .build(); | ||
|
|
||
| @BeforeClass | ||
| public static void setupAuditAlias() { | ||
| try (Client client = cluster.getInternalNodeClient()) { | ||
| client.admin().indices().create(new CreateIndexRequest(BACKING_INDEX)).actionGet(); | ||
| client.admin() | ||
| .indices() | ||
| .aliases( | ||
| new IndicesAliasesRequest().addAliasAction( | ||
| IndicesAliasesRequest.AliasActions.add().index(BACKING_INDEX).alias(AUDIT_ALIAS).writeIndex(true) | ||
| ) | ||
| ) | ||
| .actionGet(); | ||
| } | ||
| } | ||
|
|
||
| private long countAuditDocs(Client client) { | ||
| SearchResponse response = client.search( | ||
| new SearchRequest(AUDIT_ALIAS).source(new SearchSourceBuilder().query(QueryBuilders.matchAllQuery()).size(0)) | ||
| ).actionGet(); | ||
|
|
||
| return response.getHits().getTotalHits().value(); | ||
| } | ||
|
|
||
| private void generateAuditEvent(String path) { | ||
| try (TestRestClient restClient = cluster.getRestClient(cluster.getAdminCertificate())) { | ||
| restClient.get(path); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Tests the core functionality of the alias detection change. | ||
| * | ||
| * <p><b>Validates:</b> The {@code metadata.hasAlias(indexName)} branch correctly | ||
| * identifies the audit target as an alias and returns true without attempting | ||
| * index creation.</p> | ||
| * | ||
| * <p><b>Without this change:</b> The sink would try to create an index named | ||
| * {@code security-audit-write-alias}, which would fail because an alias with | ||
| * that name already exists.</p> | ||
| */ | ||
| @Test | ||
| public void testRecognizesAuditTargetAsWriteAlias() { | ||
| try (Client client = cluster.getInternalNodeClient()) { | ||
| generateAuditEvent("_cluster/health"); | ||
|
|
||
| await().until(() -> countAuditDocs(client) > 0); | ||
|
|
||
| GetAliasesResponse aliasesResponse = client.admin().indices().getAliases(new GetAliasesRequest(AUDIT_ALIAS)).actionGet(); | ||
|
|
||
| assertThat("Write alias must exist in cluster metadata", aliasesResponse.getAliases().isEmpty(), is(false)); | ||
|
|
||
| String concreteIndex = aliasesResponse.getAliases().keySet().iterator().next(); | ||
| assertThat("Alias must resolve to a backing index", concreteIndex, not(equalTo(AUDIT_ALIAS))); | ||
|
|
||
| boolean backendIndexExists = client.admin().indices().exists(new IndicesExistsRequest(concreteIndex)).actionGet().isExists(); | ||
|
|
||
| assertThat("Backing index must exist physically", backendIndexExists, is(true)); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Tests that audit events are successfully written through the alias. | ||
| * | ||
| * <p><b>Validates:</b> The sink writes events to the alias, which OpenSearch | ||
| * routes to the write index. Multiple events accumulate correctly.</p> | ||
| */ | ||
| @Test | ||
| public void testWritesEventsToAliasSuccessfully() { | ||
| try (Client client = cluster.getInternalNodeClient()) { | ||
| long before = countAuditDocs(client); | ||
|
|
||
| generateAuditEvent("_cluster/health"); | ||
| generateAuditEvent("_cluster/stats"); | ||
| generateAuditEvent("_nodes/info"); | ||
|
|
||
| await().until(() -> countAuditDocs(client) - before >= 3); | ||
|
|
||
| long after = countAuditDocs(client); | ||
| assertThat("Multiple events must be written through alias", after - before, greaterThan(2L)); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Verifies that audit documents written via a write alias contain all mandatory fields, | ||
| * correct values, and do not include irrelevant transport-layer fields. | ||
| * | ||
| * <p><b>Validates:</b> When an audit event is generated and routed through a write alias, | ||
| * the resulting document contains the required fields: | ||
| * <ul> | ||
| * <li>{@code audit_category}</li> | ||
| * <li>{@code audit_request_origin} (value: {@code "REST"})</li> | ||
| * <li>{@code @timestamp}</li> | ||
| * <li>{@code audit_rest_request_method}</li> | ||
| * <li>{@code audit_rest_request_path}</li> | ||
| * <li>{@code audit_request_layer} (value: {@code "REST"})</li> | ||
| * </ul> | ||
| * and does <b>not</b> include {@code audit_transport_request_type}, | ||
| * confirming that transport-specific fields are absent for REST events.</p> | ||
| * | ||
| * <p><b>Why it's important:</b> This ensures that using a write alias does not alter | ||
| * the content or structure of audit documents, preserving compliance and correctness.</p> | ||
| */ | ||
| @Test | ||
| public void testAuditDocumentsViaAliasContainMandatoryFields() { | ||
|
Collaborator
There was a problem hiding this comment. Is this a test which covers the alias logic? It seems to be this covers logic from a higher level.
Contributor
Author
There was a problem hiding this comment. This test validates the end-to-end write path through the alias branch of createIndexIfAbsent(). The other two tests verify that the alias is recognized (testRecognizesAuditTargetAsWriteAlias) and that documents are counted (testWritesEventsToAliasSuccessfully), but neither inspects the document content. This test closes the loop by confirming that a document written through the alias pipeline — createIndexIfAbsent() → prepareIndex(aliasName) → execute() — arrives in the backing index structurally intact with all mandatory fields and correct values. Without it, we'd know documents are delivered but not that they're correct. |
||
| try (Client client = cluster.getInternalNodeClient()) { | ||
| long before = countAuditDocs(client); | ||
| generateAuditEvent("_cluster/health"); | ||
|
|
||
| await().until(() -> countAuditDocs(client) > before); | ||
|
|
||
| SearchResponse response = client.search( | ||
| new SearchRequest(AUDIT_ALIAS).source( | ||
| new SearchSourceBuilder().query(QueryBuilders.matchAllQuery()) | ||
| .size(1) | ||
| .sort("@timestamp", org.opensearch.search.sort.SortOrder.DESC) | ||
| ) | ||
| ).actionGet(); | ||
|
|
||
| Map<String, Object> doc = response.getHits().getAt(0).getSourceAsMap(); | ||
|
|
||
| assertThat(doc, hasKey("audit_category")); | ||
| assertThat(doc, hasKey("audit_request_origin")); | ||
| assertThat(doc, hasKey("@timestamp")); | ||
| assertThat(doc, hasKey("audit_rest_request_method")); | ||
| assertThat(doc, hasKey("audit_rest_request_path")); | ||
| assertThat(doc, hasKey("audit_request_layer")); | ||
| assertThat(doc.get("audit_request_layer"), is("REST")); | ||
| assertThat(doc.get("audit_request_origin"), is("REST")); | ||
| assertThat(doc, not(hasKey("audit_transport_request_type"))); | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
nit: prefer to use RestClient here instead of internalNodeClient