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.

Sign up for GitHub

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

Jump to bottom

Fixes #21763: Add an audit command generic method #1369

Merged
merged 1 commit into from Sep 16, 2022
Merged

Fixes #21763: Add an audit command generic method #1369

merged 1 commit into from Sep 16, 2022

Conversation

peckpeck
Copy link
Member

@peckpeck peckpeck requested a review from Fdall September 14, 2022 14:52
Fdall
Fdall reviewed Sep 14, 2022
View reviewed changes
tree/30_generic_methods/audit_from_command.cf Outdated

# @name Audit from command
# @description Execute an audit only command reports depending on exit code
# @documentation Execute an audit only command reports depending on
Copy link
Contributor

@Fdall Fdall Sep 14, 2022 •
edited

Choose a reason for hiding this comment

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

Suggested change
# @documentation Execute an audit only command reports depending on
# @documentation Execute an audit only command and reports depending on

Fdall
Fdall reviewed Sep 14, 2022
View reviewed changes
tree/30_generic_methods/audit_from_command.cf
commands:
pass2.!pass3::
"${command}"
contain => in_shell,
Copy link
Contributor

@Fdall Fdall Sep 14, 2022 •
edited

Choose a reason for hiding this comment

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

Doesn't it require changes on the reporting side? I am confused on how the reporting can be correct as it will be flagged as enforce mode in audit (because of the push_dry_run_mode(false))

Copy link
Member Author

Choose a reason for hiding this comment

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

the pop is done before the log so the log reports properly with the right mode
and oops ... the log line is missing here

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok 👍

@peckpeck
Copy link
Member Author

PR updated with a new commit

1 similar comment
@peckpeck
Copy link
Member Author

PR updated with a new commit

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/ncf/pull/1369
-- Your faithful QA
Kant merge: "In law a man is guilty when he violates the rights of others. In ethics he is guilty if he only thinks of doing so."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/59774/console)

@peckpeck
Copy link
Member Author

OK, squash merging this PR

@peckpeck peckpeck force-pushed the arch_21763/add_an_audit_command_generic_method branch from f7daeb6 to 5102fb3 Compare September 16, 2022 06:59
@peckpeck peckpeck merged commit 5102fb3 into Normation:branches/rudder/7.2 Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fdall Fdall Fdall approved these changes

Assignees
No one assigned
Labels
qa: Can't merge
Projects
None yet
Milestone
No milestone
3 participants
@peckpeck @Normation-Quality-Assistant @Fdall