Skip to content

OCPBUGS-55192: Add IngressController .spec.domain validation #2308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 19, 2025
Merged

OCPBUGS-55192: Add IngressController .spec.domain validation #2308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion openapi/generated_openapi/zz_generated.openapi.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion openapi/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29844,7 +29844,7 @@
"$ref": "#/definitions/io.k8s.api.core.v1.LocalObjectReference"
},
"domain": {
"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.",
"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 is additionally validated at the IngressController object level. For the maximum length of the domain value itself, the shortest possible variant of the prefix and the ingress controller name was considered for example \"router-a.\"",
"type": "string"
},
"endpointPublishingStrategy": {
Expand Down
149 changes: 149 additions & 0 deletions operator/v1/tests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,3 +563,152 @@ tests:
tuningOptions:
connectTimeout: "4 s"
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))+)$'"
- name: Should be able to create an IngressController with valid domain
initial: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: ic-spec-domain-test
namespace: openshift-ingress-operator
spec:
domain: "this-label-has-exactly-63-characters-validating-at-the-boundary.com"
expected: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: ic-spec-domain-test
namespace: openshift-ingress-operator
spec:
httpEmptyRequestsPolicy: Respond
idleConnectionTerminationPolicy: Immediate
domain: "this-label-has-exactly-63-characters-validating-at-the-boundary.com"
- name: Should not be able to create an IngressController with invalid domain
initial: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: ic-spec-domain-test
namespace: openshift-ingress-operator
spec:
domain: "*.foo.com"
expectedError: "domain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character"
- name: Should not be able to create an IngressController with domain label exceeding 63 characters
initial: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: ic-spec-domain-test
namespace: openshift-ingress-operator
spec:
domain: "foo.this-label-exceeds-63-characters-for-the-purpose-of-testing-the-domain-validation.com"
expectedError: "each DNS label must not exceed 63 characters"
- name: Should not be able to create an IngressController with the domain exceeding 244 characters
initial: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: a
namespace: openshift-ingress-operator
spec:
domain: "this-domain.has-exactly-245-characters.for-the-purpose-of-testing.the-spec-domain-field-length-validation.it-exceeds-the-limit-set-on-the-spec-domain-field.by-one-character-to-test-the-error.message-from-this-validation.otherwise-it-is-valid.com"
expectedError: "Too long: may not be more than 244 bytes"
- name: Should not be able to create an IngressController with the combined canonical domain exceeding 253 characters
initial: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: ic-name-containing-exactly-40-characters
namespace: openshift-ingress-operator
spec:
domain: "this-domain.has-208-characters.which-on.its-own-would-not-exceed.the-limit-of-253-chars.but-combined-with-the-ingress-controller-name.with-40-chars.and-the-router-prefix.ends-up-as-a-too-long.canonical-domain"
expectedError: "The combined 'router-' + metadata.name + '.' + .spec.domain cannot exceed 253 characters"
onUpdate:
- name: Should be able to update invalid domain to a valid domain
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/domain/x-kubernetes-validations
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/domain/maxLength
initial: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: ic-spec-domain-test
namespace: openshift-ingress-operator
spec:
domain: "*.foo.com"
updated: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: ic-spec-domain-test
namespace: openshift-ingress-operator
spec:
domain: "123-foo.com"
expected: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: ic-spec-domain-test
namespace: openshift-ingress-operator
spec:
httpEmptyRequestsPolicy: Respond
idleConnectionTerminationPolicy: Immediate
domain: "123-foo.com"
- name: Should not be able to update already invalid domain to another invalid domain
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/domain/x-kubernetes-validations
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/domain/maxLength
initial: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: ic-spec-domain-test
namespace: openshift-ingress-operator
spec:
domain: "*.foo.com"
updated: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: ic-spec-domain-test
namespace: openshift-ingress-operator
spec:
domain: "foo.*.com"
expectedError: "domain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character"
- name: Should be able to update other fields while retaining invalid domain due to ratcheting
initialCRDPatches:
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/domain/x-kubernetes-validations
- op: remove
path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/domain/maxLength
initial: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: ic-spec-domain-test
namespace: openshift-ingress-operator
spec:
domain: "*.foo.com"
updated: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: ic-spec-domain-test
namespace: openshift-ingress-operator
spec:
domain: "*.foo.com"
replicas: 3
expected: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: ic-spec-domain-test
namespace: openshift-ingress-operator
spec:
httpEmptyRequestsPolicy: Respond
idleConnectionTerminationPolicy: Immediate
domain: "*.foo.com"
replicas: 3
17 changes: 17 additions & 0 deletions operator/v1/types_ingress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
//
// Compatibility level 1: Stable within a major release for a minimum of 12 months or 3 minor releases (whichever is longer).
// +openshift:compatibility-gen:level=1
// +kubebuilder:validation:XValidation:rule="!has(self.spec.domain) || size('router-' + self.metadata.name + '.' + self.spec.domain) <= 253",message="The combined 'router-' + metadata.name + '.' + .spec.domain cannot exceed 253 characters"
type IngressController struct {
metav1.TypeMeta `json:",inline"`

Expand Down Expand Up @@ -68,6 +69,22 @@ type IngressControllerSpec struct {
//
// If empty, defaults to ingress.config.openshift.io/cluster .spec.domain.
//
// The 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.
//
// The 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 is additionally validated at the IngressController object
// level. For the maximum length of the domain value itself, the shortest
// possible variant of the prefix and the ingress controller name was considered
// for example "router-a."
//
// +kubebuilder:validation:MaxLength=244
// +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"
// +kubebuilder:validation:XValidation:rule="self.split('.').all(label, size(label) <= 63)",message="each DNS label must not exceed 63 characters"
Copy link
Contributor Author

@grzpiotrowski grzpiotrowski Oct 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So to summarize so far.
The current approach I have is to validate at the level of the IngressController struct, to check for the length (253 max) of the combined domain there, since the ingress controller name can vary, impacting how long the .spec.domain can be in consequence.

I have two validations of the dns domain on the .spec.domain field.
One that uses format.dns1123Subdomain() and the other to check if each label does not exceed 63 chars limit, since the function above doesn't include this.

I needed to set some max length on the .spec.domain field, as to satisfy the validation budget. It is (I think) somewhat superficial, since the real length restriction is checked including the IC.name. I set it to 244 chars, which would be the shortest IC.name (1 char), resulting in the router-a. prefix (9 chars) for the router's canonical domain. Hence 244 chars max for the .spec.domain
Though the format.dns1123Subdomain() also checks for the max length of 253 chars, but this wouldn't come into play here.

// +optional
Domain string `json:"domain,omitempty"`

Expand Down
25 changes: 25 additions & 0 deletions operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,27 @@ spec:
updated.

If empty, defaults to ingress.config.openshift.io/cluster .spec.domain.

The 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.

The 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 is additionally validated at the IngressController object
level. For the maximum length of the domain value itself, the shortest
possible variant of the prefix and the ingress controller name was considered
for example "router-a."
maxLength: 244
type: string
x-kubernetes-validations:
- message: domain must consist of lower case alphanumeric characters,
'-' or '.', and must start and end with an alphanumeric character
rule: '!format.dns1123Subdomain().validate(self).hasValue()'
- message: each DNS label must not exceed 63 characters
rule: self.split('.').all(label, size(label) <= 63)
endpointPublishingStrategy:
description: |-
endpointPublishingStrategy is used to publish the ingress controller
Expand Down Expand Up @@ -3234,6 +3254,11 @@ spec:
type: object
type: object
type: object
x-kubernetes-validations:
- message: The combined 'router-' + metadata.name + '.' + .spec.domain cannot
exceed 253 characters
rule: '!has(self.spec.domain) || size(''router-'' + self.metadata.name +
''.'' + self.spec.domain) <= 253'
served: true
storage: true
subresources:
Expand Down
25 changes: 25 additions & 0 deletions ...ated.featuregated-crd-manifests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,27 @@ spec:
updated.

If empty, defaults to ingress.config.openshift.io/cluster .spec.domain.

The 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.

The 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 is additionally validated at the IngressController object
level. For the maximum length of the domain value itself, the shortest
possible variant of the prefix and the ingress controller name was considered
for example "router-a."
maxLength: 244
type: string
x-kubernetes-validations:
- message: domain must consist of lower case alphanumeric characters,
'-' or '.', and must start and end with an alphanumeric character
rule: '!format.dns1123Subdomain().validate(self).hasValue()'
- message: each DNS label must not exceed 63 characters
rule: self.split('.').all(label, size(label) <= 63)
endpointPublishingStrategy:
description: |-
endpointPublishingStrategy is used to publish the ingress controller
Expand Down Expand Up @@ -3217,6 +3237,11 @@ spec:
type: object
type: object
type: object
x-kubernetes-validations:
- message: The combined 'router-' + metadata.name + '.' + .spec.domain cannot
exceed 253 characters
rule: '!has(self.spec.domain) || size(''router-'' + self.metadata.name +
''.'' + self.spec.domain) <= 253'
served: true
storage: true
subresources:
Expand Down
Loading