diff --git a/apis/v1/gateway_types.go b/apis/v1/gateway_types.go index 10eb5bee06..58d9751866 100644 --- a/apis/v1/gateway_types.go +++ b/apis/v1/gateway_types.go @@ -246,7 +246,7 @@ type GatewaySpec struct { // Addresses requested for this Gateway. This is optional and behavior can // depend on the implementation. If a value is set in the spec and the // requested address is invalid or unavailable, the implementation MUST - // indicate this in the associated entry in GatewayStatus.Addresses. + // indicate this in an associated entry in GatewayStatus.Conditions. // // The Addresses field represents a request for the address(es) on the // "outside of the Gateway", that traffic bound for this Gateway will use. @@ -268,8 +268,8 @@ type GatewaySpec struct { // +listType=atomic // // +kubebuilder:validation:MaxItems=16 - // +kubebuilder:validation:XValidation:message="IPAddress values must be unique",rule="self.all(a1, a1.type == 'IPAddress' ? self.exists_one(a2, a2.type == a1.type && a2.value == a1.value) : true )" - // +kubebuilder:validation:XValidation:message="Hostname values must be unique",rule="self.all(a1, a1.type == 'Hostname' ? self.exists_one(a2, a2.type == a1.type && a2.value == a1.value) : true )" + // +kubebuilder:validation:XValidation:message="IPAddress values must be unique",rule="self.all(a1, a1.type == 'IPAddress' && has(a1.value) ? self.exists_one(a2, a2.type == a1.type && has(a2.value) && a2.value == a1.value) : true )" + // +kubebuilder:validation:XValidation:message="Hostname values must be unique",rule="self.all(a1, a1.type == 'Hostname' && has(a1.value) ? self.exists_one(a2, a2.type == a1.type && has(a2.value) && a2.value == a1.value) : true )" Addresses []GatewaySpecAddress `json:"addresses,omitempty"` // Infrastructure defines infrastructure level attributes about this Gateway instance. @@ -884,7 +884,7 @@ type RouteGroupKind struct { // GatewaySpecAddress describes an address that can be bound to a Gateway. // -// +kubebuilder:validation:XValidation:message="Hostname value must only contain valid characters (matching ^(\\*\\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$)",rule="self.type == 'Hostname' ? self.value.matches(r\"\"\"^(\\*\\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$\"\"\"): true" +// +kubebuilder:validation:XValidation:message="Hostname value must be empty or contain only valid characters (matching ^(\\*\\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$)",rule="self.type == 'Hostname' ? (!has(self.value) || self.value.matches(r\"\"\"^(\\*\\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$\"\"\")): true" type GatewaySpecAddress struct { // Type of the address. // diff --git a/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml b/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml index 563669ba35..dc78ec615e 100644 --- a/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml +++ b/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml @@ -64,7 +64,7 @@ spec: Addresses requested for this Gateway. This is optional and behavior can depend on the implementation. If a value is set in the spec and the requested address is invalid or unavailable, the implementation MUST - indicate this in the associated entry in GatewayStatus.Addresses. + indicate this in an associated entry in GatewayStatus.Conditions. The Addresses field represents a request for the address(es) on the "outside of the Gateway", that traffic bound for this Gateway will use. @@ -119,20 +119,22 @@ spec: type: string type: object x-kubernetes-validations: - - message: Hostname value must only contain valid characters (matching - ^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$) - rule: 'self.type == ''Hostname'' ? self.value.matches(r"""^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$"""): + - message: Hostname value must be empty or contain only valid characters + (matching ^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$) + rule: 'self.type == ''Hostname'' ? (!has(self.value) || self.value.matches(r"""^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$""")): true' maxItems: 16 type: array x-kubernetes-list-type: atomic x-kubernetes-validations: - message: IPAddress values must be unique - rule: 'self.all(a1, a1.type == ''IPAddress'' ? self.exists_one(a2, - a2.type == a1.type && a2.value == a1.value) : true )' + rule: 'self.all(a1, a1.type == ''IPAddress'' && has(a1.value) ? + self.exists_one(a2, a2.type == a1.type && has(a2.value) && a2.value + == a1.value) : true )' - message: Hostname values must be unique - rule: 'self.all(a1, a1.type == ''Hostname'' ? self.exists_one(a2, - a2.type == a1.type && a2.value == a1.value) : true )' + rule: 'self.all(a1, a1.type == ''Hostname'' && has(a1.value) ? + self.exists_one(a2, a2.type == a1.type && has(a2.value) && a2.value + == a1.value) : true )' allowedListeners: description: |- AllowedListeners defines which ListenerSets can be attached to this Gateway. @@ -1639,7 +1641,7 @@ spec: Addresses requested for this Gateway. This is optional and behavior can depend on the implementation. If a value is set in the spec and the requested address is invalid or unavailable, the implementation MUST - indicate this in the associated entry in GatewayStatus.Addresses. + indicate this in an associated entry in GatewayStatus.Conditions. The Addresses field represents a request for the address(es) on the "outside of the Gateway", that traffic bound for this Gateway will use. @@ -1694,20 +1696,22 @@ spec: type: string type: object x-kubernetes-validations: - - message: Hostname value must only contain valid characters (matching - ^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$) - rule: 'self.type == ''Hostname'' ? self.value.matches(r"""^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$"""): + - message: Hostname value must be empty or contain only valid characters + (matching ^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$) + rule: 'self.type == ''Hostname'' ? (!has(self.value) || self.value.matches(r"""^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$""")): true' maxItems: 16 type: array x-kubernetes-list-type: atomic x-kubernetes-validations: - message: IPAddress values must be unique - rule: 'self.all(a1, a1.type == ''IPAddress'' ? self.exists_one(a2, - a2.type == a1.type && a2.value == a1.value) : true )' + rule: 'self.all(a1, a1.type == ''IPAddress'' && has(a1.value) ? + self.exists_one(a2, a2.type == a1.type && has(a2.value) && a2.value + == a1.value) : true )' - message: Hostname values must be unique - rule: 'self.all(a1, a1.type == ''Hostname'' ? self.exists_one(a2, - a2.type == a1.type && a2.value == a1.value) : true )' + rule: 'self.all(a1, a1.type == ''Hostname'' && has(a1.value) ? + self.exists_one(a2, a2.type == a1.type && has(a2.value) && a2.value + == a1.value) : true )' allowedListeners: description: |- AllowedListeners defines which ListenerSets can be attached to this Gateway. diff --git a/config/crd/standard/gateway.networking.k8s.io_gateways.yaml b/config/crd/standard/gateway.networking.k8s.io_gateways.yaml index 98914347d2..43ab793f8a 100644 --- a/config/crd/standard/gateway.networking.k8s.io_gateways.yaml +++ b/config/crd/standard/gateway.networking.k8s.io_gateways.yaml @@ -64,7 +64,7 @@ spec: Addresses requested for this Gateway. This is optional and behavior can depend on the implementation. If a value is set in the spec and the requested address is invalid or unavailable, the implementation MUST - indicate this in the associated entry in GatewayStatus.Addresses. + indicate this in an associated entry in GatewayStatus.Conditions. The Addresses field represents a request for the address(es) on the "outside of the Gateway", that traffic bound for this Gateway will use. @@ -119,20 +119,22 @@ spec: type: string type: object x-kubernetes-validations: - - message: Hostname value must only contain valid characters (matching - ^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$) - rule: 'self.type == ''Hostname'' ? self.value.matches(r"""^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$"""): + - message: Hostname value must be empty or contain only valid characters + (matching ^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$) + rule: 'self.type == ''Hostname'' ? (!has(self.value) || self.value.matches(r"""^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$""")): true' maxItems: 16 type: array x-kubernetes-list-type: atomic x-kubernetes-validations: - message: IPAddress values must be unique - rule: 'self.all(a1, a1.type == ''IPAddress'' ? self.exists_one(a2, - a2.type == a1.type && a2.value == a1.value) : true )' + rule: 'self.all(a1, a1.type == ''IPAddress'' && has(a1.value) ? + self.exists_one(a2, a2.type == a1.type && has(a2.value) && a2.value + == a1.value) : true )' - message: Hostname values must be unique - rule: 'self.all(a1, a1.type == ''Hostname'' ? self.exists_one(a2, - a2.type == a1.type && a2.value == a1.value) : true )' + rule: 'self.all(a1, a1.type == ''Hostname'' && has(a1.value) ? + self.exists_one(a2, a2.type == a1.type && has(a2.value) && a2.value + == a1.value) : true )' gatewayClassName: description: |- GatewayClassName used for this Gateway. This is the name of a @@ -1175,7 +1177,7 @@ spec: Addresses requested for this Gateway. This is optional and behavior can depend on the implementation. If a value is set in the spec and the requested address is invalid or unavailable, the implementation MUST - indicate this in the associated entry in GatewayStatus.Addresses. + indicate this in an associated entry in GatewayStatus.Conditions. The Addresses field represents a request for the address(es) on the "outside of the Gateway", that traffic bound for this Gateway will use. @@ -1230,20 +1232,22 @@ spec: type: string type: object x-kubernetes-validations: - - message: Hostname value must only contain valid characters (matching - ^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$) - rule: 'self.type == ''Hostname'' ? self.value.matches(r"""^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$"""): + - message: Hostname value must be empty or contain only valid characters + (matching ^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$) + rule: 'self.type == ''Hostname'' ? (!has(self.value) || self.value.matches(r"""^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$""")): true' maxItems: 16 type: array x-kubernetes-list-type: atomic x-kubernetes-validations: - message: IPAddress values must be unique - rule: 'self.all(a1, a1.type == ''IPAddress'' ? self.exists_one(a2, - a2.type == a1.type && a2.value == a1.value) : true )' + rule: 'self.all(a1, a1.type == ''IPAddress'' && has(a1.value) ? + self.exists_one(a2, a2.type == a1.type && has(a2.value) && a2.value + == a1.value) : true )' - message: Hostname values must be unique - rule: 'self.all(a1, a1.type == ''Hostname'' ? self.exists_one(a2, - a2.type == a1.type && a2.value == a1.value) : true )' + rule: 'self.all(a1, a1.type == ''Hostname'' && has(a1.value) ? + self.exists_one(a2, a2.type == a1.type && has(a2.value) && a2.value + == a1.value) : true )' gatewayClassName: description: |- GatewayClassName used for this Gateway. This is the name of a diff --git a/conformance/tests/gateway-static-addresses.go b/conformance/tests/gateway-static-addresses.go index fcee1b3f99..89654c9ed8 100644 --- a/conformance/tests/gateway-static-addresses.go +++ b/conformance/tests/gateway-static-addresses.go @@ -135,8 +135,8 @@ var GatewayStaticAddresses = suite.ConformanceTest{ kubernetes.GatewayStatusMustHaveListeners(t, s.Client, s.TimeoutConfig, gwNN, finalExpectedListenerState) require.Len(t, currentGW.Spec.Addresses, 1, "expected only 1 address left specified on Gateway") statusAddresses := extractStatusAddresses(currentGW.Status.Addresses) - require.NotContains(t, statusAddresses, unusableAddress.Value, "should contain the unusable address") - require.NotContains(t, statusAddresses, invalidAddress.Value, "should contain the invalid address") + require.NotContains(t, statusAddresses, unusableAddress.Value, "should not contain the unusable address") + require.NotContains(t, statusAddresses, invalidAddress.Value, "should not contain the invalid address") require.Contains(t, statusAddresses, usableAddress.Value, "should contain the usable address") for _, addr := range currentGW.Status.Addresses { if usableAddress.Value != addr.Value { diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go index bfa005bef0..fd0b2ab150 100644 --- a/pkg/generated/openapi/zz_generated.openapi.go +++ b/pkg/generated/openapi/zz_generated.openapi.go @@ -4196,7 +4196,7 @@ func schema_sigsk8sio_gateway_api_apis_v1_GatewaySpec(ref common.ReferenceCallba }, }, SchemaProps: spec.SchemaProps{ - Description: "Addresses requested for this Gateway. This is optional and behavior can depend on the implementation. If a value is set in the spec and the requested address is invalid or unavailable, the implementation MUST indicate this in the associated entry in GatewayStatus.Addresses.\n\nThe Addresses field represents a request for the address(es) on the \"outside of the Gateway\", that traffic bound for this Gateway will use. This could be the IP address or hostname of an external load balancer or other networking infrastructure, or some other address that traffic will be sent to.\n\nIf no Addresses are specified, the implementation MAY schedule the Gateway in an implementation-specific manner, assigning an appropriate set of Addresses.\n\nThe implementation MUST bind all Listeners to every GatewayAddress that it assigns to the Gateway and add a corresponding entry in GatewayStatus.Addresses.\n\nSupport: Extended\n\n", + Description: "Addresses requested for this Gateway. This is optional and behavior can depend on the implementation. If a value is set in the spec and the requested address is invalid or unavailable, the implementation MUST indicate this in an associated entry in GatewayStatus.Conditions.\n\nThe Addresses field represents a request for the address(es) on the \"outside of the Gateway\", that traffic bound for this Gateway will use. This could be the IP address or hostname of an external load balancer or other networking infrastructure, or some other address that traffic will be sent to.\n\nIf no Addresses are specified, the implementation MAY schedule the Gateway in an implementation-specific manner, assigning an appropriate set of Addresses.\n\nThe implementation MUST bind all Listeners to every GatewayAddress that it assigns to the Gateway and add a corresponding entry in GatewayStatus.Addresses.\n\nSupport: Extended\n\n", Type: []string{"array"}, Items: &spec.SchemaOrArray{ Schema: &spec.Schema{ diff --git a/pkg/test/cel/gateway_test.go b/pkg/test/cel/gateway_test.go index ba85b50f2e..0bff6093b6 100644 --- a/pkg/test/cel/gateway_test.go +++ b/pkg/test/cel/gateway_test.go @@ -584,6 +584,54 @@ func TestValidateGateway(t *testing.T) { }, wantErrors: []string{"IPAddress values must be unique", "Hostname values must be unique"}, }, + { + desc: "optional ip address or hostname values", + mutate: func(gw *gatewayv1.Gateway) { + gw.Spec.Addresses = []gatewayv1.GatewaySpecAddress{ + { + Type: ptrTo(gatewayv1.IPAddressType), + }, + { + Type: ptrTo(gatewayv1.HostnameAddressType), + }, + } + }, + }, + { + desc: "multiple optional ip address or hostname values alongside defined values", + mutate: func(gw *gatewayv1.Gateway) { + gw.Spec.Addresses = []gatewayv1.GatewaySpecAddress{ + { + Type: ptrTo(gatewayv1.HostnameAddressType), + }, + { + Type: ptrTo(gatewayv1.IPAddressType), + }, + { + Type: ptrTo(gatewayv1.IPAddressType), + Value: "1.2.3.4", + }, + { + Type: ptrTo(gatewayv1.HostnameAddressType), + Value: "foo.bar", + }, + { + Type: ptrTo(gatewayv1.IPAddressType), + }, + { + Type: ptrTo(gatewayv1.HostnameAddressType), + }, + { + Type: ptrTo(gatewayv1.IPAddressType), + Value: "2.3.4.5", + }, + { + Type: ptrTo(gatewayv1.HostnameAddressType), + Value: "bar.bar", + }, + } + }, + }, } for _, tc := range testCases {