Skip to content
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

Improve Health Status of security-auditlog Index in Single Node Clusters #5030

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@
package org.opensearch.security.auditlog.sink;

import java.io.IOException;
import java.util.Map;

import com.google.common.collect.ImmutableMap;
import org.opensearch.ResourceAlreadyExistsException;
import org.opensearch.action.DocWriteRequest;
import org.opensearch.action.admin.indices.create.CreateIndexRequest;
import org.opensearch.action.index.IndexRequestBuilder;
import org.opensearch.action.support.WriteRequest.RefreshPolicy;
import org.opensearch.client.Client;
Expand Down Expand Up @@ -51,6 +55,29 @@ public void close() throws IOException {

}

private boolean indexExists(String indexName) {
return clientProvider
Ganesh-RB marked this conversation as resolved.
Show resolved Hide resolved
.admin()
.indices()
.prepareExists(indexName)
.execute()
.actionGet()
.isExists();
}

private boolean createIndexIfAbsent(String indexName){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this could be moved to common/utils class.

try {
final Map<String, Object> indexSettings = ImmutableMap.of("index.number_of_shards", 1, "index.auto_expand_replicas", "0-all");
Ganesh-RB marked this conversation as resolved.
Show resolved Hide resolved
final CreateIndexRequest createIndexRequest = new CreateIndexRequest(indexName).settings(indexSettings);
final boolean ok = clientProvider.admin().indices().create(createIndexRequest).actionGet().isAcknowledged();
log.info("Index {} created?: {}", indexName, ok);
return ok;
} catch (ResourceAlreadyExistsException resourceAlreadyExistsException) {
log.info("Index {} already exists", indexName);
Copy link
Member

@cwperks cwperks Jan 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't lastUsedIndexName be set here?

What happens in the case of node reboot and the audit log index exists? lastUsedIndexName would be null when the node starts. Shouldn't this catch clause be used to populate it if it is null and the index already exists?

Copy link
Member

@cwperks cwperks Jan 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI I experimented around with passing the clusterService to the internal opensearch sinks which can be used to check if an index exists without having to issue an Exists request. Take a look at the change on my fork cwperks#44. That change also shows an example of how to use the integration test framework in this repo to write an integration test for this case.

return false;
}
}

public boolean doStore(final AuditMessage msg, String indexName) {

if (Boolean.parseBoolean(
Expand All @@ -62,6 +89,10 @@ public boolean doStore(final AuditMessage msg, String indexName) {
return true;
}

if(!indexExists(indexName)){
createIndexIfAbsent(indexName);
}

try (StoredContext ctx = threadPool.getThreadContext().stashContext()) {
try {
final IndexRequestBuilder irb = clientProvider.prepareIndex(indexName)
Expand Down
Loading