Add OCI signing as part of existing publish pipeline#290
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
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 newgcb publishaction (helmchartoci) to push/sign/verify charts (including non-vtag 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.
be560e0 to
5f3f2d5
Compare
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>
5f3f2d5 to
f6939e0
Compare
There was a problem hiding this comment.
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
gcbPublishOptionsnow has an injectableRunnerintended to control external command execution, but image signing insignOCIImagesstill callscosign.Sign, which shells out viashell.Commandand ignoreso.Runner. That means tests can’t fully fake cosign invocations and production can’t consistently centralize runner behavior. Consider plumbingo.Runnerthrough image signing as well (either by adding a runner-aware helper inpkg/sign/cosignthat preserves existing flags/defaults, or by switching toSignWithOptionswith 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) |
| 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") | ||
| } | ||
| } |
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>
f6939e0 to
8c208dd
Compare
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.