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

Include handler in exception handlers as well as report handlers #290

Merged
merged 2 commits into from
Dec 1, 2017

Conversation

drrk
Copy link
Contributor

@drrk drrk commented Oct 16, 2017

Description

Run audits on CCR fail.

Issues Resolved

Fixes #289

Check List

Signed-off-by: Kimball Johnson <kj@chef.io>
Copy link
Contributor

@adamleff adamleff left a comment

Choose a reason for hiding this comment

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

This seems like a reasonable change to me. It could probably use a bit of manual testing to confirm expected behavior.

Only drawback is if the CCR failed during the actual audit cookbook itself (i.e. installing the inspec gem), the errors generated by the handler failure may noisily shadow the actual error that caused the handler to fail to work properly in the first place (i.e. the gem failure will be many many many lines north of the handler failure that would be generated if inspec wasn't present on the system). However, it would be no less broken (and no additional data lost) as compared to the current state of affairs.

@chris-rock chris-rock requested a review from a team December 1, 2017 13:56
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.

While I think this should really be fixed in Chef client by the introduction of a new audit_handlers concept, this is a good temporary solution.

@chris-rock chris-rock merged commit 0039bfe into chef-boneyard:master Dec 1, 2017
@tas50 tas50 added Status: Incomplete A pull request that is not ready to be merged as noted by the author. and removed in progress labels Jan 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Incomplete A pull request that is not ready to be merged as noted by the author.
Development

Successfully merging this pull request may close these issues.

5 participants