Skip to content

fix(chart): skip bundled CRDs for standalone panels - #701

Open
Baylem wants to merge 2 commits into
GameplanePanel:masterfrom
Baylem:fix/standalone-chart-crds
Open

Baylem wants to merge 2 commits into
GameplanePanel:masterfrom
Baylem:fix/standalone-chart-crds

Conversation

@Baylem

@Baylem Baylem commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Follow-up to #630 and the maintainer's decision.

Standalone Helm installs currently require --skip-crds because the parent chart's native CRDs bypass template conditions. Move the unchanged CRDs into a bundled subchart gated by operator.enabled. The existing standalone values now omit CRDs without the extra CLI flag; default operator installs retain native CRD installation and uninstall retention. The combined API-only upgrade hook and its SQLite strategy migration remain intact.

Add regression coverage for all nine CRDs, standalone and remote profiles, packaged charts, and legacy --reuse-values. Update CRD generation paths and installation guidance.

Summary by CodeRabbit

  • New Features
    • Helm installations now manage bundled CRDs through a conditional chart dependency, enabled with the operator.
    • Standalone panel installations can omit bundled CRDs and skip CRD apply resources without --skip-crds; existing CRDs remain untouched.
  • Bug Fixes
    • Updated chart checks to verify CRD behavior across installation modes, upgrades, and packaged charts.
  • Documentation
    • Updated installation guidance to reflect CRD handling for standalone, operator-enabled, and API-only deployments.

Signed-off-by: Baylem <zelathis@gmail.com>
@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 42 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: GameplanePanel/Gameplane/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 75cfa3a6-2a32-4151-9f61-32a66348fc1c
📥 Commits

Reviewing files that changed from the base of the PR and between 2f1047f and 564220c.

📒 Files selected for processing: 2
  • charts/tests/check_gateway.py
  • charts/tests/check_standalone.py

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: GameplanePanel/Gameplane/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e4da9dbe-4143-4b53-962a-c17b7e24b751
📥 Commits

Reviewing files that changed from the base of the PR and between 869ea6b and 2f1047f.

📒 Files selected for processing: 24
  • .github/workflows/ci.yaml
  • CHANGELOG.md
  • Makefile
  • charts/gameplane/Chart.yaml
  • charts/gameplane/charts/gameplane-crds/Chart.yaml
  • charts/gameplane/charts/gameplane-crds/crds/gameplane.local_backups.yaml
  • charts/gameplane/charts/gameplane-crds/crds/gameplane.local_backupschedules.yaml
  • charts/gameplane/charts/gameplane-crds/crds/gameplane.local_clusters.yaml
  • charts/gameplane/charts/gameplane-crds/crds/gameplane.local_gameservers.yaml
  • charts/gameplane/charts/gameplane-crds/crds/gameplane.local_gametemplates.yaml
  • charts/gameplane/charts/gameplane-crds/crds/gameplane.local_modules.yaml
  • charts/gameplane/charts/gameplane-crds/crds/gameplane.local_modulesources.yaml
  • charts/gameplane/charts/gameplane-crds/crds/gameplane.local_networkcaptures.yaml
  • charts/gameplane/charts/gameplane-crds/crds/gameplane.local_restores.yaml
  • charts/gameplane/examples/standalone-panel-values.yaml
  • charts/gameplane/templates/NOTES.txt
  • charts/gameplane/values.yaml
  • charts/tests/check_gateway.py
  • charts/tests/check_standalone.py
  • docs/install.md
  • docs/standalone-panel.md
  • hack/gen-module-schema.py
  • hack/sync-chart-crds.sh
  • test/e2e/helm_install_e2e_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout: 65
  • GitHub Check: go (capture-sidecar / arm64)
  • GitHub Check: go (api / amd64)
  • GitHub Check: go (svcutil / amd64)
  • GitHub Check: go (mcp-server / arm64)
  • GitHub Check: go (gameaction / arm64)
  • GitHub Check: go (tunnel / amd64)
  • GitHub Check: go (gameproto / arm64)
  • GitHub Check: go (telemetryschema / amd64)
  • GitHub Check: go (telemetryschema / arm64)
  • GitHub Check: go (gp-module / arm64)
  • GitHub Check: go (agent / amd64)
  • GitHub Check: go (tunnel / arm64)
  • GitHub Check: go (sentinel / arm64)
  • GitHub Check: go (api / arm64)
  • GitHub Check: go (operator / amd64)
  • GitHub Check: go (svcutil / arm64)
  • GitHub Check: go (operator / arm64)
  • GitHub Check: go (agent / arm64)
  • GitHub Check: go (mcp-server / amd64)
  • GitHub Check: go (telemetry-receiver / amd64)
  • GitHub Check: go (netguard / arm64)
  • GitHub Check: go (audit-syslog-bridge / amd64)
  • GitHub Check: go (capture-sidecar / amd64)
  • GitHub Check: go (netguard / amd64)
  • GitHub Check: go (gameaction / amd64)
  • GitHub Check: go (audit-syslog-bridge / arm64)
  • GitHub Check: go (gp-module / amd64)
  • GitHub Check: go (sentinel / amd64)
  • GitHub Check: go (telemetry-receiver / arm64)
  • GitHub Check: go (gameproto / amd64)
  • GitHub Check: lint (capture-sidecar)
  • GitHub Check: lint (sentinel)
  • GitHub Check: lint (tunnel)
  • GitHub Check: lint (operator)
  • GitHub Check: lint (svcutil)
  • GitHub Check: lint (test/e2e)
  • GitHub Check: lint (gp-module)
  • GitHub Check: lint (audit-syslog-bridge)
  • GitHub Check: lint (mcp-server)
  • GitHub Check: lint (telemetry-receiver)
  • GitHub Check: lint (api)
  • GitHub Check: lint (gameaction)
  • GitHub Check: lint (telemetryschema)
  • GitHub Check: lint (agent)
  • GitHub Check: lint (gameproto)
  • GitHub Check: lint (netguard)
  • GitHub Check: go (test/e2e)
  • GitHub Check: web e2e (mock)
  • GitHub Check: chart render
  • GitHub Check: helm lint
  • GitHub Check: build e2e images (arm64)
  • GitHub Check: submodule freshness
  • GitHub Check: api (postgres)
  • GitHub Check: docs and repository checks
  • GitHub Check: e2e bucket coverage
  • GitHub Check: workflow lint
  • GitHub Check: standalone Compose smoke
  • GitHub Check: ingress-nginx smoke (kind)
  • GitHub Check: web
  • GitHub Check: capture-sidecar CAP_NET_RAW survival
  • GitHub Check: dependency review
  • GitHub Check: build e2e images
  • GitHub Check: codegen drift
  • GitHub Check: vuln (go)
  • GitHub Check: portable-paths
🧰 Additional context used
📓 Path-based instructions: 5
CRITICAL: Every workflow MUST follow hardened-CI practices: All external actions MUST be pinned to a full 40-hex commit SHA (not a tag or branch) Each action SHA pin MUST include an inline version comment for human readability Workflows MUS...

⚙️ CodeRabbit configuration file

Files:

  • .github/workflows/ci.yaml
CRITICAL: Flag test deletions or weakening: A test removed or commented out (sign: test file size decreased) A test case removed from a table-driven test A test condition relaxed (e.g., comparison changed from == to >) A test assertion remo...

⚙️ CodeRabbit configuration file

Files:

  • test/e2e/helm_install_e2e_test.go
CRITICAL: Every new E2E test MUST: Be added to a bucket in test/e2e/buckets.sh (the CI gate fails on unbucketed tests) Call t.Parallel() to allow concurrent execution within its bucket Use per-test unique resource names (not shared fixtures...

⚙️ CodeRabbit configuration file

Files:

  • test/e2e/helm_install_e2e_test.go
CRITICAL: Behavior changes must update the relevant module's specs.md.

⚙️ CodeRabbit configuration file

Files:

  • test/e2e/helm_install_e2e_test.go
CRITICAL: Gameplane's house rules forbid in-source lint suppressions.

⚙️ CodeRabbit configuration file

Files:

  • test/e2e/helm_install_e2e_test.go
🪛 ast-grep (0.45.3)
charts/tests/check_gateway.py

[error] 16-21: Use of unsanitized data to create processes
Context: subprocess.run(
["helm", "template", release, str(chart), "--namespace", namespace, "--values", str(values_path)]
+ (["--is-upgrade"] if upgrade else [])
+ (["--include-crds"] if include_crds else []),
capture_output=True, text=True, check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(os-system-unsanitized-data)

[error] 16-21: Command coming from incoming request
Context: subprocess.run(
["helm", "template", release, str(chart), "--namespace", namespace, "--values", str(values_path)]
+ (["--is-upgrade"] if upgrade else [])
+ (["--include-crds"] if include_crds else []),
capture_output=True, text=True, check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)

charts/tests/check_standalone.py

[error] 29-30: Command coming from incoming request
Context: subprocess.run(["helm", "package", str(CHART), "--destination", directory],
capture_output=True, text=True, check=False)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)

[error] 36-36: Avoid HTML built in strings
Context: render(values, chart=packages[0], include_crds=True, upgrade=upgrade)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(html-string-from-parameters)

[error] 38-38: Avoid HTML built in strings
Context: render(values, chart=packages[0], include_crds=True, upgrade=upgrade)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(html-string-from-parameters)

[error] 44-44: Avoid HTML built in strings
Context: render(include_crds=True)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(html-string-from-parameters)

[error] 60-60: Avoid HTML built in strings
Context: render(values, upgrade=upgrade, include_crds=True)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(html-string-from-parameters)

[error] 126-126: Avoid HTML built in strings
Context: render({"api": {"enabled": False}}, include_crds=True)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(html-string-from-parameters)

[error] 131-131: Avoid HTML built in strings
Context: render({"operator": {"enabled": False}}, include_crds=True)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(html-string-from-parameters)

[error] 137-137: Avoid HTML built in strings
Context: render({"api": {"standalone": True}}, failure="api.standalone requires operator.enabled=false")
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(html-string-from-parameters)

[error] 138-138: Avoid HTML built in strings
Context: render({"operator": {"enabled": False}, "api": {"enabled": False, "standalone": True}}, failure="api.standalone requires api.enabled=true")
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(html-string-from-parameters)

[error] 139-139: Avoid HTML built in strings
Context: render({"operator": {"enabled": False}, "api": {"standalone": True, "replicas": 2}}, failure="api.standalone requires api.replicas=1")
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(html-string-from-parameters)

[error] 142-142: Avoid HTML built in strings
Context: render(example, include_crds=True, upgrade=upgrade)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(html-string-from-parameters)

[error] 146-146: Avoid HTML built in strings
Context: render(include_crds=True, upgrade=upgrade)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(html-string-from-parameters)

[error] 147-147: Avoid HTML built in strings
Context: render({"api": {"enabled": False}}, include_crds=True, upgrade=upgrade)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(html-string-from-parameters)

🪛 Ruff (0.16.9)
charts/tests/check_gateway.py

[error] 16-16: subprocess call: check for execution of untrusted input

(S603)

charts/tests/check_standalone.py

[error] 29-29: subprocess call: check for execution of untrusted input

(S603)

[error] 29-29: Starting a process with a partial executable path

(S607)

Walkthrough

The chart now supplies native CRDs through an operator-gated subchart. CRD synchronization and render checks use the updated location. Installation notes and documentation describe CRD handling across standalone and operator-enabled profiles.

Changes

Conditional CRD subchart

Layer / File(s) Summary
Define and populate the conditional CRD subchart
charts/gameplane/Chart.yaml, charts/gameplane/charts/gameplane-crds/Chart.yaml, charts/gameplane/values.yaml, hack/sync-chart-crds.sh, Makefile, hack/gen-module-schema.py
The parent chart adds a gameplane-crds dependency gated by operator.enabled. CRD synchronization writes to the subchart and crd-manifests/; schema generation reads from crd-manifests/.
Validate CRDs in chart renders and packages
charts/tests/check_gateway.py, charts/tests/check_standalone.py, .github/workflows/ci.yaml, test/e2e/helm_install_e2e_test.go
Render and package checks cover CRD presence or omission across configuration profiles and upgrades. CI checks the CRD count and sync paths. The end-to-end expected CRD list includes two additional CRDs.
Document installation profiles
charts/gameplane/examples/standalone-panel-values.yaml, charts/gameplane/templates/NOTES.txt, docs/install.md, docs/standalone-panel.md, CHANGELOG.md
The installation guidance describes when Helm installs native CRDs and when standalone mode omits CRDs and the apply hook/RBAC, without requiring --skip-crds. The changelog records the change.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested labels: type: fix, type: test, type: ci, type: docs, type: chore, area: chart, area: e2e

Merge Risk: ⚪ Minimal · up to 2f104

Operator-enabled releases retain the bundled CRDs, while operator-disabled standalone releases omit them. The reviewed workflows remain usable, with no concrete merge-blocking risk established.

Security architecture risk: 🔵 Low · up to 2f104

The standalone profile avoids unnecessary cluster-wide schemas without changing their content. The main remaining concern is a non-standalone API-only first install, which can omit schemas its API still requires. No newly expanded access-control boundary was identified.

Retained concerns

  • Medium · architecture · inferred: The new operator gate also removes native CRD bootstrap from non-standalone API-only installs. On a fresh cluster, the unchanged hook does not run during installation because its pre-install event requires an existing mismatched GameServer CRD. The API remains Kubernetes-backed and retains permissions for Gameplane custom resources, but no installation step initializes their schemas. Existing operator installations avoid this condition; the inferred impact is incomplete control-plane initialization, not an access-control bypass.
Security review details

Security Blast Radius

  • observed — The existing apply-hook identity can create and modify CRDs cluster-wide; its ClusterRole is not restricted to the nine Gameplane names. Its SQLite strategy-migration permission is separately limited to gameplane-api in the release namespace. Neither authority definition changes in the full PR comparison.

Trust Boundaries and Controls

  • observed — The new schema gate is controlled by deployment values, not a request-facing input. In standalone mode, the existing hook guard removes its privileged identity and RBAC, and regression checks assert that the API does not mount a service-account token. The PR narrows installation scope rather than granting additional workload authority.

Resilience and Maintainability Implications

  • observed — The unchanged hook orders the SQLite strategy patch before CRD application, uses a repeatable merge patch, limits Job retries and cleans successful hook resources. Multi-CRD application is not a single atomic transition, and no pre-rollback hook is registered. Recovery after partial application or interruption remains unverified; these characteristics predate this PR rather than constituting a newly introduced security finding.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. (10 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: standalone panels skip the bundled CRDs.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. (10 skipped: 10 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Signed-off-by: Baylem <zelathis@gmail.com>
@github-actions github-actions Bot added area: chart Helm chart and deployment area: e2e kind-based end-to-end suite area: shared netguard, gameaction, gameproto, svcutil and other shared packages type: fix Bug fix labels Oct 15, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: chart Helm chart and deployment area: e2e kind-based end-to-end suite area: shared netguard, gameaction, gameproto, svcutil and other shared packages type: fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant