Skip to content

Conversation

@n1v0lg
Copy link
Contributor

@n1v0lg n1v0lg commented Nov 19, 2023

This PR fixes the version check we perform to determine if a new trial can be started.

For trial licenses, #100169 decoupled version tracking from the Elasticsearch version. In short, the new logic is to allow a new trial license each time we increment the trial license version counter (with some additional caveats around BWC). This means we should allow a new trial license if the version the previous trial was started on is below the CURRENT version. This condition was previously reversed. This PR fixes the check.

I've also opted to restrict ableToStartNewTrial to not take arbitrary input and instead to always compare to CURRENT. This allows us to avoid the unnecessary complexity of handling a BWC edge case where both trial versions are below the cut-over value (this is impossible in practice).

Fixes: #102286

@n1v0lg n1v0lg added >non-issue :Security/License License functionality for commercial features labels Nov 19, 2023
@n1v0lg n1v0lg self-assigned this Nov 19, 2023
@n1v0lg
Copy link
Contributor Author

n1v0lg commented Nov 20, 2023

@elasticmachine update branch

1 similar comment
@n1v0lg
Copy link
Contributor Author

n1v0lg commented Nov 20, 2023

@elasticmachine update branch

@n1v0lg
Copy link
Contributor Author

n1v0lg commented Nov 21, 2023

@elasticmachine update branch bitte

@n1v0lg
Copy link
Contributor Author

n1v0lg commented Nov 27, 2023

@elasticmachine update branch

@n1v0lg n1v0lg marked this pull request as ready for review November 27, 2023 08:49
@n1v0lg n1v0lg requested a review from gwbrown November 27, 2023 08:49
@elasticsearchmachine elasticsearchmachine added the Team:Security Meta label for security team label Nov 27, 2023
@elasticsearchmachine
Copy link
Collaborator

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

@n1v0lg
Copy link
Contributor Author

n1v0lg commented Nov 28, 2023

@elasticmachine update branch

Copy link
Contributor

@gwbrown gwbrown left a comment

Choose a reason for hiding this comment

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

Thanks for fixing and simplifying this!

@n1v0lg
Copy link
Contributor Author

n1v0lg commented Nov 29, 2023

@elasticmachine update branch

@n1v0lg n1v0lg added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Nov 29, 2023
@elasticsearchmachine elasticsearchmachine merged commit 3175b10 into elastic:main Nov 29, 2023
@n1v0lg n1v0lg deleted the fix-license-version-check branch November 29, 2023 08:51
timgrein pushed a commit to timgrein/elasticsearch that referenced this pull request Nov 30, 2023
This PR fixes the version check we perform to determine if a new trial
can be started. 

For trial licenses, elastic#100169
decoupled version tracking from the Elasticsearch version. In short, the
new logic is to allow a new trial license each time we increment the
trial license version counter (with some additional caveats around BWC).
This means we should allow a new trial license if the version the
previous trial was started on is _below_ the `CURRENT` version. This
condition was previously reversed. This PR fixes the check.

I've also opted to restrict `ableToStartNewTrial` to not take arbitrary
input and instead to always compare to `CURRENT`. This allows us to
avoid the unnecessary complexity of handling a BWC edge case where both
trial versions are below the cut-over value (this is impossible in
practice).  

Fixes: elastic#102286
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >non-issue :Security/License License functionality for commercial features Team:Security Meta label for security team v8.12.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] TrialLicenseVersionTests testNewTrialAllowed failing

4 participants