Set metadata on initial request in API key noop test

[n1v0lg]

This PR fixes API key integration test setup for noops. In the noop
test, we use the initial update request as a reference to choose
suitable values to force non-noop updates, including metadata. However,
metadata can be null on the initial request, meaning that the
underlying API key will retain the metadata it was assigned on
creation. This can lead to test failure when the metadata on the
initial request is null, and the subsequent metadata update matches
the metadata chosen at API key creation time.

Closes #88503.

[elasticmachine]

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

[n1v0lg]

@elasticmachine run elasticsearch-ci/bwc

[ywangd]

I am not sure whether this is the right fix. The failure occurs at line 1805:

elasticsearch/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java

Line 1805 in baadfb9

assertTrue(response.isUpdated());

The associated updateRequest is prepared to have a non-null metadata and also not the same as the initial metadata at line 1801:

elasticsearch/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java

Line 1801 in baadfb9

randomValueOtherThanMany(md -> md == null || md.equals(initialRequest.getMetadata()), ApiKeyTests::randomMetadata)

So why forcing a non-null initial metadata would help?

Also, I think the following random logic is incorrect:

elasticsearch/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java

Lines 1774 to 1781 in baadfb9

	randomValueOtherThanMany(
	rd -> (RoleDescriptorRequestValidator.validate(rd) != null) && initialRequest.getRoleDescriptors().contains(rd) == false,
	() -> RoleDescriptorTests.randomRoleDescriptor(false)
	),
	randomValueOtherThanMany(
	rd -> (RoleDescriptorRequestValidator.validate(rd) != null) && initialRequest.getRoleDescriptors().contains(rd) == false,
	() -> RoleDescriptorTests.randomRoleDescriptor(false)
	)

We want it the new descriptor to be (1) valid and (2) not contained by the initial descriptors. So I think it should be something like the follows?

randomValueOtherThanMany(
    rd -> RoleDescriptorRequestValidator.validate(rd) != null || initialRequest.getRoleDescriptors().contains(rd),
    () -> RoleDescriptorTests.randomRoleDescriptor(false)
),

[n1v0lg]

So why forcing a non-null initial metadata would help?

If metadata on initialRequest is null, then the metadata assigned during creation stays on the doc (since null metadata means don't update).

Suppose:

created.metadata = {"bar": "foo"}
initialRequest.metadata = null
then it can happen that on line 1805, metadata is {"bar": "foo"} matching the metadata on the doc and resulting in false for noop because we're comparing to initialRequest.metadata when picking a distinct metadata value

[n1v0lg]

We want it the new descriptor to be (1) valid and (2) not contained by the initial descriptors. So I think it should be something like the follows?

Absolutely, De Morgan's law! Thanks for catching will fix.

[ywangd]

So why forcing a non-null initial metadata would help?

If metadata on initialRequest is null, then the metadata assigned during creation stays on the doc (since null metadata means don't update).

Suppose:

created.metadata = {"bar": "foo"} initialRequest.metadata = null then it can happen that on line 1805, metadata is {"bar": "foo"} matching the metadata on the doc and resulting in false for noop because we're comparing to initialRequest.metadata when picking a distinct metadata value

Thanks for the explanation. I forgot about there is the initial creation!

[ywangd]

LGTM

[n1v0lg]

@elasticmachine run elasticsearch-ci/part-1