Support metadata on API keys (#70292)

[ywangd]

This PR adds metadata support for API keys. Metadata are of type
Map<String, Object> and can be optionally provided at API key creation time.
It is returned as part of GetApiKey response. It is also stored as part of
the authentication object to transfer throw the wire.
Note that it is not yet searchable and not exposed to any ingest processors.
They will be handled by separate PRs.

[ywangd]

@tvernum I updated this PR since the system index backport #71370 is now merged. I am requesting your review because there are quite many changes just to leverage the new feature of system indices. This single commit touches 19 files. Some of them are trivial, but there are also important changes. Some of them should even get "forward-ported" to master. It would be great if I can get a second pair of eyes to go through them.

The main reason for the relative large size of changes is to guard all possible scenarios where things can go wrong. The logic in pseudo code is like the follows:

if request has non empty metadata
  if Coordinating Node is before 7.13
    REJECT in RestCreateApiKeyAction
  else
    if Cluster has Node before 7.3
       REJECT in ApiKeyGenerator
    else
       SUCCESS
else if request metadata is an empty map
  if Coordinating Node is before 7.13
    REJECT in RestCreateApiKeyAction
  else
    if Cluster has Node before 7.3
      Do not create the metadata_flattened field in ApiKeyService#newDocument
      SUCCESS
    else
      SUCCESS
else
  if Coordinating Node is before 7.13
    SUCCESS
  else
    if Cluster has Node before 7.3
      Do not create the metadata_flattened field in ApiKeyService#newDocument
      SUCCESS
    else
      SUCCESS       

Please let me know if you'd like to have a high-bandwidth sync about the changes. It might be easier to talk through them.

[tvernum]

I don't think this is the right check. I think you want to be looking at the version of the mapping on the security index not the version of the nodes in the cluster.
The requirement you're trying to solve for is actually that you only write the metadata_flattened field if the mapping supports it, you don't really care about node versions.

[ywangd]

I thought about using SecurityIndexManager#checkMappingVersion, but was not sure about its condition of currentIndexState.mappingVersion == null. This condition means the check returns true if no mappingVersion is avaiable. But it is questionable in this case, because the cluster may have a node prior to 7.3 so that once mappingVersion is available, it will not be up-to-date. So basically we need check smallestNonClientNodeVersion again in this case. That's why I chose to just check smallestNonClientNodeVersion in the current PR. But I now realise it has its own issue because an old node can join the cluster after the mapping is updated. In this case, the check of smallestNonClientNodeVersion only will not be accurate.

So I guess the comprehensive solution should be SecurityIndexManager#checkMappingVersion plus smallestNonClientNodeVersion?

[tvernum]

To be clear, the only circumstance where this would be an issue is in a mixed cluster with some nodes < 7.3.0 that doesn't have a security index yet. Is that right?
It seems like a relatively low probability occurrence.

However, I think the correct fix is, if there is no current mapping, then ask the System Index Descriptor what mapping it would install on the current cluster and use that mapping for the checks.
That is, the question we care about is: "If I try and write to the security index, which version mapping will it have?"

[ywangd]

Also another issue is that the securityIndexManager.checkMappingVersion(Version.V_7_13_0::onOrBefore) may fail because the current mapping is indeed not up-to-date. But it will be up-to-date right before the current API key gets created because of SecurityIndexManager#prepareIndexIfNeededThenExecute. But again, the twist is SecurityIndexManager#prepareIndexIfNeededThenExecute may not update the mapping because the cluster could have an old node. Now the question is back again to check smallestNonClientNodeVersion.

[ywangd]

So I think the complete check should be something like:

if (securityIndexManager.checkMappingVersion(Version.V_7_13_0::onOrBefore) || smallestNonClientNodeVersion.onOrAfter(FLATTENED_FIELD_TYPE_INTRODUCED)) {
   // proceed normally and SUCCESS
} else {
   if (request has non-empty metadata) {
      // FAIL
   } else {
      // do not add the empty `metadata_flattened` field and SUCCESS
   }
}

What do you think?

[tvernum]

The issue is that you're replicating the internal logic of System indices.
The only reason you think you care about smallestNonClientNodeVersion is because that's what System Indices uses to determine which mapping it will install. Don't duplicate that logic, just ask the system indices descriptor to tell you what's going to happen.

[ywangd]

OK, I'll add a new method to SecurityIndexManager to encapsulate this logic.

[ywangd]

Just want to note that there will still be edge cases for things to go wrong mainly because the check here and the actual update are not an atomic operation. It is theoretically possible that an old node joins the cluster just in between the check and the update. When that happens, the API key creation will fail with an error like "new field is not allow with mapping dynamic=strict". But I think it is fine since (1) it is a very rare case and (2) the operation at least fails instead of doing the wrong thing.

[tvernum]

It is theoretically possible that an old node joins the cluster

We don't support adding old nodes to an existing cluster. There may be cases where it works, but it's never supported.

[ywangd]

That's good to know. Is it explicitly stated anywhere? I referred to the rolling upgrade guide and it is not clear to me or maybe it is just my reading.

[ywangd]

@tvernum I address all your comments with this commit 44f5b4b
I think it is largely simplified. Thanks a lot for the suggestions!