Reset ci with sbom and cosign

[SamYuan1990]

related with #2235

[SamYuan1990]

@sunya-ch and @nikimanoledaki
do we really need cosign for helm and binary? ref https://docs.sigstore.dev/quickstart/quickstart-ci/

[SamYuan1990]

related with #2235

[nikimanoledaki]

Thanks @SamYuan1990. There are three main issues:

1. Please reorder the steps so that it's: build, make push, SBOM, cosign sign. Cosign can only sign a digest that exists after a push. Right now it fails.
2. Add --yes to all five cosign calls.
3. Remove oci:// from CHART_REF. Only keep it for helm push - cosign needs a bare registry/repo:tag.

[nikimanoledaki]

Suggested change

	- cron: 0 6 * * * # 每天早上6点（UTC）
	- cron: 0 6 * * * # daily at 6:00 UTC

[nikimanoledaki]

cosign attach sbom has been deprecated. Please use the newer equivalent:

cosign attest --predicate sbom.spdx.json --type spdxjson <image>

[SamYuan1990]

Thanks @SamYuan1990. There are three main issues:

1. Please reorder the steps so that it's: build, make push, SBOM, cosign sign. Cosign can only sign a digest that exists after a push. Right now it fails.
2. Add --yes to all five cosign calls.
3. Remove oci:// from CHART_REF. Only keep it for helm push - cosign needs a bare registry/repo:tag.

hi @nikimanoledaki ,

For "reorder the steps", in my point of view:
build, meta(like openSSF, in our case SBOM/SLAS...., cosign), push(binary and openSSF)

As "pipeline as code", having a reusable workflow for both release, pr build and daily build as implementation level. We can "test" steps for release in daily changes.
The different between release and pr are parameters and push to register or not, as secret values as container hub login, we have to test those in a release, as trigger level.

Hence, at implementation level, reusable workflow build image, build binary, build helm will have their own response for build(asset) and meta(as openSSF asset), receive parameters for build tag.(such like version)

at invoker/trigger level, the workflow, push(means code merge to default branch), release(means a release tag), .... represent the reason as event trigger, owns value(as build tag, version) to implementation level, and response for push or not.

For cosign, I will change to https://docs.sigstore.dev/quickstart/quickstart-ci/ , and have some comments in code. The code will be built on github action as sigstore/cosign-installer and official documents, makes easy for people/agent to understand and maintain.

WDYT?

[nikimanoledaki]

Thanks @SamYuan1990, I agree with creating a reusable workflow if it works for every scenario, and I also agree with using sigstore/cosign-installer which makes it easier for maintainers 👍

[nikimanoledaki]

The docs say that signing the artifact should happen after pushing to the artifact registry: https://docs.sigstore.dev/quickstart/quickstart-ci/#signing-and-verifying-a-container-image

[SamYuan1990]

The docs say that signing the artifact should happen after pushing to the artifact registry: https://docs.sigstore.dev/quickstart/quickstart-ci/#signing-and-verifying-a-container-image

I suppose it's about what are we going to "release"/provide.
before the rewrite, we use docker/build-push-action, but during rewrite it changes to make files.( I don't see any benefits on makefile).
So far, I suppose we just cosign for container's sbom.

Another founding as we can't test cosign unless this been merged, as it's require a write permission on GHA token.

[vprashar2929]

nit: cleanup extra comments

[SamYuan1990]

Well, I want to keep the comments, we can discuss on the format.

The reason I want to keep those comments as I hope either coding agent or people can know on how to use this action.yaml.

Any suggestions?

[vprashar2929]

Better to place it in either CLAUDE.md/AGENTS.md or maybe in our ci docs https://github.com/sustainable-computing-io/kepler/tree/main/docs/developer

[vprashar2929]

@SamYuan1990 Couple of things that look broken with this PR:

1. Release never actually signs. The composites default to false. Release never sets sign: true So the sign steps don't run.
2. Aren't we uploading the wrong signature files? 🤔 Cosign is writing .sigstore.json bundles but the release artifact globs still look for the old .cert AFAIU they won't match so signature won't show up on the GH releases.
3. We stopped publishing images on main. The existing workflow built and pushed to Quay via publish-image The new workflow only builds it doesn't push. Can you elaborate on why this change was done?
4. We used to helm push to oci://... Now we only package and attach the chart to GH release. Why this was done? Please elaborate more
5. The existing workflow on main also runs tests/codecov and scorecard. With your changes they won't run on the push to main. Can you please revert it or elaborate why the change was done?

Also putting id-token: write at the top of the workflow that runs on PRs is broader than we need. Safer pattern would be default to readonly at top level and elevate the access only on the jobs that actually sign!

Smaller nits:

1. Cosign install is done repeatedly. This could live inside the composite actions so callers stay thin.
2. Prefer docker/login-action over echo $TOKEN | docker login

[vprashar2929]

@SamYuan1990 Have you tested this on your fork?

[SamYuan1990]

@vprashar2929 , I suppose helm publish process is not changed in this PR.

[SamYuan1990]

@SamYuan1990 Couple of things that look broken with this PR:

1. Release never actually signs. The composites default to false. Release never sets sign: true So the sign steps don't run.
2. Aren't we uploading the wrong signature files? 🤔 Cosign is writing .sigstore.json bundles but the release artifact globs still look for the old .cert AFAIU they won't match so signature won't show up on the GH releases.
3. We stopped publishing images on main. The existing workflow built and pushed to Quay via publish-image The new workflow only builds it doesn't push. Can you elaborate on why this change was done?
4. We used to helm push to oci://... Now we only package and attach the chart to GH release. Why this was done? Please elaborate more
5. The existing workflow on main also runs tests/codecov and scorecard. With your changes they won't run on the push to main. Can you please revert it or elaborate why the change was done?

Also putting id-token: write at the top of the workflow that runs on PRs is broader than we need. Safer pattern would be default to readonly at top level and elevate the access only on the jobs that actually sign!

Smaller nits:

1. Cosign install is done repeatedly. This could live inside the composite actions so callers stay thin.
2. Prefer docker/login-action over echo $TOKEN | docker login

hi @vprashar2929,

I will check item 1 and item 2. For item 1 the sign is set to false as default as pr can't get id-token: write as permission limits.

For item 3, I will leave to @nikimanoledaki , as I don't know how our build process going to be, before rebuild, we have latest build and pre merge build. ref https://github.com/SamYuan1990/kepler/blob/main/.github/workflows/push.yml#L14, https://github.com/SamYuan1990/kepler/blob/main/.github/workflows/image.yml#L9

For item 4, as I checked in pr diff, seems out of this PR's scope.

For item 5, I will add tests/codecov and scorecard back.

My own fork is before rewrite, I will find a way to test on my own fork, but as we have condition in CI as if: github.repository_owner == 'sustainable-computing-io', I suppose I can't test release.

For nits and code changes, I will check and update after we got agreement on item 1,2,3.

As a summary:

Todo list:

• fix nits on duplicated Cosign if any.
• use docker/login-action instead of echo $TOKEN | docker login.
• check id-token: write
• add tests/codecov and scorecard back.
• testing on my fork for push event.(if possible?)

Out of scope:

• helm push to oci://...

BTW, I hope coding agent can help us on CI changes.

[github-actions]

This PR is stale because it has been open 60 days with no activity.