1
0
Fork 0
mirror of https://github.com/kyverno/kyverno.git synced 2025-03-13 19:28:55 +00:00

[Bugbash] Kceu22 bugbash/fix staticcheck warnings (#3917)

* cleanup: error string formating

Fixes Staticcheck ST1005
KubeCon EU 2022 BugBash

Signed-off-by: Dhaval Shah <30974879+dhavalgshah@users.noreply.github.com>

* cleanup: merge var declaration with assignment

Fixes staticcheck S1021

Kubecon EU 2022 Bugbash

Signed-off-by: Dhaval Shah <30974879+dhavalgshah@users.noreply.github.com>

* cleanup normalize yoda condition to simple compare

fixes staticcheck ST1017

Signed-off-by: Dhaval Shah <30974879+dhavalgshah@users.noreply.github.com>

* cleanup: remove extraneous err param on executeTest

err is not used anywhere except to throw Fatal inside execureTest()
fix staticcheck SA4009

Signed-off-by: Dhaval Shah <30974879+dhavalgshah@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Sambhav Kothari <sambhavs.email@gmail.com>
Signed-off-by: Dhaval Shah <30974879+dhavalgshah@users.noreply.github.com>

* fix: match validation error message to actual errors

Signed-off-by: Dhaval Shah <30974879+dhavalgshah@users.noreply.github.com>

* cleanup: more of normalize validation error messages

Signed-off-by: Dhaval Shah <30974879+dhavalgshah@users.noreply.github.com>

* cleanup: additional error message formatting fixes

Signed-off-by: Dhaval Shah <30974879+dhavalgshah@users.noreply.github.com>

Co-authored-by: Sambhav Kothari <sambhavs.email@gmail.com>
This commit is contained in:
Dhaval Shah 2022-05-15 02:34:35 +05:30 committed by GitHub
parent 4d0d719735
commit fce35b91d2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 43 additions and 50 deletions

View file

@ -41,7 +41,7 @@ func validateResourceElement(log logr.Logger, resourceElement, patternElement, o
typedResourceElement, ok := resourceElement.(map[string]interface{})
if !ok {
log.V(4).Info("Pattern and resource have different structures.", "path", path, "expected", fmt.Sprintf("%T", patternElement), "current", fmt.Sprintf("%T", resourceElement))
return path, fmt.Errorf("Pattern and resource have different structures. Path: %s. Expected %T, found %T", path, patternElement, resourceElement)
return path, fmt.Errorf("pattern and resource have different structures. Path: %s. Expected %T, found %T", path, patternElement, resourceElement)
}
return validateMap(log, typedResourceElement, typedPatternElement, originPattern, path)
// array
@ -49,7 +49,7 @@ func validateResourceElement(log logr.Logger, resourceElement, patternElement, o
typedResourceElement, ok := resourceElement.([]interface{})
if !ok {
log.V(4).Info("Pattern and resource have different structures.", "path", path, "expected", fmt.Sprintf("%T", patternElement), "current", fmt.Sprintf("%T", resourceElement))
return path, fmt.Errorf("Validation rule Failed at path %s, resource does not satisfy the expected overlay pattern", path)
return path, fmt.Errorf("validation rule failed at path %s, resource does not satisfy the expected overlay pattern", path)
}
return validateArray(log, typedResourceElement, typedPatternElement, originPattern, path)
// elementary values
@ -87,8 +87,8 @@ func validateMap(log logr.Logger, resourceMap, patternMap map[string]interface{}
// If validateResourceElement detects array element inside resource and pattern trees, it goes to validateArray
func validateArray(log logr.Logger, resourceArray, patternArray []interface{}, originPattern interface{}, path string) (string, error) {
if 0 == len(patternArray) {
return path, fmt.Errorf("Pattern Array empty")
if len(patternArray) == 0 {
return path, fmt.Errorf("pattern Array empty")
}
switch patternArray[0].(type) {
@ -109,7 +109,7 @@ func validateArray(log logr.Logger, resourceArray, patternArray []interface{}, o
}
}
} else {
return "", fmt.Errorf("Validate Array failed, array length mismatch, resource Array len is %d and pattern Array len is %d", len(resourceArray), len(patternArray))
return "", fmt.Errorf("validate Array failed, array length mismatch, resource Array len is %d and pattern Array len is %d", len(resourceArray), len(patternArray))
}
}
return "", nil

View file

@ -854,8 +854,7 @@ func Test_preProcessStrategicMergePatch_multipleAnchors(t *testing.T) {
func toJSON(t *testing.T, b []byte) interface{} {
var i interface{}
var err error
err = json.Unmarshal(b, &i)
err := json.Unmarshal(b, &i)
assert.NilError(t, err)
return i
}

View file

@ -85,7 +85,7 @@ func validateResourceElement(log logr.Logger, resourceElement, patternElement, o
typedResourceElement, ok := resourceElement.([]interface{})
if !ok {
log.V(4).Info("Pattern and resource have different structures.", "path", path, "expected", fmt.Sprintf("%T", patternElement), "current", fmt.Sprintf("%T", resourceElement))
return path, fmt.Errorf("validation rule Failed at path %s, resource does not satisfy the expected overlay pattern", path)
return path, fmt.Errorf("validation rule failed at path %s, resource does not satisfy the expected overlay pattern", path)
}
return validateArray(log, typedResourceElement, typedPatternElement, originPattern, path, ac)
// elementary values

View file

@ -533,10 +533,10 @@ func (v *validator) validatePatterns(resource unstructured.Unstructured) *respon
if pe, ok := err.(*validate.PatternError); ok {
v.log.V(3).Info("validation rule failed", "anyPattern[%d]", idx, "path", pe.Path)
if pe.Path == "" {
patternErr := fmt.Errorf("Rule %s[%d] failed: %s.", v.rule.Name, idx, err.Error())
patternErr := fmt.Errorf("rule %s[%d] failed: %s", v.rule.Name, idx, err.Error())
failedAnyPatternsErrors = append(failedAnyPatternsErrors, patternErr)
} else {
patternErr := fmt.Errorf("Rule %s[%d] failed at path %s.", v.rule.Name, idx, pe.Path)
patternErr := fmt.Errorf("rule %s[%d] failed at path %s", v.rule.Name, idx, pe.Path)
failedAnyPatternsErrors = append(failedAnyPatternsErrors, patternErr)
}
}
@ -596,10 +596,10 @@ func (v *validator) buildErrorMessage(err error, path string) string {
}
if path != "" {
return fmt.Sprintf("validation error: %s Rule %s failed at path %s", msg, v.rule.Name, path)
return fmt.Sprintf("validation error: %s rule %s failed at path %s", msg, v.rule.Name, path)
}
return fmt.Sprintf("validation error: %s Rule %s execution error: %s", msg, v.rule.Name, err.Error())
return fmt.Sprintf("validation error: %s rule %s execution error: %s", msg, v.rule.Name, err.Error())
}
func buildAnyPatternErrorMessage(rule *kyverno.Rule, errors []string) string {

View file

@ -128,7 +128,7 @@ func TestValidate_image_tag_fail(t *testing.T) {
assert.NilError(t, err)
msgs := []string{
"validation rule 'validate-tag' passed.",
"validation error: imagePullPolicy 'Always' required with tag 'latest'. Rule validate-latest failed at path /spec/containers/0/imagePullPolicy/",
"validation error: imagePullPolicy 'Always' required with tag 'latest'. rule validate-latest failed at path /spec/containers/0/imagePullPolicy/",
}
er := Validate(&PolicyContext{Policy: &policy, NewResource: *resourceUnstructured, JSONContext: context.NewContext()})
@ -308,7 +308,7 @@ func TestValidate_Fail_anyPattern(t *testing.T) {
er := Validate(&PolicyContext{Policy: &policy, NewResource: *resourceUnstructured, JSONContext: context.NewContext()})
assert.Assert(t, !er.IsSuccessful())
msgs := []string{"validation error: A namespace is required. Rule check-default-namespace[0] failed at path /metadata/namespace/. Rule check-default-namespace[1] failed at path /metadata/namespace/."}
msgs := []string{"validation error: A namespace is required. rule check-default-namespace[0] failed at path /metadata/namespace/ rule check-default-namespace[1] failed at path /metadata/namespace/"}
for index, r := range er.PolicyResponse.Rules {
assert.Equal(t, r.Message, msgs[index])
}
@ -389,7 +389,7 @@ func TestValidate_host_network_port(t *testing.T) {
resourceUnstructured, err := utils.ConvertToUnstructured(rawResource)
assert.NilError(t, err)
er := Validate(&PolicyContext{Policy: &policy, NewResource: *resourceUnstructured, JSONContext: context.NewContext()})
msgs := []string{"validation error: Host network and port are not allowed. Rule validate-host-network-port failed at path /spec/containers/0/ports/0/hostPort/"}
msgs := []string{"validation error: Host network and port are not allowed. rule validate-host-network-port failed at path /spec/containers/0/ports/0/hostPort/"}
for index, r := range er.PolicyResponse.Rules {
assert.Equal(t, r.Message, msgs[index])
@ -434,7 +434,7 @@ func TestValidate_anchor_arraymap_pass(t *testing.T) {
}
]
}
}
}
`)
rawResource := []byte(`
@ -523,7 +523,7 @@ func TestValidate_anchor_arraymap_fail(t *testing.T) {
}
]
}
}
}
`)
rawResource := []byte(`
@ -567,7 +567,7 @@ func TestValidate_anchor_arraymap_fail(t *testing.T) {
resourceUnstructured, err := utils.ConvertToUnstructured(rawResource)
assert.NilError(t, err)
er := Validate(&PolicyContext{Policy: &policy, NewResource: *resourceUnstructured, JSONContext: context.NewContext()})
msgs := []string{"validation error: Host path '/var/lib/' is not allowed. Rule validate-host-path failed at path /spec/volumes/0/hostPath/path/"}
msgs := []string{"validation error: Host path '/var/lib/' is not allowed. rule validate-host-path failed at path /spec/volumes/0/hostPath/path/"}
for index, r := range er.PolicyResponse.Rules {
assert.Equal(t, r.Message, msgs[index])
@ -938,7 +938,7 @@ func TestValidate_anchor_map_found_invalid(t *testing.T) {
resourceUnstructured, err := utils.ConvertToUnstructured(rawResource)
assert.NilError(t, err)
er := Validate(&PolicyContext{Policy: &policy, NewResource: *resourceUnstructured, JSONContext: context.NewContext()})
msgs := []string{"validation error: pod: validate run as non root user. Rule pod rule 2 failed at path /spec/securityContext/runAsNonRoot/"}
msgs := []string{"validation error: pod: validate run as non root user. rule pod rule 2 failed at path /spec/securityContext/runAsNonRoot/"}
for index, r := range er.PolicyResponse.Rules {
assert.Equal(t, r.Message, msgs[index])
@ -980,7 +980,7 @@ func TestValidate_AnchorList_pass(t *testing.T) {
}
]
}
}
}
`)
rawResource := []byte(`
@ -1003,7 +1003,7 @@ func TestValidate_AnchorList_pass(t *testing.T) {
}
]
}
}
}
`)
var policy kyverno.ClusterPolicy
@ -1055,7 +1055,7 @@ func TestValidate_AnchorList_fail(t *testing.T) {
}
]
}
}
}
`)
rawResource := []byte(`
@ -1078,7 +1078,7 @@ func TestValidate_AnchorList_fail(t *testing.T) {
}
]
}
}
}
`)
var policy kyverno.ClusterPolicy
@ -1125,7 +1125,7 @@ func TestValidate_existenceAnchor_fail(t *testing.T) {
}
]
}
}
}
`)
rawResource := []byte(`
@ -1148,7 +1148,7 @@ func TestValidate_existenceAnchor_fail(t *testing.T) {
}
]
}
}
}
`)
var policy kyverno.ClusterPolicy
@ -1195,7 +1195,7 @@ func TestValidate_existenceAnchor_pass(t *testing.T) {
}
]
}
}
}
`)
rawResource := []byte(`
@ -1218,7 +1218,7 @@ func TestValidate_existenceAnchor_pass(t *testing.T) {
}
]
}
}
}
`)
var policy kyverno.ClusterPolicy
@ -1271,7 +1271,7 @@ func TestValidate_negationAnchor_deny(t *testing.T) {
}
]
}
}
}
`)
rawResource := []byte(`
@ -1316,7 +1316,7 @@ func TestValidate_negationAnchor_deny(t *testing.T) {
resourceUnstructured, err := utils.ConvertToUnstructured(rawResource)
assert.NilError(t, err)
er := Validate(&PolicyContext{Policy: &policy, NewResource: *resourceUnstructured, JSONContext: context.NewContext()})
msgs := []string{"validation error: Host path is not allowed. Rule validate-host-path failed at path /spec/volumes/0/hostPath/"}
msgs := []string{"validation error: Host path is not allowed. rule validate-host-path failed at path /spec/volumes/0/hostPath/"}
for index, r := range er.PolicyResponse.Rules {
assert.Equal(t, r.Message, msgs[index])
@ -1359,7 +1359,7 @@ func TestValidate_negationAnchor_pass(t *testing.T) {
}
]
}
}
}
`)
rawResource := []byte(`
@ -1820,7 +1820,7 @@ func Test_VariableSubstitutionPathNotExistInAnyPattern_AllPathPresent_NonePatter
assert.Equal(t, er.PolicyResponse.Rules[0].Status, response.RuleStatusFail)
assert.Equal(t, er.PolicyResponse.Rules[0].Message,
"validation error: Rule test-path-not-exist[0] failed at path /spec/template/spec/containers/0/name/. Rule test-path-not-exist[1] failed at path /spec/template/spec/containers/0/name/.")
"validation error: rule test-path-not-exist[0] failed at path /spec/template/spec/containers/0/name/ rule test-path-not-exist[1] failed at path /spec/template/spec/containers/0/name/")
}
func Test_VariableSubstitutionValidate_VariablesInMessageAreResolved(t *testing.T) {
@ -2009,9 +2009,8 @@ func Test_denyFeatureIssue744_BlockUpdate(t *testing.T) {
},
}
var err error
for _, testcase := range testcases {
executeTest(t, err, testcase)
executeTest(t, testcase)
}
}
@ -2026,9 +2025,8 @@ func Test_denyFeatureIssue744_DenyAll(t *testing.T) {
},
}
var err error
for _, testcase := range testcases {
executeTest(t, err, testcase)
executeTest(t, testcase)
}
}
@ -2050,9 +2048,8 @@ func Test_denyFeatureIssue744_BlockFields(t *testing.T) {
},
}
var err error
for _, testcase := range testcases {
executeTest(t, err, testcase)
executeTest(t, testcase)
}
}
@ -2067,9 +2064,8 @@ func Test_BlockLabelRemove(t *testing.T) {
},
}
var err error
for _, testcase := range testcases {
executeTest(t, err, testcase)
executeTest(t, testcase)
}
}
@ -2091,15 +2087,14 @@ func Test_denyFeatureIssue744_BlockDelete(t *testing.T) {
},
}
var err error
for _, testcase := range testcases {
executeTest(t, err, testcase)
executeTest(t, testcase)
}
}
func executeTest(t *testing.T, err error, test testCase) {
func executeTest(t *testing.T, test testCase) {
var policy kyverno.ClusterPolicy
err = json.Unmarshal(test.policy, &policy)
err := json.Unmarshal(test.policy, &policy)
if err != nil {
t.Fatal(err)
}
@ -3096,8 +3091,7 @@ func Test_block_bypass(t *testing.T) {
},
}
var err error
for _, testcase := range testcases {
executeTest(t, err, testcase)
executeTest(t, testcase)
}
}

View file

@ -63,7 +63,7 @@ func validateActions(idx int, rule *kyverno.Rule, client dclient.Interface, mock
}
if utils.ContainsString(rule.MatchResources.Kinds, rule.Generation.Kind) {
return fmt.Errorf("generation kind and match resource kind should not be the same.")
return fmt.Errorf("generation kind and match resource kind should not be the same")
}
}

View file

@ -526,7 +526,7 @@ func TestValidate_failure_action_overrides(t *testing.T) {
resourceUnstructured, err := utils.ConvertToUnstructured(tc.rawResource)
assert.NilError(t, err)
msgs := []string{
"validation error: The label 'app' is required. Rule check-label-app failed at path /metadata/labels/",
"validation error: The label 'app' is required. rule check-label-app failed at path /metadata/labels/",
}
er := engine.Validate(&engine.PolicyContext{Policy: &policy, NewResource: *resourceUnstructured, JSONContext: context.NewContext()})

View file

@ -16,5 +16,5 @@ expected:
rules:
- name: prevent-mounting-default-serviceaccount
type: Validation
message: "validation error: Prevent mounting of default service account. Rule prevent-mounting-default-serviceaccount failed at path /spec/serviceAccountName/"
message: "validation error: Prevent mounting of default service account. rule prevent-mounting-default-serviceaccount failed at path /spec/serviceAccountName/"
status: fail

View file

@ -16,5 +16,5 @@ expected:
rules:
- name: validate-selinux-options
type: Validation
message: "validation error: SELinux level is required. Rule validate-selinux-options failed at path /spec/containers/0/securityContext/seLinuxOptions/"
message: "validation error: SELinux level is required. rule validate-selinux-options failed at path /spec/containers/0/securityContext/seLinuxOptions/"
status: fail

View file

@ -15,6 +15,6 @@ expected:
name: image-with-hostpath
rules:
- name: validate-hostPath
message: "validation error: Host path volumes are not allowed. Rule validate-hostPath failed at path /spec/volumes/0/hostPath/"
message: "validation error: Host path volumes are not allowed. rule validate-hostPath failed at path /spec/volumes/0/hostPath/"
type: Validation
status: fail