Build indicesAccessControl for selective indices

[ywangd]

During index authorization, the IndicesAccessControl is built for all
original indices. The list of original indices can be large for a large
size cluster and requests with liberal wildcards. Therefore computing
IndicesAccessControl for every one of them can be expensive.

The observation is that many shard/index level request only really just
target a single concrete index and hence just need IndicesAccessControl
for that single index. One twist is that the single index may be
requested with its alias or authorized as part of a data stream. In this
case, the relevant aliases and data stream also need to be considered.
Still this list of concrete index and its aliases and data stream can be
much smaller than the original indices.

This PR adds an effective allowlist for requests that we know how to
extract the single concrete target index and only compute
IndicesAccessControl for the index and relevant aliases, data stream.

[elasticmachine]

Pinging @elastic/es-security (Team:Security)

[ywangd]

There are more Transport Requests that target a single index/shard. These three are chosen to match what are currently being used in benchmarking. They also serve the purpose of demostrating the general idea of this change. I'll work on a more complete list once we agree on the approach.

[albertzaharovits]

I took a very close look at this.
My hope is that the same improvement is going to be covered in core, by #78508 . I think that is almost there, and it also avoids the serialization/deserialization costs you were mentioning in the last weekly meeting.

[ywangd]

I took a very close look at this. My hope is that the same improvement is going to be covered in core, by #78508 . I think that is almost there, and it also avoids the serialization/deserialization costs you were mentioning in the last weekly meeting.

Thanks Albert. Filtering the list of names in the core (#78508) is a superior solution in terms of performance. It covers this PR and more:

1. It does what this PR is trying to do in a more natural way
2. It reduces the looping cost (due to small set of requested indices) in IndicesAndAliasesResolver#resolveIndicesAndAliases
3. More importantly, it cut downs the de/serialisation cost for sending requests cross nodes.

I am happy to close this PR once #78508 is merged. We should also discuss whether the same change should be actively promoted to other parts in core where applicable. I think it should be. But discussion would be great to understand the impact better and also help decide on whether core should always be aware of the source names, i.e. whether the concrete index name is from an alias or even multiple aliases.

PS: This PR and other hacks locally almost screamed for the change in #78508. But I just didn't notice it. A lesson learned on how to tackle an issue in a more broader context (than just security).

[tvernum]

Can we implement something in core that unifies these? It seems like

public interface ShardIndicesRequest extends IndicesRequest {
    public ShardId shardId();
}

would be helpful.

[tvernum]

This bothers me.
Can we add getIndexMetadata to IndexAbstraction.Index so we don't have to rely on the implementation detail of pulling a index out of the singleton list?

Or add getNumberOfAliases to IndexAbstraction? (I assume we don't call getAliases().size() because it's not very efficient).

Or we could make IndexAbstraction.getAliases() return Collection<String> and make IndexAbstraction.Index.getAliases cheaper to call.

Actually, since we call getAliases below (in method 1) we should just call it here and keep a local copy of the result.

[tvernum]

Suggested change

	effectiveIndices = new HashSet<>();
	effectiveIndices = new HashSet<>(totalAliases + 2);
	// aliases + targetIndexName + parentDataStream

[tvernum]

Collectors.toUnmodifiableSet() is very inefficient. If you're trying to squeeze the milliseconds here, you're better off constructing the set by hand like in method 1.

Suggested change

	effectiveIndices = indices.stream().filter(name -> {
	if (name.equals(targetIndexName)) {
	return true;
	}
	final IndexAbstraction indexAbstraction = aliasAndIndexLookup.get(name);
	if (indexAbstraction.getType() != IndexAbstraction.Type.CONCRETE_INDEX) {
	return indexAbstraction.getIndices().stream().anyMatch(im -> im.getIndex().getName().equals(targetIndexName));
	}
	return false;
	}).collect(Collectors.toUnmodifiableSet());
	List<String> indexNames = new ArrayList<>(indices.size());
	indices.stream().filter(name -> {
	if (name.equals(targetIndexName)) {
	return true;
	}
	final IndexAbstraction indexAbstraction = aliasAndIndexLookup.get(name);
	if (indexAbstraction.getType() != IndexAbstraction.Type.CONCRETE_INDEX) {
	return indexAbstraction.getIndices().stream().anyMatch(im -> im.getIndex().getName().equals(targetIndexName));
	}
	return false;
	}).forEach(effectiveIndices::add);
	effectiveIndices = Set.copyOf(indexNames);

I've suggested using a list to collect the names here, because I think it's going to be more efficient (but I haven't benchmarked it).
Set.copyOf constructs a new HashSet even if the argument is already a HashSet, so you're better off just using an ArrayList for raw efficiency while collecting and then paying the uniqueness cost once during copyOf.

[tvernum]

If we're going to introduce a copyOf here, then we should change the caller to use Set.of rather than Sets.newHashSet and reduce the amount of copying that happens.

[ywangd]

Closing since changes in the core (#78508) is a better pattern.