fix(trustedresources): preserve the input document by somaz94 · Pull Request #3245 · tektoncd/cli

somaz94 · 2026-09-14T08:17:22Z

Changes

tkn task sign and tkn pipeline sign marshalled the parsed resource back to
YAML, so the whole document was rewritten rather than annotated. Keys were
reordered, sequences reflowed, and zero valued fields of the embedded Kubernetes
types were emitted, including the resources: {} that Tekton rejects on apply
with strict decoding error: unknown field "spec.steps[0].resources".

Sign now takes the document the resource was read from and inserts the
signature annotation into it directly, so the signed file differs from the input
by that annotation alone. Both reported symptoms come from the same cause, so
this covers them together.

The document is parsed only to locate the insertion point. Re-serializing the
parsed tree was the simpler option but does not round trip: a folded scalar
(description: >-) comes back with its line breaks collapsed, which is exactly
the kind of unrelated diff the catalog verify-generated check trips over.

Two things worth flagging for review:

Sign takes an extra argument. All three call sites already had the file
contents to hand, so the change is one argument at each.
Signing now fails, rather than silently reformatting, on a document whose
metadata or annotations is written in flow style, since there is no line
to insert. Everything else is handled: existing annotations, no annotations,
annotations: {}, an empty annotations: key, a leading ---, and leading
comments. Happy to fall back to the old behaviour there instead if you prefer.

go.yaml.in/yaml/v3 was already vendored as an indirect dependency, so
go mod tidy moves one line and go mod vendor produces no change.

Verified by reverting the fix: the new tests then report resources: {} present
and the step sequence reformatted from - name: echo into - image: ubuntu /
name: echo, reproducing both issues exactly.

Submitter Checklist

Includes tests (if functionality changed/added)
Run the code checkers with make check
Regenerate the manpages, docs and go formatting with make generated
Commit messages follow commit message best practices

Release Notes

`tkn task sign` and `tkn pipeline sign` now only add the signature annotation instead of rewriting the whole YAML document, which also stops them emitting an invalid `resources: {}` into steps.

tekton-robot · 2026-09-14T08:17:26Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign chmouel after the PR has been reviewed.
You can assign the PR to them by writing /assign @chmouel in a comment when ready.

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

somaz94 · 2026-09-16T03:05:08Z

/kind bug

Copilot

Copilot review overview

🟡 Changes recommended

Re-signing creates duplicate signature keys, and inserted annotations do not consistently preserve CRLF endings.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Replace existing signature instead of adding duplicate annotation · New

What changed in this PR

Preserves Task and Pipeline YAML formatting when adding trusted-resource signatures.

Changes:

Inserts signature annotations directly into source YAML.
Passes original document bytes through signing call sites.
Adds preservation and verification tests.
Promotes YAML v3 to a direct dependency.

File	Description
`pkg/trustedresources/sign.go`	Signs while preserving the source document.
`pkg/trustedresources/sign_test.go`	Tests preservation and verification.
`pkg/trustedresources/annotation.go`	Implements insertion; must replace existing signatures and preserve CRLF endings.
`pkg/trustedresources/annotation_test.go`	Tests insertion; preservation checks should be order-sensitive.
`pkg/cmd/task/sign.go`	Passes Task source bytes.
`pkg/cmd/pipeline/sign.go`	Passes Pipeline source bytes.
`go.mod`	Declares YAML v3 as a direct dependency.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

divyansh42 · 2026-09-23T18:58:53Z

/retest

Copilot

Copilot review overview

🟡 Changes recommended

Critical issues with YAML aliases and unsigned unknown fields must be resolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2

Open (2)

Reject anchored mappings with unsupported alias semantics · New
Prevent silently discarded fields from remaining unsigned · New

Resolved since last review (1)

Replace existing signature instead of adding duplicate annotation

tkn task sign and tkn pipeline sign marshalled the parsed resource back to YAML, which rewrote the whole document: it reordered keys, reflowed sequences, and emitted zero valued fields of the embedded Kubernetes types, including the resources: {} that Tekton rejects on apply. Sign now takes the document the resource was read from and inserts the signature annotation into it, so the signed file differs from the input by that annotation alone. The document is parsed only to locate the insertion point: re-serializing the parsed tree collapses the line breaks of a folded scalar such as description: >-. Fixes tektoncd#2895 Fixes tektoncd#2894 Signed-off-by: somaz <genius5711@gmail.com> Assisted-by: Claude Opus 5 (via Claude Code)

Signing a document that already carried a signature inserted a second tekton.dev/signature key, which strict YAML decoders reject. Replace the existing entry instead, and refuse one that spans several lines rather than rewrite part of it. The new signature was also computed with the old annotation still on the object, while Verify removes it before checking, so a re-signed file did not verify. Drop the old signature before signing. Inserted lines now use the document's own line ending, so a CRLF file is not left with mixed endings, and the preservation tests compare whole documents, so a reordered line fails them too. Signed-off-by: somaz <genius5711@gmail.com> Assisted-by: Claude Opus 5.5 (via Claude Code)

