PROJQUAY-11867: fix(apis): restrict secretRef to tls component only - #1281
PROJQUAY-11867: fix(apis): restrict secretRef to tls component only#1281Rachimypala-RM wants to merge 3 commits into
Conversation
|
Hi @Rachimypala-RM. Thanks for your PR. I'm waiting for a quay member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (3)
Summary by CodeRabbitRelease Notes
Walkthrough
ChangessecretRef override restricted to TLS component
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🤖 Review · Started 5:39 PM UTC |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apis/quay/v1/quayregistry_types.go`:
- Line 160: The kubebuilder validation marker with the CEL rule restricting
secretRef to the tls component has been added to the quayregistry_types.go
source file but the generated CRD has not been regenerated to include this
constraint. Regenerate the CRD manifest files by running the appropriate
kubebuilder code generation command (typically make manifests or similar in your
project build system) to ensure the new XValidation rule from the marker at line
160 is properly included in the generated CRD output, so the admission
validation is enforced at runtime.
🪄 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 UI
Review profile: CHILL
Plan: Enterprise
Run ID: 5a9774fa-209b-41d2-93bc-57298f6abd4e
📒 Files selected for processing (1)
apis/quay/v1/quayregistry_types.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
quay/quay(manual)
|
🤖 Finished Review · ✅ Success · Started 5:47 PM UTC · Completed 5:57 PM UTC |
jbpratt
changed the title
|
@Rachimypala-RM: This pull request references [Jira Issue PROJQUAY-11867](https://redhat.atlassian.net/browse/PROJQUAY-11867), which is invalid:
Comment 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. |
ReviewFindingsMedium
Low
Info
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1281 +/- ##
==========================================
+ Coverage 65.35% 65.41% +0.05%
==========================================
Files 26 26
Lines 4327 4334 +7
==========================================
+ Hits 2828 2835 +7
Misses 1499 1499
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
fullsend-ai-review
Bot
added
the
requires-manual-review
|
/jira refresh |
|
@jbpratt: This pull request references [Jira Issue PROJQUAY-11867](https://redhat.atlassian.net/browse/PROJQUAY-11867), which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
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. |
There was a problem hiding this comment.
@Rachimypala-RM could we also get some unit tests to validate this fix? Thank you!
d877d33 to
1c1aaa9
Compare
secretRef was exposed on the generic Component struct without any kind-level guard, allowing it to be set on components such as postgres, redis, and clair where it has no effect. The only consumer of SecretRef in the codebase is GetTLSSecretRef, which already gates on kind == tls. Add a supportsSecretRef allowlist (tls only), a new CEL admission rule that rejects secretRef on any component whose kind is not tls, and a ValidateOverrides runtime check that fires during reconciliation as a belt-and-suspenders guard. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The XValidation marker added to Component in quayregistry_types.go was not reflected in the generated CRD manifests. Manually patched both bundle/manifests/quayregistries.crd.yaml and config/crd/bases/quay.redhat.com_quayregistries.yaml to include the new x-kubernetes-validations entry that rejects secretRef on any component kind other than tls, matching the source marker exactly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…triction Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1c1aaa9 to
d31058c
Compare
|
/ok-to-test |
|
PR needs rebase. 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. |
4 participants
secretRef was exposed on the generic Component struct without any kind-level guard, allowing it to be set on components such as postgres, redis, and clair where it has no effect. The only consumer of SecretRef in the codebase is GetTLSSecretRef, which already gates on kind == tls.
Add a supportsSecretRef allowlist (tls only), a new CEL admission rule that rejects secretRef on any component whose kind is not tls, and a ValidateOverrides runtime check that fires during reconciliation as a belt-and-suspenders guard.