From 91b7a1b9aca1f0ad55cdbf4e28443e13f4c1ff22 Mon Sep 17 00:00:00 2001 From: shuting Date: Mon, 20 May 2019 15:14:01 -0700 Subject: [PATCH] - handle operation remove case: if path does not exist - remove duplicate log - support validate in CLI --- pkg/engine/mutation.go | 1 + pkg/engine/mutation/patches.go | 9 ++++- pkg/engine/mutation/patches_test.go | 18 ++++----- pkg/engine/validation.go | 1 - pkg/kyverno/apply/apply.go | 63 +++++++++++++++++------------ 5 files changed, 55 insertions(+), 37 deletions(-) diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index 63bf86358e..1474c4364f 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -13,6 +13,7 @@ import ( func Mutate(policy kubepolicy.Policy, rawResource []byte, gvk metav1.GroupVersionKind) ([]mutation.PatchBytes, []byte) { var policyPatches []mutation.PatchBytes var processedPatches []mutation.PatchBytes + var err error patchedDocument := rawResource for _, rule := range policy.Spec.Rules { diff --git a/pkg/engine/mutation/patches.go b/pkg/engine/mutation/patches.go index db8316d6dc..068125c4fa 100644 --- a/pkg/engine/mutation/patches.go +++ b/pkg/engine/mutation/patches.go @@ -29,6 +29,9 @@ func ProcessPatches(patches []kubepolicy.Patch, resource []byte) ([]PatchBytes, patchedDocument, err = applyPatch(patchedDocument, patchRaw) if err != nil { // TODO: continue on error if one of the patches fails, will add the failure event in such case + if patch.Operation == "remove" { + continue + } log.Printf("Patch failed: patch number = %d, patch Operation = %s, err: %v", i, patch.Operation, err) continue } @@ -66,5 +69,9 @@ func applyPatch(resource []byte, patchRaw []byte) ([]byte, error) { return nil, err } - return patch.Apply(resource) + patchedDocument, err := patch.Apply(resource) + if err != nil { + return resource, err + } + return patchedDocument, err } diff --git a/pkg/engine/mutation/patches_test.go b/pkg/engine/mutation/patches_test.go index 825a5723cb..e4d26e5a1a 100644 --- a/pkg/engine/mutation/patches_test.go +++ b/pkg/engine/mutation/patches_test.go @@ -35,7 +35,7 @@ const endpointsDocument string = `{ func TestProcessPatches_EmptyPatches(t *testing.T) { var empty []types.Patch - patches, err := ProcessPatches(empty, []byte(endpointsDocument)) + patches, _, err := ProcessPatches(empty, []byte(endpointsDocument)) assert.NilError(t, err) assert.Assert(t, len(patches) == 0) } @@ -51,13 +51,13 @@ func makeAddIsMutatedLabelPatch() types.Patch { func TestProcessPatches_EmptyDocument(t *testing.T) { var patches []types.Patch patches = append(patches, makeAddIsMutatedLabelPatch()) - patchesBytes, err := ProcessPatches(patches, nil) + patchesBytes, _, err := ProcessPatches(patches, nil) assert.Assert(t, err != nil) assert.Assert(t, len(patchesBytes) == 0) } func TestProcessPatches_AllEmpty(t *testing.T) { - patchesBytes, err := ProcessPatches(nil, nil) + patchesBytes, _, err := ProcessPatches(nil, nil) assert.Assert(t, err != nil) assert.Assert(t, len(patchesBytes) == 0) } @@ -66,7 +66,7 @@ func TestProcessPatches_AddPathDoesntExist(t *testing.T) { patch := makeAddIsMutatedLabelPatch() patch.Path = "/metadata/additional/is-mutated" patches := []types.Patch{patch} - patchesBytes, err := ProcessPatches(patches, []byte(endpointsDocument)) + patchesBytes, _, err := ProcessPatches(patches, []byte(endpointsDocument)) assert.NilError(t, err) assert.Assert(t, len(patchesBytes) == 0) } @@ -74,7 +74,7 @@ func TestProcessPatches_AddPathDoesntExist(t *testing.T) { func TestProcessPatches_RemovePathDoesntExist(t *testing.T) { patch := types.Patch{Path: "/metadata/labels/is-mutated", Operation: "remove"} patches := []types.Patch{patch} - patchesBytes, err := ProcessPatches(patches, []byte(endpointsDocument)) + patchesBytes, _, err := ProcessPatches(patches, []byte(endpointsDocument)) assert.NilError(t, err) assert.Assert(t, len(patchesBytes) == 0) } @@ -83,7 +83,7 @@ func TestProcessPatches_AddAndRemovePathsDontExist_EmptyResult(t *testing.T) { patch1 := types.Patch{Path: "/metadata/labels/is-mutated", Operation: "remove"} patch2 := types.Patch{Path: "/spec/labels/label3", Operation: "add", Value: "label3Value"} patches := []types.Patch{patch1, patch2} - patchesBytes, err := ProcessPatches(patches, []byte(endpointsDocument)) + patchesBytes, _, err := ProcessPatches(patches, []byte(endpointsDocument)) assert.NilError(t, err) assert.Assert(t, len(patchesBytes) == 0) } @@ -93,7 +93,7 @@ func TestProcessPatches_AddAndRemovePathsDontExist_ContinueOnError_NotEmptyResul patch2 := types.Patch{Path: "/spec/labels/label2", Operation: "remove", Value: "label2Value"} patch3 := types.Patch{Path: "/metadata/labels/label3", Operation: "add", Value: "label3Value"} patches := []types.Patch{patch1, patch2, patch3} - patchesBytes, err := ProcessPatches(patches, []byte(endpointsDocument)) + patchesBytes, _, err := ProcessPatches(patches, []byte(endpointsDocument)) assert.NilError(t, err) assert.Assert(t, len(patchesBytes) == 1) assertEqStringAndData(t, `{"path":"/metadata/labels/label3","op":"add","value":"label3Value"}`, patchesBytes[0]) @@ -102,7 +102,7 @@ func TestProcessPatches_AddAndRemovePathsDontExist_ContinueOnError_NotEmptyResul func TestProcessPatches_RemovePathDoesntExist_EmptyResult(t *testing.T) { patch := types.Patch{Path: "/metadata/labels/is-mutated", Operation: "remove"} patches := []types.Patch{patch} - patchesBytes, err := ProcessPatches(patches, []byte(endpointsDocument)) + patchesBytes, _, err := ProcessPatches(patches, []byte(endpointsDocument)) assert.NilError(t, err) assert.Assert(t, len(patchesBytes) == 0) } @@ -111,7 +111,7 @@ func TestProcessPatches_RemovePathDoesntExist_NotEmptyResult(t *testing.T) { patch1 := types.Patch{Path: "/metadata/labels/is-mutated", Operation: "remove"} patch2 := types.Patch{Path: "/metadata/labels/label2", Operation: "add", Value: "label2Value"} patches := []types.Patch{patch1, patch2} - patchesBytes, err := ProcessPatches(patches, []byte(endpointsDocument)) + patchesBytes, _, err := ProcessPatches(patches, []byte(endpointsDocument)) assert.NilError(t, err) assert.Assert(t, len(patchesBytes) == 1) assertEqStringAndData(t, `{"path":"/metadata/labels/label2","op":"add","value":"label2Value"}`, patchesBytes[0]) diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index a6b55cadaf..c3f4d788cd 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -56,7 +56,6 @@ func Validate(policy kubepolicy.Policy, rawResource []byte, gvk metav1.GroupVers } } - log.Println("Validation is successful") return nil } diff --git a/pkg/kyverno/apply/apply.go b/pkg/kyverno/apply/apply.go index dd4815dc8b..dfd8a0ca36 100644 --- a/pkg/kyverno/apply/apply.go +++ b/pkg/kyverno/apply/apply.go @@ -27,32 +27,7 @@ func NewCmdApply(in io.Reader, out, errout io.Writer) *cobra.Command { Short: "Apply policy on the resource", Example: applyExample, Run: func(cmd *cobra.Command, args []string) { - // TODO: add pre-checks for policy and resource manifest - // order for policy and resource in args could be disordered - - if len(args) != 2 { - log.Printf("Missing policy and/or resource manifest.") - return - } - - // extract policy - policyDir := validateDir(args[0]) - policy, err := extractPolicy(policyDir) - if err != nil { - log.Printf("failed to extract policy: %v", err) - os.Exit(1) - } - - // fmt.Printf("policy name=%s, rule name=%s, %s/%s\n", policy.ObjectMeta.Name, policy.Spec.Rules[0].Name, - // policy.Spec.Rules[0].ResourceDescription.Kind, *policy.Spec.Rules[0].ResourceDescription.Name) - - // extract rawResource - resourceDir := validateDir(args[1]) - rawResource, gvk, err := extractResource(resourceDir) - if err != nil { - log.Printf("failed to load resource: %v", err) - os.Exit(1) - } + policy, rawResource, gvk := complete(args) _, patchedDocument := engine.Mutate(*policy, rawResource, *gvk) out, err := prettyPrint(patchedDocument) @@ -62,12 +37,48 @@ func NewCmdApply(in io.Reader, out, errout io.Writer) *cobra.Command { return } + if err := engine.Validate(*policy, rawResource, *gvk); err != nil { + fmt.Println(err) + return + } + fmt.Printf("%v\n", string(out)) }, } return cmd } +func complete(args []string) (*kubepolicy.Policy, []byte, *metav1.GroupVersionKind) { + // TODO: add pre-checks for policy and resource manifest + // order for policy and resource in args could be disordered + + if len(args) != 2 { + log.Printf("Missing policy and/or resource manifest.") + return nil, nil, nil + } + + // extract policy + policyDir := validateDir(args[0]) + policy, err := extractPolicy(policyDir) + if err != nil { + log.Printf("failed to extract policy: %v", err) + os.Exit(1) + } + + // fmt.Printf("policy name=%s, rule name=%s, %s/%s\n", policy.ObjectMeta.Name, policy.Spec.Rules[0].Name, + // policy.Spec.Rules[0].ResourceDescription.Kind, *policy.Spec.Rules[0].ResourceDescription.Name) + + // extract rawResource + resourceDir := validateDir(args[1]) + rawResource, gvk, err := extractResource(resourceDir) + if err != nil { + log.Printf("failed to load resource: %v", err) + os.Exit(1) + } + + return policy, rawResource, gvk +} + func extractPolicy(fileDir string) (*kubepolicy.Policy, error) { policy := &kubepolicy.Policy{}