Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 46 additions & 2 deletions app/controlplane/pkg/biz/workflowcontract.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/go-kratos/kratos/v2/log"
"github.com/google/uuid"
"google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/proto"
)

var workflowContractTracer = otelx.Tracer("chainloop-controlplane", "biz/workflowcontract")
Expand Down Expand Up @@ -497,8 +498,10 @@ 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.
return !bytes.Equal(latest.Version.Schema.Raw, incoming.Raw), nil
// only when the stored contract differs from the incoming one. The
// comparison is semantic (formatting-insensitive) so dry-run apply and real
// apply agree regardless of which client serialized the bytes.
return !ContractRawEqual(latest.Version.Schema.Raw, incoming.Raw), nil
}

// ValidateContractPolicies checks that policies and policy groups referenced by the contract
Expand Down Expand Up @@ -877,6 +880,47 @@ func UnmarshalAndValidateRawContract(raw []byte, format unmarshal.RawFormat) (*C
return nil, NewErrValidation(fmt.Errorf("contract validation failed:\n v2 Contract format error: %w\n v1 CraftingSchema format error: %w", v2Err, v1Err))
}

// ContractRawEqual reports whether two raw contract bodies represent the same
// contract once parsed, ignoring serialization/formatting differences such as
// indentation, key ordering, or YAML vs JSON. This is the canonical
// change-detection comparison: re-applying the same contract through any client
// path (verbatim `wf contract apply` or the batch `apply` splitter, which
// re-marshals YAML) must compare equal, while a genuine content edit must not.
//
// Identical bytes are trivially equal. If either body cannot be parsed and
// validated, it falls back to a raw byte comparison so a malformed body never
// silently compares equal to a different one.
func ContractRawEqual(a, b []byte) bool {
// Fast path: identical bytes are trivially equal.
if bytes.Equal(a, b) {
return true
}

ca, err := identifyUnMarshalAndValidateRawContract(a)
if err != nil {
return false
}

cb, err := identifyUnMarshalAndValidateRawContract(b)
if err != nil {
return false
}

return ca.semanticEqual(cb)
}

// semanticEqual compares two parsed contracts by their canonical proto form,
// so formatting-only differences do not register as changes. When both sides
// use the v2 schema the v2 messages are compared; otherwise the v1
// representation (always populated by UnmarshalAndValidateRawContract) is used.
func (c *Contract) semanticEqual(other *Contract) bool {
if c.isV2Schema() && other.isV2Schema() {
return proto.Equal(c.Schemav2, other.Schemav2)
}

return proto.Equal(c.Schema, other.Schema)
}

// Will try to figure out the format of the raw contract and validate it
func identifyUnMarshalAndValidateRawContract(raw []byte) (*Contract, error) {
format, err := unmarshal.IdentifyFormat(raw)
Expand Down
23 changes: 23 additions & 0 deletions app/controlplane/pkg/biz/workflowcontract_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package biz_test

import (
"bytes"
"context"
"fmt"
"os"
Expand All @@ -28,6 +29,7 @@ import (
"github.com/google/uuid"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
yaml "gopkg.in/yaml.v3"
)

func (s *workflowContractIntegrationTestSuite) TestUpdate() {
Expand Down Expand Up @@ -160,6 +162,27 @@ func (s *workflowContractIntegrationTestSuite) TestRevisionWouldChange() {
s.False(changed)
})

s.Run("semantically identical schema with different serialization would not change the revision", func() {
// Re-serialize the stored contract through a yaml.v3 Node with a different
// indentation, mirroring what the batch apply path (SplitYAMLDocuments)
// does: the bytes differ (re-indented/reflowed) but the contract is
// semantically unchanged. This is the exact dry-run drift bug.
var node yaml.Node
require.NoError(s.T(), yaml.Unmarshal(currentRaw, &node))
var buf bytes.Buffer
enc := yaml.NewEncoder(&buf)
enc.SetIndent(2)
require.NoError(s.T(), enc.Encode(&node))
require.NoError(s.T(), enc.Close())
reSerialized := buf.Bytes()
// Sanity check: the reproduction is only meaningful if the bytes differ.
s.NotEqual(currentRaw, reSerialized)

changed, err := s.WorkflowContract.RevisionWouldChange(ctx, s.org.ID, s.contractOrg1.ID.String(), reSerialized)
require.NoError(s.T(), err)
s.False(changed)
})

s.Run("different schema would change the revision", func() {
changed, err := s.WorkflowContract.RevisionWouldChange(ctx, s.org.ID, s.contractOrg1.ID.String(), updatedRaw.Raw)
require.NoError(s.T(), err)
Expand Down
123 changes: 122 additions & 1 deletion app/controlplane/pkg/biz/workflowcontract_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//
// Copyright 2024 The Chainloop Authors.
// Copyright 2024-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.
Expand Down Expand Up @@ -93,3 +93,124 @@ func TestIdentifyAndValidateRawContract(t *testing.T) {
})
}
}

func TestContractRawEqual(t *testing.T) {
// v2 contract with 2-space indented sequences, as a user would hand-write it
v2TwoSpace := []byte(`apiVersion: chainloop.dev/v1
kind: Contract
metadata:
name: my-contract
spec:
materials:
- name: my-image
type: CONTAINER_IMAGE
optional: false
- name: source-code
type: ARTIFACT
optional: true
envAllowList:
- NODE_ENV
`)

// Same semantic contract, but re-indented the way yaml.v3 reflows sequences
// (4-space indentation under the key). This mimics what the batch apply path
// produces after round-tripping through a yaml.v3 Node.
v2Reindented := []byte(`apiVersion: chainloop.dev/v1
kind: Contract
metadata:
name: my-contract
spec:
materials:
- name: my-image
type: CONTAINER_IMAGE
optional: false
- name: source-code
type: ARTIFACT
optional: true
envAllowList:
- NODE_ENV
`)

// Same semantic contract expressed as JSON.
v2JSON := []byte(`{
"apiVersion": "chainloop.dev/v1",
"kind": "Contract",
"metadata": {"name": "my-contract"},
"spec": {
"materials": [
{"name": "my-image", "type": "CONTAINER_IMAGE", "optional": false},
{"name": "source-code", "type": "ARTIFACT", "optional": true}
],
"envAllowList": ["NODE_ENV"]
}
}`)

// A genuine content change: a material type differs.
v2Changed := []byte(`apiVersion: chainloop.dev/v1
kind: Contract
metadata:
name: my-contract
spec:
materials:
- name: my-image
type: SBOM_CYCLONEDX_JSON
optional: false
- name: source-code
type: ARTIFACT
optional: true
envAllowList:
- NODE_ENV
`)

testCases := []struct {
name string
a []byte
b []byte
wantEqual bool
}{
{
name: "identical bytes",
a: v2TwoSpace,
b: v2TwoSpace,
wantEqual: true,
},
{
name: "same contract, different YAML indentation",
a: v2TwoSpace,
b: v2Reindented,
wantEqual: true,
},
{
name: "same contract, YAML vs JSON",
a: v2TwoSpace,
b: v2JSON,
wantEqual: true,
},
{
name: "genuine content change is detected",
a: v2TwoSpace,
b: v2Changed,
wantEqual: false,
},
{
name: "unparseable input falls back to raw byte comparison (equal)",
a: []byte("not a contract"),
b: []byte("not a contract"),
wantEqual: true,
},
{
name: "unparseable input falls back to raw byte comparison (different)",
a: []byte("not a contract"),
b: []byte("also not a contract"),
wantEqual: false,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.wantEqual, ContractRawEqual(tc.a, tc.b))
// equality must be symmetric
assert.Equal(t, tc.wantEqual, ContractRawEqual(tc.b, tc.a))
})
}
}
8 changes: 5 additions & 3 deletions app/controlplane/pkg/data/workflowcontract.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package data

import (
"bytes"
"context"
"fmt"
"time"
Expand Down Expand Up @@ -263,8 +262,11 @@ func (r *WorkflowContractRepo) Update(ctx context.Context, orgID uuid.UUID, name
return handleError(err)
}

// Create a revision only if we are providing a new contract and it has changed
if opts.Contract != nil && !bytes.Equal(lv.RawBody, opts.Contract.Raw) {
// Create a revision only if we are providing a new contract and it has changed.
// The comparison is semantic (formatting-insensitive) so re-applying the same
// contract serialized differently (e.g. the batch apply path re-marshals YAML)
// does not create a spurious revision.
if opts.Contract != nil && !biz.ContractRawEqual(lv.RawBody, opts.Contract.Raw) {
// TODO: Add pessimist locking to make sure we are incrementing the latest revision
lv, err = tx.WorkflowContractVersion.Create().
SetRawBody(opts.Contract.Raw).
Expand Down
Loading