From 47780bf37f7f4ffddee60b41db896c51b09f5cce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Fri, 14 Oct 2022 17:20:30 +0200 Subject: [PATCH] fix: improve banned types management in reports (#4953) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché Signed-off-by: Charles-Edouard Brétéché Signed-off-by: Charles-Edouard Brétéché --- pkg/clients/dclient/discovery.go | 8 +++++++- pkg/controllers/report/resource/controller.go | 19 ++++++++++-------- pkg/utils/report/support.go | 20 +++++++++++++++++++ .../resource/validation/validation.go | 9 +++++++-- 4 files changed, 45 insertions(+), 11 deletions(-) create mode 100644 pkg/utils/report/support.go diff --git a/pkg/clients/dclient/discovery.go b/pkg/clients/dclient/discovery.go index c39d9ef4a6..fa35ecec53 100644 --- a/pkg/clients/dclient/discovery.go +++ b/pkg/clients/dclient/discovery.go @@ -152,7 +152,13 @@ func (c serverPreferredResources) findResource(apiVersion string, kind string) ( logger.Error(err, "failed to parse GV", "groupVersion", serverResource.GroupVersion) return nil, schema.GroupVersionResource{}, err } - + // We potentially need to fix Group and Version with what the list is for + if resource.Group == "" { + resource.Group = gv.Group + } + if resource.Version == "" { + resource.Version = gv.Version + } return &resource, gv.WithResource(resource.Name), nil } } diff --git a/pkg/controllers/report/resource/controller.go b/pkg/controllers/report/resource/controller.go index b20d1b5ce3..4b629807c8 100644 --- a/pkg/controllers/report/resource/controller.go +++ b/pkg/controllers/report/resource/controller.go @@ -126,24 +126,27 @@ func (c *controller) updateDynamicWatchers(ctx context.Context) error { return err } kinds := utils.BuildKindSet(logger, utils.RemoveNonValidationPolicies(logger, append(clusterPolicies, policies...)...)...) - gvrs := map[string]schema.GroupVersionResource{} + gvrs := map[schema.GroupVersionKind]schema.GroupVersionResource{} for _, kind := range kinds.List() { apiVersion, kind := kubeutils.GetKindFromGVK(kind) apiResource, gvr, err := c.client.Discovery().FindResource(apiVersion, kind) if err != nil { logger.Error(err, "failed to get gvr from kind", "kind", kind) - } else if apiVersion == "" && kind == "Event" { - logger.Info("Event cannot be an owner, skipping", "apiVersion", apiVersion, "kind", kind) } else { - if pkgutils.ContainsString(apiResource.Verbs, "list") && pkgutils.ContainsString(apiResource.Verbs, "watch") { - gvrs[kind] = gvr + gvk := schema.GroupVersionKind{Group: apiResource.Group, Version: apiResource.Version, Kind: apiResource.Kind} + if !reportutils.IsGvkSupported(gvk) { + logger.Info("kind is not supported", "gvk", gvk) } else { - logger.Info("list/watch not supported for kind", "kind", kind) + if pkgutils.ContainsString(apiResource.Verbs, "list") && pkgutils.ContainsString(apiResource.Verbs, "watch") { + gvrs[gvk] = gvr + } else { + logger.Info("list/watch not supported for kind", "kind", kind) + } } } } dynamicWatchers := map[schema.GroupVersionResource]*watcher{} - for kind, gvr := range gvrs { + for gvk, gvr := range gvrs { // if we already have one, transfer it to the new map if c.dynamicWatchers[gvr] != nil { dynamicWatchers[gvr] = c.dynamicWatchers[gvr] @@ -156,7 +159,7 @@ func (c *controller) updateDynamicWatchers(ctx context.Context) error { } else { w := &watcher{ watcher: watchInterface, - gvk: gvr.GroupVersion().WithKind(kind), + gvk: gvk, hashes: map[types.UID]Resource{}, } go func() { diff --git a/pkg/utils/report/support.go b/pkg/utils/report/support.go new file mode 100644 index 0000000000..2b1bb259dd --- /dev/null +++ b/pkg/utils/report/support.go @@ -0,0 +1,20 @@ +package report + +import ( + corev1 "k8s.io/api/core/v1" + eventsv1 "k8s.io/api/events/v1" + eventsv1beta1 "k8s.io/api/events/v1beta1" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// bannedOwners are GVKs that are not allowed to be owners of other resources +var bannedOwners = map[schema.GroupVersionKind]struct{}{ + corev1.SchemeGroupVersion.WithKind("Event"): {}, + eventsv1.SchemeGroupVersion.WithKind("Event"): {}, + eventsv1beta1.SchemeGroupVersion.WithKind("Event"): {}, +} + +func IsGvkSupported(gvk schema.GroupVersionKind) bool { + _, exists := bannedOwners[gvk] + return !exists +} diff --git a/pkg/webhooks/resource/validation/validation.go b/pkg/webhooks/resource/validation/validation.go index 7e311d351d..c9be937c15 100644 --- a/pkg/webhooks/resource/validation/validation.go +++ b/pkg/webhooks/resource/validation/validation.go @@ -20,6 +20,7 @@ import ( admissionv1 "k8s.io/api/admission/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" ) type ValidationHandler interface { @@ -163,17 +164,21 @@ func (v *validationHandler) handleAudit( if !v.admissionReports { return } - // we don't need reports for deletions and when it's about sub resources + // we don't need reports for deletions and when it's about sub resources if request.Operation == admissionv1.Delete || request.SubResource != "" { return } + // check if the resource supports reporting + if !reportutils.IsGvkSupported(schema.GroupVersionKind(request.Kind)) { + return + } responses, err := v.buildAuditResponses(resource, request, namespaceLabels) if err != nil { v.log.Error(err, "failed to build audit responses") } responses = append(responses, engineResponses...) report := reportutils.NewAdmissionReport(resource, request, request.Kind, responses...) - // if it's not a creation, the resource already exists, we can set the owner + // if it's not a creation, the resource already exists, we can set the owner if request.Operation != admissionv1.Create { gv := metav1.GroupVersion{Group: request.Kind.Group, Version: request.Kind.Version} controllerutils.SetOwner(report, gv.String(), request.Kind.Kind, resource.GetName(), resource.GetUID())