apiextensions: allow core name changes if not established

staging/src/k8s.io/kube-apiextensions-server/pkg/apis/apiextensions/validation/validation.go

@@ -46,7 +46,7 @@ func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinitio
 // ValidateCustomResourceDefinitionUpdate statically validates
 func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomResourceDefinition) field.ErrorList {
 	allErrs := genericvalidation.ValidateObjectMetaUpdate(&obj.ObjectMeta, &oldObj.ObjectMeta, field.NewPath("metadata"))
-	allErrs = append(allErrs, ValidateCustomResourceDefinitionSpecUpdate(&obj.Spec, &oldObj.Spec, field.NewPath("spec"))...)
+	allErrs = append(allErrs, ValidateCustomResourceDefinitionSpecUpdate(&obj.Spec, &oldObj.Spec, apiextensions.IsCRDConditionTrue(oldObj, apiextensions.Established), field.NewPath("spec"))...)
 	allErrs = append(allErrs, ValidateCustomResourceDefinitionStatus(&obj.Status, field.NewPath("status"))...)
 	return allErrs
 }
@@ -64,22 +64,21 @@ func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi
 
 	if len(spec.Group) == 0 {
 		allErrs = append(allErrs, field.Required(fldPath.Child("group"), ""))
-	}
+	} else if errs := validationutil.IsDNS1123Subdomain(spec.Group); len(errs) > 0 {
-	if errs := validationutil.IsDNS1123Subdomain(spec.Group); len(errs) > 0 {
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("group"), spec.Group, strings.Join(errs, ",")))
-	}
+	} else if len(strings.Split(spec.Group, ".")) < 2 {
-	if len(strings.Split(spec.Group, ".")) < 2 {
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("group"), spec.Group, "should be a domain with at least one dot"))
 	}
 
 	if len(spec.Version) == 0 {
 		allErrs = append(allErrs, field.Required(fldPath.Child("version"), ""))
-	}
+	} else if errs := validationutil.IsDNS1035Label(spec.Version); len(errs) > 0 {
-	if errs := validationutil.IsDNS1035Label(spec.Version); len(errs) > 0 {
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("version"), spec.Version, strings.Join(errs, ",")))
 	}
 
 	switch spec.Scope {
+	case "":
+		allErrs = append(allErrs, field.Required(fldPath.Child("scope"), ""))
 	case apiextensions.ClusterScoped, apiextensions.NamespaceScoped:
 	default:
 		allErrs = append(allErrs, field.NotSupported(fldPath.Child("scope"), spec.Scope, []string{string(apiextensions.ClusterScoped), string(apiextensions.NamespaceScoped)}))
@@ -105,17 +104,19 @@ func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi
 }
 
 // ValidateCustomResourceDefinitionSpecUpdate statically validates
-func ValidateCustomResourceDefinitionSpecUpdate(spec, oldSpec *apiextensions.CustomResourceDefinitionSpec, fldPath *field.Path) field.ErrorList {
+func ValidateCustomResourceDefinitionSpecUpdate(spec, oldSpec *apiextensions.CustomResourceDefinitionSpec, established bool, fldPath *field.Path) field.ErrorList {
 	allErrs := ValidateCustomResourceDefinitionSpec(spec, fldPath)
 
-	// these all affect the storage, so you can't change them
+	if established {
-	genericvalidation.ValidateImmutableField(spec.Group, oldSpec.Group, fldPath.Child("group"))
+		// these effect the storage and cannot be changed therefore
-	genericvalidation.ValidateImmutableField(spec.Version, oldSpec.Version, fldPath.Child("version"))
+		allErrs = append(allErrs, genericvalidation.ValidateImmutableField(spec.Version, oldSpec.Version, fldPath.Child("version"))...)
-	genericvalidation.ValidateImmutableField(spec.Scope, oldSpec.Scope, fldPath.Child("scope"))
+		allErrs = append(allErrs, genericvalidation.ValidateImmutableField(spec.Scope, oldSpec.Scope, fldPath.Child("scope"))...)
-	genericvalidation.ValidateImmutableField(spec.Names.Kind, oldSpec.Names.Kind, fldPath.Child("names", "kind"))
+		allErrs = append(allErrs, genericvalidation.ValidateImmutableField(spec.Names.Kind, oldSpec.Names.Kind, fldPath.Child("names", "kind"))...)
+	}
 
-	// this affects the expected resource name, so you can't change it either
+	// these affects the resource name, which is always immutable, so this can't be updated.
-	genericvalidation.ValidateImmutableField(spec.Names.Plural, oldSpec.Names.Plural, fldPath.Child("names", "plural"))
+	allErrs = append(allErrs, genericvalidation.ValidateImmutableField(spec.Group, oldSpec.Group, fldPath.Child("group"))...)
+	allErrs = append(allErrs, genericvalidation.ValidateImmutableField(spec.Names.Plural, oldSpec.Names.Plural, fldPath.Child("names", "plural"))...)
 
 	return allErrs
 }

staging/src/k8s.io/kube-apiextensions-server/pkg/apis/apiextensions/validation/validation_test.go

@@ -29,11 +29,17 @@ type validationMatch struct {
 	errorType field.ErrorType
 }
 
-func required(path *field.Path) validationMatch {
+func required(path ...string) validationMatch {
-	return validationMatch{path: path, errorType: field.ErrorTypeRequired}
+	return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeRequired}
 }
-func invalid(path *field.Path) validationMatch {
+func invalid(path ...string) validationMatch {
-	return validationMatch{path: path, errorType: field.ErrorTypeInvalid}
+	return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeInvalid}
+}
+func unsupported(path ...string) validationMatch {
+	return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeNotSupported}
+}
+func immutable(path ...string) validationMatch {
+	return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeInvalid}
 }
 
 func (v validationMatch) matches(err *field.Error) bool {
@@ -58,7 +64,12 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
 				},
 			},
 			errors: []validationMatch{
-				invalid(field.NewPath("metadata", "name")),
+				invalid("metadata", "name"),
+				required("spec", "version"),
+				required("spec", "scope"),
+				required("spec", "names", "singular"),
+				required("spec", "names", "kind"),
+				required("spec", "names", "listKind"),
 			},
 		},
 		{
@@ -67,13 +78,14 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
 				ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
 			},
 			errors: []validationMatch{
-				required(field.NewPath("spec", "group")),
+				invalid("metadata", "name"),
-				required(field.NewPath("spec", "version")),
+				required("spec", "group"),
-				{path: field.NewPath("spec", "scope"), errorType: field.ErrorTypeNotSupported},
+				required("spec", "version"),
-				required(field.NewPath("spec", "names", "plural")),
+				required("spec", "scope"),
-				required(field.NewPath("spec", "names", "singular")),
+				required("spec", "names", "plural"),
-				required(field.NewPath("spec", "names", "kind")),
+				required("spec", "names", "singular"),
-				required(field.NewPath("spec", "names", "listKind")),
+				required("spec", "names", "kind"),
+				required("spec", "names", "listKind"),
 			},
 		},
 		{
@@ -101,17 +113,20 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
 				},
 			},
 			errors: []validationMatch{
-				invalid(field.NewPath("spec", "group")),
+				invalid("metadata", "name"),
-				invalid(field.NewPath("spec", "version")),
+				invalid("spec", "group"),
-				{path: field.NewPath("spec", "scope"), errorType: field.ErrorTypeNotSupported},
+				invalid("spec", "version"),
-				invalid(field.NewPath("spec", "names", "plural")),
+				unsupported("spec", "scope"),
-				invalid(field.NewPath("spec", "names", "singular")),
+				invalid("spec", "names", "plural"),
-				invalid(field.NewPath("spec", "names", "kind")),
+				invalid("spec", "names", "singular"),
-				invalid(field.NewPath("spec", "names", "listKind")),
+				invalid("spec", "names", "kind"),
-				invalid(field.NewPath("status", "acceptedNames", "plural")),
+				invalid("spec", "names", "listKind"), // invalid format
-				invalid(field.NewPath("status", "acceptedNames", "singular")),
+				invalid("spec", "names", "listKind"), // kind == listKind
-				invalid(field.NewPath("status", "acceptedNames", "kind")),
+				invalid("status", "acceptedNames", "plural"),
-				invalid(field.NewPath("status", "acceptedNames", "listKind")),
+				invalid("status", "acceptedNames", "singular"),
+				invalid("status", "acceptedNames", "kind"),
+				invalid("status", "acceptedNames", "listKind"), // invalid format
+				invalid("status", "acceptedNames", "listKind"), // kind == listKind
 			},
 		},
 		{
@@ -138,21 +153,25 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
 				},
 			},
 			errors: []validationMatch{
-				invalid(field.NewPath("spec", "group")),
+				invalid("metadata", "name"),
-				invalid(field.NewPath("spec", "names", "listKind")),
+				invalid("spec", "group"),
-				invalid(field.NewPath("status", "acceptedNames", "listKind")),
+				required("spec", "scope"),
+				invalid("spec", "names", "listKind"),
+				invalid("status", "acceptedNames", "listKind"),
 			},
 		},
 	}
 
 	for _, tc := range tests {
 		errs := ValidateCustomResourceDefinition(tc.resource)
+		seenErrs := make([]bool, len(errs))
 
 		for _, expectedError := range tc.errors {
 			found := false
-			for _, err := range errs {
+			for i, err := range errs {
-				if expectedError.matches(err) {
+				if expectedError.matches(err) && !seenErrs[i] {
 					found = true
+					seenErrs[i] = true
 					break
 				}
 			}
@@ -161,5 +180,281 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
 				t.Errorf("%s: expected %v at %v, got %v", tc.name, expectedError.errorType, expectedError.path.String(), errs)
 			}
 		}
+
+		for i, seen := range seenErrs {
+			if !seen {
+				t.Errorf("%s: unexpected error: %v", tc.name, errs[i])
+			}
+		}
+	}
+}
+
+func TestValidateCustomResourceDefinitionUpdate(t *testing.T) {
+	tests := []struct {
+		name     string
+		old      *apiextensions.CustomResourceDefinition
+		resource *apiextensions.CustomResourceDefinition
+		errors   []validationMatch
+	}{
+		{
+			name: "unchanged",
+			old: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            "plural.group.com",
+					ResourceVersion: "42",
+				},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group:   "group.com",
+					Version: "version",
+					Scope:   apiextensions.ResourceScope("Cluster"),
+					Names: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural",
+						Singular: "singular",
+						Kind:     "kind",
+						ListKind: "listkind",
+					},
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{
+					AcceptedNames: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural",
+						Singular: "singular",
+						Kind:     "kind",
+						ListKind: "listkind",
+					},
+				},
+			},
+			resource: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            "plural.group.com",
+					ResourceVersion: "42",
+				},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group:   "group.com",
+					Version: "version",
+					Scope:   apiextensions.ResourceScope("Cluster"),
+					Names: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural",
+						Singular: "singular",
+						Kind:     "kind",
+						ListKind: "listkind",
+					},
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{
+					AcceptedNames: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural",
+						Singular: "singular",
+						Kind:     "kind",
+						ListKind: "listkind",
+					},
+				},
+			},
+			errors: []validationMatch{},
+		},
+		{
+			name: "unchanged-established",
+			old: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            "plural.group.com",
+					ResourceVersion: "42",
+				},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group:   "group.com",
+					Version: "version",
+					Scope:   apiextensions.ResourceScope("Cluster"),
+					Names: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural",
+						Singular: "singular",
+						Kind:     "kind",
+						ListKind: "listkind",
+					},
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{
+					AcceptedNames: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural",
+						Singular: "singular",
+						Kind:     "kind",
+						ListKind: "listkind",
+					},
+					Conditions: []apiextensions.CustomResourceDefinitionCondition{
+						{Type: apiextensions.Established, Status: apiextensions.ConditionTrue},
+					},
+				},
+			},
+			resource: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            "plural.group.com",
+					ResourceVersion: "42",
+				},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group:   "group.com",
+					Version: "version",
+					Scope:   apiextensions.ResourceScope("Cluster"),
+					Names: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural",
+						Singular: "singular",
+						Kind:     "kind",
+						ListKind: "listkind",
+					},
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{
+					AcceptedNames: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural",
+						Singular: "singular",
+						Kind:     "kind",
+						ListKind: "listkind",
+					},
+				},
+			},
+			errors: []validationMatch{},
+		},
+		{
+			name: "changes",
+			old: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            "plural.group.com",
+					ResourceVersion: "42",
+				},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group:   "group.com",
+					Version: "version",
+					Scope:   apiextensions.ResourceScope("Cluster"),
+					Names: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural",
+						Singular: "singular",
+						Kind:     "kind",
+						ListKind: "listkind",
+					},
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{
+					AcceptedNames: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural",
+						Singular: "singular",
+						Kind:     "kind",
+						ListKind: "listkind",
+					},
+					Conditions: []apiextensions.CustomResourceDefinitionCondition{
+						{Type: apiextensions.Established, Status: apiextensions.ConditionFalse},
+					},
+				},
+			},
+			resource: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            "plural.group.com",
+					ResourceVersion: "42",
+				},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group:   "abc.com",
+					Version: "version2",
+					Scope:   apiextensions.ResourceScope("Namespaced"),
+					Names: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural2",
+						Singular: "singular2",
+						Kind:     "kind2",
+						ListKind: "listkind2",
+					},
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{
+					AcceptedNames: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural2",
+						Singular: "singular2",
+						Kind:     "kind2",
+						ListKind: "listkind2",
+					},
+				},
+			},
+			errors: []validationMatch{
+				immutable("spec", "group"),
+				immutable("spec", "names", "plural"),
+			},
+		},
+		{
+			name: "changes-established",
+			old: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            "plural.group.com",
+					ResourceVersion: "42",
+				},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group:   "group.com",
+					Version: "version",
+					Scope:   apiextensions.ResourceScope("Cluster"),
+					Names: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural",
+						Singular: "singular",
+						Kind:     "kind",
+						ListKind: "listkind",
+					},
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{
+					AcceptedNames: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural",
+						Singular: "singular",
+						Kind:     "kind",
+						ListKind: "listkind",
+					},
+					Conditions: []apiextensions.CustomResourceDefinitionCondition{
+						{Type: apiextensions.Established, Status: apiextensions.ConditionTrue},
+					},
+				},
+			},
+			resource: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            "plural.group.com",
+					ResourceVersion: "42",
+				},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group:   "abc.com",
+					Version: "version2",
+					Scope:   apiextensions.ResourceScope("Namespaced"),
+					Names: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural2",
+						Singular: "singular2",
+						Kind:     "kind2",
+						ListKind: "listkind2",
+					},
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{
+					AcceptedNames: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural2",
+						Singular: "singular2",
+						Kind:     "kind2",
+						ListKind: "listkind2",
+					},
+				},
+			},
+			errors: []validationMatch{
+				immutable("spec", "group"),
+				immutable("spec", "version"),
+				immutable("spec", "scope"),
+				immutable("spec", "names", "kind"),
+				immutable("spec", "names", "plural"),
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		errs := ValidateCustomResourceDefinitionUpdate(tc.resource, tc.old)
+		seenErrs := make([]bool, len(errs))
+
+		for _, expectedError := range tc.errors {
+			found := false
+			for i, err := range errs {
+				if expectedError.matches(err) && !seenErrs[i] {
+					found = true
+					seenErrs[i] = true
+					break
+				}
+			}
+
+			if !found {
+				t.Errorf("%s: expected %v at %v, got %v", tc.name, expectedError.errorType, expectedError.path.String(), errs)
+			}
+		}
+
+		for i, seen := range seenErrs {
+			if !seen {
+				t.Errorf("%s: unexpected error: %v", tc.name, errs[i])
+			}
+		}
 	}
 }