Sign writes the input document with the signature added, but the signature covers only the object decoded from it, and that decode drops any field the Tekton type does not define. Such a field reached the signed file unsigned, and verify decodes the same way, so changing it did not invalidate the signature. Decode the document strictly before signing and return an error for it instead. Adding the annotation to an anchored metadata or annotations mapping also changed every alias of that mapping, so a `labels: *common` alias gained a signature entry the signed object never had, and the file failed verification. Return an error rather than edit an anchored node. Signed-off-by: somaz <genius5711@gmail.com> Assisted-by: Claude Opus 5.5 (via Claude Code)

Copilot

🟡 Changes recommended

Additional YAML documents can remain unsigned, and accepted YAML layouts can produce invalid or unverifiable output.

4 open issues

2 resolved since last review

_{Review effort: Balanced}

+	"strings"
+
+	yaml "go.yaml.in/yaml/v3"
+)
+
+// insertAnnotation sets key: value in metadata.annotations of doc, replacing an
+// existing entry for key, and returns the result, leaving every other byte of
+// the document untouched.
+//
+// The document is parsed only to locate the insertion point. Re-serializing the
+// parsed tree would be simpler, but it does not round-trip: a folded scalar
+// (`description: >-`) comes back with its line breaks collapsed, which is
+// exactly the kind of unrelated diff this is meant to avoid.
+//
+// An error is returned when the document is not a mapping with a metadata
+// mapping in it, when either mapping is written in flow style, where there
+// is no line to insert, when an existing entry for key spans several lines,
+// or when a node it would edit carries a YAML anchor, since the edit would
+// then change every alias of that node too.
+func insertAnnotation(doc []byte, key, value string) ([]byte, error) {
+	var root yaml.Node
+	if err := yaml.Unmarshal(doc, &root); err != nil {
+		return nil, fmt.Errorf("error parsing document: %w", err)
+	}


+	if metadata == nil {
+		return nil, fmt.Errorf("document has no metadata")
+	}
+	if metadata.Kind != yaml.MappingNode || metadata.Style == yaml.FlowStyle || len(metadata.Content) == 0 {


+	annotationsKey, annotations := mappingEntry(metadata, "annotations")
+	if annotations != nil && annotations.Anchor != "" {
+		return nil, fmt.Errorf("annotations has a YAML anchor")
+	}


+		indent := indentOf(annotationsKey)
+		return replaceLine(doc, annotationsKey.Line, indent+"annotations:\n"+indent+"  "+key+": "+encoded)


tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Sep 14, 2026

tekton-robot requested review from divyansh42 and pradeepitm12 September 14, 2026 08:17

tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 14, 2026

somaz94 added a commit to somaz94/somaz94 that referenced this pull request Sep 14, 2026

docs: add tektoncd/cli#3245 to OSS contributions

4580e3b

tekton-robot added the kind/bug Categorizes issue or PR as related to a bug. label Sep 16, 2026

divyansh42 requested a balanced review from Copilot September 22, 2026 18:20

Copilot started reviewing on behalf of divyansh42 September 22, 2026 18:21

View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Comment thread pkg/trustedresources/annotation.go

divyansh42 requested a balanced review from Copilot September 23, 2026 18:58

Copilot started reviewing on behalf of divyansh42 September 23, 2026 18:59

View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Comment thread pkg/trustedresources/annotation.go

Comment thread pkg/trustedresources/sign.go

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 13, 2026

somaz94 force-pushed the fix/task-sign-preserve-yaml branch from 12e7210 to a2cb92f Compare October 14, 2026 01:55

tekton-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 14, 2026

somaz94 added 3 commits October 20, 2026 10:39

somaz94 force-pushed the fix/task-sign-preserve-yaml branch from a2cb92f to 3bed96c Compare October 20, 2026 01:56

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2026

divyansh42 requested a balanced review from Copilot October 20, 2026 10:27

Copilot started reviewing on behalf of divyansh42 October 20, 2026 10:27

View session

Copilot AI reviewed Oct 20, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(trustedresources): preserve the input document - #3245

fix(trustedresources): preserve the input document#3245
somaz94 wants to merge 3 commits into
tektoncd:mainfrom
somaz94:fix/task-sign-preserve-yaml

somaz94 commented Sep 14, 2026

Uh oh!

tekton-robot commented Sep 14, 2026

Uh oh!

somaz94 commented Sep 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

divyansh42 commented Sep 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		indent := indentOf(annotationsKey)
		return replaceLine(doc, annotationsKey.Line, indent+"annotations:\n"+indent+" "+key+": "+encoded)

Conversation

somaz94 commented Sep 14, 2026

Changes

Submitter Checklist

Release Notes

Uh oh!

tekton-robot commented Sep 14, 2026

Uh oh!

somaz94 commented Sep 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

divyansh42 commented Sep 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants