ci: consolidate release-pipeline bash into tested Go#6359
Conversation
b4722c3 to
eaf488a
Compare
|
eaf488a to
f304dcf
Compare
f304dcf to
7468cd6
Compare
There was a problem hiding this comment.
Pull request overview
This PR replaces large, untested release-pipeline bash + gomplate templating with a consolidated, tested Go CLI (scripts/release-tools) backed by reusable packages in scripts/camunda-core/pkg/*, and updates release workflows/docs accordingly.
Changes:
- Introduces
release-toolssubcommands (matrix updates/rendering, tag resolution, Harbor tag ops, release notes, values injection, chart metadata/images) and adds a CI workflow to vet+test the new tooling. - Adds new
camunda-coreGo packages for version-matrix rendering/writing, release-please version computation, release notes transforms, Harbor tag parsing, and Harbor tag API operations. - Removes legacy bash/scripts and gomplate templates previously used to generate release notes and version-matrix content.
Reviewed changes
Copilot reviewed 63 out of 66 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/values-injector/main.go | Removes the old standalone values-injector CLI entrypoint. |
| scripts/values-injector/main_test.go | Removes tests for the old values-injector CLI. |
| scripts/values-injector/internal/injector/injector.go | Removes the old values.yaml tag merge implementation (moved into new package). |
| scripts/values-injector/internal/chartcomponents/types.go | Removes old values-injector component type definitions. |
| scripts/values-injector/internal/chartcomponents/chart89.go | Removes old chart 8.9 type alias. |
| scripts/values-injector/internal/chartcomponents/chart88.go | Removes old chart 8.8 struct definition. |
| scripts/values-injector/internal/chartcomponents/chart87.go | Removes old chart 8.7 type alias. |
| scripts/values-injector/internal/chartcomponents/chart86.go | Removes old chart 8.6 struct definition. |
| scripts/values-injector/internal/chartcomponents/chart810.go | Removes old chart 8.10 type alias. |
| scripts/values-injector/go.sum | Removes module sums for the deleted values-injector module. |
| scripts/values-injector/go.mod | Removes the deleted values-injector Go module. |
| scripts/templates/version-matrix/VERSION-MATRIX-RELEASE.md.tpl | Removes gomplate template for per-app version-matrix README generation. |
| scripts/templates/version-matrix/VERSION-MATRIX-INDEX.md.tpl | Removes gomplate template for top-level version-matrix index README generation. |
| scripts/templates/version-matrix/.gomplate.yaml | Removes gomplate plugin wiring that previously called bash generators. |
| scripts/templates/release-notes/RELEASE-NOTES-FOOTER.md.tpl | Removes gomplate footer template; footer is now rendered by Go. |
| scripts/sort-version-matrix.go | Removes legacy Go helper used by the bash/gomplate pipeline. |
| scripts/release-tools/main.go | Adds the release-tools CLI dispatcher and subcommand wiring. |
| scripts/release-tools/chart_images.go | Adds image-set derivation command (no helm template) for chart image annotations. |
| scripts/release-tools/update_matrix.go | Adds version-matrix.json upsert/update command (including enterprise image derivation). |
| scripts/release-tools/version_matrix.go | Adds README rendering/splicing for version-matrix per-app and index readmes. |
| scripts/release-tools/resolve_tag.go | Adds rolling/concrete Harbor tag resolution + GitHub output emission. |
| scripts/release-tools/resolve_tag_test.go | Adds unit tests for resolve-tag behavior and outputs. |
| scripts/release-tools/harbor_tag.go | Adds Harbor tag operation command wrapper around new Harbor client package. |
| scripts/release-tools/image_overrides.go | Adds command to collect image override inputs and emit HAS_IMAGE_OVERRIDES + block. |
| scripts/release-tools/component_image_versions.go | Adds command to generate component-image-versions annotation block. |
| scripts/release-tools/chart_metadata.go | Adds command to parse package Chart.yaml metadata into GitHub outputs. |
| scripts/release-tools/release_version.go | Adds release-version computation command (incl. optional “fail if already released”). |
| scripts/release-tools/release_notes.go | Adds Go implementation for release notes generation + Chart.yaml annotations updates. |
| scripts/release-tools/inject_values.go | Adds Go wrapper for values.yaml image tag injection (replacing old CLI). |
| scripts/release-tools/inject_values_test.go | Adds tests for env-driven override building behavior. |
| scripts/release-tools/ghoutput.go | Adds helper for writing to $GITHUB_OUTPUT / $GITHUB_ENV with multiline support. |
| scripts/release-tools/go.mod | Adds Go module definition for scripts/release-tools (with local replace to camunda-core). |
| scripts/release-tools/go.sum | Adds dependency sums for release-tools module. |
| scripts/generate-release-notes.sh | Removes legacy release notes + version-matrix bash generator. |
| scripts/camunda-core/pkg/versionmatrix/versionmatrix.go | Extends ChartEntry model to include enterprise image set field. |
| scripts/camunda-core/pkg/versionmatrix/write.go | Adds pure JSON upsert/write logic for version-matrix entries. |
| scripts/camunda-core/pkg/versionmatrix/write_test.go | Adds unit tests for version-matrix entry upsert semantics. |
| scripts/camunda-core/pkg/versionmatrix/readme.go | Adds per-app README splicing + index rendering implementation. |
| scripts/camunda-core/pkg/valuesinjector/valuesinjector.go | Adds reusable values.yaml image tag injection library (format-preserving). |
| scripts/camunda-core/pkg/valuesinjector/valuesinjector_test.go | Ports/adapts values injector tests to the new package. |
| scripts/camunda-core/pkg/releaseplease/releaseplease.go | Adds pure release-please trace parsing + dev tag/version computation logic. |
| scripts/camunda-core/pkg/releaseplease/releaseplease_test.go | Adds tests for releaseplease version/tag computations and alpha detection. |
| scripts/camunda-core/pkg/releasenotes/releasenotes.go | Adds pure transforms for helmCLIVersion + artifacthub changes block generation. |
| scripts/camunda-core/pkg/releasenotes/releasenotes_test.go | Adds tests for Helm CLI clamping and ArtifactHub changes extraction. |
| scripts/camunda-core/pkg/harbortag/harbortag.go | Adds pure parsing/resolution logic for dev/rc and rolling Harbor tags. |
| scripts/camunda-core/pkg/harbortag/harbortag_test.go | Adds unit tests for tag parsing/resolution helpers. |
| scripts/camunda-core/pkg/harbor/harbor.go | Adds Harbor API client for digest lookup and tag add/move/delete via deleteTag endpoint. |
| scripts/camunda-core/pkg/harbor/harbor_test.go | Adds tests validating tag endpoint usage and EnsureTag behavior. |
| scripts/camunda-core/pkg/chartmeta/package.go | Adds Chart.yaml metadata readers (chart-images annotation + package metadata). |
| scripts/camunda-core/pkg/chartmeta/package_test.go | Adds tests for Chart.yaml metadata parsing and chart version “latest stable” detection. |
| scripts/camunda-core/pkg/chartmeta/metadata.go | Adds component-image-versions + imageOverrides block generation helpers. |
| scripts/camunda-core/pkg/chartmeta/metadata_test.go | Adds tests for component-image-versions and override block rendering. |
| scripts/camunda-core/pkg/chartmeta/images_test.go | Adds contract tests for non-render image set derivation from values.yaml. |
| Makefile | Adds build/install targets for release-tools and switches release note generation to Go. |
| docs/reference/release-process.md | Updates release process docs to reference release-tools release-notes. |
| .gitignore | Ignores the built release-tools binary in scripts/release-tools/. |
| .github/workflows/test-release-tooling.yaml | Adds CI workflow to vet+test the new release tooling packages and CLI. |
| .github/workflows/repo-pr-conventions.yaml | Updates comments to reference release-tools behavior for path filters. |
| .github/workflows/chart-release-public.yaml | Replaces inline bash parsing with release-tools subcommands; adjusts Harbor login usage. |
| .github/workflows/chart-release-chores.yaml | Replaces version-matrix bash generation with release-tools update/render commands. |
| .github/AGENTS.md | Updates scripts inventory to reflect release-tools replacing values-injector. |
| // Early exit if the tag already exists. | ||
| if tags, _ := capture(ctx, "git", "tag", "-l"); strings.Contains(tags, chartTag) { | ||
| fmt.Printf("[WARN] The tag %s already exists, nothing to do...\n", chartTag) | ||
| return nil | ||
| } |
| on: | ||
| pull_request: | ||
| push: | ||
| branches: | ||
| - main | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
| // Command release-tools provides the data-transformation subcommands the Camunda | ||
| // chart release pipelines use; each subcommand wraps a tested package in | ||
| // scripts/camunda-core/pkg. It is a lean stdlib-flag dispatcher (no cobra): | ||
| // | ||
| // release-tools <subcommand> [flags] | ||
| package main |
| // Package releasenotes holds the pure release-notes transforms: | ||
| // | ||
| // - HelmCLIVersion: the camunda.io/helmCLIVersion annotation value, clamped | ||
| // across the Helm v3→v4 migration by Camunda minor. | ||
| // - CliffGroups / ArtifactHubChanges: the artifacthub.io/changes annotation | ||
| // block parsed from the git-cliff RELEASE-NOTES.md sections via the | ||
| // keep-a-changelog map. | ||
| // - ParseChartIdentity / AppMinor / AppStripLastSegment: Chart.yaml field reads. | ||
| package releasenotes |
Ian-wang-liyang
left a comment
There was a problem hiding this comment.
crev review
PR #6359 replaces ~600 lines of untested release-pipeline bash across three workflows with a tested Go CLI backed by new scripts/camunda-core/pkg/* packages (chartmeta, harbor, harbortag, releasenotes, releaseplease, valuesinjector, versionmatrix). Two P1 issues need addressing before merge: (1) update-matrix --chart-yaml hard-fails on any dev artifact built before this PR because the new camunda.io/chart-images annotation did not exist then — all in-flight RC promotions will fail until those artifacts are rebuilt; add a graceful fallback. (2) TestImageSetRealChartContract will consistently fail in CI for 8.7/8.8/8.9 because test-release-tooling.yaml has no helm dependency update step and vendoredSubchartValues() fatals when the required .tgz files are absent — this is also the exact code path for the PR's primary bug fix. No chart template, values.yaml, or security surfaces were changed.
Specialists run: correctness, security, api-stability, test-adequacy, devils-advocate, verifier, escalation-assessor. Devil's-advocate hypotheses: 13 raised, 1 promoted.
Hypotheses by stance: adversarial-input=4 author-blind-spot=1 claim-vs-code=3 cross-version=1 missing-case=3 scope-discipline=1 · by disposition: covered_by_specialist=1 dropped_low_severity=1 dropped_rubric=2 dropped_ungroundable=9
Escalation: AI review sufficient (score: 0.19, threshold: 0.50). No hard NEVER rules violated; no chart templates, values.yaml, or security surfaces touched. The 2 confirmed P1 findings are CI pipeline operational gaps (not user-facing chart API breaks). Dominant escalation pressure comes from high novelty (large new Go subsystem, ~8000-line diff) and the P1 deployment migration cliff, but these stay below threshold. Per policy, human review and approval is always advisory-required regardless of the score.
| return err | ||
| } | ||
| if len(images) == 0 { | ||
| return fmt.Errorf("%s annotation in %s is empty or missing; it must be recorded at build time before promote", chartmeta.ChartImagesAnnotation, chartYAML) |
There was a problem hiding this comment.
P1 · update-matrix hard-fails on pre-PR dev artifacts, blocking RC promotion during deployment window (via correctness)
When --chart-yaml is passed (the path used by chart-promote-rc.yaml), ChartImages() reads the camunda.io/chart-images annotation from the extracted Chart.yaml. This annotation is written by the new Record chart-images annotation step introduced in chart-build-dev.yaml by this same PR.
Any dev artifact built before this PR merges will not carry this annotation. ChartImages() returns an empty slice for a missing annotation (confirmed by TestChartImagesMissing in package_test.go). The guard at lines 61–63 then hard-fails:
camunda.io/chart-images annotation in /tmp/Chart.yaml is empty or missing; it must be recorded at build time before promote
This means the Update version matrix step in chart-promote-rc.yaml will fail for every pre-existing dev artifact in Harbor until those artifacts are rebuilt by the updated pipeline. The two workflows (chart-build-dev.yaml and chart-promote-rc.yaml) must land and stabilize before any in-flight RC promotion is attempted.
Recommendation: Add a graceful fallback in runUpdateMatrix: when --chart-yaml is given but len(images) == 0, fall back to deriving the image set from --chart (the vendored chart in the working tree), with a ::warning:: log. This makes the new pipeline backward-compatible with artifacts built before this PR and eliminates the deployment ordering constraint.
return fmt.Errorf("%s annotation in %s is empty or missing; it must be recorded at build time before promote", chartmeta.ChartImagesAnnotation, chartYAML)
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 |
There was a problem hiding this comment.
P1 · TestImageSetRealChartContract will fail in CI for 8.7/8.8/8.9: helm dependency update missing from test workflow (via test-adequacy)
TestImageSetRealChartContract iterates over versions 8.10, 8.9, 8.8, 8.7. Each subtest skips only when values.yaml is absent. All four chart directories exist in the repo (full checkout), so all four subtests run.
For 8.7, 8.8, and 8.9, Chart.yaml declares non-common file:// subcharts (keycloak, postgresql, web-modeler-postgresql, elasticsearch). ImageSet() calls bundledSubcharts() to find these deps, then calls vendoredSubchartValues(chartDir, name) for each. That function:
matches, err := filepath.Glob(filepath.Join(chartDir, "charts", name+"-*.tgz"))
if err != nil || len(matches) == 0 {
return nil, fmt.Errorf("vendored subchart %s-*.tgz not found in %s/charts (run helm dependency update)", name, chartDir)
}Because test-release-tooling.yaml has no helm dependency update step, the charts/*.tgz files are absent — filepath.Glob returns (nil, nil) — triggering the error, which propagates to t.Fatalf. The CI job fails for 8.9, 8.8, and 8.7 subtests on every run.
8.10 passes because its only dependency is common, which bundledSubcharts() explicitly excludes.
This is particularly notable because the vendored-subchart image discovery path is the primary bug fix in this PR (preventing busybox over-inclusion). The test that validates this fix will consistently fail in CI as written.
Recommendation: Either (a) add a helm dependency update step in test-release-tooling.yaml before go test ./pkg/... (targeting active chart versions 8.7–8.10), or (b) update the skip guard in TestImageSetRealChartContract to skip when charts/ has no .tgz files rather than only when values.yaml is absent. Option (a) provides a fuller contract test; option (b) is simpler and avoids slow network calls in CI.
go test ./pkg/...
09f5ce2 to
1bef766
Compare
Which problem does the PR fix?
Closes #6345.
The three release-pipeline workflows (
chart-build-dev,chart-promote-rc,chart-release-public) carried large amounts of untested inline bash, plusgenerate-release-notes.sh,generate-version-matrix.shand gomplatetemplates. Untested string-mashing already shipped a bug to production (busybox
chart_enterprise_images).What's in this PR?
Consolidates the release-pipeline bash into one tested Go CLI
(
scripts/release-tools) over packages inscripts/camunda-core/pkg/*. 1:1functional behavior, with the bugfixes noted below.
helm template.chartmeta.ImageSetderives theimage set by dumb-reading
values.yaml+ the vendored subchart values (norender), so test-scenario scaffolding (e.g. a
busyboxinit container) is nolonger picked up.
chart-images,update-matrix,resolve-tag,harbor-tag,component-image-versions,image-overrides,chart-metadata,release-version,release-notes(
--main/--footer),inject-values,version-matrix.version-matrix --readme <app> --chart-version <v>splices a single version's section into the per-app README (other rows kept
verbatim);
--indexrenders the top-level index. Enterprise images are thevalues-enterprise.yamloverlay'sregistry.camunda.cloudrefs.generate-release-notes.sh,generate-version-matrix.sh,sort-version-matrix.go, the version-matrix.tpl+.gomplate.yaml,RELEASE-NOTES-FOOTER.md.tpl, andscripts/values-injector(folded intopkg/valuesinjector).Bugfixes flagged for review:
deleteArtifact→deleteTag. Tag cleanup hit the artifact-deleteendpoint, which removes the shared artifact, not just the tag. Now uses
DELETE .../tags/{tag}. Verified live against a throwaway Harbor repo.logIndexfallback. New cosign bundle format uses.verificationMaterial.tlogEntries[0].logIndex; old format used.rekorBundle.Payload.logIndex. Both are now tried.release-version --fail-if-released(set only forworkflow_dispatch) errors when the computedversion already has a release git tag, so a manual build can't package and push
over an immutable published release (especially with image-tag overrides).
Checklist
Please make sure to follow our Contributing Guide.
Before opening the PR:
make go.update-golden-only.After opening the PR: