diff --git a/pkg/apis/admissionregistration/types.go b/pkg/apis/admissionregistration/types.go index 09a5df61137..e0c026586d8 100644 --- a/pkg/apis/admissionregistration/types.go +++ b/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 } diff --git a/pkg/apis/admissionregistration/v1beta1/defaults.go b/pkg/apis/admissionregistration/v1beta1/defaults.go index a5da0bfbc8b..594fff7d986 100644 --- a/pkg/apis/admissionregistration/v1beta1/defaults.go +++ b/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) + } +} diff --git a/pkg/apis/admissionregistration/validation/validation.go b/pkg/apis/admissionregistration/validation/validation.go index 9cf318a2c79..e3a6d365896 100644 --- a/pkg/apis/admissionregistration/validation/validation.go +++ b/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 } diff --git a/pkg/apis/admissionregistration/validation/validation_test.go b/pkg/apis/admissionregistration/validation/validation_test.go index 61e84575399..39ce0f10193 100644 --- a/pkg/apis/admissionregistration/validation/validation_test.go +++ b/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{ diff --git a/pkg/apis/auditregistration/types.go b/pkg/apis/auditregistration/types.go index 5ae4aa51336..bee192e4161 100644 --- a/pkg/apis/auditregistration/types.go +++ b/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 } diff --git a/pkg/apis/auditregistration/v1alpha1/defaults.go b/pkg/apis/auditregistration/v1alpha1/defaults.go index 1884a390792..dc23c0742cd 100644 --- a/pkg/apis/auditregistration/v1alpha1/defaults.go +++ b/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) + } +} diff --git a/pkg/apis/auditregistration/validation/validation.go b/pkg/apis/auditregistration/validation/validation.go index 849d0b111a1..5934883c957 100644 --- a/pkg/apis/auditregistration/validation/validation.go +++ b/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 } diff --git a/pkg/apis/auditregistration/validation/validation_test.go b/pkg/apis/auditregistration/validation/validation_test.go index 96c6a882c5d..fde2ac17097 100644 --- a/pkg/apis/auditregistration/validation/validation_test.go +++ b/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{ diff --git a/staging/publishing/import-restrictions.yaml b/staging/publishing/import-restrictions.yaml index 4b9a3ff243b..e92287a3bfa 100644 --- a/staging/publishing/import-restrictions.yaml +++ b/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: diff --git a/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go b/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go index f7025f6080c..21c5b72f1f3 100644 --- a/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go +++ b/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"` } diff --git a/staging/src/k8s.io/api/auditregistration/v1alpha1/types.go b/staging/src/k8s.io/api/auditregistration/v1alpha1/types.go index af31cfe275a..55abb93d8bb 100644 --- a/staging/src/k8s.io/api/auditregistration/v1alpha1/types.go +++ b/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"` } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go index cd1e8c5ff50..c10d7c5faca 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go +++ b/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. diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults.go index 7bea2d69887..3e1567bb633 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults.go +++ b/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 { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go index af2415ef586..fadafce8de4 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go +++ b/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. diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go index 7e74b38ce79..74190171d6f 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go +++ b/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"))...) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go index 031cbee9425..33387c6af3a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go +++ b/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{ diff --git a/staging/src/k8s.io/apiserver/pkg/audit/util/conversion_test.go b/staging/src/k8s.io/apiserver/pkg/audit/util/conversion_test.go index 4cd23c13d19..c8f0c66da14 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/util/conversion_test.go +++ b/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, }, }, }, diff --git a/staging/src/k8s.io/apiserver/pkg/util/webhook/validation.go b/staging/src/k8s.io/apiserver/pkg/util/webhook/validation.go index 2ddb2c09ab7..7c51bdd4e07 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/webhook/validation.go +++ b/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 }