Require acknowledgement to start_trial license #30135

Tim-Brooks · 2018-04-25T14:55:47Z

This is related to #30134. It modifies the start_trial action to require
an acknowledgement parameter in the rest request to actually start the
trial license. There are backwards compatibility issues as prior ES
versions did not support this parameter. To handle this, it is assumed
that a request coming from a node prior to 6.3 is acknowledged. And
attempts to write a non-acknowledged request to a prior to 6.3 node will
throw an exception.

Additionally this PR adds messages about the trial license the user is
generating.

This is related to elastic#30134. It modifies the start_trial action to require an acknowledgement parameter in the rest request to actually start the trial license. There are backwards compatibility issues as prior ES versions did not support this parameter. To handle this, it is assumed that a request coming from a node prior to 6.3 is acknowledged. And attempts to write a non-acknowledged request to a prior to 6.3 node will throw an exception. Additionally this PR adds messages about the trial license the user is generating.

Tim-Brooks · 2018-04-25T14:56:08Z

Here is the message for a trial that needs to be acknowledge:

{
"acknowledged": false,
"trial_was_started": false,
"error_message": "Operation failed: Needs acknowledgement.",
"acknowledge": {
"message": "This API initiates a free 30-day trial for all platinum features. By starting this trial, you agree that it is subject to the terms and conditions at https://www.elastic.co/legal/trial_license/. To begin your free trial, call /start_trial again and specify the "acknowledge=true" parameter.",
"security": [
"With a trial license, X-Pack security features are available, but are not enabled by default."
]
}
}
Trial that is acknowledged:

{
"acknowledged": true,
"trial_was_started": true,
"type": "trial"
}

elasticmachine · 2018-04-25T17:10:59Z

Pinging @elastic/es-core-infra

jaymode · 2018-04-25T17:35:00Z

x-pack/docs/build.gradle.orig

@@ -0,0 +1,687 @@
+import org.elasticsearch.gradle.test.NodeInfo


remove this file

jaymode · 2018-04-25T17:37:47Z

x-pack/docs/en/rest-api/license/start-trial.asciidoc

@@ -40,7 +40,7 @@ The following example checks whether you are eligible to start a trial:

 [source,js]
 ------------------------------------------------------------
-POST _xpack/license/start_trial
+GET _xpack/license/start_trial


I think we should make start_trial start with an underscore like other APIs that perform an action (_search, _authenticate, etc), so _start_trial and _start_basic

That was discussed. Also I imagine that this "bug fix" PR to add necessary ack messages to a response would not be the place to modify a route that already exists in prior versions of ES x-pack.

jaymode · 2018-04-25T17:38:26Z

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

