feat: add artifacts.oci.disable-signing to skip signing by waveywaves · Pull Request #1419 · tektoncd/chains

waveywaves · 2025-08-19T19:51:19Z

Changes

this change allows users to disable image signing while still allowing provenance generation and attestation signing

partially fixes #1406

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Has Docs included if any changes are user facing
Has Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including
functionality, content, code)
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

feature: allow users to skip image signing with 'artifacts.oci.disable-signing: "true"'

tekton-robot · 2025-08-19T19:56:32Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/config/config.go	91.8%	90.2%	-1.6

waveywaves · 2025-08-19T20:04:31Z

cc @anithapriyanatarajan

tekton-robot · 2025-08-19T20:09:23Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/config/config.go	91.8%	92.2%	0.3

tekton-robot · 2025-08-20T05:03:21Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/config/config.go	91.8%	92.2%	0.3

pkg/config/config.go

tekton-robot · 2025-08-21T14:39:21Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/config/config.go	91.8%	92.2%	0.3

waveywaves · 2025-08-26T09:45:10Z

cc @jawed @lcarva

jkhelil

@waveywaves can you please add a e2e test with

artifacts.oci.disable-signing: "true"
artifacts.taskrun.storage: "oci"

checking that signature for image is not pushed while attestation for taskrun is pushed to oci
I am wondering what will be the taskrun attestion target repository now, as chains is using type hinting to idenitfy where to push image signature, do we need storage.oci.repository in the case of artifacts.oci.disable-signing( not sure, we need some e2e tests)
can you clarify this in doc also please

docs/config.md

- Add TestDisableOCISigning to verify that OCI image signatures are not created when disabled - Verify that attestations are still pushed to OCI registry when signing is disabled - Update documentation to clarify that type hinting still determines attestation repository - Clarify that storage.oci.repository is optional and type hinting works as expected Addresses review feedback from PR tektoncd#1419

waveywaves · 2025-09-10T23:45:03Z

@jawed I have added the necessary documentation mentioning that the artifact storage still needs to be set to OCI so that the generated attestations are pushed to the registries.

- Add TestDisableOCISigning to verify that OCI image signatures are not created when disabled - Verify that attestations are still pushed to OCI registry when signing is disabled - Update documentation to clarify that type hinting still determines attestation repository - Clarify that storage.oci.repository is optional and type hinting works as expected Addresses review feedback from PR tektoncd#1419

aThorp96 · 2025-09-15T14:59:05Z

docs/config.md

+| `artifacts.oci.signer`      | The signature backend to sign `OCI` payloads with.                                                                                                                                       | `x509`, `kms`                              | `x509`          |
+| `artifacts.oci.disable-signing` | Whether to skip signing of OCI artifacts while still allowing provenance generation and attestation signing. Note: This only disables OCI image signing; attestations are still generated. To push attestations to registries, set `artifacts.taskrun.storage` and/or `artifacts.pipelinerun.storage` to include `oci`. When enabled, attestations will still be pushed to the same location determined by type hinting (IMAGE_URL/IMAGE_DIGEST results) or `storage.oci.repository` if configured. | `"true"`, `"false"`                        | `"false"`       |


Suggestion: instead of a dedicated setting, would it make sense to instead add a new supported value to artifacts.oci.signer so a user could set artifacts.oci.signer: none? In the interest of consistency, would it also make sense in the usecase to make this setting (either the dedicated disable-signing setting or signer: none value) applicable to artifacts.taskrun and artifacts.pipelinerun as well?

I do like this direction better, I think the current default signer should stay the same and then it should be possible to disable signing this way if needed for the user.

I'll make this change and amend the commit

Add "none" as a valid signer option for OCI, TaskRun, and PipelineRun artifacts to disable signing while maintaining attestation generation and storage capabilities. Changes: - Add "none" as valid signer value in config.go for all artifact types - Update Artifact.Enabled() to return false when signer is "none" - Remove DisableSigning field from Artifact struct - Update tests to use signer: "none" instead of disable-signing field - Update documentation to list "none" as a valid signer option - Update e2e test to use artifacts.oci.signer: "none" Addresses review feedback to use signer field instead of separate disable-signing configuration.

aThorp96

This looks good to me! Thank you for the docs update as well. I have one optional docs suggestion to improve the "skimmability" of the docs.

As a general question: in what circumstance might a user want to skip signing? Would this be for performance reasons or something else?

docs/config.md

Co-authored-by: Andrew Thorp <[email protected]>

waveywaves · 2025-09-23T17:08:12Z

@aThorp96 @jkhelil can you take another look and let me know if this looks good

cc @lcarva

aThorp96

Looks good! Thanks for this

waveywaves · 2025-09-23T17:39:55Z

in what circumstance might a user want to skip signing? Would this be for performance reasons or something else?

@aThorp96 we should be able to generate provenance attestations that aren't signed by us; it should be possible to use external signers and not the prebuilt ones. This is what I understand. This opens doors for external signing infrastructure to be used and not be locked in with tekton chains signing.

anithapriyanatarajan · 2025-09-23T17:51:46Z

in what circumstance might a user want to skip signing? Would this be for performance reasons or something else?

@aThorp96 we should be able to generate provenance attestations that aren't signed by us; it should be possible to use external signers and not the prebuilt ones. This is what I understand. This opens doors for external signing infrastructure to be used and not be locked in with tekton chains signing.

@waveywaves @aThorp96 - The goal is to allow users to optionally skip signing the built images in Tekton, while still ensuring that the provenance of the build steps is generated and signed by Tekton Chains. In other words, users may choose to skip artifact.oci.signer (This gives more flexibility for users to choose different signing keys/providers for different cases) , but signing of taskruns and pipelineruns must always be enforced to be SLSA compliant. Please refer #1406 for more details

lcarva · 2025-09-23T17:54:24Z

in what circumstance might a user want to skip signing? Would this be for performance reasons or something else?

@aThorp96 we should be able to generate provenance attestations that aren't signed by us; it should be possible to use external signers and not the prebuilt ones. This is what I understand. This opens doors for external signing infrastructure to be used and not be locked in with tekton chains signing.

Actually, signing artifacts is not something Chains should be doing: #1346 (artifacts.oci.signer)

It should, however, always sign the content it generates (SLSA Provenance). artifacts.taskrun.signer controls generating and signing SLSA Provenance for TaskRuns, while artifacts.pipelinerun.signer does so for PipelineRuns. Different folks may be interested in only one of those.

I don't think there's any point, other than debugging/development reasons, to ever run Chains with all artifacts.*.signer options set to none.

@anithapriyanatarajan and I responded at the same time. I support her answer as well.

aThorp96 · 2025-09-23T18:05:58Z

Thanks all for help understanding the use-case.

[chains] should, however, always sign the content it generates (SLSA Provenance). artifacts.taskrun.signer controls generating and signing SLSA Provenance for TaskRuns, while artifacts.pipelinerun.signer does so for PipelineRuns.

@lcarva It sounds like you're saying this setting does make sense to have on artifacts.oci.signer but not on artifacts.{taskrun,pipelinerun}.signer. Is that correct, or are you saying the use-case only likely applies to artifacts.oci.signer?

lcarva · 2025-09-23T18:35:39Z

Thanks all for help understanding the use-case.

[chains] should, however, always sign the content it generates (SLSA Provenance). artifacts.taskrun.signer controls generating and signing SLSA Provenance for TaskRuns, while artifacts.pipelinerun.signer does so for PipelineRuns.

@lcarva It sounds like you're saying this setting does make sense to have on artifacts.oci.signer but not on artifacts.{taskrun,pipelinerun}.signer. Is that correct, or are you saying the use-case only likely applies to artifacts.oci.signer?

I think it makes sense to have it on at least one of artifacts.{taskrun,pipelinerun}.signer. For example, the Konflux project doesn't care about SLSA Provenance attestation for TaskRuns, so they could run Chains with artifacts.taskrun.signer=none and artifacts.pipelinerun.signer=x509.

waveywaves · 2025-09-24T11:04:07Z

@lcarva @jkhelil could you unblock the testing on this PR ? It's not going through from the looks of it

lcarva · 2025-09-24T14:19:35Z

/ok-to-test

lcarva · 2025-09-24T14:22:07Z

Not sure why the checks were cancelled... Trying to re-run them.

waveywaves · 2025-09-25T12:00:37Z

@lcarva seems like all are passing but one

lcarva

/lgtm

Thanks for all the tests!

tekton-robot · 2025-09-25T12:47:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aThorp96, lcarva

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [lcarva]

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

tekton-robot requested review from PuneetPunamiya and wlynch August 19, 2025 19:51

tekton-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Aug 19, 2025

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 19, 2025

waveywaves force-pushed the feature/configurable-artifact-signing-skip branch from 857501b to a85accf Compare August 20, 2025 04:58

lcarva reviewed Aug 20, 2025

View reviewed changes

