[FEATURE] Index pattern resolution improvements

[nibix]

Overview

Goal of this issue is to implement broad improvements regarding the index pattern resolution in the security plugin.

Index pattern resolution is a vital component of the security plugin; it extracts from action requests metadata about the indices, aliases and data streams a request is going to touch. All other components of the security plugin rely on the index pattern resolution to reliably and efficiently provide this information.

However, as described in the following sections, the index pattern resolution currently operates in ways that are less than optimal.

Current situation

Performance

CPU profiling on the security plugin code shows that the core index pattern resolution code is one of the most CPU intense tasks of the request processing performed by the security plugin.

This is the code in question:

https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java

An analysis of the code shows that many things are done unnecessarily:

• The alias resolution is expensive because it is done by filtering the whole index set (which can be quite big); yet, the result of the alias resolution is used only in a single component which is often bypassed: the PrivilegeInterceptorImpl for implementing Dashboards multi tenancy.

• Data streams are broken apart into individual indices even if the privilege is granted on the data stream itself; as data streams can have many backing indices, this can be also expensive.

• Generally, lots of copying and filtering of potentially large data structures is going on

Structure

The class IndexResolverReplacer must have white-box knowledge of the requests it wants to process. This can be seen here:

security/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java

Lines 657 to 836 in 19a41b8

	if (request instanceof BulkRequest) {
	for (DocWriteRequest ar : ((BulkRequest) request).requests()) {
	result = getOrReplaceAllIndices(ar, provider, false) && result;
	}
	} else if (request instanceof MultiGetRequest) {
	for (ListIterator<Item> it = ((MultiGetRequest) request).getItems().listIterator(); it.hasNext();) {
	Item item = it.next();
	result = getOrReplaceAllIndices(item, provider, false) && result;
	/*if(item.index() == null || item.indices() == null || item.indices().length == 0) {
	it.remove();
	}*/
	}
	} else if (request instanceof MultiSearchRequest) {
	for (ListIterator<SearchRequest> it = ((MultiSearchRequest) request).requests().listIterator(); it.hasNext();) {
	SearchRequest ar = it.next();
	result = getOrReplaceAllIndices(ar, provider, false) && result;
	/*if(ar.indices() == null || ar.indices().length == 0) {
	it.remove();
	}*/
	}
	} else if (request instanceof MultiTermVectorsRequest) {
	for (ActionRequest ar : (Iterable<TermVectorsRequest>) () -> ((MultiTermVectorsRequest) request).iterator()) {
	result = getOrReplaceAllIndices(ar, provider, false) && result;
	}
	} else if (request instanceof PutMappingRequest) {
	PutMappingRequest pmr = (PutMappingRequest) request;
	Index concreteIndex = pmr.getConcreteIndex();
	if (concreteIndex != null && (pmr.indices() == null || pmr.indices().length == 0)) {
	String[] newIndices = provider.provide(new String[] { concreteIndex.getName() }, request, true);
	if (checkIndices(request, newIndices, true, allowEmptyIndices) == false) {
	return false;
	}
	((PutMappingRequest) request).indices(newIndices);
	((PutMappingRequest) request).setConcreteIndex(null);
	} else {
	String[] newIndices = provider.provide(((PutMappingRequest) request).indices(), request, true);
	if (checkIndices(request, newIndices, false, allowEmptyIndices) == false) {
	return false;
	}
	((PutMappingRequest) request).indices(newIndices);
	}
	} else if (request instanceof RestoreSnapshotRequest) {
	if (clusterInfoHolder.isLocalNodeElectedClusterManager() == Boolean.FALSE) {
	return true;
	}
	final RestoreSnapshotRequest restoreRequest = (RestoreSnapshotRequest) request;
	final SnapshotInfo snapshotInfo = SnapshotRestoreHelper.getSnapshotInfo(restoreRequest);
	if (snapshotInfo == null) {
	log.warn(
	"snapshot repository '" + restoreRequest.repository() + "', snapshot '" + restoreRequest.snapshot() + "' not found"
	);
	provider.provide(new String[] { "*" }, request, false);
	} else {
	final List<String> requestedResolvedIndices = IndexUtils.filterIndices(
	snapshotInfo.indices(),
	restoreRequest.indices(),
	restoreRequest.indicesOptions()
	);
	final List<String> renamedTargetIndices = renamedIndices(restoreRequest, requestedResolvedIndices);
	// final Set<String> indices = new HashSet<>(requestedResolvedIndices);
	// indices.addAll(renamedTargetIndices);
	if (isDebugEnabled) {
	log.debug("snapshot: {} contains this indices: {}", snapshotInfo.snapshotId().getName(), renamedTargetIndices);
	}
	provider.provide(renamedTargetIndices.toArray(new String[0]), request, false);
	}
	} else if (request instanceof IndicesAliasesRequest) {
	for (AliasActions ar : ((IndicesAliasesRequest) request).getAliasActions()) {
	result = getOrReplaceAllIndices(ar, provider, false) && result;
	}
	} else if (request instanceof DeleteRequest) {
	String[] newIndices = provider.provide(((DeleteRequest) request).indices(), request, true);
	if (checkIndices(request, newIndices, true, allowEmptyIndices) == false) {
	return false;
	}
	((DeleteRequest) request).index(newIndices.length != 1 ? null : newIndices[0]);
	} else if (request instanceof UpdateRequest) {
	String[] newIndices = provider.provide(((UpdateRequest) request).indices(), request, true);
	if (checkIndices(request, newIndices, true, allowEmptyIndices) == false) {
	return false;
	}
	((UpdateRequest) request).index(newIndices.length != 1 ? null : newIndices[0]);
	} else if (request instanceof SingleShardRequest) {
	final SingleShardRequest<?> singleShardRequest = (SingleShardRequest<?>) request;
	final String index = singleShardRequest.index();
	String[] indices = provider.provide(index == null ? null : new String[] { index }, request, true);
	if (!checkIndices(request, indices, true, allowEmptyIndices)) {
	return false;
	}
	singleShardRequest.index(indices.length != 1 ? null : indices[0]);
	} else if (request instanceof FieldCapabilitiesIndexRequest) {
	// FieldCapabilitiesIndexRequest does not support replacing the indexes.
	// However, the indexes are always determined by FieldCapabilitiesRequest which will be reduced below
	// (implements Replaceable). So IF an index arrives here, we can be sure that we have
	// at least privileges for indices:data/read/field_caps
	FieldCapabilitiesIndexRequest fieldCapabilitiesRequest = (FieldCapabilitiesIndexRequest) request;
	String index = fieldCapabilitiesRequest.index();
	String[] newIndices = provider.provide(new String[] { index }, request, true);
	if (!checkIndices(request, newIndices, true, allowEmptyIndices)) {
	return false;
	}
	} else if (request instanceof IndexRequest) {
	String[] newIndices = provider.provide(((IndexRequest) request).indices(), request, true);
	if (checkIndices(request, newIndices, true, allowEmptyIndices) == false) {
	return false;
	}
	((IndexRequest) request).index(newIndices.length != 1 ? null : newIndices[0]);
	} else if (request instanceof Replaceable) {
	String[] newIndices = provider.provide(((Replaceable) request).indices(), request, true);
	if (checkIndices(request, newIndices, false, allowEmptyIndices) == false) {
	return false;
	}
	((Replaceable) request).indices(newIndices);
	} else if (request instanceof RolloverRequest) {
	provider.provide(((RolloverRequest) request).indices(), request, false);
	return false;
	} else if (request instanceof BulkShardRequest) {
	provider.provide(((ReplicationRequest) request).indices(), request, false);
	// replace not supported?
	} else if (request instanceof ReplicationRequest) {
	String[] newIndices = provider.provide(((ReplicationRequest) request).indices(), request, true);
	if (checkIndices(request, newIndices, true, allowEmptyIndices) == false) {
	return false;
	}
	((ReplicationRequest) request).index(newIndices.length != 1 ? null : newIndices[0]);
	} else if (request instanceof MultiGetRequest.Item) {
	String[] newIndices = provider.provide(((MultiGetRequest.Item) request).indices(), request, true);
	if (checkIndices(request, newIndices, true, allowEmptyIndices) == false) {
	return false;
	}
	((MultiGetRequest.Item) request).index(newIndices.length != 1 ? null : newIndices[0]);
	} else if (request instanceof CreateIndexRequest) {
	String[] newIndices = provider.provide(((CreateIndexRequest) request).indices(), request, true);
	if (checkIndices(request, newIndices, true, allowEmptyIndices) == false) {
	return false;
	}
	((CreateIndexRequest) request).index(newIndices.length != 1 ? null : newIndices[0]);
	} else if (request instanceof ResizeRequest) {
	// clone or shrink operations
	provider.provide(((ResizeRequest) request).indices(), request, true);
	provider.provide(((ResizeRequest) request).getTargetIndexRequest().indices(), request, true);
	} else if (request instanceof CreateDataStreamAction.Request) {
	provider.provide(((CreateDataStreamAction.Request) request).indices(), request, false);
	} else if (request instanceof ReindexRequest) {
	result = getOrReplaceAllIndices(((ReindexRequest) request).getDestination(), provider, false) && result;
	result = getOrReplaceAllIndices(((ReindexRequest) request).getSearchRequest(), provider, false) && result;
	} else if (request instanceof BaseNodesRequest) {
	// do nothing
	} else if (request instanceof MainRequest) {
	// do nothing
	} else if (request instanceof ClearScrollRequest) {
	// do nothing
	} else if (request instanceof SearchScrollRequest) {
	// do nothing
	} else if (request instanceof PutComponentTemplateAction.Request) {
	// do nothing
	} else {
	if (isDebugEnabled) {
	log.debug(request.getClass() + " not supported (It is likely not a indices related request)");
	}
	result = false;
	}
	return result;
	}

