-
Notifications
You must be signed in to change notification settings - Fork 617
CONSOLE-5163: Add labels field to Ingress componentRoutes #2845
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.
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
base: master
Are you sure you want to change the base?
Changes from 4 commits
6d5edf7
1fde3f0
b5a6dab
0935778
2c69bb1
091391a
a58bd90
2090d43
0e5ff48
97f9faf
a9dae67
6740a1b
afa3235
ea90be0
628b8ca
9bb1216
1bdeb2d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,279 @@ | ||
| apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this | ||
| name: "Ingress" | ||
| crdName: ingresses.config.openshift.io | ||
| featureGates: | ||
| - IngressComponentRouteLabels | ||
| tests: | ||
| onCreate: | ||
| - name: Should be able to create an Ingress with componentRoutes containing labels | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| ingress: shard-console | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| ingress: shard-console | ||
| - name: Should be able to create an Ingress with multiple componentRoutes with labels | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console-2 | ||
| namespace: openshift-console | ||
| hostname: console.internal.corp.example.com | ||
| labels: | ||
| ingress: shard-console2 | ||
| - name: console-3 | ||
| namespace: openshift-console | ||
| hostname: console.private.corp.example.com | ||
| labels: | ||
| ingress: shard-console3 | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console-2 | ||
| namespace: openshift-console | ||
| hostname: console.internal.corp.example.com | ||
| labels: | ||
| ingress: shard-console2 | ||
| - name: console-3 | ||
| namespace: openshift-console | ||
| hostname: console.private.corp.example.com | ||
| labels: | ||
| ingress: shard-console3 | ||
| - name: Should be able to create componentRoutes with a DNS-prefixed label key | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| example.com/my-label: value | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| example.com/my-label: value | ||
| - name: Should be able to create componentRoutes with max-length label key and value | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz012: abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz012 | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz012: abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz012 | ||
| - name: Should reject componentRoutes with invalid label key | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| "!!!bad": value | ||
| expectedError: "label keys must be valid Kubernetes qualified names" | ||
| - name: Should reject componentRoutes with invalid label value | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| ingress: "invalid value with spaces!" | ||
| expectedError: "label values must be valid Kubernetes label values" | ||
| - name: Should reject componentRoutes with reserved kubernetes.io/ label prefix | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| kubernetes.io/name: value | ||
| expectedError: "label keys must not use reserved prefixes" | ||
| - name: Should reject componentRoutes with reserved k8s.io/ label prefix | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| k8s.io/name: value | ||
| expectedError: "label keys must not use reserved prefixes" | ||
| - name: Should reject componentRoutes with more than 8 labels | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| label1: value1 | ||
| label2: value2 | ||
| label3: value3 | ||
| label4: value4 | ||
| label5: value5 | ||
| label6: value6 | ||
| label7: value7 | ||
| label8: value8 | ||
| label9: value9 | ||
| expectedError: "Too many properties" | ||
| onUpdate: | ||
| - name: Should be able to add labels to componentRoutes on update | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| updated: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| ingress: shard-console | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| ingress: shard-console | ||
| - name: Should be able to change an existing label value on update | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| ingress: shard-old | ||
| updated: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| ingress: shard-new | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| ingress: shard-new | ||
| - name: Should be able to remove labels from componentRoutes on update | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| labels: | ||
| ingress: shard-console | ||
| updated: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Ingress | ||
| spec: | ||
| domain: apps.example.com | ||
| componentRoutes: | ||
| - name: console | ||
| namespace: openshift-console | ||
| hostname: console.example.com | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -245,6 +245,25 @@ type ComponentRouteSpec struct { | |
| // the Secret specification for a serving certificate will not be needed. | ||
| // +optional | ||
| ServingCertKeyPairSecret SecretNameReference `json:"servingCertKeyPairSecret"` | ||
|
|
||
| // labels defines additional labels to be applied to the route created | ||
| // for the component. These labels are used by the IngressController to | ||
| // determine which routes it should manage. Changing labels may cause the | ||
| // route to be reassigned to a different IngressController. | ||
| // When omitted, no additional labels are applied to the component route. | ||
| // Label keys and values must conform to Kubernetes label conventions. | ||
| // Keys with the "kubernetes.io/" and "k8s.io/" prefixes are reserved | ||
| // for Kubernetes use and may not be specified. | ||
| // A maximum of 8 labels may be specified. | ||
|
Leo6Leo marked this conversation as resolved.
Outdated
|
||
| // +openshift:enable:FeatureGate=IngressComponentRouteLabels | ||
| // +optional | ||
| // +mapType=granular | ||
| // +kubebuilder:validation:MinProperties=1 | ||
| // +kubebuilder:validation:MaxProperties=8 | ||
|
Leo6Leo marked this conversation as resolved.
coderabbitai[bot] marked this conversation as resolved.
|
||
| // +kubebuilder:validation:XValidation:rule="self.all(key, !format.qualifiedName().validate(key).hasValue())",message="label keys must be valid Kubernetes qualified names" | ||
| // +kubebuilder:validation:XValidation:rule="self.all(key, !key.startsWith('kubernetes.io/') && !key.startsWith('k8s.io/'))",message="label keys must not use reserved prefixes kubernetes.io/ or k8s.io/" | ||
| // +kubebuilder:validation:XValidation:rule="self.all(key, self[key].matches('^(([A-Za-z0-9][-A-Za-z0-9_.]{0,61})?[A-Za-z0-9])?$'))",message="label values must be valid Kubernetes label values (at most 63 characters, alphanumeric, '-', '_', or '.', must start and end with alphanumeric)" | ||
|
Leo6Leo marked this conversation as resolved.
Outdated
|
||
| Labels map[string]string `json:"labels,omitempty"` | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two issues here:
|
||
| } | ||
|
|
||
| // ComponentRouteStatus contains information allowing configuration of a route's hostname and serving certificate. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests cover basic CRUD and some negatives but the CEL validations need more coverage. Please add:
ingress: "") accepted (valid per K8s label spec, non-obvious edge case)format.qualifiedName()enforcement)-starts-with-dash) rejectedopenshift.io/my-label), confirms the reserved check is not overly broadThe MinProperties/MaxProperties boundary tests are basic OpenAPI and do not need separate test cases.