pkg/config/config.go Outdated Show resolved Hide resolved

lcarva reviewed Aug 20, 2025

View reviewed changes

pkg/config/config.go Outdated Show resolved Hide resolved

waveywaves force-pushed the feature/configurable-artifact-signing-skip branch from a85accf to 431323f Compare August 21, 2025 14:34

jkhelil requested changes Aug 27, 2025

View reviewed changes

docs/config.md Show resolved Hide resolved

waveywaves force-pushed the feature/configurable-artifact-signing-skip branch from 431323f to 87b72df Compare September 10, 2025 23:32

waveywaves force-pushed the feature/configurable-artifact-signing-skip branch from 87b72df to 5cc663f Compare September 10, 2025 23:39

waveywaves force-pushed the feature/configurable-artifact-signing-skip branch 3 times, most recently from ffcb767 to 1a16d53 Compare September 11, 2025 00:24

aThorp96 reviewed Sep 15, 2025

View reviewed changes

waveywaves force-pushed the feature/configurable-artifact-signing-skip branch from 1a16d53 to b84c54f Compare September 18, 2025 15:04

waveywaves force-pushed the feature/configurable-artifact-signing-skip branch from b84c54f to 102b73a Compare September 18, 2025 15:38

aThorp96 approved these changes Sep 22, 2025

View reviewed changes

docs/config.md Outdated Show resolved Hide resolved

docs: decouple oci signer note from table

db6fcc0

Co-authored-by: Andrew Thorp <[email protected]>

aThorp96 approved these changes Sep 23, 2025

View reviewed changes

tekton-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Sep 24, 2025

lcarva approved these changes Sep 25, 2025

View reviewed changes

tekton-robot assigned lcarva Sep 25, 2025

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 25, 2025

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 25, 2025

tekton-robot merged commit 3ef21a9 into tektoncd:main Sep 25, 2025
34 of 55 checks passed

anithapriyanatarajan mentioned this pull request Mar 11, 2026

Handle signing OCI artifacts in *ARTIFACT_OUTPUTS #1578

Open

6 tasks

		\| `artifacts.oci.signer` \| The signature backend to sign `OCI` payloads with. \| `x509`, `kms` \| `x509` \|
		\| `artifacts.oci.disable-signing` \| Whether to skip signing of OCI artifacts while still allowing provenance generation and attestation signing. Note: This only disables OCI image signing; attestations are still generated. To push attestations to registries, set `artifacts.taskrun.storage` and/or `artifacts.pipelinerun.storage` to include `oci`. When enabled, attestations will still be pushed to the same location determined by type hinting (IMAGE_URL/IMAGE_DIGEST results) or `storage.oci.repository` if configured. \| `"true"`, `"false"` \| `"false"` \|

Conversation

waveywaves commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Submitter Checklist

Release Notes

Uh oh!

tekton-robot commented Aug 19, 2025

Uh oh!

waveywaves commented Aug 19, 2025

Uh oh!

tekton-robot commented Aug 19, 2025

Uh oh!

tekton-robot commented Aug 20, 2025

Uh oh!

Uh oh!

Uh oh!

tekton-robot commented Aug 21, 2025

Uh oh!

waveywaves commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkhelil left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

waveywaves commented Sep 10, 2025

Uh oh!

aThorp96 Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

waveywaves Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

waveywaves Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aThorp96 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

waveywaves commented Sep 23, 2025

Uh oh!

aThorp96 left a comment

Choose a reason for hiding this comment

Uh oh!

waveywaves commented Sep 23, 2025

Uh oh!

anithapriyanatarajan commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lcarva commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aThorp96 commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lcarva commented Sep 23, 2025

Uh oh!

waveywaves commented Sep 24, 2025

Uh oh!

lcarva commented Sep 24, 2025

Uh oh!

lcarva commented Sep 24, 2025

Uh oh!

waveywaves commented Sep 25, 2025

Uh oh!

lcarva left a comment

Choose a reason for hiding this comment

Uh oh!

tekton-robot commented Sep 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

waveywaves commented Aug 19, 2025 •

edited

Loading

waveywaves commented Aug 26, 2025 •

edited

Loading

jkhelil left a comment •

edited

Loading

waveywaves Sep 18, 2025 •

edited

Loading

waveywaves Sep 18, 2025 •

edited

Loading

anithapriyanatarajan commented Sep 23, 2025 •

edited

Loading

lcarva commented Sep 23, 2025 •

edited

Loading

aThorp96 commented Sep 23, 2025 •

edited

Loading