chore(chart): reduce crd size by removing description#2106
chore(chart): reduce crd size by removing description#2106yashmehrotra wants to merge 1 commit intomainfrom
Conversation
WalkthroughA new make target removes description fields from generated CRD YAML files in the build pipeline. The scrapeplugins CRD schema is restructured with simplified field definitions and descriptions removed throughout. Changes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ 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 |
BenchstatBase: ✅ No significant performance changes detectedFull benchstat output |
yashmehrotra
force-pushed
the
strip-crds
branch
from
59b94b0 to
bb2e471
Compare
![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 the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 192-195: The Makefile target strip-crd-descriptions depends on an
undeclared, unpinned yq and doesn’t guard against an empty glob; add a pinned
YQ_VERSION variable and a bootstrap step that mirrors existing tools
(download/extract the specific yq binary into a tools path and expose it via a
YQ_BIN variable), update the target to call $(YQ_BIN) instead of plain yq, and
harden the loop over chart/crds/*.yaml (e.g., skip non-existent matches or use a
safe find pattern) so it fails gracefully when the directory is empty; also
update the repo setup/CI docs/config to document the required YQ_VERSION.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 39db49d9-4e6a-4c6d-b2e4-00e4a369a8d1
📒 Files selected for processing (3)
Makefilechart/crds/configs.flanksource.com_scrapeconfigs.yamlchart/crds/configs.flanksource.com_scrapeplugins.yaml
| strip-crd-descriptions: | ||
| @for f in chart/crds/*.yaml; do \ | ||
| yq 'del(.. | select(has("description")).description)' "$$f" > "$$f.tmp" && mv "$$f.tmp" "$$f"; \ | ||
| done |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify yq is referenced/installed anywhere else in the repo
rg -nP '\byq\b' -g '!**/vendor/**' -g '!**/node_modules/**'Repository: flanksource/config-db
Length of output: 176
🏁 Script executed:
# Check the Makefile structure and how other tools are installed
head -200 Makefile | tail -100Repository: flanksource/config-db
Length of output: 2824
🏁 Script executed:
# Check if chart/crds directory exists and has yaml files
fd '\.yaml$' chart/crds 2>/dev/null | head -20Repository: flanksource/config-db
Length of output: 172
🏁 Script executed:
# Look for other tool installations in Makefile
rg -n 'controller-gen|kustomize' Makefile -B2 -A2Repository: flanksource/config-db
Length of output: 2126
🏁 Script executed:
# Check if there's any documentation or references to yq versions
rg -n 'yq|jq' Makefile -i | head -20Repository: flanksource/config-db
Length of output: 167
🏁 Script executed:
# Look for any go.mod or requirements that might hint at yq usage
cat go.mod | grep -i yq || echo "No yq in go.mod"Repository: flanksource/config-db
Length of output: 80
🌐 Web query:
mikefarah yq del select has description syntax vs python yq
💡 Result:
In mikefarah/yq (Go implementation), to delete array elements that have a "description" key, use: del(.[] | select(has("description"))) This works because: - .[] iterates over array elements - has("description") checks if the key exists in the object (returns true even if value is null/empty) - select(has("description")) filters to those elements - del deletes the matching elements from the array Examples from official docs: - del(.[] | select(. == "*at")) deletes array elements matching wildcard - del(.. | select(has("name")).name) recursively deletes "name" keys For objects in an array of objects: yq 'del(.path.to.array[] | select(has("description")))' file.yaml In python-yq (kislyuk/yq, jq wrapper), syntax is identical since it pipes YAML to jq: del(.[] | select(has("description"))) No syntax differences found for this operation; both support jq-like select and has. mikefarah/yq adds native YAML features (comments, etc.), but core filter syntax matches jq. Sample input: items: - name: foo description: bar - name: baz Command: yq 'del(.items[] | select(has("description")))' file.yaml Output: items: - name: baz
Citations:
- 1: https://mikefarah.gitbook.io/yq/operators/select
- 2: https://mikefarah.gitbook.io/yq/operators/delete.md
- 3: How to remove a certian element in an array mikefarah/yq#1606
- 4: Deleting object incorrectly deleting attribute, how to delete object? mikefarah/yq#1160
- 5: https://mikefarah.gitbook.io/yq/operators/delete
- 6: https://mikefarah.gitbook.io/yq/operators/has
- 7: https://mikefarah.gitbook.io/yq/operators/has.md
Undeclared yq dependency and missing version pinning.
The strip-crd-descriptions target relies on yq being installed but doesn't bootstrap it like other tools (controller-gen, kustomize, etc.) do in this Makefile. Additionally, there's no version specification for yq—contributors might have different versions with varying behavior and flag support.
The glob pattern chart/crds/*.yaml could also fail ungracefully if the directory becomes empty; defensive programming would guard against this.
🔧 Suggested hardening
strip-crd-descriptions:
- `@for` f in chart/crds/*.yaml; do \
- yq 'del(.. | select(has("description")).description)' "$$f" > "$$f.tmp" && mv "$$f.tmp" "$$f"; \
- done
+ `@command` -v yq >/dev/null 2>&1 || { echo "ERROR: yq is required"; exit 1; }
+ `@shopt` -s nullglob 2>/dev/null || true; \
+ for f in chart/crds/*.yaml; do \
+ [ -e "$$f" ] || continue; \
+ yq -i 'del(.. | select(has("description")).description)' "$$f"; \
+ doneAlso document the required yq version in your setup/CI configuration.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| strip-crd-descriptions: | |
| @for f in chart/crds/*.yaml; do \ | |
| yq 'del(.. | select(has("description")).description)' "$$f" > "$$f.tmp" && mv "$$f.tmp" "$$f"; \ | |
| done | |
| strip-crd-descriptions: | |
| `@command` -v yq >/dev/null 2>&1 || { echo "ERROR: yq is required"; exit 1; } | |
| `@shopt` -s nullglob 2>/dev/null || true; \ | |
| for f in chart/crds/*.yaml; do \ | |
| [ -e "$$f" ] || continue; \ | |
| yq -i 'del(.. | select(has("description")).description)' "$$f"; \ | |
| done |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 192 - 195, The Makefile target strip-crd-descriptions
depends on an undeclared, unpinned yq and doesn’t guard against an empty glob;
add a pinned YQ_VERSION variable and a bootstrap step that mirrors existing
tools (download/extract the specific yq binary into a tools path and expose it
via a YQ_BIN variable), update the target to call $(YQ_BIN) instead of plain yq,
and harden the loop over chart/crds/*.yaml (e.g., skip non-existent matches or
use a safe find pattern) so it fails gracefully when the directory is empty;
also update the repo setup/CI docs/config to document the required YQ_VERSION.
1 participant
Summary by CodeRabbit
Release Notes
Chores
Refactor