Automatically update Chart.yaml version for release#320
Automatically update Chart.yaml version for release#320peterj merged 3 commits intoagentregistry-dev:mainfrom
Chart.yaml version for release#320Conversation
nikolasmatt
changed the title
nikolasmatt
changed the title
Chart.yaml version for release
There was a problem hiding this comment.
Pull request overview
This PR refactors the Helm chart/release workflow so the chart’s Chart.yaml is generated from a template at build time, and the GitHub release workflow delegates chart packaging/pushing/checksumming to Makefile targets.
Changes:
- Add
charts-generateto generatecharts/agentregistry/Chart.yamlfromChart-template.yamlusingenvsubst, and wire it into other chart-related Make targets. - Introduce a
charts-releaseMake target to run chart test → push → checksum, and update the GitHub release workflow to use it. - Update documentation and
.gitignoreto reflect thatChart.yamlis generated and not committed.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
Makefile |
Adds chart generation/release targets and updates install/push/test flows to rely on generated Chart.yaml. |
DEVELOPMENT.md |
Documents the new generated-Chart.yaml workflow and release target usage. |
charts/agentregistry/Chart-template.yaml |
Converts chart version/appVersion (and Artifact Hub image annotation) to use CHART_VERSION. |
.gitignore |
Ignores the generated charts/agentregistry/Chart.yaml. |
.github/workflows/release.yml |
Simplifies version handling and uses make charts-release for chart publishing. |
Comments suppressed due to low confidence (2)
Makefile:316
install-agentregistrynow depends oncharts-generate(which writes to$(HELM_CHART_DIR)), but the recipe still hardcodescharts/agentregistryin thehelm upgrade --installcommand. If a caller overridesHELM_CHART_DIR,charts-generatewill generateChart.yamlin the overridden directory whilehelmstill installs fromcharts/agentregistry, which can fail due to a missing/oldChart.yaml. Use$(HELM_CHART_DIR)consistently in the install command (or explicitly disallow overriding for this target).
install-agentregistry: charts-generate ## Build images and Helm install AgentRegistry into the Kind cluster (BUILD=false to skip image builds)
ifeq ($(BUILD),true)
install-agentregistry: docker-server docker-agentgateway
endif
@JWT_KEY=$$(kubectl --context $(KIND_CLUSTER_CONTEXT) -n $(KIND_NAMESPACE) \
charts/agentregistry/Chart-template.yaml:28
artifacthub.io/imagesnow points to.../server:${CHART_VERSION}(no leadingv). However, this repo’s Docker image tags are built/pushed using$(VERSION)(which typically includes thevprefix, especially for tag-based releases likev0.3.0). This likely makes the Artifact Hub image reference incorrect/nonexistent. Consider templating the image tag separately (e.g. use${VERSION}orv${CHART_VERSION}) so the metadata matches the actual published image tags.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| # Generate Chart.yaml from Chart-template.yaml using envsubst. | ||
| .PHONY: charts-generate | ||
| charts-generate: ## Generate Chart.yaml from Chart-template.yaml (uses CHART_VERSION, default derived from git tags) | ||
| @echo "Generating $(HELM_CHART_DIR)/Chart.yaml (version=$(CHART_VERSION))..." | ||
| CHART_VERSION=$(CHART_VERSION) envsubst '$$CHART_VERSION' \ | ||
| < $(HELM_CHART_DIR)/Chart-template.yaml \ | ||
| > $(HELM_CHART_DIR)/Chart.yaml |
There was a problem hiding this comment.
charts-generate invokes envsubst but the Makefile never checks that envsubst is installed. This will currently fail with a generic "command not found" error on machines/CI images without gettext. Consider adding a preflight check (similar to _helm-check) that verifies envsubst is available and prints a clear install hint, or make ENV_SUBST ?= envsubst configurable and check that executable.
| # Required env vars for the push step: HELM_REGISTRY_PASSWORD (and optionally HELM_REGISTRY_USERNAME). | ||
| # Override version: make charts-release CHART_VERSION=1.2.3 |
There was a problem hiding this comment.
The comments for charts-release mention required env vars for the push step (e.g. HELM_REGISTRY_PASSWORD), but charts-push no longer performs a helm registry login and doesn't read these env vars. Either restore an explicit login/logout in charts-push (using those env vars) or update the comments/docs to state that the caller must be logged in before running charts-push/charts-release to avoid confusion.
| # Required env vars for the push step: HELM_REGISTRY_PASSWORD (and optionally HELM_REGISTRY_USERNAME). | |
| # Override version: make charts-release CHART_VERSION=1.2.3 | |
| # Caller must be logged in to the Helm OCI registry before running charts-push/charts-release. | |
| # For example: HELM_REGISTRY_USERNAME=... HELM_REGISTRY_PASSWORD=... helm registry login $(HELM_REGISTRY) |
| # Full Helm release pipeline: test → push (→ lint → package → generate + deps) → checksum. | ||
| # Required env vars for the push step: HELM_REGISTRY_PASSWORD (and optionally HELM_REGISTRY_USERNAME). | ||
| # Override version: make charts-release CHART_VERSION=1.2.3 | ||
| .PHONY: charts-release | ||
| charts-release: charts-test charts-push charts-checksum ## Full Helm release: lint, test, package, checksum, and push |
There was a problem hiding this comment.
charts-release: charts-test charts-push charts-checksum is described as an ordered pipeline, but Make does not guarantee prerequisite execution order under parallel builds (make -j), and charts-checksum does not depend on charts-package/charts-push to ensure the .tgz exists. To make this robust, enforce sequencing in the charts-release recipe (invoke sub-targets in order) or add explicit dependencies (e.g. charts-checksum: charts-package) and/or mark the target(s) .NOTPARALLEL.
| ```bash | ||
| # Generate with version derived from the latest git tag (e.g. 0.3.0) | ||
| make charts-generate | ||
|
|
||
| # Generate with an explicit version | ||
| make charts-generate CHART_VERSION=0.4.0 | ||
| ``` | ||
|
|
||
| `CHART_VERSION` defaults to the output of `git describe --tags --abbrev=0` with the leading `v` stripped. If there are no tags, set it explicitly. | ||
|
|
||
| Any Makefile target that needs `Chart.yaml` (e.g. `charts-lint`, `charts-test`, `charts-package`, `install-agentregistry`) declares `charts-generate` as a prerequisite and will generate it automatically. You only need to run `make charts-generate` directly if you're invoking `helm` commands by hand. |
There was a problem hiding this comment.
The default CHART_VERSION derivation described here doesn’t match the Makefile. The Makefile sets VERSION via git describe --tags --always (or a fallback) and then strips a leading v to get CHART_VERSION. This can include additional -<n>-g<sha> suffixes, and it does not use git describe --abbrev=0 as written. Please update this section to reflect the actual derivation (or change the Makefile to match the documented behavior).
| done | ||
| - name: Build Release Artifacts | ||
| run: make release-cli | ||
|
|
There was a problem hiding this comment.
The release workflow now runs make charts-release, which requires envsubst (gettext) to generate Chart.yaml. There’s no step ensuring envsubst is installed on the runner, so this job can fail depending on the base image. Add an explicit install step (e.g. install gettext/gettext-base) or adjust chart generation to avoid this external dependency in CI.
| - name: Install envsubst | |
| run: | | |
| sudo apt-get update | |
| sudo apt-get install -y gettext-base |
bb6daed
Description
Fixes #312
Removes the need to manually update
Chart.yamlwith the release version.Moves most of the helmchart release logic from the GH workflow file to make targets
Change Type
Changelog
/kind cleanup
Additional Notes