@@ -0,0 +1,67 @@
+/*


remove this file

jaymode · 2018-04-25T17:38:56Z

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

@@ -0,0 +1,55 @@
+/*


remove this file

jaymode · 2018-04-25T17:39:08Z

...k/plugin/core/src/main/java/org/elasticsearch/license/PostStartTrialRequestBuilder.java.orig

@@ -0,0 +1,17 @@
+/*


remove this file

jaymode · 2018-04-25T17:39:38Z

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

@@ -0,0 +1,48 @@
+/*


remove this file

jaymode · 2018-04-25T17:39:55Z

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

@@ -0,0 +1,59 @@
+/*


remove this file

jaymode · 2018-04-25T17:40:26Z

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

@@ -0,0 +1,79 @@
+/*


remove this file

jaymode · 2018-04-25T17:40:40Z

x-pack/plugin/core/src/test/java/org/elasticsearch/license/StartTrialLicenseTests.java.orig

@@ -0,0 +1,113 @@
+/*


remove this file

jaymode · 2018-04-25T17:41:34Z

x-pack/plugin/src/test/resources/rest-api-spec/test/license/20_put_license.yml

@@ -133,7 +133,8 @@ teardown:

  - do:
      catch: forbidden
-      xpack.license.post_start_trial: {}
+      xpack.license.post_start_trial:


should it still be post_start_trial and not changed to get_start_trial? Or just start_trial

No it should be post_start_trial. We're testing that this call fails.

jaymode

Left one comment, but otherwise LGTM

jaymode · 2018-04-25T18:09:18Z

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

@@ -57,10 +76,57 @@ public Status getStatus() {
    @Override
    public void readFrom(StreamInput in) throws IOException {
        status = in.readEnum(Status.class);
+        if (in.getVersion().onOrAfter(Version.V_6_3_0)) {


the readFrom version is different than the writeTo. Doesn't seem correct to me

Tim-Brooks · 2018-04-26T02:53:40Z

run sample packaging tests

Tim-Brooks · 2018-04-26T20:31:20Z

run gradle build tests

This is related to elastic#30134. It modifies the start_trial action to require an acknowledgement parameter in the rest request to actually start the trial license. There are backwards compatibility issues as prior ES versions did not support this parameter. To handle this, it is assumed that a request coming from a node prior to 6.3 is acknowledged. And attempts to write a non-acknowledged request to a prior to 6.3 node will throw an exception. Additionally this PR adds messages about the trial license the user is generating.

* master: (24 commits) Watcher: Ensure mail message ids are unique per watch action (#30112) REST: Remove GET support for clear cache indices (#29525) SQL: Correct error message (#30138) Require acknowledgement to start_trial license (#30135) Fix a bug in FieldCapabilitiesRequest#equals and hashCode. (#30181) SQL: Add BinaryMathProcessor to named writeables list (#30127) Tests: Use buildDir as base for generated-resources (#30191) Fix SliceBuilderTests#testRandom failures Build: Fix deb version to use tilde with prerelease versions (#29000) Fix edge cases in CompositeKeyExtractorTests (#30175) Document time unit limitations for date histograms (#30177) Add support for field capabilities to the high-level REST client. (#29664) Remove licenses missed by the migration (#30128) [DOCS] Updates docker installation package details (#30110) Fix TermsSetQueryBuilder.doEquals() method (#29629) [Monitoring] Remove unhelpful Monitoring tests (#30144) [Test] Fix RenameProcessorTests.testRenameExistingFieldNullValue() (#29655) add copyright/scope configuration for intellij to Contributing Guide (#29688) [test] include oss tar in packaging tests (#30155) TEST: Update settings should go through cluster state (#29682) ...

This commit is a follow up to elastic#30135. It updates the stream compatibility versions in the start_trial requests and responses to reflect that fact that this work has been backported to 6.3.

* master: (7173 commits) Bump changelog version to 6.4 (elastic#30217) [DOCS] Adds native realm security settings (elastic#30186) Test: Switch painless test to 1 shard CCS: Drop http address from remote cluster info (elastic#29568) Reindex: Fold "from old" tests into reindex module (elastic#30142) Convert FieldCapabilitiesResponse to a ToXContentObject. (elastic#30182) [DOCS] Added 'on a single shard' to description of max_thread_count. Closes 28518 (elastic#29686) [TEST] Redirect links to new locations (elastic#30179) Move repository-s3 fixture tests to QA test project (elastic#29372) Fail snapshot operations early on repository corruption (elastic#30140) Docs: Document `failures` on reindex and friends Build global ordinals terms bucket from matching ordinals (elastic#30166) Watcher: Ensure mail message ids are unique per watch action (elastic#30112) REST: Remove GET support for clear cache indices (elastic#29525) SQL: Correct error message (elastic#30138) Require acknowledgement to start_trial license (elastic#30135) Fix a bug in FieldCapabilitiesRequest#equals and hashCode. (elastic#30181) SQL: Add BinaryMathProcessor to named writeables list (elastic#30127) Tests: Use buildDir as base for generated-resources (elastic#30191) Fix SliceBuilderTests#testRandom failures ...

This commit is a follow up to #30135. It updates the stream compatibility versions in the start_trial requests and responses to reflect that fact that this work has been backported to 6.3.

Tim-Brooks added >bug review v7.0.0 v6.3.0 :Security/License License functionality for commercial features labels Apr 25, 2018

Tim-Brooks requested review from skearns64, jaymode, jasontedor and lcawl April 25, 2018 14:55

hub-cap added :Security/License License functionality for commercial features and removed :Security/License License functionality for commercial features labels Apr 25, 2018

jaymode requested changes Apr 25, 2018

View reviewed changes

Delete files

a5d0c81

jaymode approved these changes Apr 25, 2018

View reviewed changes

Fix version

d92138b

bleskes added v6.3.1 v6.4.0 and removed v6.3.0 v6.3.1 labels Apr 26, 2018

Merge remote-tracking branch 'upstream/master' into start_trial_messages

784718b

jasontedor added blocker and removed >bug labels Apr 26, 2018

Tim-Brooks added v6.3.0 and removed v6.4.0 labels Apr 26, 2018

Tim-Brooks merged commit 592481e into elastic:master Apr 27, 2018

Tim-Brooks mentioned this pull request Apr 27, 2018

Update versions for start_trial after backport #30218

Merged

jpountz added the >enhancement label Jun 13, 2018

russcam mentioned this pull request Jul 9, 2018

Add acknowledge request parameter to Start Trial API elastic/elasticsearch-net#3324

Closed

Tim-Brooks deleted the start_trial_messages branch November 14, 2018 14:55

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Require acknowledgement to start_trial license #30135

Require acknowledgement to start_trial license #30135

Tim-Brooks commented Apr 25, 2018 •

edited

Loading

Tim-Brooks commented Apr 25, 2018

elasticmachine commented Apr 25, 2018

jaymode Apr 25, 2018

jaymode Apr 25, 2018

Tim-Brooks Apr 25, 2018 •

edited by jaymode

Loading

jaymode Apr 25, 2018

jaymode Apr 25, 2018

jaymode Apr 25, 2018

jaymode Apr 25, 2018

jaymode Apr 25, 2018

jaymode Apr 25, 2018

jaymode Apr 25, 2018

jaymode Apr 25, 2018

Tim-Brooks Apr 25, 2018

jaymode left a comment

jaymode Apr 25, 2018

Tim-Brooks commented Apr 26, 2018

Tim-Brooks commented Apr 26, 2018

		@@ -0,0 +1,687 @@
		import org.elasticsearch.gradle.test.NodeInfo

Require acknowledgement to start_trial license #30135

Require acknowledgement to start_trial license #30135

Conversation

Tim-Brooks commented Apr 25, 2018 • edited Loading

Tim-Brooks commented Apr 25, 2018

elasticmachine commented Apr 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tim-Brooks Apr 25, 2018 • edited by jaymode Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaymode left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tim-Brooks commented Apr 26, 2018

Tim-Brooks commented Apr 26, 2018

Tim-Brooks commented Apr 25, 2018 •

edited

Loading

Tim-Brooks Apr 25, 2018 •

edited by jaymode

Loading