This is a fragile way to implement things and requires a big maintenance effort. Newly actions added to the core must/shoule be reviewed whether they need special treatment here.

do_not_fail_on_forbidden

The class IndexResolverReplacer additionally implements the index replace algorithm required by the do_not_fail_on_forbidden mode. As the class does not implement it in a universal way, there are many issues with that mode as described in #3905.

New approach

Goal of this issue is to completely replace the IndexResolverReplacer implementation which:

• Leverages core interfaces to efficiently get the "true" resolved indices from a request
• Avoids unnecessary index resolutions
• Can be leveraged to implement the refined index authorization as described in [RFC] Refined index authorization (replacement for do_not_fail_on_forbidden) #3905

Strategy

• Conceive core interfaces to be implemented by action requests to provide true index information
• Create prototype implementation (might also leverage caching of resolution result in request object)
• Review usage of IndexResolverReplacer in security plugin and conceive a new light weight replacement
• Implement refined index authorization (gated by feature flag)

[cwperks]

[Triage] Thank you for filing this issue @nibix . I'm very supportive of reworking this area of the code for performance, maintainability and testability. I recently made a change to the Index resolution process for RolloverRequests and I think the big if <-> else block can be brittle if new requests types are added that do not extend IndicesRequest.Replaceable.

[aparajita31pandey]

@nibix Indeed IndexResolverReplacer.java it's currently quite difficult to extend or customize.
With this refactor I'd expect the core logic would remain intact and and new version would contain something similar to originalRequest field or similar . We're currently using this functionality via a JAR in our codebase.

[nibix]

@aparajita31pandey

Interesting. Could you elaborate on your use-case and how you are using it?

[aparajita31pandey]

Hey @nibix , We have a requirement to derive indices for all types of ActionRequest. We are using originalRequest to derive indices from any type of Action Request object. Would new plan provide something similar ?

[nibix]

@aparajita31pandey

So, the properties you need, and which are provided to you by originalIndices are:

• Index patterns are not resolved, i.e., when a search request is done on index_*, I also get reported index_*
• Date math is not resolved
• Aliases are not resolved, i.e., when i am searching on alias_a, I also get alias_a and not its member indices

Do I understand that right? Is it for some kind of logging?

Did you consider just directly using IndicesRequest.indices()?