From 7ea2930fa4a5cf8578be7e3a783ef24e64ca5ad9 Mon Sep 17 00:00:00 2001 From: Shuting Zhao Date: Tue, 26 May 2020 13:56:07 -0700 Subject: [PATCH] - fix violations re-create on the same resource - skip background processing if a resource is to be deleted --- pkg/policy/apply.go | 46 ++++++++++++++++++++++------- pkg/policy/existing.go | 4 +++ pkg/policyviolation/clusterpv.go | 11 ++++++- pkg/policyviolation/common.go | 14 +++++++-- pkg/policyviolation/namespacedpv.go | 11 ++++++- 5 files changed, 72 insertions(+), 14 deletions(-) diff --git a/pkg/policy/apply.go b/pkg/policy/apply.go index 6ec7666c70..9387401d4c 100644 --- a/pkg/policy/apply.go +++ b/pkg/policy/apply.go @@ -32,7 +32,7 @@ func applyPolicy(policy kyverno.ClusterPolicy, resource unstructured.Unstructure }() var engineResponses []response.EngineResponse - var engineResponse response.EngineResponse + var engineResponseMutation, engineResponseValidation response.EngineResponse var err error // build context ctx := context.NewContext() @@ -41,15 +41,14 @@ func applyPolicy(policy kyverno.ClusterPolicy, resource unstructured.Unstructure logger.Error(err, "enable to add transform resource to ctx") } //MUTATION - engineResponse, err = mutation(policy, resource, ctx, logger) - engineResponses = append(engineResponses, engineResponse) + engineResponseMutation, err = mutation(policy, resource, ctx, logger) if err != nil { logger.Error(err, "failed to process mutation rule") } //VALIDATION - engineResponse = engine.Validate(engine.PolicyContext{Policy: policy, Context: ctx, NewResource: resource}) - engineResponses = append(engineResponses, engineResponse) + engineResponseValidation = engine.Validate(engine.PolicyContext{Policy: policy, Context: ctx, NewResource: resource}) + engineResponses = append(engineResponses, mergeRuleRespose(engineResponseMutation, engineResponseValidation)) //TODO: GENERATION return engineResponses @@ -81,27 +80,31 @@ func getFailedOverallRuleInfo(resource unstructured.Unstructured, engineResponse // resource does not match so there was a mutation rule violated for index, rule := range engineResponse.PolicyResponse.Rules { log.V(4).Info("veriying if policy rule was applied before", "rule", rule.Name) - if len(rule.Patches) == 0 { + + patches := dropKyvernoAnnotation(rule.Patches, log) + if len(patches) == 0 { continue } - patch, err := jsonpatch.DecodePatch(utils.JoinPatches(rule.Patches)) + + patch, err := jsonpatch.DecodePatch(utils.JoinPatches(patches)) if err != nil { - log.Error(err, "failed to decode JSON patch", "patches", rule.Patches) + log.Error(err, "failed to decode JSON patch", "patches", patches) return response.EngineResponse{}, err } // apply the patches returned by mutate to the original resource patchedResource, err := patch.Apply(rawResource) if err != nil { - log.Error(err, "failed to apply JSON patch", "patches", rule.Patches) + log.Error(err, "failed to apply JSON patch", "patches", patches) return response.EngineResponse{}, err } if !jsonpatch.Equal(patchedResource, rawResource) { log.V(4).Info("policy rule conditions not satisfied by resource", "rule", rule.Name) engineResponse.PolicyResponse.Rules[index].Success = false - engineResponse.PolicyResponse.Rules[index].Message = fmt.Sprintf("mutation json patches not found at resource path %s", extractPatchPath(rule.Patches, log)) + engineResponse.PolicyResponse.Rules[index].Message = fmt.Sprintf("mutation json patches not found at resource path %s", extractPatchPath(patches, log)) } } + return engineResponse, nil } @@ -125,3 +128,26 @@ func extractPatchPath(patches [][]byte, log logr.Logger) string { } return strings.Join(resultPath, ";") } + +func dropKyvernoAnnotation(patches [][]byte, log logr.Logger) (resultPathes [][]byte) { + for _, patch := range patches { + var data jsonPatch + if err := json.Unmarshal(patch, &data); err != nil { + log.Error(err, "failed to decode the generate patch", "patch", string(patch)) + continue + } + + value := fmt.Sprintf("%v", data.Value) + if strings.Contains(value, engine.PodTemplateAnnotation) { + continue + } + + resultPathes = append(resultPathes, patch) + } + return +} + +func mergeRuleRespose(mutation, validation response.EngineResponse) response.EngineResponse { + mutation.PolicyResponse.Rules = append(mutation.PolicyResponse.Rules, validation.PolicyResponse.Rules...) + return mutation +} diff --git a/pkg/policy/existing.go b/pkg/policy/existing.go index fdef1bb34c..6153dbeba2 100644 --- a/pkg/policy/existing.go +++ b/pkg/policy/existing.go @@ -167,6 +167,10 @@ func getResourcesPerNamespace(kind string, client *client.Client, namespace stri } // filter based on name for _, r := range list.Items { + if r.GetDeletionTimestamp() != nil { + continue + } + // match name if rule.MatchResources.Name != "" { if !wildcard.Match(rule.MatchResources.Name, r.GetName()) { diff --git a/pkg/policyviolation/clusterpv.go b/pkg/policyviolation/clusterpv.go index 4cb26ea216..8fe4920cd2 100644 --- a/pkg/policyviolation/clusterpv.go +++ b/pkg/policyviolation/clusterpv.go @@ -93,8 +93,17 @@ func (cpv *clusterPV) createPV(newPv *kyverno.ClusterPolicyViolation) error { if err != nil { return fmt.Errorf("failed to retry getting resource for policy violation %s/%s: %v", newPv.Name, newPv.Spec.Policy, err) } + + if obj.GetDeletionTimestamp() != nil { + return nil + } + // set owner reference to resource - ownerRef := createOwnerReference(obj) + ownerRef, ok := createOwnerReference(obj) + if !ok { + return nil + } + newPv.SetOwnerReferences([]metav1.OwnerReference{ownerRef}) // create resource diff --git a/pkg/policyviolation/common.go b/pkg/policyviolation/common.go index e2211edb13..e8e77308a7 100644 --- a/pkg/policyviolation/common.go +++ b/pkg/policyviolation/common.go @@ -14,9 +14,19 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" ) -func createOwnerReference(resource *unstructured.Unstructured) metav1.OwnerReference { +func createOwnerReference(resource *unstructured.Unstructured) (metav1.OwnerReference, bool) { controllerFlag := true blockOwnerDeletionFlag := true + + apiversion := resource.GetAPIVersion() + kind := resource.GetKind() + name := resource.GetName() + uid := resource.GetUID() + + if apiversion == "" || kind == "" || name == "" || uid == "" { + return metav1.OwnerReference{}, false + } + ownerRef := metav1.OwnerReference{ APIVersion: resource.GetAPIVersion(), Kind: resource.GetKind(), @@ -25,7 +35,7 @@ func createOwnerReference(resource *unstructured.Unstructured) metav1.OwnerRefer Controller: &controllerFlag, BlockOwnerDeletion: &blockOwnerDeletionFlag, } - return ownerRef + return ownerRef, true } func retryGetResource(client *client.Client, rspec kyverno.ResourceSpec) (*unstructured.Unstructured, error) { diff --git a/pkg/policyviolation/namespacedpv.go b/pkg/policyviolation/namespacedpv.go index 3b0202c6df..ba061defce 100644 --- a/pkg/policyviolation/namespacedpv.go +++ b/pkg/policyviolation/namespacedpv.go @@ -92,8 +92,17 @@ func (nspv *namespacedPV) createPV(newPv *kyverno.PolicyViolation) error { if err != nil { return fmt.Errorf("failed to retry getting resource for policy violation %s/%s: %v", newPv.Name, newPv.Spec.Policy, err) } + + if obj.GetDeletionTimestamp() != nil { + return nil + } + // set owner reference to resource - ownerRef := createOwnerReference(obj) + ownerRef, ok := createOwnerReference(obj) + if !ok { + return nil + } + newPv.SetOwnerReferences([]metav1.OwnerReference{ownerRef}) // create resource