do not allow subresources in initializer rules

pkg/apis/admissionregistration/types.go

@@ -64,6 +64,7 @@ type Initializer struct {
 
 	// Rules describes what resources/subresources the initializer cares about.
 	// The initializer cares about an operation if it matches _any_ Rule.
+	// Rule.Resources must not include subresources.
 	Rules []Rule
 
 	// FailurePolicy defines what happens if the responsible initializer controller
@@ -97,7 +98,10 @@ type Rule struct {
 	// '*/scale' means all scale subresources.
 	// '*/*' means all resources and their subresources.
 	//
-	// If '*' or '*/*' is present, the length of the slice must be one.
+	// If wildcard is present, the validation rule will ensure resources do not
+	// overlap with each other.
+	//
+	// Depending on the enclosing object, subresources might not be allowed.
 	// Required.
 	Resources []string
 }

pkg/apis/admissionregistration/v1alpha1/types.go

@@ -66,6 +66,7 @@ type Initializer struct {
 
 	// Rules describes what resources/subresources the initializer cares about.
 	// The initializer cares about an operation if it matches _any_ Rule.
+	// Rule.Resources must not include subresources.
 	Rules []Rule `json:"rules,omitempty" protobuf:"bytes,2,rep,name=rules"`
 
 	// FailurePolicy defines what happens if the responsible initializer controller
@@ -99,7 +100,10 @@ type Rule struct {
 	// '*/scale' means all scale subresources.
 	// '*/*' means all resources and their subresources.
 	//
-	// If '*' or '*/*' is present, the length of the slice must be one.
+	// If wildcard is present, the validation rule will ensure resources do not
+	// overlap with each other.
+	//
+	// Depending on the enclosing object, subresources might not be allowed.
 	// Required.
 	Resources []string `json:"resources,omitempty" protobuf:"bytes,3,rep,name=resources"`
 }

pkg/apis/admissionregistration/validation/validation.go

@@ -49,7 +49,8 @@ func validateInitializer(initializer *admissionregistration.Initializer, fldPath
 	}
 
 	for i, rule := range initializer.Rules {
-		allErrors = append(allErrors, validateRule(&rule, fldPath.Child("rules").Index(i))...)
+		notAllowSubresources := false
+		allErrors = append(allErrors, validateRule(&rule, fldPath.Child("rules").Index(i), notAllowSubresources)...)
 	}
 	// TODO: relax the validation rule when admissionregistration is beta.
 	if initializer.FailurePolicy != nil && *initializer.FailurePolicy != admissionregistration.Ignore {
@@ -102,10 +103,10 @@ func validateResources(resources []string, fldPath *field.Path) field.ErrorList
 		}
 		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])))
+			allErrors = append(allErrors, field.Invalid(fldPath.Index(i), resSub, fmt.Sprintf("if '%s/*' is present, must not specify %s", res, resSub)))
 		}
 		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])))
+			allErrors = append(allErrors, field.Invalid(fldPath.Index(i), resSub, fmt.Sprintf("if '*/%s' is present, must not specify %s", sub, resSub)))
 		}
 		if sub == "*" {
 			resourcesWithWildcardSubresoures[res] = struct{}{}
@@ -123,7 +124,26 @@ func validateResources(resources []string, fldPath *field.Path) field.ErrorList
 	return allErrors
 }
 
-func validateRule(rule *admissionregistration.Rule, fldPath *field.Path) field.ErrorList {
+func validateResourcesNoSubResources(resources []string, fldPath *field.Path) field.ErrorList {
+	var allErrors field.ErrorList
+	if len(resources) == 0 {
+		allErrors = append(allErrors, field.Required(fldPath, ""))
+	}
+	for i, resource := range resources {
+		if resource == "" {
+			allErrors = append(allErrors, field.Required(fldPath.Index(i), ""))
+		}
+		if strings.Contains(resource, "/") {
+			allErrors = append(allErrors, field.Invalid(fldPath.Index(i), resource, "must not specify subresources"))
+		}
+	}
+	if len(resources) > 1 && hasWildcard(resources) {
+		allErrors = append(allErrors, field.Invalid(fldPath, resources, "if '*' is present, must not specify other resources"))
+	}
+	return allErrors
+}
+
+func validateRule(rule *admissionregistration.Rule, fldPath *field.Path, allowSubResource bool) field.ErrorList {
 	var allErrors field.ErrorList
 	if len(rule.APIGroups) == 0 {
 		allErrors = append(allErrors, field.Required(fldPath.Child("apiGroups"), ""))
@@ -143,7 +163,11 @@ func validateRule(rule *admissionregistration.Rule, fldPath *field.Path) field.E
 			allErrors = append(allErrors, field.Required(fldPath.Child("apiVersions").Index(i), ""))
 		}
 	}
+	if allowSubResource {
 		allErrors = append(allErrors, validateResources(rule.Resources, fldPath.Child("resources"))...)
+	} else {
+		allErrors = append(allErrors, validateResourcesNoSubResources(rule.Resources, fldPath.Child("resources"))...)
+	}
 	return allErrors
 }
 
