Decouple trial license resets from product version #100169

gwbrown · 2023-10-02T23:44:20Z

Prior to this commit, a new trial license was allowed for a cluster for each major version. This was a reasonable strategy when we were releasing major versions on a highly regular cadence, but since those releases have slowed, we may wish to offer new trial licenses that are not tied to a major version release.

This commit introduces the concept of a Trial Licence Era, which is independent from the Elasticsearch version. In order to avoid complications with serialization, the TrialLicenseEra class is serialized in a way that is wire-compatible with the general Version class, and maintains the existing behavior for trials started in released versions. Thorough unit tests are included to verify that TrialLicenseEra and Version are compatible in both streamed and XContent representations.

Prior to this commit, a new trial license was allowed for a cluster for each major version. This was a reasonable strategy when we were releasing major versions on a highly regular cadence, but since those releases have slowed, we may wish to offer new trial licenses that are not tied to a major version release. This commit introduces the concept of a Trial Licence Era, which is independent from the Elasticsearch version. In order to avoid complications with serialization, the TrialLicenseEra class is serialized in a way that is wire-compatible with the general Version class, and maintains the existing behavior for trials started in released versions. Thorough unit tests are included to verify that TrialLicenseEra and Version are compatible in both streamed and XContent representations.

…l-from-version

elasticsearchmachine · 2023-10-02T23:45:26Z

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

jakelandis · 2023-10-03T22:58:35Z

x-pack/plugin/core/src/test/java/org/elasticsearch/license/internal/TrialLicenseEraTests.java

+    // This simulates a node of a version that supports transport eras receiving a message which is still using old-school Versions
+    private TrialLicenseEra versionToEraSerialization(Version version) throws IOException {
+        try (BytesStreamOutput output = new BytesStreamOutput()) {
+            output.setTransportVersion(TransportVersion.current());


Should the test also include Transport version prior to the introduction of TransportVersion ?

I don't think that using a different version there changes anything in this test; being a unit test this can't really-actually check BWC so all it's really doing is feeding the output of writeTo of one into the StreamInput constructor of the other. If I'm wrong and there's some logic we should make sure is tested here please point me to it; this is an unorthodox thing I'm doing and I want to make sure all the bases are covered.

jakelandis · 2023-10-03T23:01:04Z

x-pack/plugin/core/src/main/java/org/elasticsearch/license/internal/TrialLicenseEra.java

+ * that's already used a trial and let it expire. This class controls when we do that. The serialization of this class is designed to
+ * maintain compatibility with old-school Elasticsearch versions (specifically the {@link org.elasticsearch.Version} class).
+ */
+public class TrialLicenseEra implements ToXContentFragment, Writeable {


nit: (feel free to ignore) I would prefer TrialLicenseVersion over TrialLicenseEra custom concept scoped versions will become (if not already) common place and I think Era will be the outlier w.r.t. naming.

Hah, I used Era instead of Version to try to avoid arguments about introducing yet another Version class, but maybe those have already been had. I'm very open to calling it Version.

+1 to using the Version suffix, it's all the rage

jakelandis

LGTM

…l-from-version

rjernst

I have a couple thoughts on simplifying the serialized forms.

rjernst · 2023-10-03T23:24:12Z

x-pack/plugin/core/src/test/java/org/elasticsearch/license/internal/TrialLicenseEraTests.java

+
+    public void testVersionCanParseAllEras() {
+        for (int i = 2; i <= TrialLicenseEra.CURRENT.asInt(); i++) {
+            Version.fromString(new TrialLicenseEra(i).asVersionString());


Assume that Version will eventually go away, so I'm not sure what this test is adding anything useful. While we need to support bwc, we should not be beholden to maintain a semver-like string going forward.

I can remove this test, I found it necessary to gain enough confidence in this solution to be comfortable PR'ing it but it might not be valuable going forward. We might need to maintain the string format, if for some reason we need old versions of ES to be able to read the XContent emitted by newer ones (see the other thread on that point).

rjernst · 2023-10-09T22:03:50Z

x-pack/plugin/core/src/main/java/org/elasticsearch/license/internal/TrialLicenseVersion.java

+    }
+
+    public TrialLicenseVersion(StreamInput in) throws IOException {
+        this.era = (in.readVInt() - 99) / 1000000; // Copied from the constructor of Version


In other new *Version forks from Version we have used the exact same serialization but diverging going forward (a bigger integer). Rather than try to reduce this down to the major version number from previous Version instances, can we pin the latest version id as the "current", and not do any manipulation to the id when read?

Thanks, that is a better way of doing it - frankly this is the first thing that occurred to me and upon reflection that's obviously a better way of doing it. I'll make the updates.

rjernst · 2023-10-09T22:06:08Z

x-pack/plugin/core/src/main/java/org/elasticsearch/license/internal/TrialLicenseVersion.java

+
+    // pkg-private for testing
+    String asVersionString() {
+        return this + ".0.0";


Can we use this bwc format only in bwc cases? Does the license maintain the version bit as a string or version id?

A string, unfortunately:

elasticsearch/x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicensesMetadata.java

Line 155 in 878127c

builder.field(Fields.TRIAL_LICENSE, trialVersion.toString());

(⬆️ code is from main, to be clear).

Note this is LicensesMetadata, rather than in the license itself, so we only store one of these for the whole cluster but it's still in the persistent cluster state. We can probably switch to emitting an integer, are there are any cases where we'd need an old version of ES to be able to parse XContent emitted by a newer version? The main case I'm aware of where we need to read a LicensesMetadata XContent is restoring a snapshot, and you can't restore a snapshot taken by a newer version than you're currently on AFAIK.

As long as we only upgrade the license metadata format after the min version of the cluster has stabilized in the upgrade, we should be ok.

…l-from-version

- Fix a few more leftover `era`s - Remove serialization tests linked to `Version` - Use integer as-is instead of extracting major version - Disallow starting a trial license in a mixed-version cluster

rjernst

LGTM. Only remaining nit is there are still quite a few uses of the "era" terminology in method and variable names.

rjernst · 2023-10-31T22:08:49Z

x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicensesMetadata.java


-    Version getMostRecentTrialVersion() {
-        return trialVersion;
+    TrialLicenseVersion getMostRecentTrialEra() {


leftover "era" terminology?

rjernst · 2023-10-31T22:09:01Z

x-pack/plugin/core/src/main/java/org/elasticsearch/license/ClusterStateLicenseService.java

                    Metadata currentMetadata = currentState.metadata();
                    LicensesMetadata licensesMetadata = currentMetadata.custom(LicensesMetadata.TYPE);
-                    Version trialVersion = null;
+                    TrialLicenseVersion trialEra = null;


leftover "era" terminology?

rjernst · 2023-10-31T22:09:23Z

x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartBasicClusterTask.java

            }
-            Version trialVersion = currentLicensesMetadata != null ? currentLicensesMetadata.getMostRecentTrialVersion() : null;
-            updatedLicensesMetadata = new LicensesMetadata(selfGeneratedLicense, trialVersion);
+            TrialLicenseVersion trialEra = currentLicensesMetadata != null ? currentLicensesMetadata.getMostRecentTrialEra() : null;


leftover "era"

rjernst · 2023-10-31T22:10:32Z

x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartupSelfGeneratedLicenseTask.java

        License selfGeneratedLicense = SelfGeneratedLicense.create(specBuilder, currentState.nodes());
-        Version trialVersion = currentLicenseMetadata.getMostRecentTrialVersion();
-        LicensesMetadata newLicenseMetadata = new LicensesMetadata(selfGeneratedLicense, trialVersion);
+        TrialLicenseVersion trialEra = currentLicenseMetadata.getMostRecentTrialEra();


leftover "era"

…l-from-version

gwbrown · 2023-11-02T21:55:44Z

Failure is #97260

gwbrown · 2023-11-02T21:55:52Z

@elasticmachine update branch

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

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

This commit is a follow-up to the changes converting the trial license refresh logic to use `TrialLicenseVersion` (#100169), converting the remaining uses of `Version`, which are used to ensure wire compatibility, with `TransportVersion`. Incidentally, this commit also removes code for compatibility with 7.6.0 and earlier, as those versions can no longer be in a cluster with current-version nodes.

You are allowed to initiate a trial only if your cluster has not already activated a trial for the current major product version. For example, if you have already activated a trial for v6.0, you cannot start a new trial until v7.0. `LicensesMetadata` is a ClusterState custom that contains the license information for a cluster. If the cluster has exercised a trial there is a trial version field populated. During a major upgrade, we want to allow users to run a new trial to test the features on the new major. This PR updates the trial version in the license to allow to start a new trial on 9.0. When 9.0 is released we'll have no clusters that has already exercised the trial so this PR also removes some of the BWC handling that was added when [trial license version was decoupled from stack version](#100169).

gwbrown added 4 commits September 28, 2023 17:16

Clean up some missed version -> era

880e7d6

Correct tests & add clarifying comments

3f54f9b

Merge remote-tracking branch 'origin/main' into license/decouple-tria…

2191282

…l-from-version

gwbrown added >refactoring :Security/License License functionality for commercial features v8.11.0 labels Oct 2, 2023

gwbrown requested review from jakelandis and rjernst October 2, 2023 23:44

gwbrown mentioned this pull request Oct 2, 2023

Decouple trial license resets from product version #100040

Closed

gwbrown marked this pull request as ready for review October 2, 2023 23:45

elasticsearchmachine added the Team:Security Meta label for security team label Oct 2, 2023

gwbrown changed the title ~~License/decouple trial from version~~ Decouple trial license resets from product version Oct 2, 2023

jakelandis reviewed Oct 3, 2023

View reviewed changes

jakelandis approved these changes Oct 3, 2023

View reviewed changes

mattc58 added v8.12.0 and removed v8.11.0 labels Oct 4, 2023

gwbrown added 2 commits October 4, 2023 16:25

Era -> Version

6b9ad0e

Merge remote-tracking branch 'origin/main' into license/decouple-tria…

7704e3a

…l-from-version

rjernst reviewed Oct 9, 2023

View reviewed changes

gwbrown added 2 commits October 27, 2023 12:08

Merge remote-tracking branch 'origin/main' into license/decouple-tria…

41ebb01

…l-from-version

Address review feedback, specifically:

41989f6

- Fix a few more leftover `era`s - Remove serialization tests linked to `Version` - Use integer as-is instead of extracting major version - Disallow starting a trial license in a mixed-version cluster

gwbrown requested a review from rjernst October 27, 2023 22:23

rjernst approved these changes Oct 31, 2023

View reviewed changes

gwbrown added 3 commits November 1, 2023 16:29

Yet more era->version

08b4d25

Merge remote-tracking branch 'origin/main' into license/decouple-tria…

2f248c2

…l-from-version

Remove another unnecessary test & another version->era

f1fd28a

Merge branch 'main' into license/decouple-trial-from-version

1487461

gwbrown added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Nov 2, 2023

elasticsearchmachine merged commit 900f109 into elastic:main Nov 2, 2023

gwbrown deleted the license/decouple-trial-from-version branch November 2, 2023 22:55

gwbrown mentioned this pull request Nov 3, 2023

Remove remaining Version usages in licensing #101800

Merged

This was referenced Nov 16, 2023

[CI] TrialLicenseVersionTests testNewTrialAllowed failing #102286

Closed

Fix trial license version check #102362

Merged

gwbrown mentioned this pull request Dec 6, 2023

[CI] LicensesMetadataSerializationTests testLicenseTombstoneWithUsedTrialFromXContext failing #103093

Closed

jfreden mentioned this pull request Jan 15, 2025

Bump TrialLicenseVersion to allow starting new trial on 9.0 #120198

Merged

Decouple trial license resets from product version #100169

Decouple trial license resets from product version #100169

Uh oh!

Conversation

gwbrown commented Oct 2, 2023

Uh oh!

elasticsearchmachine commented Oct 2, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakelandis Oct 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakelandis left a comment

Choose a reason for hiding this comment

Uh oh!

rjernst left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gwbrown Oct 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rjernst left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gwbrown commented Nov 2, 2023

Uh oh!

gwbrown commented Nov 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jakelandis Oct 3, 2023 •

edited

Loading

gwbrown Oct 9, 2023 •

edited

Loading