support for new override labels #527
Conversation
auditors/capabilities/fixtures/capabilities-some-allowed-mix-old-labels.yml
Outdated
Show resolved
Hide resolved
auditors/netpols/fixtures/namespace-allow-missing-default-deny-ingress-old-label.yml
Show resolved
Hide resolved
dani-santos-code
force-pushed
the
ds/remove-tainted-annotations
branch
5 times, most recently
from
8544ca5
to
ea2c44a
Compare
dani-santos-code
force-pushed
the
ds/remove-tainted-annotations
branch
from
ea2c44a
to
7d5b684
Compare
There was a problem hiding this comment.
Backwards compatible is awesome! I wonder if this would be a good opportunity to clean up the annotation format (for the new annotations). I think we could still keep it backwards compatible since the override logic is contained in a single package.
The syntax for container / pod / namespace currently is:
container.audit.kubernetes.io/[container name].[override identifier](on the pod resource)audit.kubernetes.io/pod.[override identifier](on the pod resource)audit.kubernetes.io/namespace.[override identifier](on the namespace resource)
The requirements for label syntax are here
eg. maybe it could be
container.kubeaudit.io/[container name].[override identifier]pod.kubeaudit.io/[override identifier]namespace.kubeaudit.io/[override identifier]
Or, since in most cases the label applies to the resource it's specified on, we could have kubeaudit.io/[override identifier] as the format and only have the container case be different (since container resources don't allow labels we put it on the pod) container.kubeaudit.io/[container name].[override identifier]
auditors/netpols/fixtures/namespace-allow-missing-default-deny-ingress-old-label.yml
Outdated
Show resolved
Hide resolved
dani-santos-code
dismissed stale reviews from genevieveluyt and cailyn-codes
via
eba0895
|
Excellent point and great suggestions! Thank you! ❤️ I like applied your feedback here: 896f2b6 more specifically: https://github.com/Shopify/kubeaudit/blob/896f2b6e50553209a65958a9a8edfc07f02f50c8/pkg/override/override.go |
printer.go
Outdated
| @@ -48,6 +49,13 @@ func WithColor(color bool) PrintOption { | |||
| } | |||
| } | |||
|
|
|||
| // WithDeprecationWarning allows us to send a message about deprecations to end users | |||
| func WithDeprecationWarning(warning string) PrintOption { | |||
There was a problem hiding this comment.
All of the other print options are format options that the user can specify (eg. if they're using kubeaudit as a package; as a cli we pass on flags to the printer) so it's a bit weird to have an option that isn't intended to be used by an end user. Maybe we can just print the warning to StdErr in root.go?
There was a problem hiding this comment.
true! doesn't make sense indeed. let's go with stdErr in root. Addressed here: 211c9a8
Co-authored-by: Genevieve Luyt <11131143+genevieveluyt@users.noreply.github.com>
dani-santos-code
changed the title
Description
This PR addresses feedback from the kubernetes community on using unregistered annotations.
We have purchased a domain (
kubeaudit.io) as suggested for our own purpose: override labels.Backwards compatibility testing
This should be backwards compatible. I added 3 tests for the 3 old labels that should capture that. I also ran kubeaudit locally using both the old and the new labels and correctly applied the override label. I also tested using a random label (e.g kubeau.io) and it didn't work, as expected. 🎉
Deprecation Warning
Now we print a general deprecation warning to let users know that it will be deprecated.
Actual deprecation
Left comments in the code base with TODOs so we can come back and delete the deprecated annotations when we think it's the right time. When should we do so?
Fixes # #457
Type of change
How Has This Been Tested?
Checklist: