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

improve compliance refresh token handling #85

Merged
merged 3 commits into from Aug 25, 2016

Conversation

Projects
None yet
4 participants
@chris-rock
Collaborator

chris-rock commented Aug 23, 2016

Description

This PR is doing the following:

  • extract inspec gem installation
  • ensures the refresh token is only exchanged once

Issues Resolved

Customers reported that refresh_token handling was not working as expected and lead to authentication errors for some requests.

Check List

@jeremymv2

View changes

libraries/inspec.rb Outdated
action :install do
converge_by 'install/update inspec' do
chef_gem 'inspec' do
version version if version != 'latest'

This comment has been minimized.

@jeremymv2

jeremymv2 Aug 25, 2016

Contributor

@chris-rock I get an WARN on line 15 version

Recipe: audit::_inspec

 * inspec[inspec] action install[2016-08-25T13:28:53+00:00] WARN: /var/chef/cache/cookbooks/audit/libraries/inspec.rb:15:in `block (3 levels) in <class:ChefInspec>': property version is declared in both chef_gem[inspec] and inspec[inspec] action :install. Use new_resource.version instead. At /var/chef/cache/cookbooks/audit/libraries/inspec.rb:15:in `block (3 levels) in <class:ChefInspec>'
[2016-08-25T13:28:53+00:00] WARN: /var/chef/cache/cookbooks/audit/libraries/inspec.rb:15:in `block (3 levels) in <class:ChefInspec>': property version is declared in both chef_gem[inspec] and inspec[inspec] action :install. Use new_resource.version instead. At /var/chef/cache/cookbooks/audit/libraries/inspec.rb:15:in `block (3 levels) in <class:ChefInspec>'

This doc seems to support the WARN message above:
https://docs.chef.io/custom_resources.html#new-resource-property

When I test changing line 15 to the following I no longer get the WARNing, and the gem is installed correctly at the expected version.
version new_resource.version if new_resource.version != 'latest'

This comment has been minimized.

@chris-rock

chris-rock Aug 25, 2016

Collaborator

👍

@jeremymv2

View changes

libraries/profile.rb Outdated
path = tar_path
access_token = token
puts "ACCESSTOKEN #{access_token}"

This comment has been minimized.

@jeremymv2

jeremymv2 Aug 25, 2016

Contributor

Being a little nit-picky; line 58 looks to be a left-over debugging artifact. As-is it is appended to the end of current line in console/logs during converges and doesn't have any context and is empty when ['audit']['collector'] == chef-server. Perhaps, if this is valuable/desirable info to have in log files (which I'm not totally convinced it is), then a line like: Chef::Log.info "Using Access Token: #{access_token}" if access_token would be more delightful for the users?

@jeremymv2

View changes

libraries/report.rb Outdated
converge_by "report compliance profiles' results" do
reports, ownermap = compound_report(profiles)
puts "ACCESSTOKEN #{token}"

This comment has been minimized.

@jeremymv2

jeremymv2 Aug 25, 2016

Contributor

same comment as before about ACCESSTOKEN output.

This comment has been minimized.

@chris-rock

chris-rock Aug 25, 2016

Collaborator

👍 Thanks for highlighting this @jeremymv2

@jeremymv2

This comment has been minimized.

Contributor

jeremymv2 commented Aug 25, 2016

Added a few comments above. Otherwise this PR looks awesome and will be super helpful. Tested the PR itself and works like charm! Very nice @chris-rock!

👍

@chris-rock chris-rock merged commit 5dd214f into master Aug 25, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@chris-rock chris-rock deleted the chris-rock/compliance_refresh_token branch Aug 25, 2016

@iennae iennae removed the in progress label Aug 25, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment