-
Notifications
You must be signed in to change notification settings - Fork 780
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
feat: log details on log denies #2813
Conversation
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #2813 +/- ##
==========================================
+ Coverage 52.73% 52.86% +0.12%
==========================================
Files 132 132
Lines 11629 11632 +3
==========================================
+ Hits 6133 6149 +16
+ Misses 5013 5005 -8
+ Partials 483 478 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Why don't you think |
FWIW audit already reports details: gatekeeper/pkg/audit/manager.go Line 1115 in b7230e0
|
pkg/logging/logging.go
Outdated
@@ -34,6 +34,7 @@ const ( | |||
Mutator = "mutator" | |||
DebugLevel = 2 // r.log.Debug(foo) == r.log.V(logging.DebugLevel).Info(foo) | |||
ExecutionStats = "execution_stats" | |||
ViolationMetadata = "violation_metadata" |
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 consistent with how audit handles the details
field:
gatekeeper/pkg/audit/manager.go
Line 1115 in b7230e0
logging.Details, details, |
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.
Oh I missed that! 🤦🏼
Deduping this comment with the audit
logging. I figured that logging the details of the rego evaluations for every object for every audit run would be too noisy. At present, we only log details for a violation
in audit so this should be consistent w the audit behavior now:
gatekeeper/pkg/audit/manager.go
Lines 1106 to 1131 in b7230e0
func logViolation(l logr.Logger, | |
constraint *unstructured.Unstructured, | |
enforcementAction util.EnforcementAction, resourceGroupVersionKind schema.GroupVersionKind, rnamespace, rname, message string, details interface{}, rlabels map[string]string, | |
) { | |
userConstraintAnnotations := constraint.GetAnnotations() | |
delete(userConstraintAnnotations, "kubectl.kubernetes.io/last-applied-configuration") | |
l.Info( | |
message, | |
logging.Details, details, | |
logging.EventType, "violation_audited", | |
logging.ConstraintGroup, constraint.GroupVersionKind().Group, | |
logging.ConstraintAPIVersion, constraint.GroupVersionKind().Version, | |
logging.ConstraintKind, constraint.GetKind(), | |
logging.ConstraintName, constraint.GetName(), | |
logging.ConstraintNamespace, constraint.GetNamespace(), | |
logging.ConstraintAction, enforcementAction, | |
logging.ConstraintAnnotations, userConstraintAnnotations, | |
logging.ResourceGroup, resourceGroupVersionKind.Group, | |
logging.ResourceAPIVersion, resourceGroupVersionKind.Version, | |
logging.ResourceKind, resourceGroupVersionKind.Kind, | |
logging.ResourceNamespace, rnamespace, | |
logging.ResourceName, rname, | |
logging.ResourceLabels, rlabels, | |
) | |
} |
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
@maxsmythe PTAL when you have some time 🙏🏼 ; this is a one liner now hehe |
Co-authored-by: Jaydipkumar Arvindbhai Gabani <gabanijaydip@gmail.com> Signed-off-by: alex <8968914+acpana@users.noreply.github.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
Smallest of patches to log the details of a violation. I don't think this has a place in
audit
but am open to reconsider.Addresses #2790
Example: