Skip to content

Enforce license for cross-cluster API key APIs#96307

Merged
ywangd merged 4 commits intoelastic:mainfrom
ywangd:rcs-speicali-api-key-license
May 25, 2023
Merged

Enforce license for cross-cluster API key APIs#96307
ywangd merged 4 commits intoelastic:mainfrom
ywangd:rcs-speicali-api-key-license

Conversation

@ywangd
Copy link
Member

@ywangd ywangd commented May 24, 2023

Enforce the same license level as the advanced remote cluster security feature for these APIs.

ywangd added 3 commits May 23, 2023 21:24
Enforce the same license level as the advanced remote cluster security
feature for these APIs.
@ywangd ywangd added >non-issue :Security/License License functionality for commercial features v8.9.0 labels May 24, 2023
@ywangd ywangd requested a review from n1v0lg May 24, 2023 01:14
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Security Meta label for security team label May 24, 2023
Comment on lines +85 to +95
@Override
protected Exception checkFeatureAvailable(RestRequest request) {
final Exception failedFeature = super.checkFeatureAvailable(request);
if (failedFeature != null) {
return failedFeature;
} else if (ADVANCED_REMOTE_CLUSTER_SECURITY_FEATURE.checkWithoutTracking(licenseState)) {
return null;
} else {
return LicenseUtils.newComplianceException(ADVANCED_REMOTE_CLUSTER_SECURITY_FEATURE.getName());
}
}
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 method is duplicated in RestUpdateCrossClusterApiKeyAction. If we get to add more REST APIs for cross-cluster API keys, we can extract a superclass to have this method in one place. For now I am keeping it this way. Please let me know if you think otherwise.

final Exception failedFeature = super.checkFeatureAvailable(request);
if (failedFeature != null) {
return failedFeature;
} else if (ADVANCED_REMOTE_CLUSTER_SECURITY_FEATURE.checkWithoutTracking(licenseState)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I went back and forth with whether this should check or checkWithoutTracking. At the end, I went with checkWithoutTracking because it is more consistent to track actual usage of cross-cluster access. Otherwise, the stats can be skewed by just playing with these APIs.

.nodes(1)
.apply(commonClusterConfig)
.setting("xpack.license.self_generated.type", "basic")
.setting("xpack.license.self_generated.type", "trial")
Copy link
Member Author

Choose a reason for hiding this comment

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

Need to change the initial license for FC to be trial. Otherwise the whole test suite fails because it cannot create the cross-cluster API key. Fortunately, this change does not really impact the essence of what we are trying to test here.

Copy link
Contributor

@n1v0lg n1v0lg left a comment

Choose a reason for hiding this comment

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

LGTM

@ywangd ywangd merged commit 870ca07 into elastic:main May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>non-issue :Security/License License functionality for commercial features Team:Security Meta label for security team v8.9.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants