From 8edb00d7146296430ff9333c27d00ba96143a1bd Mon Sep 17 00:00:00 2001 From: Shuting Zhao Date: Mon, 9 Dec 2019 19:28:34 -0800 Subject: [PATCH 1/2] - skip processing mutate rule if condition is not met; - update debugging info --- pkg/engine/overlay.go | 11 +++++------ pkg/engine/overlayCondition.go | 10 +++++----- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/pkg/engine/overlay.go b/pkg/engine/overlay.go index c772c2cb7a..b300479df2 100644 --- a/pkg/engine/overlay.go +++ b/pkg/engine/overlay.go @@ -35,15 +35,14 @@ func processOverlay(rule kyverno.Rule, resource unstructured.Unstructured) (resp // condition key is not present in the resource, don't apply this rule // consider as success case conditionNotPresent: - glog.V(3).Infof("Resource %s/%s/%s: %s", resource.GetKind(), resource.GetNamespace(), resource.GetName(), overlayerr.ErrorMsg()) + glog.V(3).Infof("Skip applying rule '%s' on resource '%s/%s/%s': %s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), overlayerr.ErrorMsg()) response.Success = true return response, resource // conditions are not met, don't apply this rule - // consider as failure case conditionFailure: - glog.Errorf("Resource %s/%s/%s does not meet the conditions in the rule %s with overlay pattern %s", resource.GetKind(), resource.GetNamespace(), resource.GetName(), rule.Name, rule.Mutation.Overlay) + glog.V(3).Infof("Skip applying rule '%s' on resource '%s/%s/%s': %s", rule.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), overlayerr.ErrorMsg()) //TODO: send zero response and not consider this as applied? - response.Success = false + response.Success = true response.Message = overlayerr.ErrorMsg() return response, resource // rule application failed @@ -106,12 +105,12 @@ func processOverlayPatches(resource, overlay interface{}) ([][]byte, overlayErro // anchor key does not exist in the resource, skip applying policy case conditionNotPresent: glog.V(4).Infof("Mutate rule: skip applying policy: %v at %s", overlayerr, path) - return nil, newOverlayError(overlayerr.statusCode, fmt.Sprintf("policy not applied: %v at %s", overlayerr.ErrorMsg(), path)) + return nil, newOverlayError(overlayerr.statusCode, fmt.Sprintf("Policy not applied, condition tag not present: %v at %s", overlayerr.ErrorMsg(), path)) // anchor key is not satisfied in the resource, skip applying policy case conditionFailure: // anchor key is not satisfied in the resource, skip applying policy glog.V(4).Infof("Mutate rule: failed to validate condition at %s, err: %v", path, overlayerr) - return nil, newOverlayError(overlayerr.statusCode, fmt.Sprintf("Conditions are not met at %s, %v", path, overlayerr)) + return nil, newOverlayError(overlayerr.statusCode, fmt.Sprintf("Policy not applied, conditions are not met at %s, %v", path, overlayerr)) } } diff --git a/pkg/engine/overlayCondition.go b/pkg/engine/overlayCondition.go index ffd8f5ef4d..12c22992d3 100755 --- a/pkg/engine/overlayCondition.go +++ b/pkg/engine/overlayCondition.go @@ -110,8 +110,8 @@ func validateConditionAnchorMap(resourceMap, anchors map[string]interface{}, pat // resource - A: B2 func compareOverlay(resource, overlay interface{}, path string) (string, overlayError) { if reflect.TypeOf(resource) != reflect.TypeOf(overlay) { - glog.V(4).Infof("Found anchor on different types of element: overlay %T, resource %T\nSkip processing overlay.", overlay, resource) - return path, newOverlayError(conditionFailure, fmt.Sprintf("Found anchor on different types of element: overlay %T, resource %T\nSkip processing overlay.", overlay, resource)) + glog.V(4).Infof("Found anchor on different types of element: overlay %T, resource %T", overlay, resource) + return path, newOverlayError(conditionFailure, fmt.Sprintf("Found anchor on different types of element: overlay %T, resource %T", overlay, resource)) } switch typedOverlay := overlay.(type) { @@ -122,7 +122,7 @@ func compareOverlay(resource, overlay interface{}, path string) (string, overlay curPath := path + noAnchorKey + "/" resourceVal, ok := typedResource[noAnchorKey] if !ok { - return curPath, newOverlayError(conditionFailure, fmt.Sprintf("field %s is not present", noAnchorKey)) + return curPath, newOverlayError(conditionFailure, fmt.Sprintf("Field %s is not present", noAnchorKey)) } if newPath, err := compareOverlay(resourceVal, overlayVal, curPath); !reflect.DeepEqual(err, overlayError{}) { return newPath, err @@ -140,10 +140,10 @@ func compareOverlay(resource, overlay interface{}, path string) (string, overlay case string, float64, int, int64, bool, nil: if !ValidateValueWithPattern(resource, overlay) { glog.V(4).Infof("Mutate rule: failed validating value %v with overlay %v", resource, overlay) - return path, newOverlayError(conditionFailure, fmt.Sprintf("failed validating value %v with overlay %v", resource, overlay)) + return path, newOverlayError(conditionFailure, fmt.Sprintf("Failed validating value %v with overlay %v", resource, overlay)) } default: - return path, newOverlayError(conditionFailure, fmt.Sprintf("overlay has unknown type %T, value %v", overlay, overlay)) + return path, newOverlayError(conditionFailure, fmt.Sprintf("Overlay has unknown type %T, value %v", overlay, overlay)) } return "", overlayError{} From 4c2a16904ce3a4a790d9674dff743de6926dffe1 Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Tue, 10 Dec 2019 09:15:50 -0800 Subject: [PATCH 2/2] update tests --- pkg/engine/overlayCondition_test.go | 6 +++--- pkg/engine/overlay_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/engine/overlayCondition_test.go b/pkg/engine/overlayCondition_test.go index 9b78c74c07..af0cd3a42d 100644 --- a/pkg/engine/overlayCondition_test.go +++ b/pkg/engine/overlayCondition_test.go @@ -194,7 +194,7 @@ func TestMeetConditions_anchosInSameObject(t *testing.T) { json.Unmarshal(overlayRaw, &overlay) _, err := meetConditions(resource, overlay) - assert.Error(t, err, "[overlayError:0] failed validating value 443 with overlay 444") + assert.Error(t, err, "[overlayError:0] Failed validating value 443 with overlay 444") } func TestMeetConditions_anchorOnPeer(t *testing.T) { @@ -444,7 +444,7 @@ func TestMeetConditions_anchorsOnPeer_two(t *testing.T) { json.Unmarshal(overlayRaw, &overlay) _, err := meetConditions(resource, overlay) - assert.Error(t, err, "[overlayError:0] failed validating value true with overlay false") + assert.Error(t, err, "[overlayError:0] Failed validating value true with overlay false") overlayRaw = []byte(`{ "spec": { @@ -594,7 +594,7 @@ func TestMeetConditions_anchorsOnPeer_multiple(t *testing.T) { json.Unmarshal(overlayRaw, &overlay) _, err = meetConditions(resource, overlay) - assert.Error(t, err, "[overlayError:0] failed validating value ENV_VALUE with overlay ENV_VALUE1") + assert.Error(t, err, "[overlayError:0] Failed validating value ENV_VALUE with overlay ENV_VALUE1") } func TestMeetConditions_AtleastOneExist(t *testing.T) { diff --git a/pkg/engine/overlay_test.go b/pkg/engine/overlay_test.go index 54cadfc7ed..fc93f7e052 100644 --- a/pkg/engine/overlay_test.go +++ b/pkg/engine/overlay_test.go @@ -494,7 +494,7 @@ func TestProcessOverlayPatches_ImagePullPolicy(t *testing.T) { json.Unmarshal(overlayRaw, &overlay) patches, err = processOverlayPatches(resource, overlay) - assert.Error(t, err, "[overlayError:0] Conditions are not met at /spec/template/metadata/labels/app/, [overlayError:0] failed validating value nginx with overlay nginx1") + assert.Error(t, err, "[overlayError:0] Policy not applied, conditions are not met at /spec/template/metadata/labels/app/, [overlayError:0] Failed validating value nginx with overlay nginx1") assert.Assert(t, len(patches) == 0) } @@ -807,7 +807,7 @@ func TestProcessOverlayPatches_anchorOnPeer(t *testing.T) { json.Unmarshal(overlayRaw, &overlay) patches, err = processOverlayPatches(resource, overlay) - assert.Error(t, err, "[overlayError:0] Conditions are not met at /subsets/0/ports/0/port/, [overlayError:0] failed validating value 443 with overlay 444") + assert.Error(t, err, "[overlayError:0] Policy not applied, conditions are not met at /subsets/0/ports/0/port/, [overlayError:0] Failed validating value 443 with overlay 444") assert.Assert(t, len(patches) == 0) }