Skip to content

chore(chart): reduce crd size by removing description #2106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yashmehrotra wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore(chart): reduce crd size by removing description #2106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ docker-push:
manifests: generate gen-schemas ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
#$(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
$(CONTROLLER_GEN) crd paths="./api/..." output:crd:artifacts:config=chart/crds
$(MAKE) strip-crd-descriptions


.PHONY: generate
Expand Down Expand Up @@ -188,6 +189,11 @@ build-debug:
install:
cp ./.bin/$(NAME) /usr/local/bin/

strip-crd-descriptions:
@for f in chart/crds/*.yaml; do \
yq 'del(.. | select(has("description")).description)' "$$f" > "$$f.tmp" && mv "$$f.tmp" "$$f"; \
done
Comment on lines +192 to +195
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 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 -100

Repository: 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 -20

Repository: flanksource/config-db

Length of output: 172

🏁 Script executed:

# Look for other tool installations in Makefile
rg -n 'controller-gen|kustomize' Makefile -B2 -A2

Repository: 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 -20

Repository: 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:

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"; \
+	done

Also 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.

Suggested change
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.


install-crd: manifests
kubectl apply -f chart/crds

Expand Down Expand Up @@ -295,4 +301,4 @@ bench:

.PHONY: modernize
modernize:
go fix ./...
go fix ./...
Loading
Loading