@@ -154,7 +178,7 @@ func ValidateInitializerConfigurationUpdate(newIC, oldIC *admissionregistration.
 func ValidateExternalAdmissionHookConfiguration(e *admissionregistration.ExternalAdmissionHookConfiguration) field.ErrorList {
 	allErrors := genericvalidation.ValidateObjectMeta(&e.ObjectMeta, false, genericvalidation.NameIsDNSSubdomain, field.NewPath("metadata"))
 	for i, hook := range e.ExternalAdmissionHooks {
-		allErrors = append(allErrors, validateExternalAdmissionHook(&hook, field.NewPath("externalAdmissionHook").Index(i))...)
+		allErrors = append(allErrors, validateExternalAdmissionHook(&hook, field.NewPath("externalAdmissionHooks").Index(i))...)
 	}
 	return allErrors
 }
@@ -212,7 +236,8 @@ func validateRuleWithOperations(ruleWithOperations *admissionregistration.RuleWi
 			allErrors = append(allErrors, field.NotSupported(fldPath.Child("operations").Index(i), operation, supportedOperations.List()))
 		}
 	}
-	allErrors = append(allErrors, validateRule(&ruleWithOperations.Rule, fldPath)...)
+	allowSubResource := true
+	allErrors = append(allErrors, validateRule(&ruleWithOperations.Rule, fldPath, allowSubResource)...)
 	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 for APIGroups or APIVersions",
+			name: "wildcard cannot be mixed with other strings for APIGroups or APIVersions or Resources",
 			config: getInitializerConfiguration(
 				[]admissionregistration.Initializer{
 					{
@@ -190,15 +190,15 @@ 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`,
+			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]`,
 		},
 		{
-			name: `resource "*" can co-exist with resources that have subresources`,
+			name: "Subresource not allowed",
 			config: getInitializerConfiguration(
 				[]admissionregistration.Initializer{
 					{
@@ -207,95 +207,12 @@ func TestValidateInitializerConfiguration(t *testing.T) {
 							{
 								APIGroups:   []string{"a"},
 								APIVersions: []string{"a"},
-								Resources:   []string{"*", "a/b", "a/*", "*/b"},
+								Resources:   []string{"a/b"},
 							},
 						},
 					},
 				}),
-		},
+			expectedError: ` "a/b": must not specify subresources`,
-		{
-			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\"",
@@ -357,7 +274,7 @@ func TestValidateExternalAdmissionHookConfiguration(t *testing.T) {
 						Name: "",
 					},
 				}),
-			expectedError: `externalAdmissionHook[1].name: Invalid value: "k8s.io": should be a domain with at least two dots, externalAdmissionHook[2].name: Required value`,
+			expectedError: `externalAdmissionHooks[1].name: Invalid value: "k8s.io": should be a domain with at least two dots, externalAdmissionHooks[2].name: Required value`,
 		},
 		{
 			name: "Operations must not be empty or nil",
@@ -385,7 +302,7 @@ func TestValidateExternalAdmissionHookConfiguration(t *testing.T) {
 						},
 					},
 				}),
