Skip to content

Commit 2aa449e

Browse files
grzpiotrowskigrzpiotrowski
committed
Add IngressController .spec.domain API validation
This commit fixes OCPBUGS-55192. https://issues.redhat.com/browse/OCPBUGS-55192 Add ratcheting validation of the .spec.domain field of ingress controller. Domain must consist of DNS labels separated by periods, where each label contains only lowercase alphanumeric characters and hyphens, must start and end with an alphanumeric character, and must not exceed 63 characters. The domain must not exceed 253 characters. As part of the ingressController's implementation, cluster-ingress-operator always composes the router's canonical hostname from: `"router-" + ingressController.Name + ingressController.Status.Domain`. For this reason, the maximum length of the .spec.domain field can vary based on the ingressController name set by the user (if it is not "default"). Validation of the IC's .spec.domain field introduced by this commit takes into consideration the actual length of the ingressController.Name, by adding a CEL validation rule on the IngressController object. * operator/v1/types_ingress.go (IngressControllerSpec): Add ratcheting validation of the Domain field. * operator/v1/tests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml Add test cases for the ingress controller .spec.domain field validation Generated files: * openapi/openapi.json * openapi/generated_openapi/zz_generated.openapi.go * operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers.crd.yaml * operator/v1/zz_generated.featuregated-crd-manifests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml * operator/v1/zz_generated.featuregated-crd-manifests/ingresscontrollers.operator.openshift.io/SetEIPForNLBIngressController.yaml * operator/v1/zz_generated.swagger_doc_generated.go
1 parent 230d0e0 commit 2aa449e

File tree

8 files changed

+240
-6
lines changed

8 files changed

+240
-6
lines changed

‎openapi/generated_openapi/zz_generated.openapi.go‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎openapi/openapi.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29524,7 +29524,7 @@
2952429524
"type": "string"
2952529525
},
2952629526
"metadata": {
29527-
"description": "metadata is the standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata",
29527+
"description": "metadata is the standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata // +kubebuilder:validation:XValidation:rule=\"!has(self.name) || size(self.name) <= 63\",message=\"metadata.name must not exceed 63 characters\"",
2952829528
"default": {},
2952929529
"$ref": "#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"
2953029530
},
@@ -29844,7 +29844,7 @@
2984429844
"$ref": "#/definitions/io.k8s.api.core.v1.LocalObjectReference"
2984529845
},
2984629846
"domain": {
29847-
"description": "domain is a DNS name serviced by the ingress controller and is used to configure multiple features:\n\n* For the LoadBalancerService endpoint publishing strategy, domain is\n used to configure DNS records. See endpointPublishingStrategy.\n\n* When using a generated default certificate, the certificate will be valid\n for domain and its subdomains. See defaultCertificate.\n\n* The value is published to individual Route statuses so that end-users\n know where to target external DNS records.\n\ndomain must be unique among all IngressControllers, and cannot be updated.\n\nIf empty, defaults to ingress.config.openshift.io/cluster .spec.domain.",
29847+
"description": "domain is a DNS name serviced by the ingress controller and is used to configure multiple features:\n\n* For the LoadBalancerService endpoint publishing strategy, domain is\n used to configure DNS records. See endpointPublishingStrategy.\n\n* When using a generated default certificate, the certificate will be valid\n for domain and its subdomains. See defaultCertificate.\n\n* The value is published to individual Route statuses so that end-users\n know where to target external DNS records.\n\ndomain must be unique among all IngressControllers, and cannot be updated.\n\nIf empty, defaults to ingress.config.openshift.io/cluster .spec.domain.\n\nThe domain value must be a valid DNS name. It must consist of lowercase alphanumeric characters, '-' or '.', and each label must start and end with an alphanumeric character and not exceed 63 characters. Maximum length of a valid DNS domain is 253 characters.\n\nThe implementation may add a prefix such as \"router-default-\" to the domain when constructing the router canonical hostname. To ensure the resulting hostname does not exceed the DNS maximum length of 253 characters, the domain length additionally validated at the IngressController object level.",
2984829848
"type": "string"
2984929849
},
2985029850
"endpointPublishingStrategy": {

‎operator/v1/tests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,3 +565,152 @@ tests:
565565
tuningOptions:
566566
connectTimeout: "4 s"
567567
expectedError: "IngressController.operator.openshift.io \"default\" is invalid: spec.tuningOptions.connectTimeout: Invalid value: \"4 s\": spec.tuningOptions.connectTimeout in body should match '^(0|([0-9]+(\\.[0-9]+)?(ns|us|µs|μs|ms|s|m|h))+)$'"
568+
- name: Should be able to create an IngressController with valid domain
569+
initial: |
570+
apiVersion: operator.openshift.io/v1
571+
kind: IngressController
572+
metadata:
573+
name: ic-spec-domain-test
574+
namespace: openshift-ingress-operator
575+
spec:
576+
domain: "foo.com"
577+
expected: |
578+
apiVersion: operator.openshift.io/v1
579+
kind: IngressController
580+
metadata:
581+
name: ic-spec-domain-test
582+
namespace: openshift-ingress-operator
583+
spec:
584+
httpEmptyRequestsPolicy: Respond
585+
idleConnectionTerminationPolicy: Immediate
586+
domain: "foo.com"
587+
- name: Should not be able to create an IngressController with invalid domain
588+
initial: |
589+
apiVersion: operator.openshift.io/v1
590+
kind: IngressController
591+
metadata:
592+
name: ic-spec-domain-test
593+
namespace: openshift-ingress-operator
594+
spec:
595+
domain: "*.foo.com"
596+
expectedError: "domain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character"
597+
- name: Should not be able to create an IngressController with domain label exceeding 63 characters
598+
initial: |
599+
apiVersion: operator.openshift.io/v1
600+
kind: IngressController
601+
metadata:
602+
name: ic-spec-domain-test
603+
namespace: openshift-ingress-operator
604+
spec:
605+
domain: "foo.this-label-exceeds-63-characters-for-the-purpose-of-testing-the-domain-validation.com"
606+
expectedError: "each DNS label must not exceed 63 characters"
607+
- name: Should not be able to create the default IngressController with the domain exceeding 253 characters
608+
initial: |
609+
apiVersion: operator.openshift.io/v1
610+
kind: IngressController
611+
metadata:
612+
name: default
613+
namespace: openshift-ingress-operator
614+
spec:
615+
domain: "foo.this-domain.has-exactly-277-characters.for-the-purpose-of-testing.the-domain-length-validation.it-is-an-extraordinarily-long-and-twisted-domain.but-it-would-be-completely-valid.if-not-for-its-gargantuan-length.and-now-there-is-even-more-text.to-go-well-beyond-the-limit.com"
616+
expectedError: "Too long: may not be more than 253 bytes"
617+
- name: Should not be able to create an IngressController with the canonical domain exceeding 253 characters
618+
initial: |
619+
apiVersion: operator.openshift.io/v1
620+
kind: IngressController
621+
metadata:
622+
name: ic-name-containing-exactly-40-characters
623+
namespace: openshift-ingress-operator
624+
spec:
625+
domain: "foo.this-domain.has-exactly-235-characters.which-on.its-own-would-not-exceed.the-limit-of-253-characters.but-combined-with-the-ingress-controller-name.with-40-characters.and-the-router-dash-prefix.ends-up-as-a-too-long.canonical-domain"
626+
expectedError: "The combined 'router-' + IngressController.Name + '.' + .spec.domain cannot exceed 253 characters"
627+
onUpdate:
628+
- name: Should be able to update invalid domain to a valid domain
629+
initialCRDPatches:
630+
- op: remove
631+
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/domain/x-kubernetes-validations
632+
- op: remove
633+
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/domain/maxLength
634+
initial: |
635+
apiVersion: operator.openshift.io/v1
636+
kind: IngressController
637+
metadata:
638+
name: ic-spec-domain-test
639+
namespace: openshift-ingress-operator
640+
spec:
641+
domain: "*.foo.com"
642+
updated: |
643+
apiVersion: operator.openshift.io/v1
644+
kind: IngressController
645+
metadata:
646+
name: ic-spec-domain-test
647+
namespace: openshift-ingress-operator
648+
spec:
649+
domain: "123-foo.com"
650+
expected: |
651+
apiVersion: operator.openshift.io/v1
652+
kind: IngressController
653+
metadata:
654+
name: ic-spec-domain-test
655+
namespace: openshift-ingress-operator
656+
spec:
657+
httpEmptyRequestsPolicy: Respond
658+
idleConnectionTerminationPolicy: Immediate
659+
domain: "123-foo.com"
660+
- name: Should not be able to update already invalid domain to another invalid domain
661+
initialCRDPatches:
662+
- op: remove
663+
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/domain/x-kubernetes-validations
664+
- op: remove
665+
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/domain/maxLength
666+
initial: |
667+
apiVersion: operator.openshift.io/v1
668+
kind: IngressController
669+
metadata:
670+
name: ic-spec-domain-test
671+
namespace: openshift-ingress-operator
672+
spec:
673+
domain: "*.foo.com"
674+
updated: |
675+
apiVersion: operator.openshift.io/v1
676+
kind: IngressController
677+
metadata:
678+
name: ic-spec-domain-test
679+
namespace: openshift-ingress-operator
680+
spec:
681+
domain: "foo.*.com"
682+
expectedError: "domain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character"
683+
- name: Should be able to update other fields while retaining invalid domain due to ratcheting
684+
initialCRDPatches:
685+
- op: remove
686+
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/domain/x-kubernetes-validations
687+
- op: remove
688+
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/domain/maxLength
689+
initial: |
690+
apiVersion: operator.openshift.io/v1
691+
kind: IngressController
692+
metadata:
693+
name: ic-spec-domain-test
694+
namespace: openshift-ingress-operator
695+
spec:
696+
domain: "*.foo.com"
697+
updated: |
698+
apiVersion: operator.openshift.io/v1
699+
kind: IngressController
700+
metadata:
701+
name: ic-spec-domain-test
702+
namespace: openshift-ingress-operator
703+
spec:
704+
domain: "*.foo.com"
705+
replicas: 3
706+
expected: |
707+
apiVersion: operator.openshift.io/v1
708+
kind: IngressController
709+
metadata:
710+
name: ic-spec-domain-test
711+
namespace: openshift-ingress-operator
712+
spec:
713+
httpEmptyRequestsPolicy: Respond
714+
idleConnectionTerminationPolicy: Immediate
715+
domain: "*.foo.com"
716+
replicas: 3

‎operator/v1/types_ingress.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,13 @@ import (
3535
//
3636
// Compatibility level 1: Stable within a major release for a minimum of 12 months or 3 minor releases (whichever is longer).
3737
// +openshift:compatibility-gen:level=1
38+
// +kubebuilder:validation:XValidation:rule="!has(self.spec.domain) || size('router-' + self.metadata.name + '.' + self.spec.domain) <= 253",message="The combined 'router-' + IngressController.Name + '.' + .spec.domain cannot exceed 253 characters"
3839
type IngressController struct {
3940
metav1.TypeMeta `json:",inline"`
4041

4142
// metadata is the standard object's metadata.
4243
// More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata
44+
// // +kubebuilder:validation:XValidation:rule="!has(self.name) || size(self.name) <= 63",message="metadata.name must not exceed 63 characters"
4345
metav1.ObjectMeta `json:"metadata,omitempty"`
4446

4547
// spec is the specification of the desired behavior of the IngressController.
@@ -68,6 +70,20 @@ type IngressControllerSpec struct {
6870
//
6971
// If empty, defaults to ingress.config.openshift.io/cluster .spec.domain.
7072
//
73+
// The domain value must be a valid DNS name. It must consist of lowercase
74+
// alphanumeric characters, '-' or '.', and each label must start and end
75+
// with an alphanumeric character and not exceed 63 characters. Maximum
76+
// length of a valid DNS domain is 253 characters.
77+
//
78+
// The implementation may add a prefix such as "router-default-" to the domain
79+
// when constructing the router canonical hostname. To ensure the resulting
80+
// hostname does not exceed the DNS maximum length of 253 characters,
81+
// the domain length additionally validated at the IngressController object
82+
// level.
83+
//
84+
// +kubebuilder:validation:MaxLength=253
85+
// +kubebuilder:validation:XValidation:rule="!format.dns1123Subdomain().validate(self).hasValue()",message="domain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character"
86+
// +kubebuilder:validation:XValidation:rule="self.split('.').all(label, !format.dns1123Label().validate(label).hasValue())",message="each DNS label must not exceed 63 characters"
7187
// +optional
7288
Domain string `json:"domain,omitempty"`
7389

‎operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers.crd.yaml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,25 @@ spec:
164164
updated.
165165
166166
If empty, defaults to ingress.config.openshift.io/cluster .spec.domain.
167+
168+
The domain value must be a valid DNS name. It must consist of lowercase
169+
alphanumeric characters, '-' or '.', and each label must start and end
170+
with an alphanumeric character and not exceed 63 characters. Maximum
171+
length of a valid DNS domain is 253 characters.
172+
173+
The implementation may add a prefix such as "router-default-" to the domain
174+
when constructing the router canonical hostname. To ensure the resulting
175+
hostname does not exceed the DNS maximum length of 253 characters,
176+
the domain length additionally validated at the IngressController object
177+
level.
178+
maxLength: 253
167179
type: string
180+
x-kubernetes-validations:
181+
- message: domain must consist of lower case alphanumeric characters,
182+
'-' or '.', and must start and end with an alphanumeric character
183+
rule: '!format.dns1123Subdomain().validate(self).hasValue()'
184+
- message: each DNS label must not exceed 63 characters
185+
rule: self.split('.').all(label, !format.dns1123Label().validate(label).hasValue())
168186
endpointPublishingStrategy:
169187
description: |-
170188
endpointPublishingStrategy is used to publish the ingress controller
@@ -3234,6 +3252,11 @@ spec:
32343252
type: object
32353253
type: object
32363254
type: object
3255+
x-kubernetes-validations:
3256+
- message: The combined 'router-' + IngressController.Name + '.' + .spec.domain
3257+
cannot exceed 253 characters
3258+
rule: '!has(self.spec.domain) || size(''router-'' + self.metadata.name +
3259+
''.'' + self.spec.domain) <= 253'
32373260
served: true
32383261
storage: true
32393262
subresources:

‎operator/v1/zz_generated.featuregated-crd-manifests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,25 @@ spec:
165165
updated.
166166
167167
If empty, defaults to ingress.config.openshift.io/cluster .spec.domain.
168+
169+
The domain value must be a valid DNS name. It must consist of lowercase
170+
alphanumeric characters, '-' or '.', and each label must start and end
171+
with an alphanumeric character and not exceed 63 characters. Maximum
172+
length of a valid DNS domain is 253 characters.
173+
174+
The implementation may add a prefix such as "router-default-" to the domain
175+
when constructing the router canonical hostname. To ensure the resulting
176+
hostname does not exceed the DNS maximum length of 253 characters,
177+
the domain length additionally validated at the IngressController object
178+
level.
179+
maxLength: 253
168180
type: string
181+
x-kubernetes-validations:
182+
- message: domain must consist of lower case alphanumeric characters,
183+
'-' or '.', and must start and end with an alphanumeric character
184+
rule: '!format.dns1123Subdomain().validate(self).hasValue()'
185+
- message: each DNS label must not exceed 63 characters
186+
rule: self.split('.').all(label, !format.dns1123Label().validate(label).hasValue())
169187
endpointPublishingStrategy:
170188
description: |-
171189
endpointPublishingStrategy is used to publish the ingress controller
@@ -3101,6 +3119,11 @@ spec:
31013119
type: object
31023120
type: object
31033121
type: object
3122+
x-kubernetes-validations:
3123+
- message: The combined 'router-' + IngressController.Name + '.' + .spec.domain
3124+
cannot exceed 253 characters
3125+
rule: '!has(self.spec.domain) || size(''router-'' + self.metadata.name +
3126+
''.'' + self.spec.domain) <= 253'
31043127
served: true
31053128
storage: true
31063129
subresources:

0 commit comments

Comments
 (0)