From ab3e7a73ec085d66a8d0e15e1dcaa4766b440810 Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Fri, 26 May 2017 16:05:51 -0700 Subject: [PATCH] validation of subresources --- .../validation/validation.go | 69 ++++++++++-- .../validation/validation_test.go | 106 +++++++++++++++++- 2 files changed, 161 insertions(+), 14 deletions(-) diff --git a/pkg/apis/admissionregistration/validation/validation.go b/pkg/apis/admissionregistration/validation/validation.go index 5425b16bebb..aa8a0e0a68f 100644 --- a/pkg/apis/admissionregistration/validation/validation.go +++ b/pkg/apis/admissionregistration/validation/validation.go @@ -17,6 +17,7 @@ limitations under the License. package validation import ( + "fmt" "strings" genericvalidation "k8s.io/apimachinery/pkg/api/validation" @@ -66,6 +67,62 @@ func hasWildcard(slice []string) bool { return false } +func validateResources(resources []string, fldPath *field.Path) field.ErrorList { + var allErrors field.ErrorList + if len(resources) == 0 { + allErrors = append(allErrors, field.Required(fldPath, "")) + } + + // */x + resourcesWithWildcardSubresoures := sets.String{} + // x/* + subResoucesWithWildcardResource := sets.String{} + // */* + hasDoubleWildcard := false + // * + hasSingleWildcard := false + // x + hasResourceWithoutSubresource := false + + for i, resSub := range resources { + if resSub == "" { + allErrors = append(allErrors, field.Required(fldPath.Index(i), "")) + continue + } + if resSub == "*/*" { + hasDoubleWildcard = true + } + if resSub == "*" { + hasSingleWildcard = true + } + parts := strings.SplitN(resSub, "/", 2) + if len(parts) == 1 { + hasResourceWithoutSubresource = resSub != "*" + continue + } + res, sub := parts[0], parts[1] + if _, ok := resourcesWithWildcardSubresoures[res]; ok { + allErrors = append(allErrors, field.Invalid(fldPath.Index(i), resources[i], fmt.Sprintf("if '%s/*' is present, must not specify %s", res, resources[i]))) + } + if _, ok := subResoucesWithWildcardResource[sub]; ok { + allErrors = append(allErrors, field.Invalid(fldPath.Index(i), resources[i], fmt.Sprintf("if '*/%s' is present, must not specify %s", sub, resources[i]))) + } + if sub == "*" { + resourcesWithWildcardSubresoures[res] = struct{}{} + } + if res == "*" { + subResoucesWithWildcardResource[sub] = struct{}{} + } + } + if len(resources) > 1 && hasDoubleWildcard { + allErrors = append(allErrors, field.Invalid(fldPath, resources, "if '*/*' is present, must not specify other resources")) + } + if hasSingleWildcard && hasResourceWithoutSubresource { + allErrors = append(allErrors, field.Invalid(fldPath, resources, "if '*' is present, must not specify other resources without subresources")) + } + return allErrors +} + func validateRule(rule *admissionregistration.Rule, fldPath *field.Path) field.ErrorList { var allErrors field.ErrorList if len(rule.APIGroups) == 0 { @@ -86,17 +143,7 @@ func validateRule(rule *admissionregistration.Rule, fldPath *field.Path) field.E allErrors = append(allErrors, field.Required(fldPath.Child("apiVersions").Index(i), "")) } } - if len(rule.Resources) == 0 { - allErrors = append(allErrors, field.Required(fldPath.Child("resources"), "")) - } - if len(rule.Resources) > 1 && hasWildcard(rule.Resources) { - allErrors = append(allErrors, field.Invalid(fldPath.Child("Resources"), rule.Resources, "if '*' is present, must not specify other resources")) - } - for i, resource := range rule.Resources { - if resource == "" { - allErrors = append(allErrors, field.Required(fldPath.Child("resources").Index(i), "")) - } - } + allErrors = append(allErrors, validateResources(rule.Resources, fldPath.Child("resources"))...) return allErrors } diff --git a/pkg/apis/admissionregistration/validation/validation_test.go b/pkg/apis/admissionregistration/validation/validation_test.go index e700d6a3807..d25cb880a7b 100644 --- a/pkg/apis/admissionregistration/validation/validation_test.go +++ b/pkg/apis/admissionregistration/validation/validation_test.go @@ -181,7 +181,7 @@ func TestValidateInitializerConfiguration(t *testing.T) { expectedError: "resources[1]: Required value", }, { - name: "wildcard cannot be mixed with other strings", + name: "wildcard cannot be mixed with other strings for APIGroups or APIVersions", config: getInitializerConfiguration( []admissionregistration.Initializer{ { @@ -190,12 +190,112 @@ func TestValidateInitializerConfiguration(t *testing.T) { { APIGroups: []string{"a", "*"}, APIVersions: []string{"a", "*"}, - Resources: []string{"a", "*"}, + Resources: []string{"a"}, }, }, }, }), - expectedError: `initializers[0].rules[0].apiGroups: Invalid value: []string{"a", "*"}: if '*' is present, must not specify other API groups, initializers[0].rules[0].apiVersions: Invalid value: []string{"a", "*"}: if '*' is present, must not specify other API versions, initializers[0].rules[0].Resources: Invalid value: []string{"a", "*"}: if '*' is present, must not specify other resources`, + expectedError: `initializers[0].rules[0].apiGroups: Invalid value: []string{"a", "*"}: if '*' is present, must not specify other API groups, initializers[0].rules[0].apiVersions: Invalid value: []string{"a", "*"}: if '*' is present, must not specify other API versions`, + }, + { + name: `resource "*" can co-exist with resources that have subresources`, + config: getInitializerConfiguration( + []admissionregistration.Initializer{ + { + Name: "initializer.k8s.io", + Rules: []admissionregistration.Rule{ + { + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"*", "a/b", "a/*", "*/b"}, + }, + }, + }, + }), + }, + { + name: `resource "*" cannot mix with resources that don't have subresources`, + config: getInitializerConfiguration( + []admissionregistration.Initializer{ + { + Name: "initializer.k8s.io", + Rules: []admissionregistration.Rule{ + { + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"*", "a"}, + }, + }, + }, + }), + expectedError: `if '*' is present, must not specify other resources without subresources`, + }, + { + name: "resource a/* cannot mix with a/x", + config: getInitializerConfiguration( + []admissionregistration.Initializer{ + { + Name: "initializer.k8s.io", + Rules: []admissionregistration.Rule{ + { + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"a/*", "a/x"}, + }, + }, + }, + }), + expectedError: `initializers[0].rules[0].resources[1]: Invalid value: "a/x": if 'a/*' is present, must not specify a/x`, + }, + { + name: "resource a/* can mix with a", + config: getInitializerConfiguration( + []admissionregistration.Initializer{ + { + Name: "initializer.k8s.io", + Rules: []admissionregistration.Rule{ + { + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"a/*", "a"}, + }, + }, + }, + }), + }, + { + name: "resource */a cannot mix with x/a", + config: getInitializerConfiguration( + []admissionregistration.Initializer{ + { + Name: "initializer.k8s.io", + Rules: []admissionregistration.Rule{ + { + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"*/a", "x/a"}, + }, + }, + }, + }), + expectedError: `initializers[0].rules[0].resources[1]: Invalid value: "x/a": if '*/a' is present, must not specify x/a`, + }, + { + name: "resource */* cannot mix with other resources", + config: getInitializerConfiguration( + []admissionregistration.Initializer{ + { + Name: "initializer.k8s.io", + Rules: []admissionregistration.Rule{ + { + APIGroups: []string{"a"}, + APIVersions: []string{"a"}, + Resources: []string{"*/*", "a"}, + }, + }, + }, + }), + expectedError: `initializers[0].rules[0].resources: Invalid value: []string{"*/*", "a"}: if '*/*' is present, must not specify other resources`, }, { name: "FailurePolicy can only be \"Ignore\"",