Add Labels to GSM Bundle - #5347
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughGSM bundles now support validated labels. Labels remain during cluster expansion and are applied to generated Kubernetes Secrets with the default requester label. ChangesGSM label propagation
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: psalajova The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
pkg/api/gsm.go (1)
36-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument
GSMBundle.Labels.Add a Go doc comment for this exported API field. State the
key:valuesyntax and that labels apply to generated Secrets.🤖 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 `@pkg/api/gsm.go` at line 36, Add a Go doc comment immediately above the exported GSMBundle.Labels field, documenting that labels use key:value syntax and are applied to generated Secrets.Source: Coding guidelines
🤖 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 `@cmd/ci-secret-bootstrap/main.go`:
- Around line 1553-1558: Update bundle validation to reject configured labels
using the reserved api.DPTPRequesterLabel key, preventing bundles from
overriding the bootstrap requester label. In the label merge near the labels map
and bundle.Labels loop, assign api.DPTPRequesterLabel to
api.CISecretBootstrapName after processing all configured labels as a defensive
safeguard. Add a test covering the reserved-key collision and confirming the
bootstrap requester value is preserved.
In `@pkg/api/gsm_test.go`:
- Around line 740-783: Update the expectedConfig for the “resolved bundle
doesn't drop labels” test case to retain the managed-clusters ClusterGroups
entry from the input GSMConfig, while preserving the expanded Targets and Labels
assertions.
In `@pkg/api/gsm.go`:
- Around line 453-458: Update GSMConfig.Validate() to split each bundle label
into exactly one key and value, then validate the key with
validation.IsQualifiedName and the value with validation.IsValidLabelValue,
rejecting empty or malformed components such as :cluster, bad key:value, and
key:bad:value before Secret creation. Add table-driven tests covering invalid
label keys and values.
---
Nitpick comments:
In `@pkg/api/gsm.go`:
- Line 36: Add a Go doc comment immediately above the exported GSMBundle.Labels
field, documenting that labels use key:value syntax and are applied to generated
Secrets.
🪄 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: 9bd0648f-1b5e-4c99-a4a5-e3dcd4b6b922
⛔ Files ignored due to path filters (1)
pkg/api/zz_generated.deepcopy.gois excluded by!**/zz_generated*
📒 Files selected for processing (4)
cmd/ci-secret-bootstrap/main.gocmd/ci-secret-bootstrap/main_test.gopkg/api/gsm.gopkg/api/gsm_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/release(manual)openshift/ci-docs(manual)openshift/release-controller(manual)openshift/ci-chat-bot(manual)
467ad2e to
1089aeb
Compare
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 `@pkg/api/constant.go`:
- Around line 45-57: Update validateBundle to track each parsed label key and
return a configuration error when a duplicate key appears, before Secret
generation runs. Keep ParseLabel’s per-entry validation unchanged and preserve
labels assignment only for validated, unique keys.
- Around line 43-45: The exported ParseLabel documentation should describe the
label parameter, both returned key and value strings, and the whitespace
behavior while preserving its validation summary. Also update the relevant
labels field documentation in openshift/ci-docs to mention the exact key:value
syntax.
🪄 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: 50e0f87a-e9f7-4267-ab0c-6ab435313cba
⛔ Files ignored due to path filters (1)
pkg/api/zz_generated.deepcopy.gois excluded by!**/zz_generated*
📒 Files selected for processing (5)
cmd/ci-secret-bootstrap/main.gocmd/ci-secret-bootstrap/main_test.gopkg/api/constant.gopkg/api/gsm.gopkg/api/gsm_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/release(manual)openshift/ci-docs(manual)openshift/release-controller(manual)openshift/ci-chat-bot(manual)
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/api/gsm_test.go
- pkg/api/gsm.go
- cmd/ci-secret-bootstrap/main.go
- cmd/ci-secret-bootstrap/main_test.go
1089aeb to
1688c69
Compare
1688c69 to
0abb04b
Compare
|
@psalajova: The following test 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. |
1 participant
Adds a labels field to GSM bundle definitions in gsm-config.yaml, allowing bundles with
sync_to_cluster: trueto attach custom Kubernetes labels to the secrets they create on clusters.This is needed for the Vault-to-GSM migration (https://redhat.atlassian.net/browse/DPTP-4893). Some Vault secrets currently use
secretsync/target-labelsmetadata (#5189) to set labels on synced K8s secrets -- most notablyargocd.argoproj.io/secret-type:clusterfor ArgoCD cluster secret discovery. Without this field, those secrets would lose their labels after migration to GSM.Example of such bundle in gsm-config.yaml
Intended workflow is:
sync_to_cluster: true, it fetches the secret data from GSM and creates/updates K8s Secrets on the target clustersObjectMeta.Labels(alongside the existingdptp.openshift.io/requester: ci-secret-bootstraplabel)argocd.argoproj.io/secret-type: cluster) find them automaticallySummary
GSM bundle definitions can now include Kubernetes Secret labels.
When
sync_to_cluster: true,ci-secret-bootstrapapplies valid configured labels to generated or updated Secrets. It retains the existing bootstrap label and ignores invalid labels.GSMBundlepreserves labels during cluster-group expansion and${CLUSTER}substitution. Validation rejects malformed labels and the reservedDPTPRequesterLabelkey.This enables migrated Vault secrets, including ArgoCD cluster secrets, to be discovered automatically by Kubernetes controllers.