Skip to content
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

Upload profiles to Chef Compliance via Chef resource #122

Merged
merged 6 commits into from
Oct 27, 2016

Conversation

vjeffrey
Copy link

@vjeffrey vjeffrey commented Oct 26, 2016

tada! it works @chris-rock .....i have a bunch of requires in the inspec resource....not sure how to get around that?

using this to test it: https://github.com/vjeffrey/testing-audit

@vjeffrey vjeffrey force-pushed the vj/upload-profiles branch 5 times, most recently from d4cc042 to 5fd0cda Compare October 26, 2016 18:41
@vjeffrey vjeffrey changed the title wip: upload profiles upload profiles Oct 26, 2016
require 'bundles/inspec-compliance/api'
require 'bundles/inspec-compliance/target'
_success, msg, access_token = Compliance::API.get_token_via_refresh_token(server, refresh_token, true)

Copy link
Contributor

@jeremymv2 jeremymv2 Oct 26, 2016

Choose a reason for hiding this comment

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

The access_token (and inspec lib namespaces) will also be required when sending reports directly to Compliance so the all the lib requires should probably be moved in this resource so that it's as DRY as possible.

Copy link
Contributor

@jeremymv2 jeremymv2 Oct 26, 2016

Choose a reason for hiding this comment

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

This was solved in the past since there was a compliance library (libraries/compliance.rb). All libraries get compiled and automatically included in the cookbook resources. That would perhaps be a decent place for all the requires and the compliance API methods.

Copy link
Author

Choose a reason for hiding this comment

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

👍 sounds like a plan!

@vjeffrey vjeffrey mentioned this pull request Oct 26, 2016
4 tasks

# upload profile
inspec p do
profile_name p
Copy link
Contributor

Choose a reason for hiding this comment

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

inspec p do
   profile_name p

This feels a little strange to me since the resource is inspec - to me that would on the surface mean that it controls the installation and configuration of inspec (gem/tools). This might just be pernickety, but mixing in inspec profiles uploads might make more sense in a separate resource perhaps.

Copy link
Author

Choose a reason for hiding this comment

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

ya, that makes sense. 👍

@chris-rock
Copy link
Contributor

@vjeffrey Do you intend to add the upload example to our example directory?

Copy link
Contributor

@chris-rock chris-rock left a comment

Choose a reason for hiding this comment

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

I like the simple wrapper for the cookbook that reuses all the InSpec core functionality. Awesome @vjeffrey Just added a minor questions.

raise "Profile archive file #{path} does not exist." unless ::File.exist?(path)
profile = Inspec::Profile.for_target(path, {})
error_count = 0
lambda { |msg|
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this lamda used somewhere?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would be great to tie this warnings and errors to Chef::Log.error and Chef::Log.warn

Copy link
Author

Choose a reason for hiding this comment

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

i think that lambda was just cruft leftover. i'll remove and test in a bit. oh ya, good point on the errors. i'll go through and update those! :)

@vjeffrey vjeffrey force-pushed the vj/upload-profiles branch 5 times, most recently from 495ad10 to fa964ed Compare October 27, 2016 13:48
Victoria Jeffrey added 5 commits October 27, 2016 09:48
Signed-off-by: Victoria Jeffrey <vjeffrey@chef.io>
Signed-off-by: Victoria Jeffrey <vjeffrey@chef.io>
Signed-off-by: Victoria Jeffrey <vjeffrey@chef.io>
@@ -0,0 +1,107 @@
# Put files/directories that should be ignored in this file when uploading
Copy link
Contributor

Choose a reason for hiding this comment

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

lets remove this file

Copy link
Author

Choose a reason for hiding this comment

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

👍

Signed-off-by: Victoria Jeffrey <vjeffrey@chef.io>
@vjeffrey
Copy link
Author

@chris-rock: updated!

@chris-rock chris-rock changed the title upload profiles Upload profiles to Chef Compliance via Chef resource Oct 27, 2016
@chris-rock chris-rock mentioned this pull request Oct 27, 2016
@chris-rock
Copy link
Contributor

Awesome @vjeffrey Lets cover the unit tests in another PR #128

@chris-rock chris-rock merged commit 3296f21 into master Oct 27, 2016
@chris-rock chris-rock deleted the vj/upload-profiles branch October 27, 2016 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants