validation of subresources

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, validateResources(rule.Resources, fldPath.Child("resources"))...)
-		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), ""))
-		}
-	}
 	return allErrors
 }
 

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\"",