Add port to ServiceReference of Admission Webhooks, ConversionWebhooks and AuditSync with defaulter and validator

pkg/apis/admissionregistration/types.go

@@ -311,8 +311,6 @@ type WebhookClientConfig struct {
 	//
 	// If the webhook is running within the cluster, then you should use `service`.
 	//
-	// Port 443 will be used if it is open, otherwise it is an error.
-	//
 	// +optional
 	Service *ServiceReference
 
@@ -335,4 +333,9 @@ type ServiceReference struct {
 	// this service.
 	// +optional
 	Path *string
+
+	// If specified, the port on the service that hosting webhook.
+	// `Port` should be a valid port number (1-65535, inclusive).
+	// +optional
+	Port int32
 }

pkg/apis/admissionregistration/v1beta1/defaults.go

@@ -20,6 +20,7 @@ import (
 	admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	utilpointer "k8s.io/utils/pointer"
 )
 
 func addDefaultingFuncs(scheme *runtime.Scheme) error {
@@ -56,3 +57,10 @@ func SetDefaults_Rule(obj *admissionregistrationv1beta1.Rule) {
 		obj.Scope = &s
 	}
 }
+
+// SetDefaults_ServiceReference sets defaults for Webhook's ServiceReference
+func SetDefaults_ServiceReference(obj *admissionregistrationv1beta1.ServiceReference) {
+	if obj.Port == nil {
+		obj.Port = utilpointer.Int32Ptr(443)
+	}
+}

pkg/apis/admissionregistration/validation/validation.go

@@ -250,7 +250,7 @@ func validateWebhook(hook *admissionregistration.Webhook, fldPath *field.Path) f
 	case cc.URL != nil:
 		allErrors = append(allErrors, webhook.ValidateWebhookURL(fldPath.Child("clientConfig").Child("url"), *cc.URL, true)...)
 	case cc.Service != nil:
-		allErrors = append(allErrors, webhook.ValidateWebhookService(fldPath.Child("clientConfig").Child("service"), cc.Service.Name, cc.Service.Namespace, cc.Service.Path)...)
+		allErrors = append(allErrors, webhook.ValidateWebhookService(fldPath.Child("clientConfig").Child("service"), cc.Service.Name, cc.Service.Namespace, cc.Service.Path, cc.Service.Port)...)
 	}
 	return allErrors
 }

pkg/apis/admissionregistration/validation/validation_test.go

@@ -579,6 +579,42 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 			}, true),
 			expectedError: `clientConfig.service.path: Invalid value: "/apis/foo.bar/v1alpha1/--bad": segment[3]: a DNS-1123 subdomain`,
 		},
+		{
+			name: "invalid port 0",
+			config: newValidatingWebhookConfiguration(
+				[]admissionregistration.Webhook{
+					{
+						Name: "webhook.k8s.io",
+						ClientConfig: admissionregistration.WebhookClientConfig{
+							Service: &admissionregistration.ServiceReference{
+								Namespace: "ns",
+								Name:      "n",
+								Path:      strPtr("https://apis/foo.bar"),
+								Port:      0,
+							},
+						},
+					},
+				}, true),
+			expectedError: `Invalid value: 0: port must be a valid number between 1 and 65535, inclusive`,
+		},
+		{
+			name: "invalid port >65535",
+			config: newValidatingWebhookConfiguration(
+				[]admissionregistration.Webhook{
+					{
+						Name: "webhook.k8s.io",
+						ClientConfig: admissionregistration.WebhookClientConfig{
+							Service: &admissionregistration.ServiceReference{
+								Namespace: "ns",
+								Name:      "n",
+								Path:      strPtr("https://apis/foo.bar"),
+								Port:      65536,
+							},
+						},
+					},
+				}, true),
+			expectedError: `Invalid value: 65536: port must be a valid number between 1 and 65535, inclusive`,
+		},
 		{
 			name: "timeout seconds cannot be greater than 30",
 			config: newValidatingWebhookConfiguration([]admissionregistration.Webhook{

pkg/apis/auditregistration/types.go

@@ -168,8 +168,6 @@ type WebhookClientConfig struct {
 	//
 	// If the webhook is running within the cluster, then you should use `service`.
 	//
-	// Port 443 will be used if it is open, otherwise it is an error.
-	//
 	// +optional
 	Service *ServiceReference
 
@@ -193,4 +191,9 @@ type ServiceReference struct {
 	// this service.
 	// +optional
 	Path *string
+
+	// If specified, the port on the service that hosting webhook.
+	// `Port` should be a valid port number (1-65535, inclusive).
+	// +optional
+	Port int32
 }

pkg/apis/auditregistration/v1alpha1/defaults.go

@@ -54,3 +54,10 @@ func SetDefaults_AuditSink(obj *auditregistrationv1alpha1.AuditSink) {
 		obj.Spec.Webhook.Throttle = DefaultThrottle()
 	}
 }
+
+// SetDefaults_ServiceReference sets defaults for AuditSync Webhook's ServiceReference
+func SetDefaults_ServiceReference(obj *auditregistrationv1alpha1.ServiceReference) {
+	if obj.Port == nil {
+		obj.Port = utilpointer.Int32Ptr(443)
+	}
+}

pkg/apis/auditregistration/validation/validation.go

@@ -55,7 +55,7 @@ func ValidateWebhook(w auditregistration.Webhook, fldPath *field.Path) field.Err
 	case cc.URL != nil:
 		allErrs = append(allErrs, webhook.ValidateWebhookURL(fldPath.Child("clientConfig").Child("url"), *cc.URL, false)...)
 	case cc.Service != nil:
-		allErrs = append(allErrs, webhook.ValidateWebhookService(fldPath.Child("clientConfig").Child("service"), cc.Service.Name, cc.Service.Namespace, cc.Service.Path)...)
+		allErrs = append(allErrs, webhook.ValidateWebhookService(fldPath.Child("clientConfig").Child("service"), cc.Service.Name, cc.Service.Namespace, cc.Service.Path, cc.Service.Port)...)
 	}
 	return allErrs
 }

pkg/apis/auditregistration/validation/validation_test.go

@@ -228,6 +228,34 @@ func TestValidateWebhookConfiguration(t *testing.T) {
 			},
 			expectedError: `clientConfig.service.path: Invalid value: "foo/": must start with a '/'`,
 		},
