Add OCI signing as part of existing publish pipeline by SgtCoDFish · Pull Request #290 · cert-manager/release

SgtCoDFish · 2026-04-17T11:09:01Z

The aim is for this to replace the need for hack/push_and_sign_chart.sh and to make charts.jetstack.io downstream of the OCI registry.

⚠️ This is COMPLETELY UNTESTED and mostly AI generated and so probably can't be merged in its current state. But it's a start!

cert-manager-prow · 2026-04-17T11:09:08Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign thatsmrtalbot for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot

Pull request overview

This PR extends the existing release publish pipeline to publish Helm charts to an OCI registry and sign/verify those OCI artifacts (intended to replace hack/push_and_sign_chart.sh and make charts.jetstack.io downstream of the OCI registry).

Changes:

Add cosign helpers for signing/verifying with explicit option flags.
Add Helm OCI helpers (helm push, crane copy) and a new gcb publish action (helmchartoci) to push/sign/verify charts (including non-v tag copy).
Update the Cloud Build publish job to install/use cosign v3, crane, and helm; plumb a new substitution/flag for the OCI registry target.

Reviewed changes

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

Show a summary per file

File	Description
pkg/sign/cosign/cosign.go	Adds optioned cosign sign/verify wrappers used by OCI chart signing.
pkg/release/helm/oci.go	Introduces helper wrappers for `helm push` to OCI and `crane copy` tag operations.
gcb/publish/cloudbuild.yaml	Installs cosign v3, crane, and helm; passes new OCI-related args/substitution into `cmrel gcb publish`.
cmd/cmrel/cmd/publish.go	Adds CLI flag + build substitution for the published Helm chart OCI registry.
cmd/cmrel/cmd/gcb_publish.go	Adds new publish action `helmchartoci` implementing push/sign/verify logic; adds helm/crane paths + OCI registry options.
cmd/cmrel/cmd/const.go	Adds a default OCI registry constant for Helm chart publishing.

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

Copilot · 2026-04-17T12:40:50Z

+	if err := signOCIImages(ctx, o, pushedContent); err != nil {
 		return fmt.Errorf("failed to sign images: %w", err)
 	}

 	return nil


With the Cloud Build config upgrading cosign to v3, the container image signing path (now signOCIImages) still relies on cosign.Sign(...), which uses cosign defaults. Elsewhere (chart signing) you explicitly disable cosign v3 defaults like --use-signing-config / --new-bundle-format / tlog behavior to preserve existing behavior. Consider aligning image signing with the chart signing flags (e.g., add an optioned signing helper) to avoid behavior changes or failures after the cosign v3 upgrade.

Copilot · 2026-04-17T12:40:51Z

+- name: docker.io/library/golang:1.26-alpine@sha256:c2a1f7b2095d046ae14b286b18413a05bb82c9bca9b25fe7ff5efef0f0826166
+  entrypoint: sh
+  args:
+  - -c
+  - |
+    apk add --no-cache curl
+    curl https://raw.githubusercontent.com/helm/helm/main/scripts/get-helm-3 | sh
+    mv /usr/local/bin/helm /go/bin/helm


The Helm installer step downloads and executes an unpinned script from the Helm repo’s main branch (curl ... | sh). This is a supply-chain risk and can make builds non-reproducible; additionally, piping into sh may fail if the script expects bash. Prefer installing a pinned Helm release artifact (or a pinned script commit) and verify checksums/signatures; if you keep the script approach, invoke it with bash and pin the commit/ref.

Copilot · 2026-04-17T12:40:51Z

+	ociURL := fmt.Sprintf("oci://%s", o.PublishedHelmChartOCIRegistry)
+
+	// Push chart to OCI registry (helm automatically pushes .prov if it exists)
+	log.Printf("Pushing chart to %s", ociURL)
+	if err := helm.PushChartToOCI(ctx, chartPath, ociURL, o.HelmPath); err != nil {


published-helm-chart-oci-registry is described as an OCI registry, but pushHelmChartOCI always prefixes it with oci://. If a caller passes an oci://... value (which is common in Helm docs), this will produce an invalid URL (oci://oci://...) and also break the cosign/crane refs built from the same string. Consider normalizing the input once (e.g., trim optional oci:// prefix, validate it’s host/path only) before constructing ociURL, chartRef, and destRef.

Copilot · 2026-04-17T12:40:51Z

 var publishActionMap map[string]publishAction = map[string]publishAction{
-	"helmchartpr":         pushHelmChartPR,
+	"helmchartoci":        pushHelmChartOCI,
 	"githubrelease":       pushGitHubRelease,
 	"pushcontainerimages": pushContainerImages,
+
+	// helmchartpr has been deprecated in favour of helmchartoci
+	// "helmchartpr":         pushHelmChartPR,
 }


The action map removes helmchartpr entirely but the comment says it is “deprecated”. As written, users specifying --publish-actions=helmchartpr will now error out, which is a breaking CLI change rather than a deprecation. Consider keeping helmchartpr as an alias to the new OCI implementation (and log a warning), or implement an explicit migration path in canonicalizeAndVerifyPublishActions.

Copilot · 2026-04-17T12:40:52Z

+func pushHelmChartOCI(ctx context.Context, o *gcbPublishOptions, rel *release.Unpacked) error {
+	log.Printf("Pushing Helm chart to OCI registry %q", o.PublishedHelmChartOCIRegistry)
+
+	// Verify tools are available
+	log.Printf("Verifying helm installation")
+	if err := shell.Command(ctx, "", o.HelmPath, "version"); err != nil {
+		return fmt.Errorf("failed to verify helm installation: %w", err)
+	}


New publish behavior is introduced in pushHelmChartOCI (helm push, crane copy, cosign sign/verify) but there are no tests covering the command construction/branching (skip-signing, non-v tag copy). Since this package already has tests, consider adding unit tests by refactoring command execution behind an interface so you can assert the expected helm/crane/cosign invocations without running external binaries.

Copilot · 2026-04-17T12:40:52Z

+		"--tlog-upload=" + fmt.Sprintf("%t", opts.TlogUpload),
+		"--new-bundle-format=" + fmt.Sprintf("%t", opts.NewBundleFormat),
+		"--use-signing-config=" + fmt.Sprintf("%t", opts.UseSigningConfig),


SignWithOptions builds boolean flag values via fmt.Sprintf("%t", ...). Using strconv.FormatBool avoids pulling in fmt just for boolean formatting and is simpler/cheaper.

This bug has been latent since commit 7ba3cf2 added the cmctl check. It caused tests to panic Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>

The aim is for this to replace the need for hack/push_and_sign_chart.sh and to eventually make charts.jetstack.io downstream of the OCI registry. Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

cmd/cmrel/cmd/gcb_publish.go:734

gcbPublishOptions now has an injectable Runner intended to control external command execution, but image signing in signOCIImages still calls cosign.Sign, which shells out via shell.Command and ignores o.Runner. That means tests can’t fully fake cosign invocations and production can’t consistently centralize runner behavior. Consider plumbing o.Runner through image signing as well (either by adding a runner-aware helper in pkg/sign/cosign that preserves existing flags/defaults, or by switching to SignWithOptions with explicitly chosen options).

func signOCIImages(ctx context.Context, o *gcbPublishOptions, allContentToSign []string) error {
	if o.SkipSigning {
		log.Println("Skipping signing container images / manifest lists as skip-signing is set")
		return nil
	}

+		log.Printf("Warning: .prov file not found for release %s - this should only happen for releases earlier than v1.6.0", rel.ReleaseVersion)
+	}
+
+	ociURL := fmt.Sprintf("oci://%s", o.PublishedHelmChartOCIRegistry)


+	// Use the first chart (cert-manager releases have so far only had one chart)
+	chart := rel.Charts[0]
+	chartPath := chart.Path()


 var publishActionMap map[string]publishAction = map[string]publishAction{
-	"helmchartpr":         pushHelmChartPR,
+	"helmchartoci":        pushHelmChartOCI,
 	"githubrelease":       pushGitHubRelease,
 	"pushcontainerimages": pushContainerImages,
+


+func TestHelmChartOCIIsRegisteredPublishAction(t *testing.T) {
+	if _, ok := publishActionMap["helmchartoci"]; !ok {
+		t.Error("expected 'helmchartoci' to be a registered publish action")
+	}
+	if _, ok := publishActionMap["helmchartpr"]; ok {
+		t.Error("expected 'helmchartpr' to no longer be registered after deprecation")
+	}
+}


cert-manager-prow Bot added the dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. label Apr 17, 2026

cert-manager-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 17, 2026

erikgb requested a review from Copilot April 17, 2026 12:35

Copilot started reviewing on behalf of erikgb April 17, 2026 12:35 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

cert-manager-prow Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 9, 2026

SgtCoDFish force-pushed the oci-sign-during-release branch from be560e0 to 5f3f2d5 Compare June 23, 2026 08:49

cert-manager-prow Bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 23, 2026

SgtCoDFish added 2 commits June 23, 2026 10:42

fix panic in TestValidate_Semver

fc0711e

This bug has been latent since commit 7ba3cf2 added the cmctl check. It caused tests to panic Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>

add OCI signing as part of existing publish pipeline

f6939e0

The aim is for this to replace the need for hack/push_and_sign_chart.sh and to eventually make charts.jetstack.io downstream of the OCI registry. Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>

SgtCoDFish force-pushed the oci-sign-during-release branch from 5f3f2d5 to f6939e0 Compare June 23, 2026 09:50

cert-manager-prow Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 23, 2026

SgtCoDFish requested a review from Copilot June 23, 2026 09:50

Copilot started reviewing on behalf of SgtCoDFish June 23, 2026 09:51 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add OCI signing as part of existing publish pipeline#290

Add OCI signing as part of existing publish pipeline#290
SgtCoDFish wants to merge 2 commits into
cert-manager:masterfrom
SgtCoDFish:oci-sign-during-release

SgtCoDFish commented Apr 17, 2026

Uh oh!

cert-manager-prow Bot commented Apr 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

SgtCoDFish commented Apr 17, 2026

Uh oh!

cert-manager-prow Bot commented Apr 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants