From 24f9d89f70d7ea0e052ab889f7d506daa9e40bc6 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Tue, 23 Jun 2026 11:58:12 +0200 Subject: [PATCH] fix(cli): preserve verbatim contract bytes when splitting apply documents The batch `apply` path re-serialized each YAML document through a yaml.v3 Node, which re-indented and reflowed the content. This produced different bytes than `wf contract apply` (which sends the file verbatim), so the server's raw-byte change detection reported spurious contract revisions and false drift on `apply --dry-run` for a contract that had not changed. SplitYAMLDocuments now splits the stream on document markers and keeps each document's original bytes verbatim, so every apply path sends exactly what the user authored. Re-applying the same file is idempotent regardless of path, while genuine edits (including comment or formatting changes) still produce a new revision. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 606de1a5-e4ab-480b-8009-722416f5e18d --- app/cli/pkg/action/apply.go | 53 ++++++--- app/cli/pkg/action/apply_test.go | 110 +++++++++++++++++++ app/controlplane/pkg/biz/workflowcontract.go | 4 +- 3 files changed, 150 insertions(+), 17 deletions(-) create mode 100644 app/cli/pkg/action/apply_test.go diff --git a/app/cli/pkg/action/apply.go b/app/cli/pkg/action/apply.go index dd5977e53..675d5333d 100644 --- a/app/cli/pkg/action/apply.go +++ b/app/cli/pkg/action/apply.go @@ -18,9 +18,7 @@ package action import ( "bytes" "context" - "errors" "fmt" - "io" "os" "path/filepath" "strings" @@ -163,22 +161,16 @@ func CollectYAMLFiles(path string) ([]string, error) { // SplitYAMLDocuments splits a potentially multi-document YAML file into individual documents, // extracting kind and name from each. func SplitYAMLDocuments(rawData []byte) ([]*YAMLDoc, error) { - decoder := yaml.NewDecoder(bytes.NewReader(rawData)) - var docs []*YAMLDoc - for { + for _, raw := range splitYAMLOnDocumentMarkers(rawData) { + // Detect empty or comment-only documents so leading/trailing separators + // and stray comments do not produce spurious resources. var node yaml.Node - if err := decoder.Decode(&node); err != nil { - if errors.Is(err, io.EOF) { - break - } + if err := yaml.Unmarshal(raw, &node); err != nil { return nil, fmt.Errorf("decoding YAML document: %w", err) } - - // Marshal node back to bytes for the per-resource apply - docBytes, err := yaml.Marshal(&node) - if err != nil { - return nil, fmt.Errorf("marshalling YAML node: %w", err) + if node.Kind == 0 { + continue } // Extract kind and name via partial unmarshal @@ -188,7 +180,7 @@ func SplitYAMLDocuments(rawData []byte) ([]*YAMLDoc, error) { Name string `yaml:"name"` } `yaml:"metadata"` } - if err := yaml.Unmarshal(docBytes, &header); err != nil { + if err := yaml.Unmarshal(raw, &header); err != nil { return nil, fmt.Errorf("extracting document kind: %w", err) } @@ -200,12 +192,41 @@ func SplitYAMLDocuments(rawData []byte) ([]*YAMLDoc, error) { return nil, fmt.Errorf("missing 'metadata.name' field in YAML document of kind %q", header.Kind) } + // Keep the document's original bytes verbatim (no re-indenting, comment + // loss, or reflow) so the batch apply path sends exactly what the user + // authored — identical to the bytes `wf contract apply` sends. Otherwise + // the server sees different bytes for the same file and reports spurious + // contract revisions. docs = append(docs, &YAMLDoc{ Kind: header.Kind, Name: header.Metadata.Name, - RawData: docBytes, + RawData: raw, }) } return docs, nil } + +// splitYAMLOnDocumentMarkers splits a multi-document YAML stream into its +// individual documents, returning each document's original bytes verbatim. +// Documents are separated by a "---" marker on its own line (per the YAML +// spec, a directives-end marker at column 0); the marker line itself is not +// part of any document. +func splitYAMLOnDocumentMarkers(rawData []byte) [][]byte { + var docs [][]byte + var cur []byte + for _, line := range bytes.SplitAfter(rawData, []byte("\n")) { + if len(line) == 0 { + continue + } + if bytes.Equal(bytes.TrimRight(line, " \t\r\n"), []byte("---")) { + docs = append(docs, cur) + cur = nil + continue + } + cur = append(cur, line...) + } + docs = append(docs, cur) + + return docs +} diff --git a/app/cli/pkg/action/apply_test.go b/app/cli/pkg/action/apply_test.go new file mode 100644 index 000000000..bc7a145f4 --- /dev/null +++ b/app/cli/pkg/action/apply_test.go @@ -0,0 +1,110 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package action + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSplitYAMLDocuments(t *testing.T) { + t.Run("single document is preserved verbatim", func(t *testing.T) { + // Hand-written contract with 2-space indentation and comments. The batch + // apply path must send these bytes unchanged so the server sees exactly + // what `wf contract apply` (verbatim os.ReadFile) sends. + in := []byte(`apiVersion: chainloop.dev/v1 +kind: Contract +metadata: + name: my-contract +spec: + materials: + - name: sbom # keep this comment + type: SBOM_CYCLONEDX_JSON +`) + docs, err := SplitYAMLDocuments(in) + require.NoError(t, err) + require.Len(t, docs, 1) + assert.Equal(t, "Contract", docs[0].Kind) + assert.Equal(t, "my-contract", docs[0].Name) + // The document bytes must be byte-identical to the input: no re-indenting, + // no comment loss, no reflow. + assert.Equal(t, string(in), string(docs[0].RawData)) + }) + + t.Run("multiple documents keep their own verbatim bytes", func(t *testing.T) { + doc1 := `apiVersion: chainloop.dev/v1 +kind: Contract +metadata: + name: first +spec: + materials: + - name: sbom + type: SBOM_CYCLONEDX_JSON +` + doc2 := `apiVersion: chainloop.dev/v1 +kind: Contract +metadata: + name: second # second contract +spec: {} +` + in := []byte(doc1 + "---\n" + doc2) + + docs, err := SplitYAMLDocuments(in) + require.NoError(t, err) + require.Len(t, docs, 2) + assert.Equal(t, "first", docs[0].Name) + assert.Equal(t, "second", docs[1].Name) + assert.Equal(t, doc1, string(docs[0].RawData)) + assert.Equal(t, doc2, string(docs[1].RawData)) + }) + + t.Run("empty and comment-only documents are skipped", func(t *testing.T) { + in := []byte(`# just a comment, no document here +--- +apiVersion: chainloop.dev/v1 +kind: Contract +metadata: + name: only-one +spec: {} +`) + docs, err := SplitYAMLDocuments(in) + require.NoError(t, err) + require.Len(t, docs, 1) + assert.Equal(t, "only-one", docs[0].Name) + }) + + t.Run("document without kind returns an error", func(t *testing.T) { + in := []byte(`apiVersion: chainloop.dev/v1 +metadata: + name: no-kind +`) + _, err := SplitYAMLDocuments(in) + require.Error(t, err) + assert.Contains(t, err.Error(), "kind") + }) + + t.Run("document without metadata.name returns an error", func(t *testing.T) { + in := []byte(`apiVersion: chainloop.dev/v1 +kind: Contract +spec: {} +`) + _, err := SplitYAMLDocuments(in) + require.Error(t, err) + assert.Contains(t, err.Error(), "name") + }) +} diff --git a/app/controlplane/pkg/biz/workflowcontract.go b/app/controlplane/pkg/biz/workflowcontract.go index 5f0fa625d..61e5992b1 100644 --- a/app/controlplane/pkg/biz/workflowcontract.go +++ b/app/controlplane/pkg/biz/workflowcontract.go @@ -497,7 +497,9 @@ func (uc *WorkflowContractUseCase) RevisionWouldChange(ctx context.Context, orgI } // Same rule used by the data layer on Update: a new revision is created - // only when the stored raw body differs from the incoming one. + // only when the stored raw body differs from the incoming one. The CLI sends + // verbatim contract bytes on every apply path, so dry-run apply and real + // apply see identical bytes for the same file. return !bytes.Equal(latest.Version.Schema.Raw, incoming.Raw), nil }