Skip to content
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.

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

Add v7 restCompat for invalidating API key with the id field #78664

Merged
merged 5 commits into from
Oct 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion x-pack/plugin/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ tasks.named("yamlRestTestV7CompatTransform").configure{ task ->
task.skipTest("rollup/put_job/Test basic put_job", "rollup was an experimental feature, also see #41227")
task.skipTest("rollup/start_job/Test start job twice", "rollup was an experimental feature, also see #41227")
task.skipTest("ml/trained_model_cat_apis/Test cat trained models", "A type field was added to cat.ml_trained_models #73660, this is a backwards compatible change. Still this is a cat api, and we don't support them with rest api compatibility. (the test would be very hard to transform too)")
task.skipTest("api_key/10_basic/Test invalidate api keys with single id", "waiting for https://github.com/elastic/elasticsearch/pull/78664")

task.replaceKeyInDo("license.delete", "xpack-license.delete")
task.replaceKeyInDo("license.get", "xpack-license.get")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
package org.elasticsearch.xpack.security.rest.action.apikey;

import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.RestApiVersion;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.ConstructingObjectParser;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;
Expand Down Expand Up @@ -42,11 +44,7 @@ public final class RestInvalidateApiKeyAction extends SecurityBaseRestHandler {
});

static {
PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("realm_name"));
PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("username"));
PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("name"));
PARSER.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), new ParseField("owner"));
PARSER.declareStringArray(ConstructingObjectParser.optionalConstructorArg(), new ParseField("ids"));
initObjectParser(PARSER, false);
}

public RestInvalidateApiKeyAction(Settings settings, XPackLicenseState licenseState) {
Expand All @@ -61,7 +59,7 @@ public List<Route> routes() {
@Override
protected RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClient client) throws IOException {
try (XContentParser parser = request.contentParser()) {
final InvalidateApiKeyRequest invalidateApiKeyRequest = PARSER.parse(parser, null);
final InvalidateApiKeyRequest invalidateApiKeyRequest = getObjectParser(request).parse(parser, null);
return channel -> client.execute(InvalidateApiKeyAction.INSTANCE, invalidateApiKeyRequest,
new RestBuilderListener<InvalidateApiKeyResponse>(channel) {
@Override
Expand All @@ -78,4 +76,43 @@ public RestResponse buildResponse(InvalidateApiKeyResponse invalidateResp,
public String getName() {
return "xpack_security_invalidate_api_key";
}

private ConstructingObjectParser<InvalidateApiKeyRequest, Void> getObjectParser(RestRequest request) {
if (request.getRestApiVersion() == RestApiVersion.V_7) {
final ConstructingObjectParser<InvalidateApiKeyRequest, Void> objectParser = new ConstructingObjectParser<>(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason this isn't a static final field like the non-compat parser?
I'm not particularly fussed either way, it just surprised me.

Copy link
Member Author

Choose a reason for hiding this comment

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

My thinking is to minimize the overhead of restCompat to normal v8 usage (at the same time accept a bit more overhead for restCompat). So this v7 parser does not have to exist if client never uses v7 rest compat. It's probably a marginal saving. But recent focus on performance issue led me to this.

"invalidate_api_key_v7", a -> {
final String id = (String) a[5];
@SuppressWarnings("unchecked")
final List<String> ids = (List<String>) a[4];
if (id != null && ids != null) {
throw new IllegalArgumentException("Must use either [id] or [ids], not both at the same time");
}
final String[] idsArray;
if (Strings.hasText(id)) {
idsArray = new String[] { id };
} else if (ids != null) {
idsArray = ids.toArray(String[]::new);
} else {
idsArray = null;
}
return new InvalidateApiKeyRequest((String) a[0], (String) a[1], (String) a[2],
(a[3] == null) ? false : (Boolean) a[3], idsArray);
});
initObjectParser(objectParser, true);
return objectParser;
} else {
return PARSER;
}
}

private static void initObjectParser(ConstructingObjectParser<InvalidateApiKeyRequest, Void> objectParser, boolean restCompatMode) {
objectParser.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("realm_name"));
objectParser.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("username"));
objectParser.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("name"));
objectParser.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), new ParseField("owner"));
objectParser.declareStringArray(ConstructingObjectParser.optionalConstructorArg(), new ParseField("ids"));
if (restCompatMode) {
objectParser.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("id").withAllDeprecated("ids"));
}
}
}