Merge pull request #82707 from liggitt/allow-1.16-review-versions

Allow v1 review versions in 1.17+

pkg/apis/admissionregistration/validation/BUILD

@@ -23,6 +23,7 @@ go_library(
     importpath = "k8s.io/kubernetes/pkg/apis/admissionregistration/validation",
     deps = [
         "//pkg/apis/admissionregistration:go_default_library",
+        "//pkg/apis/admissionregistration/v1:go_default_library",
         "//pkg/apis/admissionregistration/v1beta1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/validation:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation:go_default_library",

pkg/apis/admissionregistration/validation/validation.go

@@ -28,7 +28,8 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/util/webhook"
 	"k8s.io/kubernetes/pkg/apis/admissionregistration"
-	"k8s.io/kubernetes/pkg/apis/admissionregistration/v1beta1"
+	admissionregistrationv1 "k8s.io/kubernetes/pkg/apis/admissionregistration/v1"
+	admissionregistrationv1beta1 "k8s.io/kubernetes/pkg/apis/admissionregistration/v1beta1"
 )
 
 func hasWildcard(slice []string) bool {
@@ -155,8 +156,8 @@ func validateRule(rule *admissionregistration.Rule, fldPath *field.Path, allowSu
 // AcceptedAdmissionReviewVersions contains the list of AdmissionReview versions the *prior* version of the API server understands.
 // 1.15: server understands v1beta1; accepted versions are ["v1beta1"]
 // 1.16: server understands v1, v1beta1; accepted versions are ["v1beta1"]
-// 1.17: server understands v1, v1beta1; accepted versions are ["v1","v1beta1"]
+// 1.17+: server understands v1, v1beta1; accepted versions are ["v1","v1beta1"]
-var AcceptedAdmissionReviewVersions = []string{v1beta1.SchemeGroupVersion.Version}
+var AcceptedAdmissionReviewVersions = []string{admissionregistrationv1.SchemeGroupVersion.Version, admissionregistrationv1beta1.SchemeGroupVersion.Version}
 
 func isAcceptedAdmissionReviewVersion(v string) bool {
 	for _, version := range AcceptedAdmissionReviewVersions {

pkg/apis/admissionregistration/validation/validation_test.go

@@ -65,7 +65,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 					SideEffects:  &unknownSideEffect,
 				},
 			}, false),
-			expectedError: `webhooks[0].admissionReviewVersions: Required value: must specify one of v1beta1`,
+			expectedError: `webhooks[0].admissionReviewVersions: Required value: must specify one of v1, v1beta1`,
 		}, {
 			name: "should fail on bad AdmissionReviewVersion value",
 			config: newValidatingWebhookConfiguration([]admissionregistration.ValidatingWebhook{
@@ -415,7 +415,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 					SideEffects:  &unknownSideEffect,
 				},
 			}, false),
-			expectedError: `webhooks[0].admissionReviewVersions: Required value: must specify one of v1beta1`,
+			expectedError: `webhooks[0].admissionReviewVersions: Required value: must specify one of v1, v1beta1`,
 		},
 		{
 			name: "SideEffects are required",
@@ -1034,7 +1034,7 @@ func TestValidateMutatingWebhookConfiguration(t *testing.T) {
 					SideEffects:  &unknownSideEffect,
 				},
 			}, false),
-			expectedError: `webhooks[0].admissionReviewVersions: Required value: must specify one of v1beta1`,
+			expectedError: `webhooks[0].admissionReviewVersions: Required value: must specify one of v1, v1beta1`,
 		}, {
 			name: "should fail on bad AdmissionReviewVersion value",
 			config: newMutatingWebhookConfiguration([]admissionregistration.MutatingWebhook{
@@ -1384,7 +1384,7 @@ func TestValidateMutatingWebhookConfiguration(t *testing.T) {
 					SideEffects:  &unknownSideEffect,
 				},
 			}, false),
-			expectedError: `webhooks[0].admissionReviewVersions: Required value: must specify one of v1beta1`,
+			expectedError: `webhooks[0].admissionReviewVersions: Required value: must specify one of v1, v1beta1`,
 		},
 		{
 			name: "SideEffects are required",
@@ -1795,7 +1795,7 @@ func TestValidateMutatingWebhookConfigurationUpdate(t *testing.T) {
 		expectedError string
 	}{
 		{
-			name: "should pass on valid new AdmissionReviewVersion",
+			name: "should pass on valid new AdmissionReviewVersion (v1beta1)",
 			config: newMutatingWebhookConfiguration([]admissionregistration.MutatingWebhook{
 				{
 					Name:                    "webhook.k8s.io",
@@ -1813,6 +1813,25 @@ func TestValidateMutatingWebhookConfigurationUpdate(t *testing.T) {
 			}, true),
 			expectedError: ``,
 		},
+		{
+			name: "should pass on valid new AdmissionReviewVersion (v1)",
+			config: newMutatingWebhookConfiguration([]admissionregistration.MutatingWebhook{
+				{
+					Name:                    "webhook.k8s.io",
+					ClientConfig:            validClientConfig,
+					SideEffects:             &unknownSideEffect,
+					AdmissionReviewVersions: []string{"v1"},
+				},
+			}, true),
+			oldconfig: newMutatingWebhookConfiguration([]admissionregistration.MutatingWebhook{
+				{
+					Name:         "webhook.k8s.io",
+					ClientConfig: validClientConfig,
+					SideEffects:  &unknownSideEffect,
+				},
+			}, true),
+			expectedError: ``,
+		},
 		{
 			name: "should pass on invalid AdmissionReviewVersion with invalid previous versions",
 			config: newMutatingWebhookConfiguration([]admissionregistration.MutatingWebhook{

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/BUILD

@@ -14,6 +14,7 @@ go_library(
     deps = [
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers:go_default_library",
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library",
+        "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1:go_default_library",
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library",
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema:go_default_library",
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting:go_default_library",

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

@@ -33,7 +33,8 @@ import (
 	"k8s.io/apiserver/pkg/util/webhook"
 
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
-	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
 	apiservervalidation "k8s.io/apiextensions-apiserver/pkg/apiserver/validation"
 	apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features"
@@ -313,8 +314,8 @@ func validateEnumStrings(fldPath *field.Path, value string, accepted []string, r
 // AcceptedConversionReviewVersions contains the list of ConversionReview versions the *prior* version of the API server understands.
 // 1.15: server understands v1beta1; accepted versions are ["v1beta1"]
 // 1.16: server understands v1, v1beta1; accepted versions are ["v1beta1"]
-// TODO(liggitt): 1.17: server understands v1, v1beta1; accepted versions are ["v1","v1beta1"]
+// 1.17+: server understands v1, v1beta1; accepted versions are ["v1","v1beta1"]
-var acceptedConversionReviewVersions = sets.NewString(v1beta1.SchemeGroupVersion.Version)
+var acceptedConversionReviewVersions = sets.NewString(apiextensionsv1.SchemeGroupVersion.Version, apiextensionsv1beta1.SchemeGroupVersion.Version)
 
 func isAcceptedConversionReviewVersion(v string) bool {
 	return acceptedConversionReviewVersions.Has(v)
@@ -1007,7 +1008,7 @@ func allowedAtRootSchema(field string) bool {
 
 // requireOpenAPISchema returns true if the request group version requires a schema
 func requireOpenAPISchema(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool {
-	if requestGV == v1beta1.SchemeGroupVersion {
+	if requestGV == apiextensionsv1beta1.SchemeGroupVersion {
 		// for backwards compatibility
 		return false
 	}
@@ -1038,7 +1039,7 @@ func allowDefaults(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.Cust
 	if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceDefaulting) {
 		return false
 	}
-	if requestGV == v1beta1.SchemeGroupVersion {
+	if requestGV == apiextensionsv1beta1.SchemeGroupVersion {
 		return false
 	}
 	return true
@@ -1212,7 +1213,7 @@ func schemaHasKubernetesExtensions(s *apiextensions.JSONSchemaProps) bool {
 
 // requireStructuralSchema returns true if schemas specified must be structural
 func requireStructuralSchema(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool {
-	if requestGV == v1beta1.SchemeGroupVersion {
+	if requestGV == apiextensionsv1beta1.SchemeGroupVersion {
 		// for compatibility
 		return false
 	}
@@ -1282,7 +1283,7 @@ func schemaHasUnprunedDefaults(schema *apiextensions.JSONSchemaProps) (bool, err
 
 // requireValidPropertyType returns true if valid openapi v3 types should be required for the given API version
 func requireValidPropertyType(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool {
-	if requestGV == v1beta1.SchemeGroupVersion {
+	if requestGV == apiextensionsv1beta1.SchemeGroupVersion {
 		// for compatibility
 		return false
 	}
@@ -1297,7 +1298,7 @@ func requireValidPropertyType(requestGV schema.GroupVersion, oldCRDSpec *apiexte
 func validateAPIApproval(newCRD, oldCRD *apiextensions.CustomResourceDefinition, requestGV schema.GroupVersion) field.ErrorList {
 	// check to see if we need confirm API approval for kube group.
 
-	if requestGV == v1beta1.SchemeGroupVersion {
+	if requestGV == apiextensionsv1beta1.SchemeGroupVersion {
 		// no-op for compatibility with v1beta1
 		return nil
 	}
@@ -1323,19 +1324,19 @@ func validateAPIApproval(newCRD, oldCRD *apiextensions.CustomResourceDefinition,
 	// in v1, we require valid approval strings
 	switch newApprovalState {
 	case apihelpers.APIApprovalInvalid:
-		return field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations").Key(v1beta1.KubeAPIApprovedAnnotation), newCRD.Annotations[v1beta1.KubeAPIApprovedAnnotation], reason)}
+		return field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations").Key(apiextensionsv1beta1.KubeAPIApprovedAnnotation), newCRD.Annotations[apiextensionsv1beta1.KubeAPIApprovedAnnotation], reason)}
 	case apihelpers.APIApprovalMissing:
-		return field.ErrorList{field.Required(field.NewPath("metadata", "annotations").Key(v1beta1.KubeAPIApprovedAnnotation), reason)}
+		return field.ErrorList{field.Required(field.NewPath("metadata", "annotations").Key(apiextensionsv1beta1.KubeAPIApprovedAnnotation), reason)}
 	case apihelpers.APIApproved, apihelpers.APIApprovalBypassed:
 		// success
 		return nil
 	default:
-		return field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations").Key(v1beta1.KubeAPIApprovedAnnotation), newCRD.Annotations[v1beta1.KubeAPIApprovedAnnotation], reason)}
+		return field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations").Key(apiextensionsv1beta1.KubeAPIApprovedAnnotation), newCRD.Annotations[apiextensionsv1beta1.KubeAPIApprovedAnnotation], reason)}
 	}
 }
 
 func validatePreserveUnknownFields(crd, oldCRD *apiextensions.CustomResourceDefinition, requestGV schema.GroupVersion) field.ErrorList {
-	if requestGV == v1beta1.SchemeGroupVersion {
+	if requestGV == apiextensionsv1beta1.SchemeGroupVersion {
 		// no-op for compatibility with v1beta1
 		return nil
 	}

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

@@ -780,7 +780,7 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
 			},
 		},
 		{
-			name: "webhook conversion with preserveUnknownFields=false",
+			name: "webhook conversion with preserveUnknownFields=false, conversionReviewVersions=[v1beta1]",
 			resource: &apiextensions.CustomResourceDefinition{
 				ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
 				Spec: apiextensions.CustomResourceDefinitionSpec{
@@ -824,6 +824,51 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
 			},
 			errors: []validationMatch{},
 		},
+		{
+			name: "webhook conversion with preserveUnknownFields=false, conversionReviewVersions=[v1]",
+			resource: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group: "group.com",
+					Scope: apiextensions.ResourceScope("Cluster"),
+					Names: apiextensions.CustomResourceDefinitionNames{
+						Plural:   "plural",
+						Singular: "singular",
+						Kind:     "Plural",
+						ListKind: "PluralList",
+					},
+					Versions: []apiextensions.CustomResourceDefinitionVersion{
+						{
+							Name:    "version1",
+							Served:  true,
+							Storage: true,
+						},
+						{
+							Name:    "version2",
+							Served:  true,
+							Storage: false,
+						},
+					},
+					Conversion: &apiextensions.CustomResourceConversion{
+						Strategy: apiextensions.ConversionStrategyType("Webhook"),
+						WebhookClientConfig: &apiextensions.WebhookClientConfig{
+							URL: strPtr("https://example.com/webhook"),
+						},
+						ConversionReviewVersions: []string{"v1"},
+					},
+					Validation: &apiextensions.CustomResourceValidation{
+						OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
+							Type: "object",
+						},
+					},
+					PreserveUnknownFields: pointer.BoolPtr(false),
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{
+					StoredVersions: []string{"version1"},
+				},
+			},
+			errors: []validationMatch{},
+		},
 		{
 			name: "no_storage_version",
 			resource: &apiextensions.CustomResourceDefinition{