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 max_resource_units to enterprise license #50735

Merged
merged 12 commits into from
Jan 13, 2020

Conversation

tvernum
Copy link
Contributor

@tvernum tvernum commented Jan 8, 2020

The enterprise license type must has "max_resource_units" and may not
have "max_nodes".

This change adds support for this new field, validation that the field
is present if-and-only-if the license is enterprise and bumps the
license version number to reflect the new field.

The enterprise license type must has "max_resource_units" and may not
have "max_nodes".

This change adds support for this new field, validation that the field
is present if-and-only-if the license is enterprise and bumps the
license version number to reflect the new field.
@tvernum tvernum added >enhancement :Security/License License functionality for commercial features v8.0.0 v7.6.0 labels Jan 8, 2020
@tvernum tvernum requested a review from jkakavas January 8, 2020 11:25
@elasticmachine
Copy link
Collaborator

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

Copy link
Member

@jkakavas jkakavas left a comment

Choose a reason for hiding this comment

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

LGTM

private static void validateLimits(String type, int maxNodes, int maxResourceUnits) {
if (LicenseType.isEnterprise(type)) {
if (maxResourceUnits == -1) {
throw new IllegalStateException("maxResourceUnits must be set for enterprise licenses");
Copy link
Member

Choose a reason for hiding this comment

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

nit:

Suggested change
throw new IllegalStateException("maxResourceUnits must be set for enterprise licenses");
throw new IllegalStateException("maxResourceUnits must be set for license type [" + type + "]");

if (maxResourceUnits == -1) {
throw new IllegalStateException("maxResourceUnits must be set for enterprise licenses");
} else if (maxNodes != -1) {
throw new IllegalStateException("maxNodes may not be set for enterprise licenses");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
throw new IllegalStateException("maxNodes may not be set for enterprise licenses");
throw new IllegalStateException("maxNodes may not be set license type [" + type + "]");

if (maxNodes == -1) {
throw new IllegalStateException("maxNodes has to be set");
} else if (maxResourceUnits != -1) {
throw new IllegalStateException("maxResourceUnits may only be set for enterprise licenses");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
throw new IllegalStateException("maxResourceUnits may only be set for enterprise licenses");
throw new IllegalStateException("maxResourceUnits may only be set license type [" + type + "]");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This suggestion is backwards. I could change it to say that they "may not be set for license type [" + type + "]", but it seemed more helpful to explain when they are permitted than to just reject them for the current type.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, makes sense, sorry for the half-baked suggestion

@tvernum
Copy link
Contributor Author

tvernum commented Jan 13, 2020

@jkakavas I pushed fbfe58c to update the error messages. Do the new messages seem OK to you?

@tvernum tvernum requested a review from jkakavas January 13, 2020 06:15
Copy link
Member

@jkakavas jkakavas left a comment

Choose a reason for hiding this comment

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

LGTM

@tvernum tvernum merged commit 8727f27 into elastic:master Jan 13, 2020
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Jan 13, 2020
The enterprise license type must have "max_resource_units" and may not
have "max_nodes".

This change adds support for this new field, validation that the field
is present if-and-only-if the license is enterprise and bumps the
license version number to reflect the new field.

Includes a BWC layer to return "max_nodes: ${max_resource_units}" in
the GET license API.

Backport of: elastic#50735
tvernum added a commit that referenced this pull request Jan 14, 2020
The enterprise license type must have "max_resource_units" and may not
have "max_nodes".

This change adds support for this new field, validation that the field
is present if-and-only-if the license is enterprise and bumps the
license version number to reflect the new field.

Includes a BWC layer to return "max_nodes: ${max_resource_units}" in
the GET license API.

Backport of: #50735
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
The enterprise license type must has "max_resource_units" and may not
have "max_nodes".

This change adds support for this new field, validation that the field
is present if-and-only-if the license is enterprise and bumps the
license version number to reflect the new field.
@six5532one
Copy link

@tvernum I'd like to test that ECE can install this version of the enterprise license on clusters that support it. Is this version of the stack license supported on any 7.6 snapshot, or is there a specific tagged stack version I need to use for testing?

russcam added a commit to elastic/elasticsearch-net that referenced this pull request Feb 21, 2020
Relates: #4341, elastic/elasticsearch#49223, elastic/elasticsearch#50735

This commit adds support for Enterprise LicenseType,
and adds max_resource_units to GetLicenseResponse.
Mpdreamz pushed a commit to elastic/elasticsearch-net that referenced this pull request Feb 21, 2020
Relates: #4341, elastic/elasticsearch#49223, elastic/elasticsearch#50735

This commit adds support for Enterprise LicenseType,
and adds max_resource_units to GetLicenseResponse.
github-actions bot pushed a commit to elastic/elasticsearch-net that referenced this pull request Feb 21, 2020
Relates: #4341, elastic/elasticsearch#49223, elastic/elasticsearch#50735

This commit adds support for Enterprise LicenseType,
and adds max_resource_units to GetLicenseResponse.
Mpdreamz pushed a commit to elastic/elasticsearch-net that referenced this pull request Feb 21, 2020
Relates: #4341, elastic/elasticsearch#49223, elastic/elasticsearch#50735

This commit adds support for Enterprise LicenseType,
and adds max_resource_units to GetLicenseResponse.

Co-authored-by: Russ Cam <[email protected]>
russcam added a commit to elastic/elasticsearch-net that referenced this pull request Feb 23, 2020
Relates: #4341, elastic/elasticsearch#49223, elastic/elasticsearch#50735

This commit adds support for Enterprise LicenseType,
and adds max_resource_units to GetLicenseResponse.

(cherry picked from commit c737df6)
pgomulka added a commit that referenced this pull request Jul 27, 2021
…es (#75479)

in #50067 for _license the accept_enterprise = false was no longer supported. This
commit allows it under rest compatibility and is showing enterprise licenses as platinum
The same change had to be applied to _xpack endpoint #58217

in #50735 max_resource_units was introduced to be more accurate. For v7 requests, which do not know about enterprise license we will return max_node which will be set from max_resource_units (it assumes that one resource unit is 32GB - which corresponds to 1 node)

relates #51816
ywangd pushed a commit to ywangd/elasticsearch that referenced this pull request Jul 30, 2021
…es (elastic#75479)

in elastic#50067 for _license the accept_enterprise = false was no longer supported. This
commit allows it under rest compatibility and is showing enterprise licenses as platinum
The same change had to be applied to _xpack endpoint elastic#58217

in elastic#50735 max_resource_units was introduced to be more accurate. For v7 requests, which do not know about enterprise license we will return max_node which will be set from max_resource_units (it assumes that one resource unit is 32GB - which corresponds to 1 node)

relates elastic#51816
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Security/License License functionality for commercial features v7.6.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants