fix(vald-operator): bring Helm chart CI/Makefile parity with vald-helm-operator and vald-benchmark-operator by kpango · Pull Request #3560 · vdaas/vald

kpango · 2026-07-11T04:32:30Z

Summary

vald-operator (charts/operator/vald) was never wired into the Helm publish/lint/release automation, so it was never packaged and never appeared in the published Helm repo index (verified against gh-pages charts/index.yaml).
Added helm/package/all, helm/docs/all (helm.mk) and k8s/manifest/all (k8s.mk) aggregate Makefile targets mirroring the existing helm/schema/all idiom, so future charts only need to be listed in one place.
helm.yaml: package all 5 charts instead of 4 — fixes the ArtifactHub publishing gap for vald-operator.
helm-lint.yaml: collapsed 3 near-duplicate per-chart lint jobs into a matrix job and added previously-missing lint coverage for vald-benchmark-operator.
_release-pr.yaml / update-deps.yaml: bump vald-operator's chart version and regenerate its k8s manifests/docs on release and daily dependency updates, same as the other charts; also stage pkg/ since k8s/manifest/all now regenerates vald-operator's Go test golden file.
k8s.mk: fixed mv path bugs in k8s/manifest/operator/helm/update, k8s/manifest/operator/benchmark/update, k8s/operator/helm/deploy, k8s/operator/helm/delete, k8s/operator/benchmark/deploy, k8s/operator/benchmark/delete — helm template --output-dir names the output directory after Chart.yaml's name: field (vald-helm-operator, vald-benchmark-operator), not the chart's source path, so these targets always failed before this fix.

Verification

make helm/package/all runs end-to-end, producing all 5 chart packages including vald-operator-*.tgz.
helm lint passes on all 4 charts (vald, vald-helm-operator, vald-operator, vald-benchmark-operator).
Regenerated k8s/operator/helm and k8s/operator/benchmark manifests with the project-pinned helm v4.1.3, ran the standard make license + yamlfmt cleanup pass (same as CI does downstream), and confirmed the output byte-matches the committed manifests.
actionlint clean on all 4 modified workflow files; YAML syntax validated.
Independently reviewed by code-reviewer and vald-reviewer subagents; both flagged the same pkg/ git-add gap (fixed) and code-reviewer additionally caught the same mv-path bug in the k8s/operator/{helm,benchmark}/deploy|delete dev-convenience targets (fixed).

Known/out-of-scope items (confirmed with requester)

