Skip to content

chore(deps): bump clickhouse-operator-helm version to v0.0.6#224

Open
GrigoryPervakov wants to merge 2 commits into
mainfrom
bump-clickhouse-operator
Open

chore(deps): bump clickhouse-operator-helm version to v0.0.6#224
GrigoryPervakov wants to merge 2 commits into
mainfrom
bump-clickhouse-operator

Conversation

@GrigoryPervakov

Copy link
Copy Markdown
Member

No description provided.

@GrigoryPervakov GrigoryPervakov requested a review from a team as a code owner May 27, 2026 19:42
@changeset-bot

changeset-bot Bot commented May 27, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b68d4cf

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@GrigoryPervakov GrigoryPervakov marked this pull request as draft May 29, 2026 14:19
@GrigoryPervakov GrigoryPervakov force-pushed the bump-clickhouse-operator branch 2 times, most recently from b1a5f67 to 7446b1f Compare June 19, 2026 15:44
@GrigoryPervakov GrigoryPervakov changed the title chore(deps): bump clickhouse-operator-helm version to v0.0.5 chore(deps): bump clickhouse-operator-helm version to v0.0.6 Jun 19, 2026
@GrigoryPervakov GrigoryPervakov force-pushed the bump-clickhouse-operator branch from 7446b1f to 3972747 Compare June 19, 2026 16:16
@GrigoryPervakov GrigoryPervakov marked this pull request as ready for review June 19, 2026 16:28
@GrigoryPervakov GrigoryPervakov force-pushed the bump-clickhouse-operator branch from 3972747 to 453b1e3 Compare June 19, 2026 16:44
@github-actions

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found. This is a dependency bump (clickhouse-operator-helm 0.0.20.0.6) plus a coupled values-key rename and an integration-test wait tweak. No template-rendering breakage, secret, or malformed manifest was found. Chart.yaml (~0.0.6) and Chart.lock (0.0.6) are consistent. The findings below are verification and coverage recommendations, not merge blockers.

🟡 P2 -- recommended

  • charts/clickstack-operators/values.yaml:11 -- the clickhouse-operator override keys were renamed enableenabled (webhook, certManager, crd), but the v0.0.6 subchart is a remote OCI dependency that is not vendored, so the key spelling the chart actually reads is unverified; Helm silently ignores unknown subchart keys, so a wrong key drops the override and the subchart default applies.
    • Fix: Run helm dependency build charts/clickstack-operators then helm template and confirm the rendered output reflects crd.enabled: true and webhook/certManager disabled, and add helm-unittest coverage asserting the disabled-component cases.
    • ce-correctness-reviewer, ce-testing-reviewer, ce-maintainability-reviewer
🔵 P3 nitpicks (3)
  • integration-tests/full-stack/assert.sh:27 -- --field-selector=status.phase!=Succeeded excludes only Succeeded pods, so a Failed pod (or a Job pod that completes mid-wait) still never reaches condition=Ready and hangs the ungated final wait for the full 600s before failing under set -e; this is an inherent limitation of the chosen approach, not a regression vs the prior --all wait.
    • Fix: Scope the readiness wait to long-running app pods by label rather than filtering on status.phase against --all.
    • ce-correctness-reviewer, ce-reliability-reviewer
  • charts/clickstack-operators/values.yaml:9 -- the renamed override keys carry only a generic doc-link comment, so a future subchart bump that renames keys again would be silently ignored by Helm with no error.
    • Fix: Add an inline comment tying the webhook/certManager/crd keys to the pinned clickhouse-operator-helm version.
  • .changeset/bump-clickhouse-operator-v0.0.6.md:2 -- a patch bump is declared for a change that renames a published values key, which silently breaks any downstream override still using the old enable spelling.
    • Fix: Confirm patch is the intended bump for a user-facing values-key rename rather than minor.

Reviewers (5): ce-correctness-reviewer, ce-testing-reviewer, ce-maintainability-reviewer, ce-project-standards-reviewer, ce-reliability-reviewer (plus a dedicated chart-schema investigator).

Testing gaps:

  • charts/clickstack-operators/ has no helm-unittest coverage; the renamed enabled keys are only exercised indirectly by the full-stack integration suite.
  • No assertion verifies the disabled-component cases (webhook.enabled: false, certManager.enabled: false) actually keep those features off, so a silent key-binding mismatch would not be caught.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants