Skip to content

Add OCI signing as part of existing publish pipeline#290

Open
SgtCoDFish wants to merge 2 commits into
cert-manager:masterfrom
SgtCoDFish:oci-sign-during-release
Open

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

Conversation

@SgtCoDFish

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor

[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:

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

@cert-manager-prow cert-manager-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 17, 2026
@erikgb erikgb requested a review from Copilot April 17, 2026 12:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread cmd/cmrel/cmd/gcb_publish.go
Comment thread gcb/publish/cloudbuild.yaml
Comment thread cmd/cmrel/cmd/gcb_publish.go Outdated
Comment thread cmd/cmrel/cmd/gcb_publish.go
Comment thread cmd/cmrel/cmd/gcb_publish.go
Comment thread pkg/sign/cosign/cosign.go Outdated
@cert-manager-prow 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 SgtCoDFish force-pushed the oci-sign-during-release branch from be560e0 to 5f3f2d5 Compare June 23, 2026 08:49
@cert-manager-prow 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
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>
@SgtCoDFish SgtCoDFish force-pushed the oci-sign-during-release branch from 5f3f2d5 to f6939e0 Compare June 23, 2026 09:50
@cert-manager-prow 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 SgtCoDFish requested a review from Copilot June 23, 2026 09:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Comment thread cmd/cmrel/cmd/gcb_publish.go Outdated
Comment on lines 280 to +284
var publishActionMap map[string]publishAction = map[string]publishAction{
"helmchartpr": pushHelmChartPR,
"helmchartoci": pushHelmChartOCI,
"githubrelease": pushGitHubRelease,
"pushcontainerimages": pushContainerImages,

Comment on lines +393 to +400
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")
}
}
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 SgtCoDFish force-pushed the oci-sign-during-release branch from f6939e0 to 8c208dd Compare June 23, 2026 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants