Add accept_license property to chef-ingredient. by sersut · Pull Request #101 · chef-cookbooks/chef-ingredient

sersut · 2016-04-19T15:54:36Z

This PR add accept_license property to chef-ingredient so the users can set it to be able to reconfigure compliance, manage, reporting and analytics after the version that supports --accept-license during reconfigure ships.

@mmzyk note that this PR needs the minimum version numbers that support --accept-license CLI option. I have set them to be a minor version bump with the current existing ones. If we would like to set a different version let me know.

/cc: @chef-cookbooks/engineering-services @alexpop

alexpop · 2016-04-19T15:57:06Z

+    # Returns license acceptance CLI parameter when applicable
+    #
+    def license_acceptance_parameter
+      if new_resource.accept_license &&


Thanks for checking the version 👍

sersut · 2016-04-19T16:08:05Z

Hrmmm... Looks like there is one wrinkle for this. Do we pass the --accept-license option when version is set to :latest?

I can see two options:

Do not check for the versions and add --accept-license when property is set. It is up to the user to know if their version requires this option and add it only when necessary. Note that reconfigure is going to fail if we specify this option and if the command does not support it.
Merge this after all products are released and add the CLI option --accept-license when version is set to :latest all the time.

I am leaning more towards option 1 since it is a simpler case. If you see the reconfigure error (which has a clear error message) you add this option. There are complicated scenarios like what if you are on a previous version of chef-ingredient, what if you are using a custom repo and did not populate your local repositories with the version that supports this CLI option, etc...

Let me know what you think guys and I'll change the PR.

scotthain · 2016-04-19T16:16:28Z

Code looks good - I would also lean towards the first (simpler) option, as long as the error message is clear and there's a simple path a user can go to fix it. Looks like travis is unhappy but otherwise 👍

schisamo · 2016-04-19T17:02:32Z

+         (new_resource.product_name == 'compliance' && version_for(new_resource.version) >= version_for('1.1.3')) ||
+         (new_resource.product_name == 'manage' && version_for(new_resource.version) >= version_for('1.10.0')) ||
+         (new_resource.product_name == 'reporting' && version_for(new_resource.version) >= version_for('1.6.0')) ||
+         (new_resource.product_name == 'analytics' && version_for(new_resource.version) >= version_for('1.4.0'))


So IMO we should just lay down the required /var/opt/<PROJECT>/.license.accepted if accept_license is true. This will allow us to avoid all this crazy version checking and is fully backward compatible.

💯 to Seth's comment

Great idea. Will do that.

sersut · 2016-04-19T19:05:52Z

Updated with @schisamo's great comment!

schisamo · 2016-04-19T19:10:59Z

 - `channel`: Channel to install the products from. It can be `:stable` (default), `:current` or `:unstable`.
 - `package_source`: Full path to a location where the package is located. If present, this file is used for installing the package. Default `nil`.
 - `timeout`: The amount of time (in seconds) to wait to fetch the installer before timing out. Default: default timeout of the Chef package resource - `900` seconds.
+- `accept_license`: A boolean value that specifies if license should be accepted if it is asked for during `reconfigure`action. This option is needed only for manage, analytics, reporting and manage products. Default: `false`.


manage, analytics, reporting and manage

Something is strange in that sentence. I believe we are missing a few products also like Compliance.

argh... double manage 😄 will fix soon.

alexpop · 2016-04-20T07:25:37Z

Great work guys. I've tried this against latest compliance build(1.1.4) that already requires --accept-license for reconfigure. The resource failed because of missing directory /var/opt/chef-compliance:

 ================================================================================
 Error executing action `reconfigure` on resource 'chef_ingredient[compliance]'
 ================================================================================

 Chef::Exceptions::EnclosingDirectoryDoesNotExist
 ------------------------------------------------
 file[/var/opt/chef-compliance/.license.accepted] (/tmp/kitchen/cache/cookbooks/chef-ingredient/libraries/chef_ingredient_provider.rb line 94) had an error: Chef::Exceptions::EnclosingDirectoryDoesNotExist: Parent directory /var/opt/chef-compliance does not exist.

Creating the /var/opt/chef-compliance directory before the chef_ingredient[compliance] resource made the converge successfully complete.

sersut · 2016-04-20T17:36:14Z

@alexpop you are awesome. Fixing it immediately.

sersut · 2016-04-20T19:49:23Z

Thanks to @alexpop for verifying this change for compliance.

yzl · 2016-04-20T21:21:41Z

👍

wrightp · 2016-04-21T16:35:58Z

As we start releasing addons and other packages to include the license accept feature I anticipate that a good portion of the integration tests will start to fail.

…ker kitchen file. Proprietary Chef products—such as Chef Compliance, Chef Delivery, Chef Analytics, Reporting, and the Chef Management Console—are governed by the Chef MLSA. [The Chef MLSA must be accepted when installing or reconfiguring the product](https://docs.chef.io/chef_license.html). Chef ingredient added the [accept_license](chef-cookbooks/chef-ingredient#101) property to provide a way to automate this. This fix adds the attribute ['chef-server']['accept_license']. The default value is _false_. Individuals must explicitly change the value to true in their environment to accept the license. Make sure you set the node attribute ['chef-server']['accept_license'] = true to resolve this error.

chef-supermarket added the Signed CLA label Apr 19, 2016

alexpop reviewed Apr 19, 2016
View reviewed changes

schisamo reviewed Apr 19, 2016
View reviewed changes

sersut force-pushed the sersut/accept_license branch from f61fa5f to fc84a25 Compare April 19, 2016 19:05

chef-supermarket added the Signed CLA label Apr 19, 2016

schisamo reviewed Apr 19, 2016
View reviewed changes

Add accept_license property to chef-ingredient.

f5d9fe6

sersut force-pushed the sersut/accept_license branch from fc84a25 to f5d9fe6 Compare April 19, 2016 20:03

chef-supermarket added the Signed CLA label Apr 19, 2016

Create the directory for the .license.accepted file.

e4d04b1

chef-supermarket added the Signed CLA label Apr 20, 2016

sersut merged commit 4ee4129 into master Apr 21, 2016

sersut deleted the sersut/accept_license branch April 21, 2016 16:35

Uh oh!

Conversation

sersut commented Apr 19, 2016

Uh oh!

alexpop Apr 19, 2016

Choose a reason for hiding this comment

Uh oh!

sersut commented Apr 19, 2016

Uh oh!

scotthain commented Apr 19, 2016

Uh oh!

schisamo Apr 19, 2016 • 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!

wrightp Apr 19, 2016 • 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!

sersut Apr 19, 2016

Choose a reason for hiding this comment

Uh oh!

sersut commented Apr 19, 2016

Uh oh!

schisamo Apr 19, 2016

Choose a reason for hiding this comment

Uh oh!

sersut Apr 19, 2016

Choose a reason for hiding this comment

Uh oh!

alexpop commented Apr 20, 2016

Uh oh!

sersut commented Apr 20, 2016

Uh oh!

sersut commented Apr 20, 2016

Uh oh!

yzl commented Apr 20, 2016

Uh oh!

wrightp commented Apr 21, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

schisamo Apr 19, 2016 •

edited

Loading

wrightp Apr 19, 2016 •

edited

Loading