-
Notifications
You must be signed in to change notification settings - Fork 733
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
Add group and version to constraint logs and events #855
Conversation
Codecov Report
@@ Coverage Diff @@
## master #855 +/- ##
==========================================
- Coverage 43.96% 43.34% -0.62%
==========================================
Files 47 47
Lines 3146 3170 +24
==========================================
- Hits 1383 1374 -9
- Misses 1569 1598 +29
- Partials 194 198 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@@ -714,6 +714,8 @@ func logConstraint(l logr.Logger, constraint *unstructured.Unstructured, enforce | |||
l.Info( | |||
"audit results for constraint", | |||
logging.EventType, "constraint_audited", | |||
logging.ConstraintGroup, constraint.GroupVersionKind().Group, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to adding this for audit logs, we should also add it for
constraint controller:
https://github.com/open-policy-agent/gatekeeper/blob/master/pkg/controller/constraint/constraint_controller.go#L353-L370
and admission
https://github.com/open-policy-agent/gatekeeper/blob/master/pkg/webhook/policy.go#L240-L263
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
pkg/audit/manager.go
Outdated
logging.ConstraintName: constraint.GetName(), | ||
logging.ConstraintNamespace: constraint.GetNamespace(), | ||
logging.ConstraintAction: enforcementAction, | ||
logging.ResourceKind: rkind, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be logging resource group/version for the violating resource as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Signed-off-by: mmirecki <mmirecki@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
…t#855) Signed-off-by: mmirecki <mmirecki@redhat.com> Co-authored-by: Max Smythe <smythe@google.com> Signed-off-by: Matej Kern <matej.kern@infobip.com>
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when the PR gets merged):Fixes #748
Special notes for your reviewer: