CONSOLE-5163: Add labels field to Ingress componentRoutes#2845
Conversation
Add a labels field (map[string]string) to ComponentRouteSpec in the Ingress config API. This allows cluster admins to specify labels on component routes that IngressControllers use for route selection and sharding.
Add a labels field (map[string]string) to ComponentRouteSpec in the Ingress config API. This allows cluster admins to specify labels on component routes that IngressControllers use for route selection and sharding. The field includes: - +mapType=granular for proper strategic merge patch behavior - CEL validation for Kubernetes label key/value format compliance - MaxProperties=8 to bound the number of labels - Unit tests for valid labels, empty labels, and invalid key/value rejection
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@Leo6Leo: This pull request references CONSOLE-5163 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Hello @Leo6Leo! Some important instructions when contributing to openshift/api: |
📝 WalkthroughWalkthroughThis PR adds support for attaching labels to component-managed routes in the OpenShift 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
openshift-ci
Bot
added
the
size/L
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/v1/types_ingress.go`:
- Around line 249-260: The comment for the Labels field is missing the behavior
when omitted and doesn't document the MaxProperties=8 kubebuilder constraint;
update the comment for the Labels (the map with +optional, +mapType=granular and
+kubebuilder:validation:MaxProperties=8) to state that the field is optional,
what happens when it is omitted (e.g., no additional labels will be applied to
the created route), and explicitly document the MaxProperties=8 limit (maximum
of 8 user-supplied labels) along with the existing label key/value validation
rules so all kubebuilder markers are reflected in the field documentation.
- Around line 249-261: Add an OpenShift feature-gate annotation above the new
Labels field so the stable API field is gated; specifically, add a
kubebuilder/openShift annotation like
"+openshift:enable:FeatureGate=MyFeatureGate" immediately before the "Labels
map[string]string `json:\"labels,omitempty\"`" declaration in types_ingress.go
(replace MyFeatureGate with the chosen gate name, e.g., IngressLabels) so the
new Labels field is activated only when that feature gate is enabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: b6586099-0cdf-4a7c-bbfb-fe467ed6e955
⛔ Files ignored due to path filters (5)
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_ingresses.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.deepcopy.gois excluded by!**/zz_generated*config/v1/zz_generated.featuregated-crd-manifests/ingresses.config.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/zz_generated*openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**,!**/zz_generated*
📒 Files selected for processing (3)
config/v1/tests/ingresses.config.openshift.io/AAA_ungated.yamlconfig/v1/types_ingress.gopayload-manifests/crds/0000_10_config-operator_01_ingresses.crd.yaml
| // 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. | ||
| // Label keys and values must conform to Kubernetes label conventions: | ||
| // keys must be 1-63 characters (with optional prefix up to 253 characters), | ||
| // and values must be 0-63 characters, consisting of alphanumeric characters, | ||
| // '-', '_', or '.', and must start and end with an alphanumeric character. | ||
| // +optional | ||
| // +mapType=granular | ||
| // +kubebuilder:validation:MaxProperties=8 | ||
| // +kubebuilder:validation:XValidation:rule="self.all(key, key.matches('^([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="label keys must be valid Kubernetes label keys" | ||
| // +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)" |
There was a problem hiding this comment.
Document omitted behavior and map-size constraint in the field comment.
The Labels comment (Lines 249-256) does not state behavior when omitted and does not document the MaxProperties=8 constraint declared on Line 258.
As per coding guidelines, **/types*.go: Documentation for +optional fields must explain the behavior when the field is omitted, and all kubebuilder validation markers must be documented in the field's comment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@config/v1/types_ingress.go` around lines 249 - 260, The comment for the
Labels field is missing the behavior when omitted and doesn't document the
MaxProperties=8 kubebuilder constraint; update the comment for the Labels (the
map with +optional, +mapType=granular and
+kubebuilder:validation:MaxProperties=8) to state that the field is optional,
what happens when it is omitted (e.g., no additional labels will be applied to
the created route), and explicitly document the MaxProperties=8 limit (maximum
of 8 user-supplied labels) along with the existing label key/value validation
rules so all kubebuilder markers are reflected in the field documentation.
| // 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. | ||
| // Label keys and values must conform to Kubernetes label conventions: | ||
| // keys must be 1-63 characters (with optional prefix up to 253 characters), | ||
| // and values must be 0-63 characters, consisting of alphanumeric characters, | ||
| // '-', '_', or '.', and must start and end with an alphanumeric character. | ||
| // +optional | ||
| // +mapType=granular | ||
| // +kubebuilder:validation:MaxProperties=8 | ||
| // +kubebuilder:validation:XValidation:rule="self.all(key, key.matches('^([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="label keys must be valid Kubernetes label keys" | ||
| // +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)" | ||
| Labels map[string]string `json:"labels,omitempty"` |
There was a problem hiding this comment.
Gate the new stable v1 API field.
Labels is introduced on Line 261 without a +openshift:enable:FeatureGate=... annotation. Stable API field additions should be feature-gated before merge.
As per coding guidelines, **/types*.go: New fields on stable APIs should be introduced behind a feature gate using +openshift:enable:FeatureGate=MyFeatureGate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@config/v1/types_ingress.go` around lines 249 - 261, Add an OpenShift
feature-gate annotation above the new Labels field so the stable API field is
gated; specifically, add a kubebuilder/openShift annotation like
"+openshift:enable:FeatureGate=MyFeatureGate" immediately before the "Labels
map[string]string `json:\"labels,omitempty\"`" declaration in types_ingress.go
(replace MyFeatureGate with the chosen gate name, e.g., IngressLabels) so the
new Labels field is activated only when that feature gate is enabled.
|
@Leo6Leo: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Summary
Add a
labelsfield (map[string]string) toComponentRouteSpecin the Ingress config API (config/v1). This allows cluster admins to specify labels on component routes that IngressControllers use for route selection and sharding.Motivation
When running multiple IngressControllers (e.g., for internal vs. external traffic), each controller uses route labels/selectors to determine which routes it manages. Currently,
componentRoutesentries have no way to specify labels, so component routes (like the console) always land on the default IngressController. This makes it impossible to route console traffic through a specific shard without manual post-creation patching.Changes
labelsfield toComponentRouteSpecwith: -+optional,+mapType=granularfor proper strategic merge patchmaxProperties=8upper boundExample