From 2b4ff1ef6dfa98716173bcf063bc49436713890f Mon Sep 17 00:00:00 2001 From: Prateek Pandey Date: Sat, 12 Nov 2022 01:00:54 +0530 Subject: [PATCH] fix: synchronize source resource update to clone list resource (#5317) * fix: synchronize source resource update to clone list target resource Signed-off-by: prateekpandey14 * add kuttl test to verify the clone list synchronized behavior Signed-off-by: prateekpandey14 * refactor functions parameters Signed-off-by: prateekpandey14 * fix the kuttl test description and behavior README Signed-off-by: prateekpandey14 * Use entire content to compare Signed-off-by: prateekpandey14 --- pkg/controllers/webhook/controller.go | 1 + pkg/policy/validate.go | 88 +++++++++++++------ pkg/utils/controller/run.go | 4 +- .../resource/generation/generation.go | 2 +- pkg/webhooks/resource/updaterequest.go | 3 +- pkg/webhooks/utils/exclude.go | 2 + .../00-cluster-policy.yaml | 7 ++ .../01-trigger.yaml | 6 ++ .../02-update.yaml | 6 ++ .../03-update-source.yaml | 6 ++ .../cpol-clone-list-sync-update/README.md | 11 +++ .../cluster-policy-ready.yaml | 9 ++ .../cluster-policy.yaml | 32 +++++++ .../manifests.yaml | 21 +++++ .../sync/cpol-clone-list-sync-update/ns.yaml | 4 + .../resource-assert.yaml | 38 ++++++++ .../synchronized-target.yaml | 18 ++++ .../update-source.yaml | 9 ++ 18 files changed, 234 insertions(+), 33 deletions(-) create mode 100644 test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/00-cluster-policy.yaml create mode 100644 test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/01-trigger.yaml create mode 100644 test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/02-update.yaml create mode 100644 test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/03-update-source.yaml create mode 100644 test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/README.md create mode 100644 test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/cluster-policy-ready.yaml create mode 100644 test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/cluster-policy.yaml create mode 100644 test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/manifests.yaml create mode 100644 test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/ns.yaml create mode 100644 test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/resource-assert.yaml create mode 100644 test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/synchronized-target.yaml create mode 100644 test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/update-source.yaml diff --git a/pkg/controllers/webhook/controller.go b/pkg/controllers/webhook/controller.go index 2a5f66b6d1ef..f13f9af2b279 100644 --- a/pkg/controllers/webhook/controller.go +++ b/pkg/controllers/webhook/controller.go @@ -822,6 +822,7 @@ func (c *controller) mergeWebhook(dst *webhook, policy kyvernov1.PolicyInterface if rule.HasGenerate() { matchedGVK = append(matchedGVK, rule.MatchResources.GetKinds()...) matchedGVK = append(matchedGVK, rule.Generation.ResourceSpec.Kind) + matchedGVK = append(matchedGVK, rule.Generation.CloneList.Kinds...) continue } if (updateValidate && rule.HasValidate() || rule.HasImagesValidationChecks()) || diff --git a/pkg/policy/validate.go b/pkg/policy/validate.go index 75d46232480b..cda8cda3b116 100644 --- a/pkg/policy/validate.go +++ b/pkg/policy/validate.go @@ -27,6 +27,7 @@ import ( "github.com/pkg/errors" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/util/yaml" @@ -382,49 +383,80 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b logging.Error(err, fmt.Sprintf("source resource %s/%s/%s not found.", rule.Generation.Kind, rule.Generation.Clone.Namespace, rule.Generation.Clone.Name)) continue } - - updateSource := true - label := obj.GetLabels() - - if len(label) == 0 { - label = make(map[string]string) - label["generate.kyverno.io/clone-policy-name"] = policy.GetName() - } else { - if label["generate.kyverno.io/clone-policy-name"] != "" { - policyNames := label["generate.kyverno.io/clone-policy-name"] - if !strings.Contains(policyNames, policy.GetName()) { - policyNames = policyNames + "," + policy.GetName() - label["generate.kyverno.io/clone-policy-name"] = policyNames - } else { - updateSource = false - } - } else { - label["generate.kyverno.io/clone-policy-name"] = policy.GetName() - } + err = UpdateSourceResource(client, rule.Generation.Kind, rule.Generation.Clone.Namespace, policy.GetName(), obj) + if err != nil { + logging.Error(err, "failed to update source", "kind", obj.GetKind(), "name", obj.GetName(), "namespace", obj.GetNamespace()) + continue } - - if updateSource { - logging.V(4).Info("updating existing clone source") - obj.SetLabels(label) - _, err = client.UpdateResource(obj.GetAPIVersion(), rule.Generation.Kind, rule.Generation.Clone.Namespace, obj, false) + logging.V(4).Info("updated source", "kind", obj.GetKind(), "name", obj.GetName(), "namespace", obj.GetNamespace()) + } + if !mock && len(rule.Generation.CloneList.Kinds) != 0 { + for _, kind := range rule.Generation.CloneList.Kinds { + apiVersion, kind := kubeutils.GetKindFromGVK(kind) + resources, err := client.ListResource(apiVersion, kind, rule.Generation.CloneList.Namespace, rule.Generation.CloneList.Selector) if err != nil { - logging.Error(err, "failed to update source", "kind", obj.GetKind(), "name", obj.GetName(), "namespace", obj.GetNamespace()) + logging.Error(err, fmt.Sprintf("failed to list resources %s/%s.", kind, rule.Generation.CloneList.Namespace)) continue } - logging.V(4).Info("updated source", "kind", obj.GetKind(), "name", obj.GetName(), "namespace", obj.GetNamespace()) + for _, rName := range resources.Items { + obj, err := client.GetResource(apiVersion, kind, rule.Generation.CloneList.Namespace, rName.GetName()) + if err != nil { + logging.Error(err, fmt.Sprintf("source resource %s/%s/%s not found.", kind, rule.Generation.Clone.Namespace, rule.Generation.Clone.Name)) + continue + } + err = UpdateSourceResource(client, kind, rule.Generation.CloneList.Namespace, policy.GetName(), obj) + if err != nil { + logging.Error(err, "failed to update source", "kind", obj.GetKind(), "name", obj.GetName(), "namespace", obj.GetNamespace()) + continue + } + } } } } - if !mock && (spec.SchemaValidation == nil || *spec.SchemaValidation) { if err := openApiManager.ValidatePolicyMutation(policy); err != nil { return warnings, err } } - return warnings, nil } +func UpdateSourceResource(client dclient.Interface, kind, namespace string, policyName string, obj *unstructured.Unstructured) error { + updateSource := true + label := obj.GetLabels() + + if len(label) == 0 { + label = make(map[string]string) + label["generate.kyverno.io/clone-policy-name"] = policyName + } else { + if label["generate.kyverno.io/clone-policy-name"] != "" { + policyNames := label["generate.kyverno.io/clone-policy-name"] + if !strings.Contains(policyNames, policyName) { + policyNames = policyNames + "," + policyName + label["generate.kyverno.io/clone-policy-name"] = policyNames + } else { + updateSource = false + } + } else { + label["generate.kyverno.io/clone-policy-name"] = policyName + } + } + + if updateSource { + logging.V(4).Info("updating existing clone source labels") + obj.SetLabels(label) + obj.SetResourceVersion("") + + _, err := client.UpdateResource(obj.GetAPIVersion(), kind, namespace, obj, false) + if err != nil { + logging.Error(err, "failed to update source", "kind", obj.GetKind(), "name", obj.GetName(), "namespace", obj.GetNamespace()) + return err + } + logging.V(4).Info("updated source", "kind", obj.GetKind(), "name", obj.GetName(), "namespace", obj.GetNamespace()) + } + return nil +} + func ValidateVariables(p kyvernov1.PolicyInterface, backgroundMode bool) error { vars := hasVariables(p) if backgroundMode { diff --git a/pkg/utils/controller/run.go b/pkg/utils/controller/run.go index 50227b1514ee..b6c219683463 100644 --- a/pkg/utils/controller/run.go +++ b/pkg/utils/controller/run.go @@ -91,7 +91,7 @@ func reconcile(ctx context.Context, logger logr.Logger, obj interface{}, r recon } } logger = logger.WithValues("key", k, "namespace", ns, "name", n) - logger.Info("reconciling ...") - defer logger.Info("done", "duration", time.Since(start).String()) + logger.V(6).Info("reconciling ...") + defer logger.V(6).Info("done", "duration", time.Since(start).String()) return r(ctx, logger, k, ns, n) } diff --git a/pkg/webhooks/resource/generation/generation.go b/pkg/webhooks/resource/generation/generation.go index 90d37226a819..566206c2224f 100644 --- a/pkg/webhooks/resource/generation/generation.go +++ b/pkg/webhooks/resource/generation/generation.go @@ -82,7 +82,7 @@ func (h *generationHandler) Handle( policyContext *engine.PolicyContext, admissionRequestTimestamp time.Time, ) { - h.log.V(6).Info("update request") + h.log.V(6).Info("update request for generate policy") var engineResponses []*response.EngineResponse if (request.Operation == admissionv1.Create || request.Operation == admissionv1.Update) && len(policies) != 0 { diff --git a/pkg/webhooks/resource/updaterequest.go b/pkg/webhooks/resource/updaterequest.go index 0fab3b88ba13..9ee7baaf65bc 100644 --- a/pkg/webhooks/resource/updaterequest.go +++ b/pkg/webhooks/resource/updaterequest.go @@ -23,8 +23,6 @@ func (h *handlers) createUpdateRequests(logger logr.Logger, request *admissionv1 } func (h *handlers) handleMutateExisting(logger logr.Logger, request *admissionv1.AdmissionRequest, policies []kyvernov1.PolicyInterface, policyContext *engine.PolicyContext, admissionRequestTimestamp time.Time) { - logger.V(4).Info("update request") - if request.Operation == admissionv1.Delete { policyContext.NewResource = policyContext.OldResource } @@ -39,6 +37,7 @@ func (h *handlers) handleMutateExisting(logger logr.Logger, request *admissionv1 if !policy.GetSpec().IsMutateExisting() { continue } + logger.V(4).Info("update request for mutateExisting policy") var rules []response.RuleResponse policyContext.Policy = policy diff --git a/pkg/webhooks/utils/exclude.go b/pkg/webhooks/utils/exclude.go index cac800eef4b3..00f91aa5af2f 100644 --- a/pkg/webhooks/utils/exclude.go +++ b/pkg/webhooks/utils/exclude.go @@ -14,6 +14,8 @@ func ExcludeKyvernoResources(kind string) bool { return true case "ClusterBackgroundScanReport": return true + case "UpdateRequest": + return true case "GenerateRequest": return true default: diff --git a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/00-cluster-policy.yaml b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/00-cluster-policy.yaml new file mode 100644 index 000000000000..470e9d846af9 --- /dev/null +++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/00-cluster-policy.yaml @@ -0,0 +1,7 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +apply: +- manifests.yaml +- cluster-policy.yaml +assert: +- cluster-policy-ready.yaml \ No newline at end of file diff --git a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/01-trigger.yaml b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/01-trigger.yaml new file mode 100644 index 000000000000..c2962d7553e9 --- /dev/null +++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/01-trigger.yaml @@ -0,0 +1,6 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +apply: + - ns.yaml +assert: + - resource-assert.yaml \ No newline at end of file diff --git a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/02-update.yaml b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/02-update.yaml new file mode 100644 index 000000000000..c2962d7553e9 --- /dev/null +++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/02-update.yaml @@ -0,0 +1,6 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +apply: + - ns.yaml +assert: + - resource-assert.yaml \ No newline at end of file diff --git a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/03-update-source.yaml b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/03-update-source.yaml new file mode 100644 index 000000000000..d6aaec8940e0 --- /dev/null +++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/03-update-source.yaml @@ -0,0 +1,6 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +apply: + - update-source.yaml +assert: + - synchronized-target.yaml \ No newline at end of file diff --git a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/README.md b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/README.md new file mode 100644 index 000000000000..4e6125e799d1 --- /dev/null +++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/README.md @@ -0,0 +1,11 @@ +## Description + +This test verifies the synchronize behavior of generated resource, if the selected source resources using a matched label selector `allowedToBeCloned: "true"` gets changed, the update should be synchronized with the target resource as well. + +## Expected Behavior + +This test ensures that update of source resource(ConfigMap) match selected using `allowedToBeCloned: "true"` label get synchronized with target resource created by a ClusterPolicy `generate.cloneList` rule, otherwise the test fails. + +## Reference Issue(s) + +#4930 \ No newline at end of file diff --git a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/cluster-policy-ready.yaml b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/cluster-policy-ready.yaml new file mode 100644 index 000000000000..c0f5e53d43e7 --- /dev/null +++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/cluster-policy-ready.yaml @@ -0,0 +1,9 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: sync-secret-with-multi-clone +status: + conditions: + - reason: Succeeded + status: "True" + type: Ready \ No newline at end of file diff --git a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/cluster-policy.yaml b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/cluster-policy.yaml new file mode 100644 index 000000000000..7b1b18624ba8 --- /dev/null +++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/cluster-policy.yaml @@ -0,0 +1,32 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: sync-secret-with-multi-clone +spec: + generateExistingOnPolicyUpdate: true + rules: + - name: sync-secret + match: + any: + - resources: + kinds: + - Namespace + exclude: + any: + - resources: + namespaces: + - kube-system + - default + - kube-public + - kyverno + generate: + namespace: "{{request.object.metadata.name}}" + synchronize : true + cloneList: + namespace: default + kinds: + - v1/Secret + - v1/ConfigMap + selector: + matchLabels: + allowedToBeCloned: "true" \ No newline at end of file diff --git a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/manifests.yaml b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/manifests.yaml new file mode 100644 index 000000000000..2761bf800e7d --- /dev/null +++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/manifests.yaml @@ -0,0 +1,21 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: bootstrap-config + namespace: default + labels: + allowedToBeCloned: "true" +data: + initial_lives: "15" +--- +apiVersion: v1 +kind: Secret +metadata: + name: image-secret + namespace: default + labels: + allowedToBeCloned: "true" +type: kubernetes.io/basic-auth +stringData: + username: admin + password: t0p-Secret-super diff --git a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/ns.yaml b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/ns.yaml new file mode 100644 index 000000000000..f1ded585a826 --- /dev/null +++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/ns.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: prod \ No newline at end of file diff --git a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/resource-assert.yaml b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/resource-assert.yaml new file mode 100644 index 000000000000..4dfb4a4e59c7 --- /dev/null +++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/resource-assert.yaml @@ -0,0 +1,38 @@ +--- +apiVersion: v1 +data: + password: dDBwLVNlY3JldC1zdXBlcg== + username: YWRtaW4= +kind: Secret +metadata: + labels: + allowedToBeCloned: "true" + app.kubernetes.io/managed-by: kyverno + generate.kyverno.io/clone-policy-name: sync-secret-with-multi-clone + kyverno.io/background-gen-rule: sync-secret + kyverno.io/generated-by-kind: Namespace + kyverno.io/generated-by-name: prod + kyverno.io/generated-by-namespace: "" + policy.kyverno.io/policy-name: sync-secret-with-multi-clone + policy.kyverno.io/synchronize: enable + name: image-secret + namespace: prod +type: kubernetes.io/basic-auth +--- +apiVersion: v1 +data: + initial_lives: "15" +kind: ConfigMap +metadata: + labels: + allowedToBeCloned: "true" + app.kubernetes.io/managed-by: kyverno + generate.kyverno.io/clone-policy-name: sync-secret-with-multi-clone + kyverno.io/background-gen-rule: sync-secret + kyverno.io/generated-by-kind: Namespace + kyverno.io/generated-by-name: prod + kyverno.io/generated-by-namespace: "" + policy.kyverno.io/policy-name: sync-secret-with-multi-clone + policy.kyverno.io/synchronize: enable + name: bootstrap-config + namespace: prod diff --git a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/synchronized-target.yaml b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/synchronized-target.yaml new file mode 100644 index 000000000000..78d1ebc5410b --- /dev/null +++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/synchronized-target.yaml @@ -0,0 +1,18 @@ +--- +apiVersion: v1 +data: + initial_lives: "50" +kind: ConfigMap +metadata: + labels: + allowedToBeCloned: "true" + app.kubernetes.io/managed-by: kyverno + generate.kyverno.io/clone-policy-name: sync-secret-with-multi-clone + kyverno.io/background-gen-rule: sync-secret + kyverno.io/generated-by-kind: Namespace + kyverno.io/generated-by-name: prod + kyverno.io/generated-by-namespace: "" + policy.kyverno.io/policy-name: sync-secret-with-multi-clone + policy.kyverno.io/synchronize: enable + name: bootstrap-config + namespace: prod diff --git a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/update-source.yaml b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/update-source.yaml new file mode 100644 index 000000000000..91ed16a4fcb4 --- /dev/null +++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-update/update-source.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: bootstrap-config + namespace: default + labels: + allowedToBeCloned: "true" +data: + initial_lives: "50" \ No newline at end of file