+		{
+			name: "invalid port >65535",
+			config: auditregistration.Webhook{
+				ClientConfig: auditregistration.WebhookClientConfig{
+					Service: &auditregistration.ServiceReference{
+						Namespace: "ns",
+						Name:      "n",
+						Path:      strPtr("foo/"),
+						Port:      65536,
+					},
+				},
+			},
+			expectedError: `Invalid value: 65536: port must be a valid number between 1 and 65535, inclusive`,
+		},
+		{
+			name: "invalid port 0",
+			config: auditregistration.Webhook{
+				ClientConfig: auditregistration.WebhookClientConfig{
+					Service: &auditregistration.ServiceReference{
+						Namespace: "ns",
+						Name:      "n",
+						Path:      strPtr("foo/"),
+						Port:      0,
+					},
+				},
+			},
+			expectedError: `Invalid value: 0: port must be a valid number between 1 and 65535, inclusive`,
+		},
 		{
 			name: "path accepts slash",
 			config: auditregistration.Webhook{

staging/publishing/import-restrictions.yaml

@@ -119,6 +119,7 @@
   - k8s.io/kube-aggregator
   - k8s.io/kube-openapi
   - k8s.io/klog
+  - k8s.io/utils
 
 - baseImportPath: "./vendor/k8s.io/sample-apiserver/"
   allowedImports:
@@ -142,6 +143,7 @@
   - k8s.io/component-base
   - k8s.io/klog
   - k8s.io/kube-openapi
+  - k8s.io/utils
 
 - baseImportPath: "./vendor/k8s.io/kube-openapi/"
   allowedImports:

staging/src/k8s.io/api/admissionregistration/v1beta1/types.go

@@ -322,8 +322,6 @@ type WebhookClientConfig struct {
 	//
 	// If the webhook is running within the cluster, then you should use `service`.
 	//
-	// Port 443 will be used if it is open, otherwise it is an error.
-	//
 	// +optional
 	Service *ServiceReference `json:"service,omitempty" protobuf:"bytes,1,opt,name=service"`
 
@@ -346,4 +344,10 @@ type ServiceReference struct {
 	// this service.
 	// +optional
 	Path *string `json:"path,omitempty" protobuf:"bytes,3,opt,name=path"`
+
+	// If specified, the port on the service that hosting webhook.
+	// Default to 443 for backward compatibility.
+	// `Port` should be a valid port number (1-65535, inclusive).
+	// +optional
+	Port *int32 `json:"port,omitempty" protobuf:"varint,4,opt,name=port"`
 }

staging/src/k8s.io/api/auditregistration/v1alpha1/types.go

@@ -166,8 +166,6 @@ type WebhookClientConfig struct {
 	//
 	// If the webhook is running within the cluster, then you should use `service`.
 	//
-	// Port 443 will be used if it is open, otherwise it is an error.
-	//
 	// +optional
 	Service *ServiceReference `json:"service,omitempty" protobuf:"bytes,2,opt,name=service"`
 
@@ -191,4 +189,10 @@ type ServiceReference struct {
 	// this service.
 	// +optional
 	Path *string `json:"path,omitempty" protobuf:"bytes,3,opt,name=path"`
+
+	// If specified, the port on the service that hosting webhook.
+	// Default to 443 for backward compatibility.
+	// `Port` should be a valid port number (1-65535, inclusive).
+	// +optional
+	Port *int32 `json:"port,omitempty" protobuf:"varint,4,opt,name=port"`
 }

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

@@ -132,8 +132,6 @@ type WebhookClientConfig struct {
 	//
 	// If the webhook is running within the cluster, then you should use `service`.
 	//
-	// Port 443 will be used if it is open, otherwise it is an error.
-	//
 	// +optional
 	Service *ServiceReference
 
@@ -156,6 +154,11 @@ type ServiceReference struct {
 	// this service.
 	// +optional
 	Path *string
+
+	// If specified, the port on the service that hosting webhook.
+	// `Port` should be a valid port number (1-65535, inclusive).
+	// +optional
+	Port int32
 }
 
 // CustomResourceDefinitionVersion describes a version for CRD.

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

@@ -20,6 +20,7 @@ import (
 	"strings"
 
 	"k8s.io/apimachinery/pkg/runtime"
+	utilpointer "k8s.io/utils/pointer"
 )
 
 func addDefaultingFuncs(scheme *runtime.Scheme) error {
@@ -73,6 +74,13 @@ func SetDefaults_CustomResourceDefinitionSpec(obj *CustomResourceDefinitionSpec)
 	}
 }
 
+// SetDefaults_ServiceReference sets defaults for Webhook's ServiceReference
+func SetDefaults_ServiceReference(obj *ServiceReference) {
+	if obj.Port == nil {
+		obj.Port = utilpointer.Int32Ptr(443)
+	}
+}
+
 // hasPerVersionColumns returns true if a CRD uses per-version columns.
 func hasPerVersionColumns(versions []CustomResourceDefinitionVersion) bool {
 	for _, v := range versions {

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

@@ -140,8 +140,6 @@ type WebhookClientConfig struct {
 	//
 	// If the webhook is running within the cluster, then you should use `service`.
 	//
-	// Port 443 will be used if it is open, otherwise it is an error.
-	//
 	// +optional
 	Service *ServiceReference `json:"service,omitempty" protobuf:"bytes,1,opt,name=service"`
 
@@ -164,6 +162,12 @@ type ServiceReference struct {
 	// this service.
 	// +optional
 	Path *string `json:"path,omitempty" protobuf:"bytes,3,opt,name=path"`
+
+	// If specified, the port on the service that hosting webhook.
+	// Default to 443 for backward compatibility.
+	// `Port` should be a valid port number (1-65535, inclusive).
+	// +optional
+	Port *int32 `json:"port,omitempty" protobuf:"varint,4,opt,name=port"`
 }
 
 // CustomResourceDefinitionVersion describes a version for CRD.

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

@@ -311,7 +311,7 @@ func validateCustomResourceConversion(conversion *apiextensions.CustomResourceCo
 			case cc.URL != nil:
 				allErrs = append(allErrs, webhook.ValidateWebhookURL(fldPath.Child("webhookClientConfig").Child("url"), *cc.URL, true)...)
 			case cc.Service != nil:
-				allErrs = append(allErrs, webhook.ValidateWebhookService(fldPath.Child("webhookClientConfig").Child("service"), cc.Service.Name, cc.Service.Namespace, cc.Service.Path)...)
+				allErrs = append(allErrs, webhook.ValidateWebhookService(fldPath.Child("webhookClientConfig").Child("service"), cc.Service.Name, cc.Service.Namespace, cc.Service.Path, cc.Service.Port)...)
 			}
 		}
 		allErrs = append(allErrs, validateConversionReviewVersions(conversion.ConversionReviewVersions, requireRecognizedVersion, fldPath.Child("conversionReviewVersions"))...)

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

@@ -67,6 +67,94 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
 		resource *apiextensions.CustomResourceDefinition
 		errors   []validationMatch
 	}{
+		{
+			name: "webhookconfig: invalid port 0",
+			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:    "version",
+							Served:  true,
+							Storage: true,
+						},
+						{
+							Name:    "version2",
+							Served:  true,
+							Storage: false,
+						},
+					},
+					Conversion: &apiextensions.CustomResourceConversion{
+						Strategy: apiextensions.ConversionStrategyType("Webhook"),
+						WebhookClientConfig: &apiextensions.WebhookClientConfig{
+							Service: &apiextensions.ServiceReference{
+								Name:      "n",
+								Namespace: "ns",
+								Port:      0,
+							},
+						},
+					},
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{
+					StoredVersions: []string{"version"},
+				},
+			},
+			errors: []validationMatch{
+				invalid("spec", "conversion", "webhookClientConfig", "service", "port"),
+			},
+		},
+		{
+			name: "webhookconfig: invalid port 65536",
+			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:    "version",
+							Served:  true,
+							Storage: true,
+						},
+						{
+							Name:    "version2",
+							Served:  true,
+							Storage: false,
+						},
+					},
+					Conversion: &apiextensions.CustomResourceConversion{
+						Strategy: apiextensions.ConversionStrategyType("Webhook"),
+						WebhookClientConfig: &apiextensions.WebhookClientConfig{
+							Service: &apiextensions.ServiceReference{
+								Name:      "n",
+								Namespace: "ns",
+								Port:      65536,
+							},
+						},
+					},
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{
+					StoredVersions: []string{"version"},
+				},
+			},
+			errors: []validationMatch{
+				invalid("spec", "conversion", "webhookClientConfig", "service", "port"),
+			},
+		},
 		{
 			name: "webhookconfig: both service and URL provided",
 			resource: &apiextensions.CustomResourceDefinition{

staging/src/k8s.io/apiserver/pkg/audit/util/conversion_test.go

@@ -24,6 +24,7 @@ import (
 	auditregv1alpha1 "k8s.io/api/auditregistration/v1alpha1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apiserver/pkg/util/webhook"
+	"k8s.io/utils/pointer"
 )
 
 func TestHookClientConfigForSink(t *testing.T) {
@@ -48,6 +49,7 @@ func TestHookClientConfigForSink(t *testing.T) {
 								Name:      "test",
 								Path:      &path,
 								Namespace: "test",
+								Port:      pointer.Int32Ptr(123),
 							},
 						},
 					},
@@ -60,6 +62,7 @@ func TestHookClientConfigForSink(t *testing.T) {
 					Name:      "test",
 					Namespace: "test",
 					Path:      path,
+					Port:      123,
 				},
 			},
 		},

staging/src/k8s.io/apiserver/pkg/util/webhook/validation.go

@@ -51,7 +51,7 @@ func ValidateWebhookURL(fldPath *field.Path, URL string, forceHttps bool) field.
 	return allErrors
 }
 
-func ValidateWebhookService(fldPath *field.Path, namespace, name string, path *string) field.ErrorList {
+func ValidateWebhookService(fldPath *field.Path, namespace, name string, path *string, port int32) field.ErrorList {
 	var allErrors field.ErrorList
 
 	if len(name) == 0 {
@@ -62,6 +62,10 @@ func ValidateWebhookService(fldPath *field.Path, namespace, name string, path *s
 		allErrors = append(allErrors, field.Required(fldPath.Child("namespace"), "service namespace is required"))
 	}
 
+	if errs := validation.IsValidPortNum(int(port)); errs != nil {
+		allErrors = append(allErrors, field.Invalid(fldPath.Child("port"), port, "port is not valid:"+strings.Join(errs, ",")))
+	}
+
 	if path == nil {
 		return allErrors
 	}