charts/operator/vald/crds/valdoperatorrelease.yaml is a static controller-gen artifact with no regeneration mechanism left after the kubebuilder removal — flagged but not fixed here (owned by the operator's Go API type design, out of scope for this CI/Helm parity fix).
ArtifactHub's artifacthub-repo.yml lives on the gh-pages branch (repo ownership/verification metadata) — out of scope for this branch.

Manual follow-up required before/after merge

helm-lint.yaml's job IDs changed from lint-vald-chart / lint-vald-helm-operator-chart / lint-vald-operator-chart to a single matrix job lint-chart. If GitHub branch protection on this or downstream branches lists the old job names as required status checks, please update them post-merge.

Generated with Claude Code

… and internal/encoding/json build: consolidate gopls and oapi-codegen into go.tools (#3558) Signed-off-by: kpango <kpango@vdaas.org>

…elm-operator and vald-benchmark-operator vald-operator (charts/operator/vald) was never wired into the Helm publish/lint/release automation, so it was never packaged and never appeared in the published Helm repo index (verified against gh-pages charts/index.yaml). - add helm/package/all, helm/docs/all (helm.mk) and k8s/manifest/all (k8s.mk) aggregate targets mirroring the existing helm/schema/all idiom, so new charts only need to be listed in one place - helm.yaml: package all 5 charts instead of 4, fixing the ArtifactHub publishing gap for vald-operator - helm-lint.yaml: collapse 3 near-duplicate per-chart lint jobs into a matrix job and add previously-missing lint coverage for vald-benchmark-operator - _release-pr.yaml / update-deps.yaml: bump vald-operator's chart version and regenerate its k8s manifests/docs on release and daily dependency updates, same as the other charts; also stage pkg/ since k8s/manifest/all now regenerates pkg/operator/vald's test golden file - k8s.mk: fix mv path bugs in k8s/manifest/operator/helm/update, k8s/manifest/operator/benchmark/update, k8s/operator/helm/deploy, k8s/operator/helm/delete, k8s/operator/benchmark/deploy and k8s/operator/benchmark/delete — helm template --output-dir names the output directory after Chart.yaml's name field (vald-helm-operator, vald-benchmark-operator), not the chart's source path, so these targets always failed. Verified against a real helm template run and confirmed the regenerated manifests byte-match the committed ones after the standard make license + yamlfmt cleanup pass. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: kpango <kpango@vdaas.org>

coderabbitai · 2026-07-11T04:32:36Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)

main

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 401ba7eb-ae85-4eb1-afcf-0742e9a55231

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feature/vald-operator/helm-parity

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

vdaas-ci · 2026-07-11T04:32:39Z

[CHATOPS:HELP] ChatOps commands.

🙆‍♀️ /approve - approve
🍱 /format - format codes and add licenses
✅ /gen-test - generate test codes
🏷️ /label - add labels
🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

Copilot

Pull request overview

This PR wires the vald-operator Helm chart into the existing Helm packaging/linting/release automation and introduces aggregate Make targets to keep chart-related CI/Makefile workflows in parity across all Vald charts/operators.

Changes:

Added aggregate Make targets (helm/package/all, helm/docs/all, k8s/manifest/all) to centralize chart lists and simplify CI/release automation updates.
Updated GitHub Actions workflows to use the new aggregate targets and expanded Helm lint coverage to include operator charts via a matrix job.
Fixed Helm --output-dir path assumptions in k8s.mk for operator Helm/benchmark deploy/delete and manifest update targets.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
Makefile.d/k8s.mk	Adds `k8s/manifest/all` and fixes Helm template output-dir path handling for operator manifest generation and deploy/delete helpers.
Makefile.d/helm.mk	Adds `helm/package/all` and `helm/docs/all` aggregate targets to package/docs-generate all charts consistently.
.github/workflows/update-deps.yaml	Switches dependency-update automation to use the new aggregate manifest/docs targets.
.github/workflows/helm.yaml	Switches Helm packaging workflow to `make helm/package/all` so all charts are published.
.github/workflows/helm-lint.yaml	Refactors per-chart lint jobs into a matrix and extends lint coverage to additional operator charts.
.github/workflows/_release-pr.yaml	Updates release automation to bump `vald-operator` chart version and regenerate manifests/docs via aggregate targets; also stages `pkg/`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -117,7 +126,7 @@ k8s/manifest/operator/benchmark/update: \
 	--output-dir $(TEMP_DIR) \
 	charts/operator/benchmark
 	mkdir -p $(ROOTDIR)/k8s/tools/benchmark


+          - name: vald-benchmark-operator
+            path: charts/operator/benchmark
+            schema_target: helm/schema/operator/benchmark


…t matrix and release version bump hack/helm/charts.yaml is now the single source of truth for which Helm charts exist (name, path, schema target, whether values.yaml has a bumpable image tag). Consumed by: - helm-lint.yaml: a resolve-charts job (running in the same pinned vald-dev-container as every other job in this workflow) reads the file with yq and emits it as JSON; the lint-chart matrix job sources its chart list from that job output instead of a hardcoded array. - _release-pr.yaml: the 8 hardcoded per-chart sed lines for bumping Chart.yaml version / values.yaml image tag are replaced with two yq-driven loops over the same file. Adding a new chart in the future now only requires one entry in hack/helm/charts.yaml instead of editing both CI files. Verified the yq queries reproduce the prior hardcoded behavior exactly (including that vald-readreplica's values.yaml is a symlink to vald/values.yaml, so it's correctly bumped transitively without its own bump_values_tag entry), and that resolve-charts running inside the pinned dev container avoids a second, independent yq-provisioning path. Also passes matrix/loop values through env: instead of raw ${{ }} interpolation in run: blocks, and writes the multi-line JSON output via the heredoc-safe GITHUB_OUTPUT pattern, per code review. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: kpango <kpango@vdaas.org>

cloudflare-workers-and-pages · 2026-07-11T04:43:41Z

Deploying vald with Cloudflare Pages

Latest commit:	`fee9a99`
Status:	✅ Deploy successful!
Preview URL:	https://a6afb5ed.vald.pages.dev
Branch Preview URL:	https://feature-vald-operator-helm-p.vald.pages.dev

View logs

kpango · 2026-07-11T04:43:50Z

Follow-up commit: introduced hack/helm/charts.yaml as a single source of truth for the chart list consumed by helm-lint.yaml's lint matrix and _release-pr.yaml's version-bump step, replacing the hardcoded chart array / 8 sed lines respectively. Adding a future chart now only requires one entry in that file instead of editing two CI workflows.

Verified: yq queries reproduce prior hardcoded behavior exactly (incl. the vald-readreplica/vald values.yaml symlink case), resolve-charts runs inside the same pinned vald-dev-container as every other job (no second yq-provisioning path), actionlint clean, and matrix/loop values are passed via env: rather than raw ${{ }} interpolation in run: blocks per code review.

Independently reviewed by code-reviewer and vald-reviewer subagents; both raised no blocking issues. One raised finding (resolve-charts should run in the pinned container, not via a separate setup-yq provisioning path) and one (missing hack/helm/charts.yaml in helm-lint.yaml's path triggers) were addressed in this commit.

…ce in k8s.mk Replace the hardcoded Helm chart name segments (vald-helm-operator, vald-benchmark-operator) that mv/kubectl commands relied on with a name-agnostic glob, so k8s.mk no longer hardcodes any operator chart's Chart.yaml name field and stays correct regardless of future chart renames under the hierarchical charts/operator/{helm,vald,benchmark} layout. Doing this safely required removing the shared, memoized $(TEMP_DIR) from every affected recipe (k8s/manifest/operator/{helm,vald,benchmark}/update, k8s/operator/{helm,benchmark}/{deploy,delete}): a code review caught that $(TEMP_DIR) is a single directory created once per `make` invocation (Makefile:71), not per-recipe, so a glob wildcard against it would be unsafe under `make -jN` — concurrent recipes could write into and clean up the same directory. Each recipe now allocates its own private `mktemp -d` directory, chained into one shell invocation with `set -e` and a `trap ... EXIT` for cleanup so a failing step aborts the recipe instead of being silently swallowed by the previous `;`-only chaining. k8s/manifest/all (which aggregates these targets) is switched from prerequisite-based to recursive $(MAKE)-based sequencing so its sub-targets can never run in parallel regardless of an outer -jN flag. Also relocates the single-source-of-truth chart list introduced in the prior commit from hack/helm/charts.yaml to charts/charts.yaml (sits alongside the actual chart directories it describes; confirmed no existing tool - kubeval/conflint, kube-linter, helm-docs - globs charts/ in a way that would misinterpret it), and drops the now- redundant explicit path-trigger entry in helm-lint.yaml since charts/charts.yaml is already covered by the existing charts/** glob. Verified: ran the three rewritten k8s/manifest/operator/*/update targets concurrently via `make -j3` and confirmed three genuinely distinct temp directories with no cross-contamination; confirmed the set -e/trap pattern aborts on failure while still cleaning up the temp dir; regenerated manifests via the project-pinned helm v4.1.3 and the standard make license + yamlfmt cleanup pass still byte-match the committed baseline; actionlint and YAML syntax clean on all touched workflow files. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: kpango <kpango@vdaas.org>

kpango · 2026-07-11T05:08:05Z

Follow-up commit addressing feedback that Makefile.d/k8s.mk still hardcoded old-style flat chart names (vald-helm-operator, vald-benchmark-operator) inconsistent with the hierarchical charts/operator/{helm,vald,benchmark} layout established on feature/vald-operator/initial-development:

Replaced the hardcoded chart-name path segments with a glob wildcard ($(TEMP_DIR)/*/templates → now per-recipe tmpdir, see below), so k8s.mk no longer depends on any chart's Chart.yaml name: field at all.
A code-review pass caught that this wildcard was unsafe: $(TEMP_DIR) is a single directory memoized once per make invocation (not per-recipe), so under make -jN two recipes writing into it concurrently could race — in the worst case (vald-operator target) silently mixing another chart's YAML files into the wrong output. Fixed by giving each of the 7 affected recipes its own private mktemp -d directory instead of the shared one, chained into a single set -e + trap ... EXIT-guarded shell command so failures abort loudly and cleanup still happens.
k8s/manifest/all switched from prerequisite-based aggregation to recursive $(MAKE) calls so its sub-targets can never run in parallel regardless of an outer -jN.
Relocated hack/helm/charts.yaml → charts/charts.yaml (sits next to the charts it describes; confirmed no existing tool — kubeval/conflint, kube-linter, helm-docs — would misinterpret it), and dropped the now-redundant explicit path-trigger entry in helm-lint.yaml since it's covered by the existing charts/** glob.

Verified via make -j3 running the three affected manifest-update targets concurrently (three genuinely distinct temp dirs, no cross-contamination), confirmed the set -e/trap pattern aborts correctly on failure while still cleaning up, and re-confirmed regenerated manifests byte-match committed baseline after the standard cleanup pass. Went through two more rounds of independent code-reviewer verification on this specific change; all raised issues (the TEMP_DIR race, the k8s/manifest/all parallel-aggregation gap, and the ;-chain error-swallowing) are addressed in this commit.

kpango · 2026-07-13T03:01:51Z

/format

vdaas-ci · 2026-07-13T03:03:01Z

[FORMAT] Updating license headers and formatting go codes triggered by kpango.

Signed-off-by: Vdaas CI <vald@vdaas.org>

…/vald-operator/helm-parity

…/vald-operator/helm-parity Signed-off-by: Yusuke Kato <kpango@vdaas.org>

kpango · 2026-07-15T08:48:21Z

/format

vdaas-ci · 2026-07-15T08:49:29Z

[FORMAT] Updating license headers and formatting go codes triggered by kpango.

Signed-off-by: Vdaas CI <vald@vdaas.org>

kpango and others added 2 commits July 11, 2026 11:10

refactor(vald-operator): replace kustomize with internal/config.Merge…

1e3cb0c

… and internal/encoding/json build: consolidate gopls and oapi-codegen into go.tools (#3558) Signed-off-by: kpango <kpango@vdaas.org>

Copilot AI review requested due to automatic review settings July 11, 2026 04:32

github-actions Bot added type/ci area/makefile actions/build/amd64 Build AMD64 Docker Images size/XXXL type/feature New feature labels Jul 11, 2026

Copilot started reviewing on behalf of kpango July 11, 2026 04:32 View session

github-actions Bot added the priority/medium label Jul 11, 2026

Copilot AI reviewed Jul 11, 2026

View reviewed changes

github-actions Bot added the area/helm label Jul 11, 2026

kpango requested review from a team, datelier and kmrmt and removed request for a team July 13, 2026 03:01

vdaas-ci and others added 3 commits July 13, 2026 03:15

🤖 Update license headers / Format go codes and yaml files

9566ba4

Signed-off-by: Vdaas CI <vald@vdaas.org>

Resolve cspell issues

78dd7a7

Merge branch 'feature/vald-operator/initial-development' into feature…

218f09a

…/vald-operator/helm-parity

Matts966 force-pushed the feature/vald-operator/initial-development branch from 78dd7a7 to 1e3cb0c Compare July 13, 2026 11:37

kpango force-pushed the feature/vald-operator/initial-development branch from 1e3cb0c to b751b4b Compare July 13, 2026 11:50

kmrmt approved these changes Jul 14, 2026

View reviewed changes

datelier approved these changes Jul 15, 2026

View reviewed changes

Merge branch 'feature/vald-operator/initial-development' into feature…

2b34713

…/vald-operator/helm-parity Signed-off-by: Yusuke Kato <kpango@vdaas.org>

🤖 Update license headers / Format go codes and yaml files

fee9a99

Signed-off-by: Vdaas CI <vald@vdaas.org>

kpango requested review from datelier and kmrmt July 15, 2026 09:09

kmrmt approved these changes Jul 15, 2026

View reviewed changes

takuyaymd mentioned this pull request Jul 16, 2026

fix(k8s): correct helm/benchmark operator manifest output paths in make targets #3564

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(vald-operator): bring Helm chart CI/Makefile parity with vald-helm-operator and vald-benchmark-operator#3560

fix(vald-operator): bring Helm chart CI/Makefile parity with vald-helm-operator and vald-benchmark-operator#3560
kpango wants to merge 9 commits into
feature/vald-operator/initial-developmentfrom
feature/vald-operator/helm-parity

kpango commented Jul 11, 2026

Uh oh!

coderabbitai Bot commented Jul 11, 2026 •

edited

Loading

Review skipped

Uh oh!

vdaas-ci commented Jul 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

cloudflare-workers-and-pages Bot commented Jul 11, 2026 •

edited

Loading

Uh oh!

kpango commented Jul 11, 2026

Uh oh!

kpango commented Jul 11, 2026

Uh oh!

kpango commented Jul 13, 2026

Uh oh!

vdaas-ci commented Jul 13, 2026

Uh oh!

kpango commented Jul 15, 2026

Uh oh!

vdaas-ci commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

kpango commented Jul 11, 2026

Summary

Verification

Known/out-of-scope items (confirmed with requester)

Manual follow-up required before/after merge

Uh oh!

coderabbitai Bot commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

vdaas-ci commented Jul 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

cloudflare-workers-and-pages Bot commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying vald with Cloudflare Pages

Uh oh!

kpango commented Jul 11, 2026

Uh oh!

kpango commented Jul 11, 2026

Uh oh!

kpango commented Jul 13, 2026

Uh oh!

vdaas-ci commented Jul 13, 2026

Uh oh!

kpango commented Jul 15, 2026

Uh oh!

vdaas-ci commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

coderabbitai Bot commented Jul 11, 2026 •

edited

Loading

cloudflare-workers-and-pages Bot commented Jul 11, 2026 •

edited

Loading