fix(render): support repeating --required-resources and --extra-resources#107
fix(render): support repeating --required-resources and --extra-resources#107chaitanyapantheor wants to merge 1 commit into
Conversation
|
Warning Review limit reached
More reviews will be available in 36 minutes and 7 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR fixes issue ChangesRender and Push Command Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/crossplane/xpkg/annotations_test.go (1)
26-86: ⚡ Quick winAdd test case for empty key validation.
The
parseAnnotationsfunction validates that keys are non-empty (annotations.go line 37-39), but there's no test case exercising this validation. Consider adding:"EmptyKey": { reason: "An entry with an empty key should return an error.", args: args{kvs: []string{"=value"}}, want: want{err: cmpopts.AnyError}, },This ensures the validation path is covered.
🤖 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 `@cmd/crossplane/xpkg/annotations_test.go` around lines 26 - 86, Add a test case to TestParseAnnotations that verifies parseAnnotations rejects an empty key: insert a case named "EmptyKey" into the cases map in annotations_test.go with reason "An entry with an empty key should return an error.", args {kvs: []string{"=value"}}, and want {err: cmpopts.AnyError}; this exercises the parseAnnotations validation logic (the code that checks for empty keys) and ensures the error path is covered.
🤖 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.
Nitpick comments:
In `@cmd/crossplane/xpkg/annotations_test.go`:
- Around line 26-86: Add a test case to TestParseAnnotations that verifies
parseAnnotations rejects an empty key: insert a case named "EmptyKey" into the
cases map in annotations_test.go with reason "An entry with an empty key should
return an error.", args {kvs: []string{"=value"}}, and want {err:
cmpopts.AnyError}; this exercises the parseAnnotations validation logic (the
code that checks for empty keys) and ensures the error path is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fb337407-fde5-46b9-92c1-3275037d83b0
📒 Files selected for processing (9)
cmd/crossplane/render/op/cmd.gocmd/crossplane/render/op/cmd_test.gocmd/crossplane/render/xr/cmd.gocmd/crossplane/render/xr/cmd_test.gocmd/crossplane/xpkg/annotations.gocmd/crossplane/xpkg/annotations_test.gocmd/crossplane/xpkg/batch.gocmd/crossplane/xpkg/build.gocmd/crossplane/xpkg/push.go
8519911 to
11e8bc2
Compare
…rces The flags were declared as `string` instead of `[]string`, so Kong's CLI parser silently overwrote the value on each repetition, keeping only the last occurrence. Resources from all earlier flags were dropped with no error or warning. Change both flags to `[]string` and loop over the slice in Run(), merging resources from every provided path into a single slice. The deprecated --extra-resources flag in `render xr` is fixed in the same way. Fixes crossplane#90 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Chaitanya Maili <chaitanya.maili@pantheon.io>
11e8bc2 to
7caefa1
Compare
Summary
Fixes #90
--required-resourcesand--extra-resources(deprecated) oncrossplane render xrandcrossplane render opsilently ignored all but the last occurrence when the flag was repeated. Resources from earlier flags were dropped with no error or warning, causing functions that needed those resources to receive an empty set.Root cause: The fields were declared as
stringinstead of[]string. Kong's CLI parser silently overwrites astringfield on repeated flags, keeping only the last value.Fix: Change both flags to
[]stringand loop over all provided paths inRun(), merging resources from every file/directory into a single slice.Test Plan
MultipleRequiredResourcesFirstMissingtest torender xr— verifies an error on the first of two files propagates (previously silently dropped)MultipleExtraResourcesFirstMissingtest torender xr— same for deprecated--extra-resourcesMultipleRequiredResourcesFirstMissingtest torender opLoadRequiredResourcesErrortests to use[]stringgo test ./...)Example
🤖 Generated with Claude Code