Skip to content

Remove old InvalidateApiKeyRequest transport versions #118290

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Mar 24, 2025
Merged

Remove old InvalidateApiKeyRequest transport versions #118290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
911e99c
Remove old InvalidateApiKeyRequest transport versions
thecoop Dec 9, 2024
55c434c
Merge branch 'main' into remove-old-invalidateapikeyrequest-transports
thecoop Dec 10, 2024
911d4dc
Remove unused versions
thecoop Dec 10, 2024
68376d0
Merge branch 'main' into remove-old-invalidateapikeyrequest-transports
thecoop Jan 15, 2025
f31884a
Fix TransportVersions
thecoop Jan 15, 2025
30c3192
Merge branch 'main' into remove-old-invalidateapikeyrequest-transports
thecoop Mar 3, 2025
fd62362
Merge branch 'main' into remove-old-invalidateapikeyrequest-transports
thecoop Mar 5, 2025
ccad660
Merge branch 'main' into remove-old-invalidateapikeyrequest-transports
thecoop Mar 24, 2025
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
32 changes: 4 additions & 28 deletions ...ain/java/org/elasticsearch/xpack/core/security/action/apikey/InvalidateApiKeyRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

package org.elasticsearch.xpack.core.security.action.apikey;

import org.elasticsearch.TransportVersions;
import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.common.Strings;
Expand Down Expand Up @@ -41,19 +40,10 @@ public InvalidateApiKeyRequest(StreamInput in) throws IOException {
super(in);
realmName = textOrNull(in.readOptionalString());
userName = textOrNull(in.readOptionalString());
if (in.getTransportVersion().onOrAfter(TransportVersions.V_7_10_0)) {
ids = in.readOptionalStringArray();
} else {
final String id = in.readOptionalString();
ids = Strings.hasText(id) ? new String[] { id } : null;
}
ids = in.readOptionalStringArray();
validateIds(ids);
name = textOrNull(in.readOptionalString());
if (in.getTransportVersion().onOrAfter(TransportVersions.V_7_4_0)) {
ownedByAuthenticatedUser = in.readOptionalBoolean();
} else {
ownedByAuthenticatedUser = false;
}
ownedByAuthenticatedUser = in.readOptionalBoolean();
}

public InvalidateApiKeyRequest(
Expand Down Expand Up @@ -209,23 +199,9 @@ public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeOptionalString(realmName);
out.writeOptionalString(userName);
if (out.getTransportVersion().onOrAfter(TransportVersions.V_7_10_0)) {
out.writeOptionalStringArray(ids);
} else {
if (ids != null) {
if (ids.length == 1) {
out.writeOptionalString(ids[0]);
} else {
throw new IllegalArgumentException("a request with multi-valued field [ids] cannot be sent to an older version");
}
} else {
out.writeOptionalString(null);
}
}
out.writeOptionalStringArray(ids);
out.writeOptionalString(name);
if (out.getTransportVersion().onOrAfter(TransportVersions.V_7_4_0)) {
out.writeOptionalBoolean(ownedByAuthenticatedUser);
}
out.writeOptionalBoolean(ownedByAuthenticatedUser);
}

@Override
Expand Down
66 changes: 4 additions & 62 deletions ...ava/org/elasticsearch/xpack/core/security/action/apikey/InvalidateApiKeyRequestTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,10 @@
import java.io.IOException;
import java.util.function.Supplier;

import static org.elasticsearch.test.TransportVersionUtils.getPreviousVersion;
import static org.elasticsearch.test.TransportVersionUtils.randomVersionBetween;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;

public class InvalidateApiKeyRequestTests extends ESTestCase {
Expand Down Expand Up @@ -120,14 +118,10 @@ public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeOptionalString(realm);
out.writeOptionalString(user);
if (out.getTransportVersion().onOrAfter(TransportVersions.V_7_10_0)) {
if (Strings.hasText(apiKeyId)) {
out.writeOptionalStringArray(new String[] { apiKeyId });
} else {
out.writeOptionalStringArray(null);
}
if (Strings.hasText(apiKeyId)) {
out.writeOptionalStringArray(new String[] { apiKeyId });
} else {
out.writeOptionalString(apiKeyId);
out.writeOptionalStringArray(null);
}
out.writeOptionalString(apiKeyName);
out.writeOptionalBoolean(ownedByAuthenticatedUser);
Expand Down Expand Up @@ -160,20 +154,13 @@ public void writeTo(StreamOutput out) throws IOException {
ByteArrayOutputStream bos = new ByteArrayOutputStream();
OutputStreamStreamOutput osso = new OutputStreamStreamOutput(bos)
) {
TransportVersion streamVersion = randomVersionBetween(
Copy link
Member Author

Choose a reason for hiding this comment

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

This version doesn't seem to affect the test behaviour?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I can't work out why this was added. I assume it was an artifact from an earlier incomplete implementation that got retained unnecessarily.

random(),
TransportVersions.V_7_4_0,
getPreviousVersion(TransportVersions.V_7_10_0)
);
Dummy d = new Dummy(inputs[caseNo]);
osso.setTransportVersion(streamVersion);
d.writeTo(osso);

ByteArrayInputStream bis = new ByteArrayInputStream(bos.toByteArray());
InputStreamStreamInput issi = new InputStreamStreamInput(bis);
issi.setTransportVersion(streamVersion);

InvalidateApiKeyRequest request = new InvalidateApiKeyRequest(issi);

ActionRequestValidationException ve = request.validate();
assertNotNull(ve.getMessage(), ve);
assertEquals(expectedErrorMessages[caseNo].length, ve.validationErrors().size());
Expand All @@ -186,36 +173,6 @@ public void testSerialization() throws IOException {
final String apiKeyId = randomAlphaOfLength(5);
final boolean ownedByAuthenticatedUser = true;
InvalidateApiKeyRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, ownedByAuthenticatedUser);
{
ByteArrayOutputStream outBuffer = new ByteArrayOutputStream();
OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer);
out.setTransportVersion(randomVersionBetween(random(), TransportVersions.V_7_0_0, TransportVersions.V_7_3_0));
invalidateApiKeyRequest.writeTo(out);

InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray()));
inputStreamStreamInput.setTransportVersion(
randomVersionBetween(random(), TransportVersions.V_7_0_0, TransportVersions.V_7_3_0)
);
InvalidateApiKeyRequest requestFromInputStream = new InvalidateApiKeyRequest(inputStreamStreamInput);

assertThat(requestFromInputStream.getIds(), equalTo(invalidateApiKeyRequest.getIds()));
// old version so the default for `ownedByAuthenticatedUser` is false
assertThat(requestFromInputStream.ownedByAuthenticatedUser(), is(false));
}
{
ByteArrayOutputStream outBuffer = new ByteArrayOutputStream();
OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer);
out.setTransportVersion(randomVersionBetween(random(), TransportVersions.V_7_4_0, TransportVersions.V_7_9_0));
invalidateApiKeyRequest.writeTo(out);

InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray()));
inputStreamStreamInput.setTransportVersion(
randomVersionBetween(random(), TransportVersions.V_7_4_0, TransportVersions.V_7_9_0)
);
InvalidateApiKeyRequest requestFromInputStream = new InvalidateApiKeyRequest(inputStreamStreamInput);

assertThat(requestFromInputStream, equalTo(invalidateApiKeyRequest));
}
{
ByteArrayOutputStream outBuffer = new ByteArrayOutputStream();
OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer);
Expand All @@ -232,21 +189,6 @@ public void testSerialization() throws IOException {
}
}

public void testSerializationWillThrowWhenMultipleIdsAndOldVersionStream() {
final InvalidateApiKeyRequest invalidateApiKeyRequest = new InvalidateApiKeyRequest(
randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)),
randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)),
randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)),
false,
new String[] { randomAlphaOfLength(12), randomAlphaOfLength(12) }
);
ByteArrayOutputStream outBuffer = new ByteArrayOutputStream();
OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer);
out.setTransportVersion(randomVersionBetween(random(), TransportVersions.V_7_4_0, getPreviousVersion(TransportVersions.V_7_10_0)));
final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> invalidateApiKeyRequest.writeTo(out));
assertThat(e.getMessage(), containsString("a request with multi-valued field [ids] cannot be sent to an older version"));
}

private static String randomNullOrEmptyString() {
return randomFrom(new String[] { "", null });
}
Expand Down