-			expectedError: `externalAdmissionHook[0].rules[0].operations: Required value, externalAdmissionHook[0].rules[1].operations: Required value`,
+			expectedError: `externalAdmissionHooks[0].rules[0].operations: Required value, externalAdmissionHooks[0].rules[1].operations: Required value`,
 		},
 		{
 			name: "\"\" is NOT a valid operation",
@@ -428,7 +345,7 @@ func TestValidateExternalAdmissionHookConfiguration(t *testing.T) {
 			expectedError: `Unsupported value: "PATCH"`,
 		},
 		{
-			name: "wildcard cannot be mixed with other strings",
+			name: "wildcard operation cannot be mixed with other strings",
 			config: getExternalAdmissionHookConfiguration(
 				[]admissionregistration.ExternalAdmissionHook{
 					{
@@ -447,12 +364,130 @@ func TestValidateExternalAdmissionHookConfiguration(t *testing.T) {
 				}),
 			expectedError: `if '*' is present, must not specify other operations`,
 		},
+		{
+			name: `resource "*" can co-exist with resources that have subresources`,
+			config: getExternalAdmissionHookConfiguration(
+				[]admissionregistration.ExternalAdmissionHook{
+					{
+						Name: "webhook.k8s.io",
+						Rules: []admissionregistration.RuleWithOperations{
+							{
+								Operations: []admissionregistration.OperationType{"CREATE"},
+								Rule: 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: getExternalAdmissionHookConfiguration(
+				[]admissionregistration.ExternalAdmissionHook{
+					{
+						Name: "webhook.k8s.io",
+						Rules: []admissionregistration.RuleWithOperations{
+							{
+								Operations: []admissionregistration.OperationType{"CREATE"},
+								Rule: 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: getExternalAdmissionHookConfiguration(
+				[]admissionregistration.ExternalAdmissionHook{
+					{
+						Name: "webhook.k8s.io",
+						Rules: []admissionregistration.RuleWithOperations{
+							{
+								Operations: []admissionregistration.OperationType{"CREATE"},
+								Rule: admissionregistration.Rule{
+									APIGroups:   []string{"a"},
+									APIVersions: []string{"a"},
+									Resources:   []string{"a/*", "a/x"},
+								},
+							},
+						},
+					},
+				}),
+			expectedError: `externalAdmissionHooks[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: getExternalAdmissionHookConfiguration(
+				[]admissionregistration.ExternalAdmissionHook{
+					{
+						Name: "webhook.k8s.io",
+						Rules: []admissionregistration.RuleWithOperations{
+							{
+								Operations: []admissionregistration.OperationType{"CREATE"},
+								Rule: admissionregistration.Rule{
+									APIGroups:   []string{"a"},
+									APIVersions: []string{"a"},
+									Resources:   []string{"a/*", "a"},
+								},
+							},
+						},
+					},
+				}),
+		},
+		{
+			name: "resource */a cannot mix with x/a",
+			config: getExternalAdmissionHookConfiguration(
+				[]admissionregistration.ExternalAdmissionHook{
+					{
+						Name: "webhook.k8s.io",
+						Rules: []admissionregistration.RuleWithOperations{
+							{
+								Operations: []admissionregistration.OperationType{"CREATE"},
+								Rule: admissionregistration.Rule{
+									APIGroups:   []string{"a"},
+									APIVersions: []string{"a"},
+									Resources:   []string{"*/a", "x/a"},
+								},
+							},
+						},
+					},
+				}),
+			expectedError: `externalAdmissionHooks[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: getExternalAdmissionHookConfiguration(
+				[]admissionregistration.ExternalAdmissionHook{
+					{
+						Name: "webhook.k8s.io",
+						Rules: []admissionregistration.RuleWithOperations{
+							{
+								Operations: []admissionregistration.OperationType{"CREATE"},
+								Rule: admissionregistration.Rule{
+									APIGroups:   []string{"a"},
+									APIVersions: []string{"a"},
+									Resources:   []string{"*/*", "a"},
+								},
+							},
+						},
+					},
+				}),
+			expectedError: `externalAdmissionHooks[0].rules[0].resources: Invalid value: []string{"*/*", "a"}: if '*/*' is present, must not specify other resources`,
+		},
 		{
 			name: "FailurePolicy can only be \"Ignore\"",
 			config: getExternalAdmissionHookConfiguration(
 				[]admissionregistration.ExternalAdmissionHook{
 					{
-						Name: "initializer.k8s.io",
+						Name: "webhook.k8s.io",
 						FailurePolicy: func() *admissionregistration.FailurePolicyType {
 							r := admissionregistration.Fail
 							return &r