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

update to work with inspec 1.0 json format #99

Merged
merged 6 commits into from Sep 26, 2016

Conversation

Projects
None yet
7 participants
@vjeffrey
Collaborator

vjeffrey commented Sep 21, 2016

fixes #98

updates to 1.0 json format

tested with visibility workflow with this change https://delivery.shd.chef.co/e/Chef/#/organizations/Chef_Delivery/projects/delivery/changes/93fe5e4f-107a-4176-aaa6-0290eb9369ba/status/verify

working on those failing tests real quick -ok, fixed.

@binamov binamov added the in progress label Sep 21, 2016

@vjeffrey vjeffrey changed the title from update to work with inspec 1.0 json format to wip:update to work with inspec 1.0 json format Sep 21, 2016

@vjeffrey vjeffrey changed the title from wip:update to work with inspec 1.0 json format to update to work with inspec 1.0 json format Sep 22, 2016

@vjeffrey vjeffrey referenced this pull request Sep 22, 2016

Closed

InSpec in Visibility #1117

@@ -61,8 +55,6 @@ class ComplianceReport < Chef::Resource
else
Chef::Log.warn "#{collector} is not a supported inspec report collector"
end
raise "#{total_failed} audits have failed. Aborting chef-client run." if total_failed > 0 && node['audit']['fail_if_any_audits_failed']

This comment has been minimized.

@srenatus

srenatus Sep 22, 2016

Collaborator

Is this information something we cannot get from the updated format?

This comment has been minimized.

@vjeffrey

vjeffrey Sep 22, 2016

Collaborator

we no longer have a summary in the new format, which means we don't have failed counts, ya :/

This comment has been minimized.

@srenatus

srenatus Sep 22, 2016

Collaborator

Hmm. Since this is the only use, at least the attribute should go away. That said, can't we aggregate in the cookbook code?

This comment has been minimized.

@srenatus

srenatus Sep 23, 2016

Collaborator

this is just a quick hack, but if this functionality should be retained, we could go with something like

total_failed = reports.map do |name, report|
  report['profiles'].map do |profile|
    profile['controls'].map do |control|
      control['results'].map do |result|
        result['status'] != 'passed' ? 1 : 0
      end
    end
  end
end.flatten.reduce(:+)

where reports is the value of :reports here

This comment has been minimized.

@vjeffrey

vjeffrey Sep 26, 2016

Collaborator

thank youuuu @srenatus !!!

@srenatus

This comment has been minimized.

Collaborator

srenatus commented Sep 22, 2016

@vjeffrey: one clarification question re: failing tests (controls), besides that, LGTM

@@ -90,6 +80,7 @@ def count_controls(profiles)
return count unless profiles.is_a?(Array)
profiles.each do |profile|
profile = profile[0]

This comment has been minimized.

@stevendanna

stevendanna Sep 22, 2016

A comment here explaining the format of profiles and profile might be helpful

This comment has been minimized.

@vjeffrey

vjeffrey Sep 23, 2016

Collaborator

yup, that makes sense! i'll go do that right now.

@alexpop

This comment has been minimized.

Collaborator

alexpop commented Sep 22, 2016

Had a quick look at it, but I'll have to drop into the shell as well to test it against automate.

I think this change deserves a new pin here:
https://github.com/chef-cookbooks/audit/blob/v0.14.4/attributes/default.rb#L48

and bump the major version of the cookbook as well.

Are we planning to merge this before InSpec 1.0.0 is released?

@chris-rock

This comment has been minimized.

Collaborator

chris-rock commented Sep 22, 2016

@alexpop we can merge it on master but not release it until inspec 1.0 is available, in the meantime we can pin to the beta releases

@vjeffrey

This comment has been minimized.

Collaborator

vjeffrey commented Sep 26, 2016

@alexpop is there a reason you wanted to pin the inspec version? why not just always grab latest, here and in the viz data generator?

@vjeffrey

This comment has been minimized.

Collaborator

vjeffrey commented Sep 26, 2016

working on implementing the fix @srenatus got for us on report.rb; when controls have no tests it fails out, b/c there's no control['results'], so just trying to work around that :)

@alexpop

This comment has been minimized.

Collaborator

alexpop commented Sep 26, 2016

With commit f50d80a, we are sending the same data structure to Chef Automate. I've updated the Visibility merge request and there are no more javascript changes needed.

@vjeffrey

This comment has been minimized.

Collaborator

vjeffrey commented Sep 26, 2016

nice! thanks alex; i just finished testing it all; works great :)
one more question: why are we version pinning inspec and not just doing latest?

@vjeffrey

This comment has been minimized.

Collaborator

vjeffrey commented Sep 26, 2016

just pushed up the change to keep the 'fail_if_any_audits_fail' functionality

@alexpop

This comment has been minimized.

Collaborator

alexpop commented Sep 26, 2016

@vjeffrey we've been pinning the inspec version in attributes/default.rb for some time now. It allows us to release the audit cookbook with a compatible version of inspec pinned.

Users can still override the attribute to a different version or latest but it might not work.

@srenatus

This comment has been minimized.

Collaborator

srenatus commented Sep 26, 2016

Looks good to me 👍

@vjeffrey

This comment has been minimized.

Collaborator

vjeffrey commented Sep 26, 2016

the shd/visibility change was merged a minute ago by some nice reviewers :) so, as soon as we release inspec 1.0 i'll update the version in here and we can merge away

@vjeffrey

This comment has been minimized.

Collaborator

vjeffrey commented Sep 26, 2016

updated to 1.0.0

@alexpop

This comment has been minimized.

Collaborator

alexpop commented Sep 26, 2016

Many thanks and hugs!

@alexpop alexpop merged commit 1046897 into master Sep 26, 2016

1 check passed

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

@binamov binamov removed the in progress label Sep 26, 2016

@alexpop alexpop deleted the vj/update-to-inspec-1.0 branch Sep 26, 2016

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