From 2589a924c1f9037307397a7003ea5a02d2adeabd Mon Sep 17 00:00:00 2001 From: Nick Young Date: Wed, 1 Jun 2022 06:45:35 +0000 Subject: [PATCH 1/3] Update AddressType definition to add domain-prefixed strings as an option Signed-off-by: Nick Young --- apis/v1alpha2/gateway_types.go | 1 - apis/v1alpha2/shared_types.go | 21 ++++++++++++------- .../gateway.networking.k8s.io_gateways.yaml | 6 ++++++ .../gateway.networking.k8s.io_gateways.yaml | 6 ++++++ 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/apis/v1alpha2/gateway_types.go b/apis/v1alpha2/gateway_types.go index ae8ab5ee39..57f219c16e 100644 --- a/apis/v1alpha2/gateway_types.go +++ b/apis/v1alpha2/gateway_types.go @@ -450,7 +450,6 @@ type GatewayAddress struct { // Type of the address. // // +optional - // +kubebuilder:validation:Enum=IPAddress;Hostname;NamedAddress // +kubebuilder:default=IPAddress Type *AddressType `json:"type,omitempty"` diff --git a/apis/v1alpha2/shared_types.go b/apis/v1alpha2/shared_types.go index 753f04e618..a63cea6773 100644 --- a/apis/v1alpha2/shared_types.go +++ b/apis/v1alpha2/shared_types.go @@ -480,6 +480,20 @@ type AnnotationKey string type AnnotationValue string // AddressType defines how a network address is represented as a text string. +// This may take two possible forms: +// +// * A CamelCase string identifier (like `IPAddress` or `Hostname`) +// * A domain-prefixed string identifier (like `acme.io/CustomAddressType`) +// +// Values `IPAddress` and `Hostname` have Extended support. +// +// All other values, including domain-prefixed values have Custom support, +// and are expected to be used by implementations to implement custom behavior +// in a compatible way. +// +// +kubebuilder:validation:MinLength=1 +// +kubebuilder:validation:MaxLength=253 +// +kubebuilder:validation:Pattern=`^([a-zA-Z0-9])+$|^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\/[a-zA-Z0-9]+$` type AddressType string const ( @@ -502,11 +516,4 @@ const ( // // Support: Extended HostnameAddressType AddressType = "Hostname" - - // A NamedAddress provides a way to reference a specific IP address by name. - // For example, this may be a name or other unique identifier that refers - // to a resource on a cloud provider such as a static IP. - // - // Support: Implementation-Specific - NamedAddressType AddressType = "NamedAddress" ) diff --git a/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml b/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml index 640bce1856..bf63dc1576 100644 --- a/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml +++ b/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml @@ -82,6 +82,9 @@ spec: - IPAddress - Hostname - NamedAddress + maxLength: 253 + minLength: 1 + pattern: ^([a-zA-Z0-9])+$|^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\/[a-zA-Z0-9]+$ type: string value: description: "Value of the address. The validity of the values @@ -467,6 +470,9 @@ spec: - IPAddress - Hostname - NamedAddress + maxLength: 253 + minLength: 1 + pattern: ^([a-zA-Z0-9])+$|^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\/[a-zA-Z0-9]+$ type: string value: description: "Value of the address. The validity of the values diff --git a/config/crd/stable/gateway.networking.k8s.io_gateways.yaml b/config/crd/stable/gateway.networking.k8s.io_gateways.yaml index bd71a22f36..c0d2b913fb 100644 --- a/config/crd/stable/gateway.networking.k8s.io_gateways.yaml +++ b/config/crd/stable/gateway.networking.k8s.io_gateways.yaml @@ -82,6 +82,9 @@ spec: - IPAddress - Hostname - NamedAddress + maxLength: 253 + minLength: 1 + pattern: ^([a-zA-Z0-9])+$|^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\/[a-zA-Z0-9]+$ type: string value: description: "Value of the address. The validity of the values @@ -467,6 +470,9 @@ spec: - IPAddress - Hostname - NamedAddress + maxLength: 253 + minLength: 1 + pattern: ^([a-zA-Z0-9])+$|^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\/[a-zA-Z0-9]+$ type: string value: description: "Value of the address. The validity of the values From 8e3d6ba316b1260031e0f7da2147b5d0075cfe91 Mon Sep 17 00:00:00 2001 From: Nick Young Date: Mon, 6 Jun 2022 07:39:20 +0000 Subject: [PATCH 2/3] Added webhook validation for AddressType fields Signed-off-by: Nick Young --- apis/v1alpha2/shared_types.go | 2 +- apis/v1alpha2/validation/gateway.go | 35 +++++++++++++++++++++++- apis/v1alpha2/validation/gateway_test.go | 34 +++++++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/apis/v1alpha2/shared_types.go b/apis/v1alpha2/shared_types.go index a63cea6773..365f7eb83d 100644 --- a/apis/v1alpha2/shared_types.go +++ b/apis/v1alpha2/shared_types.go @@ -482,7 +482,7 @@ type AnnotationValue string // AddressType defines how a network address is represented as a text string. // This may take two possible forms: // -// * A CamelCase string identifier (like `IPAddress` or `Hostname`) +// * A predefined CamelCase string identifier (currently limited to `IPAddress` or `Hostname`) // * A domain-prefixed string identifier (like `acme.io/CustomAddressType`) // // Values `IPAddress` and `Hostname` have Extended support. diff --git a/apis/v1alpha2/validation/gateway.go b/apis/v1alpha2/validation/gateway.go index 33f5d5ac3a..cb3abfaf0b 100644 --- a/apis/v1alpha2/validation/gateway.go +++ b/apis/v1alpha2/validation/gateway.go @@ -18,6 +18,7 @@ package validation import ( "fmt" + "regexp" "k8s.io/apimachinery/pkg/util/validation/field" @@ -36,6 +37,11 @@ var ( gatewayv1a2.UDPProtocolType: {}, gatewayv1a2.TCPProtocolType: {}, } + + addressTypesValid = map[gatewayv1a2.AddressType]struct{}{ + gatewayv1a2.HostnameAddressType: {}, + gatewayv1a2.IPAddressType: {}, + } ) // ValidateGateway validates gw according to the Gateway API specification. @@ -51,7 +57,10 @@ func ValidateGateway(gw *gatewayv1a2.Gateway) field.ErrorList { // validateGatewaySpec validates whether required fields of spec are set according to the // Gateway API specification. func validateGatewaySpec(spec *gatewayv1a2.GatewaySpec, path *field.Path) field.ErrorList { - return validateGatewayListeners(spec.Listeners, path.Child("listeners")) + var errs field.ErrorList + errs = append(errs, validateGatewayListeners(spec.Listeners, path.Child("listeners"))...) + errs = append(errs, validateAddresses(spec.Addresses, path.Child("addresses"))...) + return errs } // validateGatewayListeners validates whether required fields of listeners are set according @@ -89,3 +98,27 @@ func validateListenerHostname(listeners []gatewayv1a2.Listener, path *field.Path } return errs } + +// validateAddresses validates each listener address +// if there are addresses set. Otherwise, returns no error. +func validateAddresses(addresses []gatewayv1a2.GatewayAddress, path *field.Path) field.ErrorList { + var errs field.ErrorList + re := regexp.MustCompile(`^([a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9]\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\/[a-zA-Z0-9]+$`) + + for i, a := range addresses { + if a.Type == nil { + continue + } + _, ok := addressTypesValid[*a.Type] + if !ok { + // Found something that's not one of the upstream AddressTypes + // Next, check for a domain-prefixed string + match := re.Match([]byte(*a.Type)) + if !match { + errs = append(errs, field.Invalid(path.Index(i).Child("type"), a.Type, "should either be a defined constant or a domain-prefixed string (example.com/Type)")) + } + } + + } + return errs +} diff --git a/apis/v1alpha2/validation/gateway_test.go b/apis/v1alpha2/validation/gateway_test.go index 1ce7fc0d0b..449a0b61bb 100644 --- a/apis/v1alpha2/validation/gateway_test.go +++ b/apis/v1alpha2/validation/gateway_test.go @@ -30,6 +30,11 @@ func TestValidateGateway(t *testing.T) { Hostname: nil, }, } + addresses := []gatewayv1a2.GatewayAddress{ + { + Type: nil, + }, + } baseGateway := gatewayv1a2.Gateway{ ObjectMeta: metav1.ObjectMeta{ Name: "foo", @@ -38,6 +43,7 @@ func TestValidateGateway(t *testing.T) { Spec: gatewayv1a2.GatewaySpec{ GatewayClassName: "foo", Listeners: listeners, + Addresses: addresses, }, } tlsConfig := gatewayv1a2.GatewayTLSConfig{} @@ -76,6 +82,34 @@ func TestValidateGateway(t *testing.T) { }, expectErrsOnFields: []string{"spec.listeners[0].hostname"}, }, + "Address present with IPAddress": { + mutate: func(gw *gatewayv1a2.Gateway) { + ip := gatewayv1a2.IPAddressType + gw.Spec.Addresses[0].Type = &ip + }, + expectErrsOnFields: []string{}, + }, + "Address present with Hostname": { + mutate: func(gw *gatewayv1a2.Gateway) { + host := gatewayv1a2.HostnameAddressType + gw.Spec.Addresses[0].Type = &host + }, + expectErrsOnFields: []string{}, + }, + "Address present with example.com/CustomAddress": { + mutate: func(gw *gatewayv1a2.Gateway) { + customAddress := gatewayv1a2.AddressType("example.com/CustomAddress") + gw.Spec.Addresses[0].Type = &customAddress + }, + expectErrsOnFields: []string{}, + }, + "Address present with invalid Type": { + mutate: func(gw *gatewayv1a2.Gateway) { + customAddress := gatewayv1a2.AddressType("CustomAddress") + gw.Spec.Addresses[0].Type = &customAddress + }, + expectErrsOnFields: []string{"spec.addresses[0].type"}, + }, } for name, tc := range testCases { From 0d6eaffa5fb764af5681a385a01f6aebed01a96a Mon Sep 17 00:00:00 2001 From: Nick Young Date: Mon, 6 Jun 2022 23:16:26 +0000 Subject: [PATCH 3/3] Update with requested changes and regenerate Signed-off-by: Nick Young --- apis/v1alpha2/shared_types.go | 3 +-- apis/v1alpha2/validation/gateway.go | 8 ++++++-- .../experimental/gateway.networking.k8s.io_gateways.yaml | 8 -------- config/crd/stable/gateway.networking.k8s.io_gateways.yaml | 8 -------- 4 files changed, 7 insertions(+), 20 deletions(-) diff --git a/apis/v1alpha2/shared_types.go b/apis/v1alpha2/shared_types.go index 365f7eb83d..949bb8ef10 100644 --- a/apis/v1alpha2/shared_types.go +++ b/apis/v1alpha2/shared_types.go @@ -488,8 +488,7 @@ type AnnotationValue string // Values `IPAddress` and `Hostname` have Extended support. // // All other values, including domain-prefixed values have Custom support, -// and are expected to be used by implementations to implement custom behavior -// in a compatible way. +// which are used in implementation-specific behaviors. // // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=253 diff --git a/apis/v1alpha2/validation/gateway.go b/apis/v1alpha2/validation/gateway.go index cb3abfaf0b..7392e0e03a 100644 --- a/apis/v1alpha2/validation/gateway.go +++ b/apis/v1alpha2/validation/gateway.go @@ -99,11 +99,15 @@ func validateListenerHostname(listeners []gatewayv1a2.Listener, path *field.Path return errs } +// domainPrefixedStringRegex is a regex used in validation to determine whether +// a provided string is a domain-prefixed string. Domain-prefixed strings are used +// to indicate custom (implementation-specific) address types. +var domainPrefixedStringRegex = regexp.MustCompile(`^([a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9]\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\/[a-zA-Z0-9]+$`) + // validateAddresses validates each listener address // if there are addresses set. Otherwise, returns no error. func validateAddresses(addresses []gatewayv1a2.GatewayAddress, path *field.Path) field.ErrorList { var errs field.ErrorList - re := regexp.MustCompile(`^([a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9]\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\/[a-zA-Z0-9]+$`) for i, a := range addresses { if a.Type == nil { @@ -113,7 +117,7 @@ func validateAddresses(addresses []gatewayv1a2.GatewayAddress, path *field.Path) if !ok { // Found something that's not one of the upstream AddressTypes // Next, check for a domain-prefixed string - match := re.Match([]byte(*a.Type)) + match := domainPrefixedStringRegex.Match([]byte(*a.Type)) if !match { errs = append(errs, field.Invalid(path.Index(i).Child("type"), a.Type, "should either be a defined constant or a domain-prefixed string (example.com/Type)")) } diff --git a/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml b/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml index bf63dc1576..3e87039c83 100644 --- a/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml +++ b/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml @@ -78,10 +78,6 @@ spec: type: default: IPAddress description: Type of the address. - enum: - - IPAddress - - Hostname - - NamedAddress maxLength: 253 minLength: 1 pattern: ^([a-zA-Z0-9])+$|^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\/[a-zA-Z0-9]+$ @@ -466,10 +462,6 @@ spec: type: default: IPAddress description: Type of the address. - enum: - - IPAddress - - Hostname - - NamedAddress maxLength: 253 minLength: 1 pattern: ^([a-zA-Z0-9])+$|^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\/[a-zA-Z0-9]+$ diff --git a/config/crd/stable/gateway.networking.k8s.io_gateways.yaml b/config/crd/stable/gateway.networking.k8s.io_gateways.yaml index c0d2b913fb..9927d0a1b3 100644 --- a/config/crd/stable/gateway.networking.k8s.io_gateways.yaml +++ b/config/crd/stable/gateway.networking.k8s.io_gateways.yaml @@ -78,10 +78,6 @@ spec: type: default: IPAddress description: Type of the address. - enum: - - IPAddress - - Hostname - - NamedAddress maxLength: 253 minLength: 1 pattern: ^([a-zA-Z0-9])+$|^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\/[a-zA-Z0-9]+$ @@ -466,10 +462,6 @@ spec: type: default: IPAddress description: Type of the address. - enum: - - IPAddress - - Hostname - - NamedAddress maxLength: 253 minLength: 1 pattern: ^([a-zA-Z0-9])+$|^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])\/[a-zA-Z0-9]+$