From 28e6f1ed6e17b2c73515a874d93cdbd6f9e41d9d Mon Sep 17 00:00:00 2001 From: Norwin Schnyder Date: Tue, 20 Aug 2024 08:20:57 +0000 Subject: [PATCH 1/3] Validation for label keys and values according to Kubernetes specification Signed-off-by: Norwin Schnyder --- .../apis/v1/gatewayinfrastructure.go | 6 +- apis/v1/gateway_types.go | 4 +- apis/v1/shared_types.go | 41 ++++++++++++- apis/v1/zz_generated.deepcopy.go | 2 +- .../gateway.networking.k8s.io_gateways.yaml | 60 +++++++++++++++---- 5 files changed, 97 insertions(+), 16 deletions(-) diff --git a/apis/applyconfiguration/apis/v1/gatewayinfrastructure.go b/apis/applyconfiguration/apis/v1/gatewayinfrastructure.go index 43ca7e63e0..7c55b1b70c 100644 --- a/apis/applyconfiguration/apis/v1/gatewayinfrastructure.go +++ b/apis/applyconfiguration/apis/v1/gatewayinfrastructure.go @@ -25,7 +25,7 @@ import ( // GatewayInfrastructureApplyConfiguration represents an declarative configuration of the GatewayInfrastructure type for use // with apply. type GatewayInfrastructureApplyConfiguration struct { - Labels map[v1.AnnotationKey]v1.AnnotationValue `json:"labels,omitempty"` + Labels map[v1.LabelKey]v1.LabelValue `json:"labels,omitempty"` Annotations map[v1.AnnotationKey]v1.AnnotationValue `json:"annotations,omitempty"` ParametersRef *LocalParametersReferenceApplyConfiguration `json:"parametersRef,omitempty"` } @@ -40,9 +40,9 @@ func GatewayInfrastructure() *GatewayInfrastructureApplyConfiguration { // and returns the receiver, so that objects can be build by chaining "With" function invocations. // If called multiple times, the entries provided by each call will be put on the Labels field, // overwriting an existing map entries in Labels field with the same key. -func (b *GatewayInfrastructureApplyConfiguration) WithLabels(entries map[v1.AnnotationKey]v1.AnnotationValue) *GatewayInfrastructureApplyConfiguration { +func (b *GatewayInfrastructureApplyConfiguration) WithLabels(entries map[v1.LabelKey]v1.LabelValue) *GatewayInfrastructureApplyConfiguration { if b.Labels == nil && len(entries) > 0 { - b.Labels = make(map[v1.AnnotationKey]v1.AnnotationValue, len(entries)) + b.Labels = make(map[v1.LabelKey]v1.LabelValue, len(entries)) } for k, v := range entries { b.Labels[k] = v diff --git a/apis/v1/gateway_types.go b/apis/v1/gateway_types.go index 2c8b3d8081..a2300c800a 100644 --- a/apis/v1/gateway_types.go +++ b/apis/v1/gateway_types.go @@ -683,7 +683,8 @@ type GatewayInfrastructure struct { // // +optional // +kubebuilder:validation:MaxProperties=8 - Labels map[AnnotationKey]AnnotationValue `json:"labels,omitempty"` + // +kubebuilder:validation:XValidation:message="Label keys must be in the form of an optional DNS subdomain prefix followed by a required name segment of up to 63 characters.",rule="self.all(key, key.matches(r\"\"\"^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?([A-Za-z0-9][-A-Za-z0-9_.]{0,61})?[A-Za-z0-9]$\"\"\"))" + Labels map[LabelKey]LabelValue `json:"labels,omitempty"` // Annotations that SHOULD be applied to any resources created in response to this Gateway. // @@ -696,6 +697,7 @@ type GatewayInfrastructure struct { // // +optional // +kubebuilder:validation:MaxProperties=8 + // +kubebuilder:validation:XValidation:message="Annotation keys must be in the form of an optional DNS subdomain prefix followed by a required name segment of up to 63 characters.",rule="self.all(key, key.matches(r\"\"\"^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?([A-Za-z0-9][-A-Za-z0-9_.]{0,61})?[A-Za-z0-9]$\"\"\"))" Annotations map[AnnotationKey]AnnotationValue `json:"annotations,omitempty"` // ParametersRef is a reference to a resource that contains the configuration diff --git a/apis/v1/shared_types.go b/apis/v1/shared_types.go index 954c605428..28822fa905 100644 --- a/apis/v1/shared_types.go +++ b/apis/v1/shared_types.go @@ -666,7 +666,7 @@ type GatewayController string // // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=253 -// +kubebuilder:validation:Pattern=`^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]/?)*$` +// +kubebuilder:validation:Pattern=`^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?([A-Za-z0-9][-A-Za-z0-9_.]{0,61})?[A-Za-z0-9]$` type AnnotationKey string // AnnotationValue is the value of an annotation in Gateway API. This is used @@ -678,6 +678,45 @@ type AnnotationKey string // +kubebuilder:validation:MaxLength=4096 type AnnotationValue string +// LabelKey is the key of a label in the Gateway API. This is used for validation +// of maps such as Gateway infrastructure labels. This matches the Kubernetes +// "qualified name" validation that is used for labels. +// +// Valid values include: +// +// * example +// * example.com +// * example.com/path +// * example.com/path.html +// +// Invalid values include: +// +// * example~ - "~" is an invalid character +// * example.com. - can not start or end with "." +// +// +kubebuilder:validation:MinLength=1 +// +kubebuilder:validation:MaxLength=253 +// +kubebuilder:validation:Pattern=`^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?([A-Za-z0-9][-A-Za-z0-9_.]{0,61})?[A-Za-z0-9]$` +type LabelKey string + +// LabelValue is the value of a label in the Gateway API. This is used for validation +// of maps such as Gateway infrastructure labels. This matches the Kubernetes +// label validation rules: +// * must be 63 characters or less (can be empty), +// * unless empty, must begin and end with an alphanumeric character ([a-z0-9A-Z]), +// * could contain dashes (-), underscores (_), dots (.), and alphanumerics between. +// +// Valid values include: +// +// * MyValue +// * my.name +// * 123-my-value +// +// +kubebuilder:validation:MinLength=0 +// +kubebuilder:validation:MaxLength=63 +// +kubebuilder:validation:Pattern=`^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$` +type LabelValue string + // AddressType defines how a network address is represented as a text string. // This may take two possible forms: // diff --git a/apis/v1/zz_generated.deepcopy.go b/apis/v1/zz_generated.deepcopy.go index 02906dc1a6..d11fd02b1f 100644 --- a/apis/v1/zz_generated.deepcopy.go +++ b/apis/v1/zz_generated.deepcopy.go @@ -619,7 +619,7 @@ func (in *GatewayInfrastructure) DeepCopyInto(out *GatewayInfrastructure) { *out = *in if in.Labels != nil { in, out := &in.Labels, &out.Labels - *out = make(map[AnnotationKey]AnnotationValue, len(*in)) + *out = make(map[LabelKey]LabelValue, len(*in)) for key, val := range *in { (*out)[key] = val } diff --git a/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml b/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml index 8f4ad10138..b8df3deb71 100644 --- a/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml +++ b/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml @@ -180,15 +180,31 @@ spec: Support: Extended maxProperties: 8 type: object + x-kubernetes-validations: + - message: Annotation keys must be in the form of an optional + DNS subdomain prefix followed by a required name segment of + up to 63 characters. + rule: self.all(key, key.matches(r"""^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?([A-Za-z0-9][-A-Za-z0-9_.]{0,61})?[A-Za-z0-9]$""")) labels: additionalProperties: description: |- - AnnotationValue is the value of an annotation in Gateway API. This is used - for validation of maps such as TLS options. This roughly matches Kubernetes - annotation validation, although the length validation in that case is based - on the entire size of the annotations struct. - maxLength: 4096 + LabelValue is the value of a label in the Gateway API. This is used for validation + of maps such as Gateway infrastructure labels. This matches the Kubernetes + label validation rules: + * must be 63 characters or less (can be empty), + * unless empty, must begin and end with an alphanumeric character ([a-z0-9A-Z]), + * could contain dashes (-), underscores (_), dots (.), and alphanumerics between. + + + Valid values include: + + + * MyValue + * my.name + * 123-my-value + maxLength: 63 minLength: 0 + pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$ type: string description: |- Labels that SHOULD be applied to any resources created in response to this Gateway. @@ -204,6 +220,10 @@ spec: Support: Extended maxProperties: 8 type: object + x-kubernetes-validations: + - message: Label keys must be in the form of an optional DNS subdomain + prefix followed by a required name segment of up to 63 characters. + rule: self.all(key, key.matches(r"""^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?([A-Za-z0-9][-A-Za-z0-9_.]{0,61})?[A-Za-z0-9]$""")) parametersRef: description: |- ParametersRef is a reference to a resource that contains the configuration @@ -1407,15 +1427,31 @@ spec: Support: Extended maxProperties: 8 type: object + x-kubernetes-validations: + - message: Annotation keys must be in the form of an optional + DNS subdomain prefix followed by a required name segment of + up to 63 characters. + rule: self.all(key, key.matches(r"""^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?([A-Za-z0-9][-A-Za-z0-9_.]{0,61})?[A-Za-z0-9]$""")) labels: additionalProperties: description: |- - AnnotationValue is the value of an annotation in Gateway API. This is used - for validation of maps such as TLS options. This roughly matches Kubernetes - annotation validation, although the length validation in that case is based - on the entire size of the annotations struct. - maxLength: 4096 + LabelValue is the value of a label in the Gateway API. This is used for validation + of maps such as Gateway infrastructure labels. This matches the Kubernetes + label validation rules: + * must be 63 characters or less (can be empty), + * unless empty, must begin and end with an alphanumeric character ([a-z0-9A-Z]), + * could contain dashes (-), underscores (_), dots (.), and alphanumerics between. + + + Valid values include: + + + * MyValue + * my.name + * 123-my-value + maxLength: 63 minLength: 0 + pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$ type: string description: |- Labels that SHOULD be applied to any resources created in response to this Gateway. @@ -1431,6 +1467,10 @@ spec: Support: Extended maxProperties: 8 type: object + x-kubernetes-validations: + - message: Label keys must be in the form of an optional DNS subdomain + prefix followed by a required name segment of up to 63 characters. + rule: self.all(key, key.matches(r"""^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?([A-Za-z0-9][-A-Za-z0-9_.]{0,61})?[A-Za-z0-9]$""")) parametersRef: description: |- ParametersRef is a reference to a resource that contains the configuration From cdc8e223a08b4514433357665b8c879e59a1d242 Mon Sep 17 00:00:00 2001 From: Norwin Schnyder Date: Wed, 21 Aug 2024 06:56:10 +0000 Subject: [PATCH 2/3] Validate prefix length of DNS subdomain in label and anootaion keys Signed-off-by: Norwin Schnyder --- apis/v1/gateway_types.go | 2 ++ .../gateway.networking.k8s.io_gateways.yaml | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/apis/v1/gateway_types.go b/apis/v1/gateway_types.go index a2300c800a..17fad4d9b3 100644 --- a/apis/v1/gateway_types.go +++ b/apis/v1/gateway_types.go @@ -684,6 +684,7 @@ type GatewayInfrastructure struct { // +optional // +kubebuilder:validation:MaxProperties=8 // +kubebuilder:validation:XValidation:message="Label keys must be in the form of an optional DNS subdomain prefix followed by a required name segment of up to 63 characters.",rule="self.all(key, key.matches(r\"\"\"^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?([A-Za-z0-9][-A-Za-z0-9_.]{0,61})?[A-Za-z0-9]$\"\"\"))" + // +kubebuilder:validation:XValidation:message="If specified, the label key's prefix must be a DNS subdomain not longer than 253 characters in total.",rule="self.all(key, key.split(\"/\")[0].size() < 253)" Labels map[LabelKey]LabelValue `json:"labels,omitempty"` // Annotations that SHOULD be applied to any resources created in response to this Gateway. @@ -698,6 +699,7 @@ type GatewayInfrastructure struct { // +optional // +kubebuilder:validation:MaxProperties=8 // +kubebuilder:validation:XValidation:message="Annotation keys must be in the form of an optional DNS subdomain prefix followed by a required name segment of up to 63 characters.",rule="self.all(key, key.matches(r\"\"\"^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?([A-Za-z0-9][-A-Za-z0-9_.]{0,61})?[A-Za-z0-9]$\"\"\"))" + // +kubebuilder:validation:XValidation:message="If specified, the annotation key's prefix must be a DNS subdomain not longer than 253 characters in total.",rule="self.all(key, key.split(\"/\")[0].size() < 253)" Annotations map[AnnotationKey]AnnotationValue `json:"annotations,omitempty"` // ParametersRef is a reference to a resource that contains the configuration diff --git a/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml b/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml index b8df3deb71..ca34e3976e 100644 --- a/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml +++ b/config/crd/experimental/gateway.networking.k8s.io_gateways.yaml @@ -185,6 +185,9 @@ spec: DNS subdomain prefix followed by a required name segment of up to 63 characters. rule: self.all(key, key.matches(r"""^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?([A-Za-z0-9][-A-Za-z0-9_.]{0,61})?[A-Za-z0-9]$""")) + - message: If specified, the annotation key's prefix must be a + DNS subdomain not longer than 253 characters in total. + rule: self.all(key, key.split("/")[0].size() < 253) labels: additionalProperties: description: |- @@ -224,6 +227,9 @@ spec: - message: Label keys must be in the form of an optional DNS subdomain prefix followed by a required name segment of up to 63 characters. rule: self.all(key, key.matches(r"""^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?([A-Za-z0-9][-A-Za-z0-9_.]{0,61})?[A-Za-z0-9]$""")) + - message: If specified, the label key's prefix must be a DNS + subdomain not longer than 253 characters in total. + rule: self.all(key, key.split("/")[0].size() < 253) parametersRef: description: |- ParametersRef is a reference to a resource that contains the configuration @@ -1432,6 +1438,9 @@ spec: DNS subdomain prefix followed by a required name segment of up to 63 characters. rule: self.all(key, key.matches(r"""^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?([A-Za-z0-9][-A-Za-z0-9_.]{0,61})?[A-Za-z0-9]$""")) + - message: If specified, the annotation key's prefix must be a + DNS subdomain not longer than 253 characters in total. + rule: self.all(key, key.split("/")[0].size() < 253) labels: additionalProperties: description: |- @@ -1471,6 +1480,9 @@ spec: - message: Label keys must be in the form of an optional DNS subdomain prefix followed by a required name segment of up to 63 characters. rule: self.all(key, key.matches(r"""^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?([A-Za-z0-9][-A-Za-z0-9_.]{0,61})?[A-Za-z0-9]$""")) + - message: If specified, the label key's prefix must be a DNS + subdomain not longer than 253 characters in total. + rule: self.all(key, key.split("/")[0].size() < 253) parametersRef: description: |- ParametersRef is a reference to a resource that contains the configuration From 3b35eef933a64cb2960f697b015695eaf50555ed Mon Sep 17 00:00:00 2001 From: Norwin Schnyder Date: Mon, 2 Sep 2024 07:25:19 +0000 Subject: [PATCH 3/3] add tests for label key and value validation Signed-off-by: Norwin Schnyder --- pkg/test/cel/gateway_experimental_test.go | 135 ++++++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 pkg/test/cel/gateway_experimental_test.go diff --git a/pkg/test/cel/gateway_experimental_test.go b/pkg/test/cel/gateway_experimental_test.go new file mode 100644 index 0000000000..aa35a3fd28 --- /dev/null +++ b/pkg/test/cel/gateway_experimental_test.go @@ -0,0 +1,135 @@ +//go:build experimental +// +build experimental + +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "context" + "fmt" + "strings" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +func TestGatewayInfrastructureLabels(t *testing.T) { + ctx := context.Background() + baseGateway := gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: metav1.NamespaceDefault, + }, + Spec: gatewayv1.GatewaySpec{ + GatewayClassName: "foo", + Listeners: []gatewayv1.Listener{ + { + Name: gatewayv1.SectionName("http"), + Protocol: gatewayv1.HTTPProtocolType, + Port: gatewayv1.PortNumber(80), + }, + }, + }, + } + + testCases := []struct { + name string + wantErrors []string + labels map[gatewayv1.LabelKey]gatewayv1.LabelValue + }{ + { + name: "valid label keys and values", + labels: map[gatewayv1.LabelKey]gatewayv1.LabelValue{ + "app": "gateway", + "tier": "frontend", + "example": "MyValue", + "example.com": "my.name", + "example.com/path": "123-my-value", + "example.com/path.html": "", + }, + }, + { + name: "invalid label key with invalid DNS prefix", + labels: map[gatewayv1.LabelKey]gatewayv1.LabelValue{ + "Example.com/key": "value", + }, + wantErrors: []string{"Label keys must be in the form of an optional DNS subdomain prefix followed by a required name segment of up to 63 characters"}, + }, + { + name: "invalid label key with invalid name", + labels: map[gatewayv1.LabelKey]gatewayv1.LabelValue{ + "key~@@@": "value", + }, + wantErrors: []string{"Label keys must be in the form of an optional DNS subdomain prefix followed by a required name segment of up to 63 characters"}, + }, + { + name: "invalid label key with DNS prefix too long", + labels: map[gatewayv1.LabelKey]gatewayv1.LabelValue{ + gatewayv1.LabelKey(strings.Repeat("a", 254) + "/key"): "value", + }, + wantErrors: []string{"If specified, the label key's prefix must be a DNS subdomain not longer than 253 characters in total."}, + }, + { + name: "invalid label key with name too long", + labels: map[gatewayv1.LabelKey]gatewayv1.LabelValue{ + gatewayv1.LabelKey(strings.Repeat("a", 64)): "value", + }, + wantErrors: []string{"Label keys must be in the form of an optional DNS subdomain prefix followed by a required name segment of up to 63 characters."}, + }, + { + name: "invalid label value with too many characters", + labels: map[gatewayv1.LabelKey]gatewayv1.LabelValue{ + "key": gatewayv1.LabelValue(strings.Repeat("a", 64)), + }, + wantErrors: []string{"Too long: may not be longer than 63"}, + }, + { + name: "invalid label value with invalid characters", + labels: map[gatewayv1.LabelKey]gatewayv1.LabelValue{ + "key": "v a l u e", + }, + wantErrors: []string{"spec.infrastructure.labels.key in body should match '^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$'"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gw := baseGateway.DeepCopy() + gw.Name = fmt.Sprintf("foo-%v", time.Now().UnixNano()) + + gw.Spec.Infrastructure = &gatewayv1.GatewayInfrastructure{Labels: tc.labels} + err := k8sClient.Create(ctx, gw) + + if (len(tc.wantErrors) != 0) != (err != nil) { + t.Fatalf("Unexpected response while creating Gateway; got err=\n%v\n;want error=%v", err, tc.wantErrors != nil) + } + + var missingErrorStrings []string + for _, wantError := range tc.wantErrors { + if !strings.Contains(strings.ToLower(err.Error()), strings.ToLower(wantError)) { + missingErrorStrings = append(missingErrorStrings, wantError) + } + } + if len(missingErrorStrings) != 0 { + t.Errorf("Unexpected response while creating Gateway; got err=\n%v\n;missing strings within error=%q", err, missingErrorStrings) + } + }) + } +}