From be13b041b68de5e8daa7b7c0df8360568b4058f9 Mon Sep 17 00:00:00 2001 From: Maxim Goncharenko Date: Mon, 20 May 2019 14:48:38 +0300 Subject: [PATCH 1/4] Fixed issue with validation error messages --- pkg/engine/generation.go | 22 +---- pkg/engine/mutation.go | 35 ++------ pkg/engine/utils.go | 14 ++-- pkg/engine/validation.go | 149 ++++++++++++++-------------------- pkg/engine/validation_test.go | 104 ++++++++++++++---------- pkg/webhooks/server.go | 2 +- 6 files changed, 140 insertions(+), 186 deletions(-) diff --git a/pkg/engine/generation.go b/pkg/engine/generation.go index 9785487347..b87ab71ff3 100644 --- a/pkg/engine/generation.go +++ b/pkg/engine/generation.go @@ -22,31 +22,15 @@ func Generate(policy kubepolicy.Policy, rawResource []byte, gvk metav1.GroupVers var generateResps []GenerationResponse - for i, rule := range policy.Spec.Rules { - - // Checks for preconditions - // TODO: Rework PolicyEngine interface that it receives not a policy, but mutation object for - // Mutate, validation for Validate and so on. It will allow to bring this checks outside of PolicyEngine - // to common part as far as they present for all: mutation, validation, generation - - err := rule.Validate() - if err != nil { - log.Printf("Rule has invalid structure: rule number = %d, rule name = %s in policy %s, err: %v\n", i, rule.Name, policy.ObjectMeta.Name, err) - continue - } - - ok, err := ResourceMeetsRules(rawResource, rule.ResourceDescription, gvk) - if err != nil { - log.Printf("Rule has invalid data: rule number = %d, rule name = %s in policy %s, err: %v\n", i, rule.Name, policy.ObjectMeta.Name, err) - continue - } + for _, rule := range policy.Spec.Rules { + ok := ResourceMeetsDescription(rawResource, rule.ResourceDescription, gvk) if !ok { log.Printf("Rule is not applicable to the request: rule name = %s in policy %s \n", rule.Name, policy.ObjectMeta.Name) continue } - generateResps, err = applyRuleGenerator(rawResource, rule.Generation) + generateResps, err := applyRuleGenerator(rawResource, rule.Generation) if err != nil { log.Printf("Failed to apply rule generator: %v", err) } else { diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index ec946f10db..a0cde8b9d0 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -13,40 +13,23 @@ import ( func Mutate(policy kubepolicy.Policy, rawResource []byte, gvk metav1.GroupVersionKind) []mutation.PatchBytes { var policyPatches []mutation.PatchBytes - for i, rule := range policy.Spec.Rules { - - // Checks for preconditions - // TODO: Rework PolicyEngine interface that it receives not a policy, but mutation object for - // Mutate, validation for Validate and so on. It will allow to bring this checks outside of PolicyEngine - // to common part as far as they present for all: mutation, validation, generation - - err := rule.Validate() - if err != nil { - log.Printf("Rule has invalid structure: rule number = %d, rule name = %s in policy %s, err: %v\n", i, rule.Name, policy.ObjectMeta.Name, err) - continue - } - - ok, err := ResourceMeetsRules(rawResource, rule.ResourceDescription, gvk) - if err != nil { - log.Printf("Rule has invalid data: rule number = %d, rule name = %s in policy %s, err: %v\n", i, rule.Name, policy.ObjectMeta.Name, err) - continue - } - - if !ok { - log.Printf("Rule is not applicable to the request: rule number = %d, rule name = %s in policy %s, err: %v\n", i, rule.Name, policy.ObjectMeta.Name, err) - continue - } - + for _, rule := range policy.Spec.Rules { if rule.Mutation == nil { continue } + ok := ResourceMeetsDescription(rawResource, rule.ResourceDescription, gvk) + if !ok { + log.Printf("Rule \"%s\" is not applicable to resource\n", rule.Name) + continue + } + // Process Overlay if rule.Mutation.Overlay != nil { overlayPatches, err := mutation.ProcessOverlay(rule.Mutation.Overlay, rawResource) if err != nil { - log.Printf("Overlay application failed: rule number = %d, rule name = %s in policy %s, err: %v\n", i, rule.Name, policy.ObjectMeta.Name, err) + log.Printf("Overlay application has failed for rule %s in policy %s, err: %v\n", rule.Name, policy.ObjectMeta.Name, err) } else { policyPatches = append(policyPatches, overlayPatches...) } @@ -57,7 +40,7 @@ func Mutate(policy kubepolicy.Policy, rawResource []byte, gvk metav1.GroupVersio if rule.Mutation.Patches != nil { processedPatches, err := mutation.ProcessPatches(rule.Mutation.Patches, rawResource) if err != nil { - log.Printf("Patches application failed: rule number = %d, rule name = %s in policy %s, err: %v\n", i, rule.Name, policy.ObjectMeta.Name, err) + log.Printf("Patches application has failed for rule %s in policy %s, err: %v\n", rule.Name, policy.ObjectMeta.Name, err) } else { policyPatches = append(policyPatches, processedPatches...) } diff --git a/pkg/engine/utils.go b/pkg/engine/utils.go index f4ce55a8cb..0646bf33e3 100644 --- a/pkg/engine/utils.go +++ b/pkg/engine/utils.go @@ -11,10 +11,10 @@ import ( "k8s.io/apimachinery/pkg/labels" ) -// ResourceMeetsRules checks requests kind, name and labels to fit the policy -func ResourceMeetsRules(resourceRaw []byte, description kubepolicy.ResourceDescription, gvk metav1.GroupVersionKind) (bool, error) { +// ResourceMeetsDescription checks requests kind, name and labels to fit the policy rule +func ResourceMeetsDescription(resourceRaw []byte, description kubepolicy.ResourceDescription, gvk metav1.GroupVersionKind) bool { if description.Kind != gvk.Kind { - return false, nil + return false } if resourceRaw != nil { @@ -24,7 +24,7 @@ func ResourceMeetsRules(resourceRaw []byte, description kubepolicy.ResourceDescr if description.Name != nil { if !wildcard.Match(*description.Name, name) { - return false, nil + return false } } @@ -32,18 +32,18 @@ func ResourceMeetsRules(resourceRaw []byte, description kubepolicy.ResourceDescr selector, err := metav1.LabelSelectorAsSelector(description.Selector) if err != nil { - return false, err + return false } labelMap := ParseLabelsFromMetadata(meta) if !selector.Matches(labelMap) { - return false, nil + return false } } } - return true, nil + return true } func ParseMetadataFromObject(bytes []byte) map[string]interface{} { diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index 2927fe0c3a..7cc1ec9e49 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -13,7 +13,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// TODO: This operators are already implemented in kubernetes +// Operator is string alias that represents selection operators enum type Operator string const ( @@ -25,66 +25,43 @@ const ( ) // TODO: Refactor using State pattern +// TODO: Return Events and pass all checks to get all validation errors (not ) // Validate handles validating admission request // Checks the target resourse for rules defined in the policy -func Validate(policy kubepolicy.Policy, rawResource []byte, gvk metav1.GroupVersionKind) bool { +func Validate(policy kubepolicy.Policy, rawResource []byte, gvk metav1.GroupVersionKind) error { var resource interface{} json.Unmarshal(rawResource, &resource) - allowed := true - for i, rule := range policy.Spec.Rules { - - // Checks for preconditions - // TODO: Rework PolicyEngine interface that it receives not a policy, but mutation object for - // Mutate, validation for Validate and so on. It will allow to bring this checks outside of PolicyEngine - // to common part as far as they present for all: mutation, validation, generation - - err := rule.Validate() - if err != nil { - log.Printf("Rule has invalid structure: rule number = %d, rule name = %s in policy %s, err: %v\n", i, rule.Name, policy.ObjectMeta.Name, err) - continue - } - - ok, err := ResourceMeetsRules(rawResource, rule.ResourceDescription, gvk) - if err != nil { - log.Printf("Rule has invalid data: rule number = %d, rule name = %s in policy %s, err: %v\n", i, rule.Name, policy.ObjectMeta.Name, err) - continue - } - - if !ok { - log.Printf("Rule is not applicable to the request: rule number = %d, rule name = %s in policy %s, err: %v\n", i, rule.Name, policy.ObjectMeta.Name, err) - continue - } - + for _, rule := range policy.Spec.Rules { if rule.Validation == nil { continue } - if !validateMap(resource, rule.Validation.Pattern) { - log.Printf("Validation with the rule %s has failed: %s\n", rule.Name, *rule.Validation.Message) - allowed = false - } else { - log.Printf("Validation rule %s is successful\n", rule.Name) + ok := ResourceMeetsDescription(rawResource, rule.ResourceDescription, gvk) + if !ok { + log.Printf("Rule \"%s\" is not applicable to resource\n", rule.Name) + continue + } + + if err := validateMap(resource, rule.Validation.Pattern); err != nil { + return fmt.Errorf("%s: %s", *rule.Validation.Message, err.Error()) } } - return allowed + log.Println("Validation is successful") + return nil } -func validateMap(resourcePart, patternPart interface{}) bool { +func validateMap(resourcePart, patternPart interface{}) error { pattern, ok := patternPart.(map[string]interface{}) - if !ok { - fmt.Printf("Validating error: expected Map, found %T\n", patternPart) - return false + return fmt.Errorf("Expected map, found %T", patternPart) } resource, ok := resourcePart.(map[string]interface{}) - if !ok { - fmt.Printf("Validating error: expected Map, found %T\n", resourcePart) - return false + return fmt.Errorf("Expected map, found %T", resourcePart) } for key, value := range pattern { @@ -92,82 +69,70 @@ func validateMap(resourcePart, patternPart interface{}) bool { key = key[1 : len(key)-1] } - if !validateMapElement(resource[key], value) { - return false + if err := validateMapElement(resource[key], value); err != nil { + return err } } - return true + return nil } -func validateArray(resourcePart, patternPart interface{}) bool { +func validateArray(resourcePart, patternPart interface{}) error { patternArray, ok := patternPart.([]interface{}) - if !ok { - fmt.Printf("Validating error: expected array, found %T\n", patternPart) - return false + return fmt.Errorf("Expected array, found %T", patternPart) } resourceArray, ok := resourcePart.([]interface{}) - if !ok { - fmt.Printf("Validating error: expected array, found %T\n", resourcePart) - return false + return fmt.Errorf("Expected array, found %T", resourcePart) } switch pattern := patternArray[0].(type) { case map[string]interface{}: anchors, err := getAnchorsFromMap(pattern) if err != nil { - fmt.Printf("Validating error: %v\n", err) - return false + return err } for _, value := range resourceArray { resource, ok := value.(map[string]interface{}) if !ok { - fmt.Printf("Validating error: expected Map, found %T\n", resourcePart) - return false + return fmt.Errorf("Expected array, found %T", resourcePart) } if skipArrayObject(resource, anchors) { continue } - if !validateMap(resource, pattern) { - return false + if err := validateMap(resource, pattern); err != nil { + return err } } - - return true default: for _, value := range resourceArray { - if !checkSingleValue(value, patternArray[0]) { - return false + if err := checkSingleValue(value, patternArray[0]); err != nil { + return err } } } - return true + return nil } -func validateMapElement(resourcePart, patternPart interface{}) bool { +func validateMapElement(resourcePart, patternPart interface{}) error { switch pattern := patternPart.(type) { case map[string]interface{}: dictionary, ok := resourcePart.(map[string]interface{}) - if !ok { - fmt.Printf("Validating error: expected %T, found %T\n", patternPart, resourcePart) - return false + return fmt.Errorf("Expected %T, found %T", patternPart, resourcePart) } return validateMap(dictionary, pattern) case []interface{}: array, ok := resourcePart.([]interface{}) - if !ok { - fmt.Printf("Validating error: expected %T, found %T\n", patternPart, resourcePart) - return false + return fmt.Errorf("Expected %T, found %T", patternPart, resourcePart) } return validateArray(array, pattern) @@ -175,14 +140,12 @@ func validateMapElement(resourcePart, patternPart interface{}) bool { str, ok := resourcePart.(string) if !ok { - fmt.Printf("Validating error: expected %T, found %T\n", patternPart, resourcePart) - return false + return fmt.Errorf("Expected %T, found %T", patternPart, resourcePart) } return checkSingleValue(str, pattern) default: - fmt.Printf("Validating error: unknown type in map: %T\n", patternPart) - return false + return fmt.Errorf("Validating error: unknown type in map: %T", patternPart) } } @@ -207,7 +170,7 @@ func skipArrayObject(object, anchors map[string]interface{}) bool { return true } - if !checkSingleValue(value, pattern) { + if err := checkSingleValue(value, pattern); err != nil { return true } } @@ -215,7 +178,7 @@ func skipArrayObject(object, anchors map[string]interface{}) bool { return false } -func checkSingleValue(value, pattern interface{}) bool { +func checkSingleValue(value, pattern interface{}) error { switch typedPattern := pattern.(type) { case string: switch typedValue := value.(type) { @@ -226,46 +189,54 @@ func checkSingleValue(value, pattern interface{}) bool { case int: return checkForOperator(float64(typedValue), typedPattern) default: - fmt.Printf("Validating error: expected string or numerical type, found %T, pattern: %s\n", value, typedPattern) - return false + return fmt.Errorf("Expected string or numerical type, found %T, pattern: %s", value, typedPattern) } case float64: num, ok := value.(float64) if !ok { - fmt.Printf("Validating error: expected float, found %T\n", value) - return false + return fmt.Errorf("Expected float, found %T", value) } - return typedPattern == num + if typedPattern != num { + return fmt.Errorf("Value %f is not equal to pattern %f", value, typedPattern) + } case int: num, ok := value.(int) if !ok { - fmt.Printf("Validating error: expected int, found %T\n", value) - return false + return fmt.Errorf("Expected int, found %T", value) } - return typedPattern == num + if typedPattern != num { + return fmt.Errorf("Value %d is not equal to pattern %d", num, typedPattern) + } default: - fmt.Printf("Validating error: expected pattern (string or numerical type), found %T\n", pattern) - return false + return fmt.Errorf("Expected pattern (string or numerical type), found %T", pattern) } + + return nil } -func checkForWildcard(value, pattern string) bool { - return wildcard.Match(pattern, value) +func checkForWildcard(value, pattern string) error { + if !wildcard.Match(pattern, value) { + return fmt.Errorf("Wildcard check has failed. Pattern: \"%s\". Value: \"%s\"", pattern, value) + } + + return nil } -func checkForOperator(value float64, pattern string) bool { +func checkForOperator(value float64, pattern string) error { operators := strings.Split(pattern, "|") for _, operator := range operators { operator = strings.Replace(operator, " ", "", -1) + + // At least one success - return nil if checkSingleOperator(value, operator) { - return true + return nil } } - return false + return fmt.Errorf("Operator check has failed. Pattern: \"%s\". Value: \"%f\"", pattern, value) } func checkSingleOperator(value float64, pattern string) bool { diff --git a/pkg/engine/validation_test.go b/pkg/engine/validation_test.go index 68d07ca73e..93f5ca7360 100644 --- a/pkg/engine/validation_test.go +++ b/pkg/engine/validation_test.go @@ -4,7 +4,9 @@ import ( "encoding/json" "testing" + kubepolicy "github.com/nirmata/kube-policy/pkg/apis/policy/v1alpha1" "gotest.tools/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestWrappedWithParentheses_StringIsWrappedWithParentheses(t *testing.T) { @@ -62,190 +64,190 @@ func TestCheckForWildcard_LeftAsteriskTest(t *testing.T) { value = "leftmiddle" middle := "middle" - assert.Assert(t, !checkForWildcard(value, pattern)) - assert.Assert(t, !checkForWildcard(middle, pattern)) + assert.Assert(t, checkForWildcard(value, pattern) != nil) + assert.Assert(t, checkForWildcard(middle, pattern) != nil) } func TestCheckForWildcard_MiddleAsteriskTest(t *testing.T) { pattern := "ab*ba" - value := "abbba" - assert.Assert(t, checkForWildcard(value, pattern)) + value := "abbeba" + assert.NilError(t, checkForWildcard(value, pattern)) value = "abbca" - assert.Assert(t, !checkForWildcard(value, pattern)) + assert.Assert(t, checkForWildcard(value, pattern) != nil) } func TestCheckForWildcard_QuestionMark(t *testing.T) { pattern := "ab?ba" value := "abbba" - assert.Assert(t, checkForWildcard(value, pattern)) + assert.NilError(t, checkForWildcard(value, pattern)) value = "abbbba" - assert.Assert(t, !checkForWildcard(value, pattern)) + assert.Assert(t, checkForWildcard(value, pattern) != nil) } func TestCheckSingleValue_CheckInt(t *testing.T) { pattern := 89 value := 89 - assert.Assert(t, checkSingleValue(value, pattern)) + assert.NilError(t, checkSingleValue(value, pattern)) value = 202 - assert.Assert(t, !checkSingleValue(value, pattern)) + assert.Assert(t, checkSingleValue(value, pattern) != nil) } func TestCheckSingleValue_CheckFloat(t *testing.T) { pattern := 89.9091 value := 89.9091 - assert.Assert(t, checkSingleValue(value, pattern)) + assert.NilError(t, checkSingleValue(value, pattern)) value = 89.9092 - assert.Assert(t, !checkSingleValue(value, pattern)) + assert.Assert(t, checkSingleValue(value, pattern) != nil) } func TestCheckSingleValue_CheckOperatorMoreEqual(t *testing.T) { pattern := " >= 89 " value := 89 - assert.Assert(t, checkSingleValue(value, pattern)) + assert.NilError(t, checkSingleValue(value, pattern)) pattern = ">=10.0001" floatValue := 89.901 - assert.Assert(t, checkSingleValue(floatValue, pattern)) + assert.NilError(t, checkSingleValue(floatValue, pattern)) } func TestCheckSingleValue_CheckOperatorMoreEqualFail(t *testing.T) { pattern := " >= 90 " value := 89 - assert.Assert(t, !checkSingleValue(value, pattern)) + assert.Assert(t, checkSingleValue(value, pattern) != nil) pattern = ">=910.0001" floatValue := 89.901 - assert.Assert(t, !checkSingleValue(floatValue, pattern)) + assert.Assert(t, checkSingleValue(floatValue, pattern) != nil) } func TestCheckSingleValue_CheckOperatorLessEqual(t *testing.T) { pattern := " <= 1 " value := 1 - assert.Assert(t, checkSingleValue(value, pattern)) + assert.NilError(t, checkSingleValue(value, pattern)) pattern = "<=10.0001" floatValue := 1.901 - assert.Assert(t, checkSingleValue(floatValue, pattern)) + assert.NilError(t, checkSingleValue(floatValue, pattern)) } func TestCheckSingleValue_CheckOperatorLessEqualFail(t *testing.T) { pattern := " <= 0.1558 " value := 1 - assert.Assert(t, !checkSingleValue(value, pattern)) + assert.Assert(t, checkSingleValue(value, pattern) != nil) pattern = "<=10.0001" floatValue := 12.901 - assert.Assert(t, !checkSingleValue(floatValue, pattern)) + assert.Assert(t, checkSingleValue(floatValue, pattern) != nil) } func TestCheckSingleValue_CheckOperatorMore(t *testing.T) { pattern := " > 10 " value := 89 - assert.Assert(t, checkSingleValue(value, pattern)) + assert.NilError(t, checkSingleValue(value, pattern)) pattern = ">10.0001" floatValue := 89.901 - assert.Assert(t, checkSingleValue(floatValue, pattern)) + assert.NilError(t, checkSingleValue(floatValue, pattern)) } func TestCheckSingleValue_CheckOperatorMoreFail(t *testing.T) { pattern := " > 89 " value := 89 - assert.Assert(t, !checkSingleValue(value, pattern)) + assert.Assert(t, checkSingleValue(value, pattern) != nil) pattern = ">910.0001" floatValue := 89.901 - assert.Assert(t, !checkSingleValue(floatValue, pattern)) + assert.Assert(t, checkSingleValue(floatValue, pattern) != nil) } func TestCheckSingleValue_CheckOperatorLess(t *testing.T) { pattern := " < 10 " value := 9 - assert.Assert(t, checkSingleValue(value, pattern)) + assert.NilError(t, checkSingleValue(value, pattern)) pattern = "<10.0001" floatValue := 9.901 - assert.Assert(t, checkSingleValue(floatValue, pattern)) + assert.NilError(t, checkSingleValue(floatValue, pattern)) } func TestCheckSingleValue_CheckOperatorLessFail(t *testing.T) { pattern := " < 10 " value := 10 - assert.Assert(t, !checkSingleValue(value, pattern)) + assert.Assert(t, checkSingleValue(value, pattern) != nil) pattern = "<10.0001" floatValue := 19.901 - assert.Assert(t, !checkSingleValue(floatValue, pattern)) + assert.Assert(t, checkSingleValue(floatValue, pattern) != nil) } func TestCheckSingleValue_CheckOperatorNotEqual(t *testing.T) { pattern := " != 10 " value := 9.99999 - assert.Assert(t, checkSingleValue(value, pattern)) + assert.NilError(t, checkSingleValue(value, pattern)) pattern = "!=10.0001" floatValue := 10.0000 - assert.Assert(t, checkSingleValue(floatValue, pattern)) + assert.NilError(t, checkSingleValue(floatValue, pattern)) } func TestCheckSingleValue_CheckOperatorNotEqualFail(t *testing.T) { pattern := " != 9.99999 " value := 9.99999 - assert.Assert(t, !checkSingleValue(value, pattern)) + assert.Assert(t, checkSingleValue(value, pattern) != nil) pattern = "!=10" floatValue := 10 - assert.Assert(t, !checkSingleValue(floatValue, pattern)) + assert.Assert(t, checkSingleValue(floatValue, pattern) != nil) } func TestCheckSingleValue_CheckOperatorEqual(t *testing.T) { pattern := " 10.000001 " value := 10.000001 - assert.Assert(t, checkSingleValue(value, pattern)) + assert.NilError(t, checkSingleValue(value, pattern)) pattern = "10.000000" floatValue := 10 - assert.Assert(t, checkSingleValue(floatValue, pattern)) + assert.NilError(t, checkSingleValue(floatValue, pattern)) } func TestCheckSingleValue_CheckOperatorEqualFail(t *testing.T) { pattern := " 10.000000 " value := 10.000001 - assert.Assert(t, !checkSingleValue(value, pattern)) + assert.Assert(t, checkSingleValue(value, pattern) != nil) pattern = "10.000001" floatValue := 10 - assert.Assert(t, !checkSingleValue(floatValue, pattern)) + assert.Assert(t, checkSingleValue(floatValue, pattern) != nil) } func TestCheckSingleValue_CheckSeveralOperators(t *testing.T) { pattern := " <-1 | 10.000001 " value := 10.000001 - assert.Assert(t, checkSingleValue(value, pattern)) + assert.NilError(t, checkSingleValue(value, pattern)) value = -30 - assert.Assert(t, checkSingleValue(value, pattern)) + assert.NilError(t, checkSingleValue(value, pattern)) value = 5 - assert.Assert(t, !checkSingleValue(value, pattern)) + assert.Assert(t, checkSingleValue(value, pattern) != nil) } func TestCheckSingleValue_CheckWildcard(t *testing.T) { pattern := "nirmata_*" value := "nirmata_awesome" - assert.Assert(t, checkSingleValue(value, pattern)) + assert.NilError(t, checkSingleValue(value, pattern)) pattern = "nirmata_*" value = "spasex_awesome" - assert.Assert(t, !checkSingleValue(value, pattern)) + assert.Assert(t, checkSingleValue(value, pattern) != nil) pattern = "g?t" value = "git" - assert.Assert(t, checkSingleValue(value, pattern)) + assert.NilError(t, checkSingleValue(value, pattern)) } func TestSkipArrayObject_OneAnchor(t *testing.T) { @@ -330,7 +332,7 @@ func TestValidateMapElement_TwoElementsInArrayOnePass(t *testing.T) { json.Unmarshal(rawPattern, &pattern) json.Unmarshal(rawMap, &resource) - assert.Assert(t, validateMapElement(resource, pattern)) + assert.NilError(t, validateMapElement(resource, pattern)) } func TestValidateMapElement_OneElementInArrayPass(t *testing.T) { @@ -341,7 +343,7 @@ func TestValidateMapElement_OneElementInArrayPass(t *testing.T) { json.Unmarshal(rawPattern, &pattern) json.Unmarshal(rawMap, &resource) - assert.Assert(t, validateMapElement(resource, pattern)) + assert.NilError(t, validateMapElement(resource, pattern)) } func TestValidateMapElement_OneElementInArrayNotPass(t *testing.T) { @@ -352,5 +354,19 @@ func TestValidateMapElement_OneElementInArrayNotPass(t *testing.T) { json.Unmarshal(rawPattern, &pattern) json.Unmarshal(rawMap, &resource) - assert.Assert(t, !validateMapElement(resource, pattern)) + assert.Assert(t, validateMapElement(resource, pattern) != nil) +} + +func TestValidate_ServiceTest(t *testing.T) { + rawPolicy := []byte(`{ "apiVersion": "kubepolicy.nirmata.io/v1alpha1", "kind": "Policy", "metadata": { "name": "policy-service" }, "spec": { "rules": [ { "name": "ps1", "resource": { "kind": "Service", "name": "game-service*" }, "mutate": { "patches": [ { "path": "/metadata/labels/isMutated", "op": "add", "value": "true" }, { "path": "/metadata/labels/secretLabel", "op": "replace", "value": "weKnow" }, { "path": "/metadata/labels/originalLabel", "op": "remove" }, { "path": "/spec/selector/app", "op": "replace", "value": "mutedApp" } ] }, "validate": { "message": "This resource is broken", "pattern": { "spec": { "ports": [ { "name": "hs", "protocol": 32 } ] } } } } ] } }`) + rawResource := []byte(`{ "kind": "Service", "apiVersion": "v1", "metadata": { "name": "game-service", "labels": { "originalLabel": "isHere", "secretLabel": "thisIsMySecret" } }, "spec": { "selector": { "app": "MyApp" }, "ports": [ { "name": "http", "protocol": "TCP", "port": 80, "targetPort": 9376 } ] } }`) + + var policy kubepolicy.Policy + json.Unmarshal(rawPolicy, &policy) + + gvk := metav1.GroupVersionKind{ + Kind: "Service", + } + + assert.Assert(t, Validate(policy, rawResource, gvk) != nil) } diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index 61a6e5cba5..e671af98fa 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -176,7 +176,7 @@ func (ws *WebhookServer) HandleValidation(request *v1beta1.AdmissionRequest) *v1 for _, policy := range policies { ws.logger.Printf("Validating resource with policy %s with %d rules", policy.ObjectMeta.Name, len(policy.Spec.Rules)) - if ok := engine.Validate(*policy, request.Object.Raw, request.Kind); !ok { + if err := engine.Validate(*policy, request.Object.Raw, request.Kind); err != nil { ws.logger.Printf("Validation has failed: %v\n", err) utilruntime.HandleError(err) allowed = false From 8f3361e96b3caae2e90e707188883d2ed1731b64 Mon Sep 17 00:00:00 2001 From: Maxim Goncharenko Date: Mon, 20 May 2019 15:41:23 +0300 Subject: [PATCH 2/4] Fixed issue with no message on errorness validation for user --- pkg/engine/validation.go | 41 +++++++++++++++++++++++----------------- pkg/webhooks/server.go | 25 ++++++++++++++---------- 2 files changed, 39 insertions(+), 27 deletions(-) diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index 7cc1ec9e49..35d40393ab 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -45,6 +45,13 @@ func Validate(policy kubepolicy.Policy, rawResource []byte, gvk metav1.GroupVers } if err := validateMap(resource, rule.Validation.Pattern); err != nil { + message := *rule.Validation.Message + if len(message) == 0 { + message = fmt.Sprintf("%v", err) + } else { + message = fmt.Sprintf("%s, %s", message, err.Error()) + } + return fmt.Errorf("%s: %s", *rule.Validation.Message, err.Error()) } } @@ -56,12 +63,12 @@ func Validate(policy kubepolicy.Policy, rawResource []byte, gvk metav1.GroupVers func validateMap(resourcePart, patternPart interface{}) error { pattern, ok := patternPart.(map[string]interface{}) if !ok { - return fmt.Errorf("Expected map, found %T", patternPart) + return fmt.Errorf("expected map, found %T", patternPart) } resource, ok := resourcePart.(map[string]interface{}) if !ok { - return fmt.Errorf("Expected map, found %T", resourcePart) + return fmt.Errorf("expected map, found %T", resourcePart) } for key, value := range pattern { @@ -80,12 +87,12 @@ func validateMap(resourcePart, patternPart interface{}) error { func validateArray(resourcePart, patternPart interface{}) error { patternArray, ok := patternPart.([]interface{}) if !ok { - return fmt.Errorf("Expected array, found %T", patternPart) + return fmt.Errorf("expected array, found %T", patternPart) } resourceArray, ok := resourcePart.([]interface{}) if !ok { - return fmt.Errorf("Expected array, found %T", resourcePart) + return fmt.Errorf("expected array, found %T", resourcePart) } switch pattern := patternArray[0].(type) { @@ -98,7 +105,7 @@ func validateArray(resourcePart, patternPart interface{}) error { for _, value := range resourceArray { resource, ok := value.(map[string]interface{}) if !ok { - return fmt.Errorf("Expected array, found %T", resourcePart) + return fmt.Errorf("expected array, found %T", resourcePart) } if skipArrayObject(resource, anchors) { @@ -125,14 +132,14 @@ func validateMapElement(resourcePart, patternPart interface{}) error { case map[string]interface{}: dictionary, ok := resourcePart.(map[string]interface{}) if !ok { - return fmt.Errorf("Expected %T, found %T", patternPart, resourcePart) + return fmt.Errorf("expected %T, found %T", patternPart, resourcePart) } return validateMap(dictionary, pattern) case []interface{}: array, ok := resourcePart.([]interface{}) if !ok { - return fmt.Errorf("Expected %T, found %T", patternPart, resourcePart) + return fmt.Errorf("expected %T, found %T", patternPart, resourcePart) } return validateArray(array, pattern) @@ -140,12 +147,12 @@ func validateMapElement(resourcePart, patternPart interface{}) error { str, ok := resourcePart.(string) if !ok { - return fmt.Errorf("Expected %T, found %T", patternPart, resourcePart) + return fmt.Errorf("expected %T, found %T", patternPart, resourcePart) } return checkSingleValue(str, pattern) default: - return fmt.Errorf("Validating error: unknown type in map: %T", patternPart) + return fmt.Errorf("validating error: unknown type in map: %T", patternPart) } } @@ -189,28 +196,28 @@ func checkSingleValue(value, pattern interface{}) error { case int: return checkForOperator(float64(typedValue), typedPattern) default: - return fmt.Errorf("Expected string or numerical type, found %T, pattern: %s", value, typedPattern) + return fmt.Errorf("expected string or numerical type, found %T, pattern: %s", value, typedPattern) } case float64: num, ok := value.(float64) if !ok { - return fmt.Errorf("Expected float, found %T", value) + return fmt.Errorf("expected float, found %T", value) } if typedPattern != num { - return fmt.Errorf("Value %f is not equal to pattern %f", value, typedPattern) + return fmt.Errorf("value %f is not equal to pattern %f", value, typedPattern) } case int: num, ok := value.(int) if !ok { - return fmt.Errorf("Expected int, found %T", value) + return fmt.Errorf("expected int, found %T", value) } if typedPattern != num { - return fmt.Errorf("Value %d is not equal to pattern %d", num, typedPattern) + return fmt.Errorf("value %d is not equal to pattern %d", num, typedPattern) } default: - return fmt.Errorf("Expected pattern (string or numerical type), found %T", pattern) + return fmt.Errorf("expected pattern (string or numerical type), found %T", pattern) } return nil @@ -218,7 +225,7 @@ func checkSingleValue(value, pattern interface{}) error { func checkForWildcard(value, pattern string) error { if !wildcard.Match(pattern, value) { - return fmt.Errorf("Wildcard check has failed. Pattern: \"%s\". Value: \"%s\"", pattern, value) + return fmt.Errorf("wildcard check has failed. Pattern: \"%s\". Value: \"%s\"", pattern, value) } return nil @@ -236,7 +243,7 @@ func checkForOperator(value float64, pattern string) error { } } - return fmt.Errorf("Operator check has failed. Pattern: \"%s\". Value: \"%f\"", pattern, value) + return fmt.Errorf("operator check has failed. Pattern: \"%s\". Value: \"%f\"", pattern, value) } func checkSingleOperator(value float64, pattern string) bool { diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index e671af98fa..4842004989 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -18,8 +18,8 @@ import ( "github.com/nirmata/kube-policy/pkg/engine/mutation" tlsutils "github.com/nirmata/kube-policy/pkg/tls" v1beta1 "k8s.io/api/admission/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" ) // WebhookServer contains configured TLS server with MutationWebhook. @@ -135,7 +135,7 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest) *v1be policies, err := ws.policyLister.List(labels.NewSelector()) if err != nil { - utilruntime.HandleError(err) + ws.logger.Printf("%v", err) return nil } @@ -168,26 +168,31 @@ func (ws *WebhookServer) HandleValidation(request *v1beta1.AdmissionRequest) *v1 policies, err := ws.policyLister.List(labels.NewSelector()) if err != nil { - utilruntime.HandleError(err) + ws.logger.Printf("%v", err) return nil } - allowed := true for _, policy := range policies { ws.logger.Printf("Validating resource with policy %s with %d rules", policy.ObjectMeta.Name, len(policy.Spec.Rules)) if err := engine.Validate(*policy, request.Object.Raw, request.Kind); err != nil { - ws.logger.Printf("Validation has failed: %v\n", err) - utilruntime.HandleError(err) - allowed = false - } else { - ws.logger.Println("Validation is successful") + message := fmt.Sprintf("validation has failed: %s", err.Error()) + ws.logger.Println(message) + + return &v1beta1.AdmissionResponse{ + Allowed: false, + Result: &metav1.Status{ + Message: message, + }, + } } } + ws.logger.Println("Validation is successful") return &v1beta1.AdmissionResponse{ - Allowed: allowed, + Allowed: true, } + } // bodyToAdmissionReview creates AdmissionReview object from request body From 0aebb2a88e7b564b506628d2d9a27a4660e61978 Mon Sep 17 00:00:00 2001 From: Maxim Goncharenko Date: Mon, 20 May 2019 17:07:09 +0300 Subject: [PATCH 3/4] Fixed int and float types mismatch --- pkg/engine/validation.go | 28 ++++++++++++++++++++++++++++ pkg/engine/validation_test.go | 14 ++++++++++++++ pkg/webhooks/server.go | 2 +- 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index 35d40393ab..77521deee7 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -151,9 +151,37 @@ func validateMapElement(resourcePart, patternPart interface{}) error { } return checkSingleValue(str, pattern) + case float64: + switch num := resourcePart.(type) { + case float64: + if num != pattern { + return fmt.Errorf("%f not equal %f", num, pattern) + } + case int64: + if float64(num) != pattern { + return fmt.Errorf("%d not equal %f", num, pattern) + } + default: + return fmt.Errorf("expected %T, found %T", patternPart, resourcePart) + } + case int64: + switch num := resourcePart.(type) { + case float64: + if num != float64(pattern) { + return fmt.Errorf("%f not equal %d", num, pattern) + } + case int64: + if float64(num) != float64(num) { + return fmt.Errorf("%d not equal %d", num, pattern) + } + default: + return fmt.Errorf("expected %T, found %T", patternPart, resourcePart) + } default: return fmt.Errorf("validating error: unknown type in map: %T", patternPart) } + + return nil } func getAnchorsFromMap(pattern map[string]interface{}) (map[string]interface{}, error) { diff --git a/pkg/engine/validation_test.go b/pkg/engine/validation_test.go index 93f5ca7360..eac1a884bf 100644 --- a/pkg/engine/validation_test.go +++ b/pkg/engine/validation_test.go @@ -370,3 +370,17 @@ func TestValidate_ServiceTest(t *testing.T) { assert.Assert(t, Validate(policy, rawResource, gvk) != nil) } + +func TestValidate_MapHasFloats(t *testing.T) { + rawPolicy := []byte(`{ "apiVersion": "kubepolicy.nirmata.io/v1alpha1", "kind": "Policy", "metadata": { "name": "policy-deployment-changed" }, "spec": { "rules": [ { "name": "First policy v2", "resource": { "kind": "Deployment", "name": "nginx-*" }, "mutate": { "patches": [ { "path": "/metadata/labels/isMutated", "op": "add", "value": "true" }, { "path": "/metadata/labels/app", "op": "replace", "value": "nginx_is_mutated" } ] }, "validate": { "message": "replicas number is wrong", "pattern": { "metadata": { "labels": { "app": "*" } }, "spec": { "replicas": 3 } } } } ] } }`) + rawResource := []byte(`{ "apiVersion": "apps/v1", "kind": "Deployment", "metadata": { "name": "nginx-deployment", "labels": { "app": "nginx" } }, "spec": { "replicas": 3, "selector": { "matchLabels": { "app": "nginx" } }, "template": { "metadata": { "labels": { "app": "nginx" } }, "spec": { "containers": [ { "name": "nginx", "image": "nginx:1.7.9", "ports": [ { "containerPort": 80 } ] } ] } } } }`) + + var policy kubepolicy.Policy + json.Unmarshal(rawPolicy, &policy) + + gvk := metav1.GroupVersionKind{ + Kind: "Deployment", + } + + assert.NilError(t, Validate(policy, rawResource, gvk)) +} diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index 4842004989..4d85a2ca02 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -149,7 +149,7 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest) *v1be if len(policyPatches) > 0 { namespace := engine.ParseNamespaceFromObject(request.Object.Raw) name := engine.ParseNameFromObject(request.Object.Raw) - ws.logger.Printf("Policy %s applied to %s %s/%s", policy.Name, request.Kind.Kind, namespace, name) + ws.logger.Printf("Mutation from policy %s has applied to %s %s/%s", policy.Name, request.Kind.Kind, namespace, name) } } From 500e8d7e164cb2d8c307b4bc4a6a5faf4a72d249 Mon Sep 17 00:00:00 2001 From: Maxim Goncharenko Date: Mon, 20 May 2019 18:28:54 +0300 Subject: [PATCH 4/4] Fixed string and float type mismatches --- pkg/engine/validation.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index 77521deee7..a6b55cadaf 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -144,13 +144,7 @@ func validateMapElement(resourcePart, patternPart interface{}) error { return validateArray(array, pattern) case string: - str, ok := resourcePart.(string) - - if !ok { - return fmt.Errorf("expected %T, found %T", patternPart, resourcePart) - } - - return checkSingleValue(str, pattern) + return checkSingleValue(resourcePart, patternPart) case float64: switch num := resourcePart.(type) { case float64: