Fixed mutation by overlays

pkg/engine/mutation.go

@@ -25,28 +25,23 @@ func Mutate(policy kubepolicy.Policy, rawResource []byte, gvk metav1.GroupVersio
 			ruleApplicationResult.AddMessagef("Rule %s is not applicable to resource\n", rule.Name)
 		} else {
 			// Process Overlay
-			if rule.Mutation.Overlay != nil {
-				overlayPatches, ruleResult := ProcessOverlay(rule.Mutation.Overlay, rawResource, gvk)
-				if result.Success != ruleResult.GetReason() {
-					ruleApplicationResult.MergeWith(&ruleResult)
-					ruleApplicationResult.AddMessagef("Overlay application has failed for rule %s in policy %s\n", rule.Name, policy.ObjectMeta.Name)
-				} else {
-					ruleApplicationResult.AddMessagef("Success")
-					allPatches = append(allPatches, overlayPatches...)
-				}
+			overlayPatches, ruleResult := ProcessOverlay(rule, rawResource, gvk)
+			if result.Success != ruleResult.GetReason() {
+				ruleApplicationResult.MergeWith(&ruleResult)
+				ruleApplicationResult.AddMessagef("Overlay application has failed for rule %s in policy %s\n", rule.Name, policy.ObjectMeta.Name)
+			} else {
+				ruleApplicationResult.AddMessagef("Success")
+				allPatches = append(allPatches, overlayPatches...)
 			}
 
 			// Process Patches
-			if rule.Mutation.Patches != nil {
-				rulePatches, ruleResult := ProcessPatches(rule.Mutation.Patches, patchedDocument)
-
-				if result.Success != ruleResult.GetReason() {
-					ruleApplicationResult.MergeWith(&ruleResult)
-					ruleApplicationResult.AddMessagef("Patches application has failed for rule %s in policy %s\n", rule.Name, policy.ObjectMeta.Name)
-				} else {
-					ruleApplicationResult.AddMessagef("Success")
-					allPatches = append(allPatches, rulePatches...)
-				}
+			rulePatches, ruleResult := ProcessPatches(rule, patchedDocument)
+			if result.Success != ruleResult.GetReason() {
+				ruleApplicationResult.MergeWith(&ruleResult)
+				ruleApplicationResult.AddMessagef("Patches application has failed for rule %s in policy %s\n", rule.Name, policy.ObjectMeta.Name)
+			} else {
+				ruleApplicationResult.AddMessagef("Success")
+				allPatches = append(allPatches, rulePatches...)
 			}
 		}
 		policyResult = result.Append(policyResult, &ruleApplicationResult)

pkg/engine/overlay.go

@@ -7,23 +7,24 @@ import (
 	"strconv"
 
 	jsonpatch "github.com/evanphx/json-patch"
+	kubepolicy "github.com/nirmata/kyverno/pkg/apis/policy/v1alpha1"
 	"github.com/nirmata/kyverno/pkg/result"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // ProcessOverlay handles validating admission request
 // Checks the target resourse for rules defined in the policy
-func ProcessOverlay(overlay interface{}, rawResource []byte, gvk metav1.GroupVersionKind) ([]PatchBytes, result.RuleApplicationResult) {
+func ProcessOverlay(rule kubepolicy.Rule, rawResource []byte, gvk metav1.GroupVersionKind) ([]PatchBytes, result.RuleApplicationResult) {
+	overlayApplicationResult := result.NewRuleApplicationResult(rule.Name)
+	if rule.Mutation == nil || rule.Mutation.Overlay == nil {
+		return nil, overlayApplicationResult
+	}
+
 	var resource interface{}
 	var appliedPatches []PatchBytes
 	json.Unmarshal(rawResource, &resource)
 
-	overlayApplicationResult := result.NewRuleApplicationResult("")
-	if overlay == nil {
-		return nil, overlayApplicationResult
-	}
-
-	patch := applyOverlay(resource, overlay, "/", &overlayApplicationResult)
+	patch := applyOverlay(resource, *rule.Mutation.Overlay, "/", &overlayApplicationResult)
 	if overlayApplicationResult.GetReason() == result.Success {
 		appliedPatches = append(appliedPatches, patch...)
 	}

pkg/engine/patches.go

@@ -14,9 +14,10 @@ type PatchBytes []byte
 
 // ProcessPatches Returns array from separate patches that can be applied to the document
 // Returns error ONLY in case when creation of resource should be denied.
-func ProcessPatches(patches []kubepolicy.Patch, resource []byte) ([]PatchBytes, result.RuleApplicationResult) {
-	res := result.RuleApplicationResult{
-		Reason: result.Success,
+func ProcessPatches(rule kubepolicy.Rule, resource []byte) ([]PatchBytes, result.RuleApplicationResult) {
+	res := result.NewRuleApplicationResult(rule.Name)
+	if rule.Mutation == nil || len(rule.Mutation.Patches) == 0 {
+		return nil, res
 	}
 
 	if len(resource) == 0 {
@@ -27,7 +28,7 @@ func ProcessPatches(patches []kubepolicy.Patch, resource []byte) ([]PatchBytes,
 
 	var allPatches []PatchBytes
 	patchedDocument := resource
-	for i, patch := range patches {
+	for i, patch := range rule.Mutation.Patches {
 		patchRaw, err := json.Marshal(patch)
 		if err != nil {
 

pkg/engine/patches_test.go

@@ -34,8 +34,8 @@ const endpointsDocument string = `{
 }`
 
 func TestProcessPatches_EmptyPatches(t *testing.T) {
-	var empty []types.Patch
-	patches, res := ProcessPatches(empty, []byte(endpointsDocument))
+	var emptyRule = types.Rule{}
+	patches, res := ProcessPatches(emptyRule, []byte(endpointsDocument))
 	assert.NilError(t, res.ToError())
 	assert.Assert(t, len(patches) == 0)
 }
@@ -48,33 +48,47 @@ func makeAddIsMutatedLabelPatch() types.Patch {
 	}
 }
 
+func makeRuleWithPatch(patch types.Patch) types.Rule {
+	patches := []types.Patch{patch}
+	return makeRuleWithPatches(patches)
+}
+
+func makeRuleWithPatches(patches []types.Patch) types.Rule {
+	mutation := types.Mutation{
+		Patches: patches,
+	}
+	return types.Rule{
+		Mutation: &mutation,
+	}
+}
+
 func TestProcessPatches_EmptyDocument(t *testing.T) {
-	var patches []types.Patch
-	patches = append(patches, makeAddIsMutatedLabelPatch())
-	patchesBytes, res := ProcessPatches(patches, nil)
+	rule := makeRuleWithPatch(makeAddIsMutatedLabelPatch())
+	patchesBytes, res := ProcessPatches(rule, nil)
 	assert.Assert(t, res.ToError() != nil)
 	assert.Assert(t, len(patchesBytes) == 0)
 }
 
 func TestProcessPatches_AllEmpty(t *testing.T) {
-	patchesBytes, res := ProcessPatches(nil, nil)
-	assert.Assert(t, res.ToError() != nil)
+	emptyRule := types.Rule{}
+	patchesBytes, res := ProcessPatches(emptyRule, nil)
+	assert.NilError(t, res.ToError())
 	assert.Assert(t, len(patchesBytes) == 0)
 }
 
 func TestProcessPatches_AddPathDoesntExist(t *testing.T) {
 	patch := makeAddIsMutatedLabelPatch()
 	patch.Path = "/metadata/additional/is-mutated"
-	patches := []types.Patch{patch}
-	patchesBytes, res := ProcessPatches(patches, []byte(endpointsDocument))
+	rule := makeRuleWithPatch(patch)
+	patchesBytes, res := ProcessPatches(rule, []byte(endpointsDocument))
 	assert.NilError(t, res.ToError())
 	assert.Assert(t, len(patchesBytes) == 0)
 }
 
 func TestProcessPatches_RemovePathDoesntExist(t *testing.T) {
 	patch := types.Patch{Path: "/metadata/labels/is-mutated", Operation: "remove"}
-	patches := []types.Patch{patch}
-	patchesBytes, res := ProcessPatches(patches, []byte(endpointsDocument))
+	rule := makeRuleWithPatch(patch)
+	patchesBytes, res := ProcessPatches(rule, []byte(endpointsDocument))
 	assert.NilError(t, res.ToError())
 	assert.Assert(t, len(patchesBytes) == 0)
 }
@@ -82,8 +96,8 @@ func TestProcessPatches_RemovePathDoesntExist(t *testing.T) {
 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, res := ProcessPatches(patches, []byte(endpointsDocument))
+	rule := makeRuleWithPatches([]types.Patch{patch1, patch2})
+	patchesBytes, res := ProcessPatches(rule, []byte(endpointsDocument))
 	assert.NilError(t, res.ToError())
 	assert.Assert(t, len(patchesBytes) == 0)
 }
@@ -92,8 +106,8 @@ func TestProcessPatches_AddAndRemovePathsDontExist_ContinueOnError_NotEmptyResul
 	patch1 := types.Patch{Path: "/metadata/labels/is-mutated", Operation: "remove"}
 	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, res := ProcessPatches(patches, []byte(endpointsDocument))
+	rule := makeRuleWithPatches([]types.Patch{patch1, patch2, patch3})
+	patchesBytes, res := ProcessPatches(rule, []byte(endpointsDocument))
 	assert.NilError(t, res.ToError())
 	assert.Assert(t, len(patchesBytes) == 1)
 	assertEqStringAndData(t, `{"path":"/metadata/labels/label3","op":"add","value":"label3Value"}`, patchesBytes[0])
@@ -101,8 +115,8 @@ 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, res := ProcessPatches(patches, []byte(endpointsDocument))
+	rule := makeRuleWithPatch(patch)
+	patchesBytes, res := ProcessPatches(rule, []byte(endpointsDocument))
 	assert.NilError(t, res.ToError())
 	assert.Assert(t, len(patchesBytes) == 0)
 }
@@ -110,8 +124,8 @@ func TestProcessPatches_RemovePathDoesntExist_EmptyResult(t *testing.T) {
 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, res := ProcessPatches(patches, []byte(endpointsDocument))
+	rule := makeRuleWithPatches([]types.Patch{patch1, patch2})
+	patchesBytes, res := ProcessPatches(rule, []byte(endpointsDocument))
 	assert.NilError(t, res.ToError())
 	assert.Assert(t, len(patchesBytes) == 1)
 	assertEqStringAndData(t, `{"path":"/metadata/labels/label2","op":"add","value":"label2Value"}`, patchesBytes[0])

pkg/kyverno/apply/apply.go

@@ -98,7 +98,7 @@ func applyPolicyOnRaw(policy *kubepolicy.Policy, rawResource []byte, gvk *metav1
 
 	err := result.ToError()
 	patchedResource := rawResource
-	if err != nil && len(patches) != 0 {
+	if err == nil && len(patches) != 0 {
 		patchedResource, err = engine.ApplyPatches(rawResource, patches)
 		if err != nil {
 			return nil, fmt.Errorf("Unable to apply mutation patches:\n%v", err)