From 79bf7369c5a7d0287bb189aa08d12624a7c3c860 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Sat, 21 Mar 2026 16:09:53 +0100 Subject: [PATCH 1/5] fix: return digest from attestation state save for OCC support The remote state manager was not receiving the digest of newly saved attestation state, causing subsequent writes during the same initialization to fail with OCC conflict errors. Fixes #2908 Signed-off-by: Miguel Martinez Trivino --- .../api/controlplane/v1/.mockery.yml | 17 + .../controlplane/v1/attestation_state.pb.go | 18 +- .../controlplane/v1/attestation_state.proto | 7 +- .../v1/attestation_state_grpc.pb.go | 2 +- .../v1/mocks/AttestationStateServiceClient.go | 372 ++++++++++++++++++ .../controlplane/v1/attestation_state.ts | 26 +- ...onStateServiceSaveResponse.jsonschema.json | 7 +- ...tationStateServiceSaveResponse.schema.json | 7 +- .../internal/service/attestationstate.go | 6 +- app/controlplane/pkg/biz/attestationstate.go | 21 +- .../biz/attestationstate_integration_test.go | 39 +- app/controlplane/pkg/data/attestationstate.go | 20 +- pkg/attestation/crafter/.mockery.yml | 18 + pkg/attestation/crafter/mocks/Collector.go | 153 +++++++ pkg/attestation/crafter/mocks/StateManager.go | 345 ++++++++++++++++ pkg/attestation/crafter/runcollectors_test.go | 131 ++++++ .../crafter/statemanager/remote/remote.go | 8 +- .../statemanager/remote/remote_test.go | 129 ++++++ 18 files changed, 1273 insertions(+), 53 deletions(-) create mode 100644 app/controlplane/api/controlplane/v1/.mockery.yml create mode 100644 app/controlplane/api/controlplane/v1/mocks/AttestationStateServiceClient.go create mode 100644 pkg/attestation/crafter/.mockery.yml create mode 100644 pkg/attestation/crafter/mocks/Collector.go create mode 100644 pkg/attestation/crafter/mocks/StateManager.go create mode 100644 pkg/attestation/crafter/runcollectors_test.go create mode 100644 pkg/attestation/crafter/statemanager/remote/remote_test.go diff --git a/app/controlplane/api/controlplane/v1/.mockery.yml b/app/controlplane/api/controlplane/v1/.mockery.yml new file mode 100644 index 000000000..e68e655df --- /dev/null +++ b/app/controlplane/api/controlplane/v1/.mockery.yml @@ -0,0 +1,17 @@ +all: false +formatter: goimports +include-auto-generated: false +log-level: info +recursive: false +require-template-schema-exists: true +template: testify +template-schema: "{{.Template}}.schema.json" +packages: + github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1: + config: + dir: "{{.InterfaceDir}}/mocks" + filename: "{{.InterfaceName}}.go" + pkgname: mocks + structname: "{{.InterfaceName}}" + interfaces: + AttestationStateServiceClient: diff --git a/app/controlplane/api/controlplane/v1/attestation_state.pb.go b/app/controlplane/api/controlplane/v1/attestation_state.pb.go index 0fc726ade..f908d82ca 100644 --- a/app/controlplane/api/controlplane/v1/attestation_state.pb.go +++ b/app/controlplane/api/controlplane/v1/attestation_state.pb.go @@ -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. @@ -238,7 +238,9 @@ func (x *AttestationStateServiceSaveRequest) GetBaseDigest() string { } type AttestationStateServiceSaveResponse struct { - state protoimpl.MessageState `protogen:"open.v1"` + state protoimpl.MessageState `protogen:"open.v1"` + // digest of the newly saved attestation state for Optimistic Concurrency Control + Digest string `protobuf:"bytes,1,opt,name=digest,proto3" json:"digest,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -273,6 +275,13 @@ func (*AttestationStateServiceSaveResponse) Descriptor() ([]byte, []int) { return file_controlplane_v1_attestation_state_proto_rawDescGZIP(), []int{3} } +func (x *AttestationStateServiceSaveResponse) GetDigest() string { + if x != nil { + return x.Digest + } + return "" +} + type AttestationStateServiceReadRequest struct { state protoimpl.MessageState `protogen:"open.v1"` WorkflowRunId string `protobuf:"bytes,1,opt,name=workflow_run_id,json=workflowRunId,proto3" json:"workflow_run_id,omitempty"` @@ -553,8 +562,9 @@ const file_controlplane_v1_attestation_state_proto_rawDesc = "" + "\x0fworkflow_run_id\x18\x01 \x01(\tB\a\xbaH\x04r\x02\x10\x01R\rworkflowRunId\x12R\n" + "\x11attestation_state\x18\x02 \x01(\v2\x1d.attestation.v1.CraftingStateB\x06\xbaH\x03\xd8\x01\x03R\x10attestationState\x12\x1f\n" + "\vbase_digest\x18\x03 \x01(\tR\n" + - "baseDigest\"%\n" + - "#AttestationStateServiceSaveResponse\"U\n" + + "baseDigest\"=\n" + + "#AttestationStateServiceSaveResponse\x12\x16\n" + + "\x06digest\x18\x01 \x01(\tR\x06digest\"U\n" + "\"AttestationStateServiceReadRequest\x12/\n" + "\x0fworkflow_run_id\x18\x01 \x01(\tB\a\xbaH\x04r\x02\x10\x01R\rworkflowRunId\"\xe8\x01\n" + "#AttestationStateServiceReadResponse\x12S\n" + diff --git a/app/controlplane/api/controlplane/v1/attestation_state.proto b/app/controlplane/api/controlplane/v1/attestation_state.proto index d6df7d428..fd90d0d8f 100644 --- a/app/controlplane/api/controlplane/v1/attestation_state.proto +++ b/app/controlplane/api/controlplane/v1/attestation_state.proto @@ -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. @@ -53,7 +53,10 @@ message AttestationStateServiceSaveRequest { string base_digest = 3; } -message AttestationStateServiceSaveResponse {} +message AttestationStateServiceSaveResponse { + // digest of the newly saved attestation state for Optimistic Concurrency Control + string digest = 1; +} message AttestationStateServiceReadRequest { string workflow_run_id = 1 [(buf.validate.field).string = {min_len: 1}]; diff --git a/app/controlplane/api/controlplane/v1/attestation_state_grpc.pb.go b/app/controlplane/api/controlplane/v1/attestation_state_grpc.pb.go index 95d041511..dbc05cb70 100644 --- a/app/controlplane/api/controlplane/v1/attestation_state_grpc.pb.go +++ b/app/controlplane/api/controlplane/v1/attestation_state_grpc.pb.go @@ -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. diff --git a/app/controlplane/api/controlplane/v1/mocks/AttestationStateServiceClient.go b/app/controlplane/api/controlplane/v1/mocks/AttestationStateServiceClient.go new file mode 100644 index 000000000..f95a530a4 --- /dev/null +++ b/app/controlplane/api/controlplane/v1/mocks/AttestationStateServiceClient.go @@ -0,0 +1,372 @@ +// Code generated by mockery; DO NOT EDIT. +// github.com/vektra/mockery +// template: testify + +package mocks + +import ( + "context" + + "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" + mock "github.com/stretchr/testify/mock" + "google.golang.org/grpc" +) + +// NewAttestationStateServiceClient creates a new instance of AttestationStateServiceClient. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewAttestationStateServiceClient(t interface { + mock.TestingT + Cleanup(func()) +}) *AttestationStateServiceClient { + mock := &AttestationStateServiceClient{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} + +// AttestationStateServiceClient is an autogenerated mock type for the AttestationStateServiceClient type +type AttestationStateServiceClient struct { + mock.Mock +} + +type AttestationStateServiceClient_Expecter struct { + mock *mock.Mock +} + +func (_m *AttestationStateServiceClient) EXPECT() *AttestationStateServiceClient_Expecter { + return &AttestationStateServiceClient_Expecter{mock: &_m.Mock} +} + +// Initialized provides a mock function for the type AttestationStateServiceClient +func (_mock *AttestationStateServiceClient) Initialized(ctx context.Context, in *v1.AttestationStateServiceInitializedRequest, opts ...grpc.CallOption) (*v1.AttestationStateServiceInitializedResponse, error) { + var tmpRet mock.Arguments + if len(opts) > 0 { + tmpRet = _mock.Called(ctx, in, opts) + } else { + tmpRet = _mock.Called(ctx, in) + } + ret := tmpRet + + if len(ret) == 0 { + panic("no return value specified for Initialized") + } + + var r0 *v1.AttestationStateServiceInitializedResponse + var r1 error + if returnFunc, ok := ret.Get(0).(func(context.Context, *v1.AttestationStateServiceInitializedRequest, ...grpc.CallOption) (*v1.AttestationStateServiceInitializedResponse, error)); ok { + return returnFunc(ctx, in, opts...) + } + if returnFunc, ok := ret.Get(0).(func(context.Context, *v1.AttestationStateServiceInitializedRequest, ...grpc.CallOption) *v1.AttestationStateServiceInitializedResponse); ok { + r0 = returnFunc(ctx, in, opts...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*v1.AttestationStateServiceInitializedResponse) + } + } + if returnFunc, ok := ret.Get(1).(func(context.Context, *v1.AttestationStateServiceInitializedRequest, ...grpc.CallOption) error); ok { + r1 = returnFunc(ctx, in, opts...) + } else { + r1 = ret.Error(1) + } + return r0, r1 +} + +// AttestationStateServiceClient_Initialized_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Initialized' +type AttestationStateServiceClient_Initialized_Call struct { + *mock.Call +} + +// Initialized is a helper method to define mock.On call +// - ctx context.Context +// - in *v1.AttestationStateServiceInitializedRequest +// - opts ...grpc.CallOption +func (_e *AttestationStateServiceClient_Expecter) Initialized(ctx interface{}, in interface{}, opts ...interface{}) *AttestationStateServiceClient_Initialized_Call { + return &AttestationStateServiceClient_Initialized_Call{Call: _e.mock.On("Initialized", + append([]interface{}{ctx, in}, opts...)...)} +} + +func (_c *AttestationStateServiceClient_Initialized_Call) Run(run func(ctx context.Context, in *v1.AttestationStateServiceInitializedRequest, opts ...grpc.CallOption)) *AttestationStateServiceClient_Initialized_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 *v1.AttestationStateServiceInitializedRequest + if args[1] != nil { + arg1 = args[1].(*v1.AttestationStateServiceInitializedRequest) + } + var arg2 []grpc.CallOption + var variadicArgs []grpc.CallOption + if len(args) > 2 { + variadicArgs = args[2].([]grpc.CallOption) + } + arg2 = variadicArgs + run( + arg0, + arg1, + arg2..., + ) + }) + return _c +} + +func (_c *AttestationStateServiceClient_Initialized_Call) Return(attestationStateServiceInitializedResponse *v1.AttestationStateServiceInitializedResponse, err error) *AttestationStateServiceClient_Initialized_Call { + _c.Call.Return(attestationStateServiceInitializedResponse, err) + return _c +} + +func (_c *AttestationStateServiceClient_Initialized_Call) RunAndReturn(run func(ctx context.Context, in *v1.AttestationStateServiceInitializedRequest, opts ...grpc.CallOption) (*v1.AttestationStateServiceInitializedResponse, error)) *AttestationStateServiceClient_Initialized_Call { + _c.Call.Return(run) + return _c +} + +// Read provides a mock function for the type AttestationStateServiceClient +func (_mock *AttestationStateServiceClient) Read(ctx context.Context, in *v1.AttestationStateServiceReadRequest, opts ...grpc.CallOption) (*v1.AttestationStateServiceReadResponse, error) { + var tmpRet mock.Arguments + if len(opts) > 0 { + tmpRet = _mock.Called(ctx, in, opts) + } else { + tmpRet = _mock.Called(ctx, in) + } + ret := tmpRet + + if len(ret) == 0 { + panic("no return value specified for Read") + } + + var r0 *v1.AttestationStateServiceReadResponse + var r1 error + if returnFunc, ok := ret.Get(0).(func(context.Context, *v1.AttestationStateServiceReadRequest, ...grpc.CallOption) (*v1.AttestationStateServiceReadResponse, error)); ok { + return returnFunc(ctx, in, opts...) + } + if returnFunc, ok := ret.Get(0).(func(context.Context, *v1.AttestationStateServiceReadRequest, ...grpc.CallOption) *v1.AttestationStateServiceReadResponse); ok { + r0 = returnFunc(ctx, in, opts...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*v1.AttestationStateServiceReadResponse) + } + } + if returnFunc, ok := ret.Get(1).(func(context.Context, *v1.AttestationStateServiceReadRequest, ...grpc.CallOption) error); ok { + r1 = returnFunc(ctx, in, opts...) + } else { + r1 = ret.Error(1) + } + return r0, r1 +} + +// AttestationStateServiceClient_Read_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Read' +type AttestationStateServiceClient_Read_Call struct { + *mock.Call +} + +// Read is a helper method to define mock.On call +// - ctx context.Context +// - in *v1.AttestationStateServiceReadRequest +// - opts ...grpc.CallOption +func (_e *AttestationStateServiceClient_Expecter) Read(ctx interface{}, in interface{}, opts ...interface{}) *AttestationStateServiceClient_Read_Call { + return &AttestationStateServiceClient_Read_Call{Call: _e.mock.On("Read", + append([]interface{}{ctx, in}, opts...)...)} +} + +func (_c *AttestationStateServiceClient_Read_Call) Run(run func(ctx context.Context, in *v1.AttestationStateServiceReadRequest, opts ...grpc.CallOption)) *AttestationStateServiceClient_Read_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 *v1.AttestationStateServiceReadRequest + if args[1] != nil { + arg1 = args[1].(*v1.AttestationStateServiceReadRequest) + } + var arg2 []grpc.CallOption + var variadicArgs []grpc.CallOption + if len(args) > 2 { + variadicArgs = args[2].([]grpc.CallOption) + } + arg2 = variadicArgs + run( + arg0, + arg1, + arg2..., + ) + }) + return _c +} + +func (_c *AttestationStateServiceClient_Read_Call) Return(attestationStateServiceReadResponse *v1.AttestationStateServiceReadResponse, err error) *AttestationStateServiceClient_Read_Call { + _c.Call.Return(attestationStateServiceReadResponse, err) + return _c +} + +func (_c *AttestationStateServiceClient_Read_Call) RunAndReturn(run func(ctx context.Context, in *v1.AttestationStateServiceReadRequest, opts ...grpc.CallOption) (*v1.AttestationStateServiceReadResponse, error)) *AttestationStateServiceClient_Read_Call { + _c.Call.Return(run) + return _c +} + +// Reset provides a mock function for the type AttestationStateServiceClient +func (_mock *AttestationStateServiceClient) Reset(ctx context.Context, in *v1.AttestationStateServiceResetRequest, opts ...grpc.CallOption) (*v1.AttestationStateServiceResetResponse, error) { + var tmpRet mock.Arguments + if len(opts) > 0 { + tmpRet = _mock.Called(ctx, in, opts) + } else { + tmpRet = _mock.Called(ctx, in) + } + ret := tmpRet + + if len(ret) == 0 { + panic("no return value specified for Reset") + } + + var r0 *v1.AttestationStateServiceResetResponse + var r1 error + if returnFunc, ok := ret.Get(0).(func(context.Context, *v1.AttestationStateServiceResetRequest, ...grpc.CallOption) (*v1.AttestationStateServiceResetResponse, error)); ok { + return returnFunc(ctx, in, opts...) + } + if returnFunc, ok := ret.Get(0).(func(context.Context, *v1.AttestationStateServiceResetRequest, ...grpc.CallOption) *v1.AttestationStateServiceResetResponse); ok { + r0 = returnFunc(ctx, in, opts...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*v1.AttestationStateServiceResetResponse) + } + } + if returnFunc, ok := ret.Get(1).(func(context.Context, *v1.AttestationStateServiceResetRequest, ...grpc.CallOption) error); ok { + r1 = returnFunc(ctx, in, opts...) + } else { + r1 = ret.Error(1) + } + return r0, r1 +} + +// AttestationStateServiceClient_Reset_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Reset' +type AttestationStateServiceClient_Reset_Call struct { + *mock.Call +} + +// Reset is a helper method to define mock.On call +// - ctx context.Context +// - in *v1.AttestationStateServiceResetRequest +// - opts ...grpc.CallOption +func (_e *AttestationStateServiceClient_Expecter) Reset(ctx interface{}, in interface{}, opts ...interface{}) *AttestationStateServiceClient_Reset_Call { + return &AttestationStateServiceClient_Reset_Call{Call: _e.mock.On("Reset", + append([]interface{}{ctx, in}, opts...)...)} +} + +func (_c *AttestationStateServiceClient_Reset_Call) Run(run func(ctx context.Context, in *v1.AttestationStateServiceResetRequest, opts ...grpc.CallOption)) *AttestationStateServiceClient_Reset_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 *v1.AttestationStateServiceResetRequest + if args[1] != nil { + arg1 = args[1].(*v1.AttestationStateServiceResetRequest) + } + var arg2 []grpc.CallOption + var variadicArgs []grpc.CallOption + if len(args) > 2 { + variadicArgs = args[2].([]grpc.CallOption) + } + arg2 = variadicArgs + run( + arg0, + arg1, + arg2..., + ) + }) + return _c +} + +func (_c *AttestationStateServiceClient_Reset_Call) Return(attestationStateServiceResetResponse *v1.AttestationStateServiceResetResponse, err error) *AttestationStateServiceClient_Reset_Call { + _c.Call.Return(attestationStateServiceResetResponse, err) + return _c +} + +func (_c *AttestationStateServiceClient_Reset_Call) RunAndReturn(run func(ctx context.Context, in *v1.AttestationStateServiceResetRequest, opts ...grpc.CallOption) (*v1.AttestationStateServiceResetResponse, error)) *AttestationStateServiceClient_Reset_Call { + _c.Call.Return(run) + return _c +} + +// Save provides a mock function for the type AttestationStateServiceClient +func (_mock *AttestationStateServiceClient) Save(ctx context.Context, in *v1.AttestationStateServiceSaveRequest, opts ...grpc.CallOption) (*v1.AttestationStateServiceSaveResponse, error) { + var tmpRet mock.Arguments + if len(opts) > 0 { + tmpRet = _mock.Called(ctx, in, opts) + } else { + tmpRet = _mock.Called(ctx, in) + } + ret := tmpRet + + if len(ret) == 0 { + panic("no return value specified for Save") + } + + var r0 *v1.AttestationStateServiceSaveResponse + var r1 error + if returnFunc, ok := ret.Get(0).(func(context.Context, *v1.AttestationStateServiceSaveRequest, ...grpc.CallOption) (*v1.AttestationStateServiceSaveResponse, error)); ok { + return returnFunc(ctx, in, opts...) + } + if returnFunc, ok := ret.Get(0).(func(context.Context, *v1.AttestationStateServiceSaveRequest, ...grpc.CallOption) *v1.AttestationStateServiceSaveResponse); ok { + r0 = returnFunc(ctx, in, opts...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*v1.AttestationStateServiceSaveResponse) + } + } + if returnFunc, ok := ret.Get(1).(func(context.Context, *v1.AttestationStateServiceSaveRequest, ...grpc.CallOption) error); ok { + r1 = returnFunc(ctx, in, opts...) + } else { + r1 = ret.Error(1) + } + return r0, r1 +} + +// AttestationStateServiceClient_Save_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Save' +type AttestationStateServiceClient_Save_Call struct { + *mock.Call +} + +// Save is a helper method to define mock.On call +// - ctx context.Context +// - in *v1.AttestationStateServiceSaveRequest +// - opts ...grpc.CallOption +func (_e *AttestationStateServiceClient_Expecter) Save(ctx interface{}, in interface{}, opts ...interface{}) *AttestationStateServiceClient_Save_Call { + return &AttestationStateServiceClient_Save_Call{Call: _e.mock.On("Save", + append([]interface{}{ctx, in}, opts...)...)} +} + +func (_c *AttestationStateServiceClient_Save_Call) Run(run func(ctx context.Context, in *v1.AttestationStateServiceSaveRequest, opts ...grpc.CallOption)) *AttestationStateServiceClient_Save_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 *v1.AttestationStateServiceSaveRequest + if args[1] != nil { + arg1 = args[1].(*v1.AttestationStateServiceSaveRequest) + } + var arg2 []grpc.CallOption + var variadicArgs []grpc.CallOption + if len(args) > 2 { + variadicArgs = args[2].([]grpc.CallOption) + } + arg2 = variadicArgs + run( + arg0, + arg1, + arg2..., + ) + }) + return _c +} + +func (_c *AttestationStateServiceClient_Save_Call) Return(attestationStateServiceSaveResponse *v1.AttestationStateServiceSaveResponse, err error) *AttestationStateServiceClient_Save_Call { + _c.Call.Return(attestationStateServiceSaveResponse, err) + return _c +} + +func (_c *AttestationStateServiceClient_Save_Call) RunAndReturn(run func(ctx context.Context, in *v1.AttestationStateServiceSaveRequest, opts ...grpc.CallOption) (*v1.AttestationStateServiceSaveResponse, error)) *AttestationStateServiceClient_Save_Call { + _c.Call.Return(run) + return _c +} diff --git a/app/controlplane/api/gen/frontend/controlplane/v1/attestation_state.ts b/app/controlplane/api/gen/frontend/controlplane/v1/attestation_state.ts index 051af9bb9..df5e982de 100644 --- a/app/controlplane/api/gen/frontend/controlplane/v1/attestation_state.ts +++ b/app/controlplane/api/gen/frontend/controlplane/v1/attestation_state.ts @@ -64,6 +64,8 @@ export interface AttestationStateServiceSaveRequest { } export interface AttestationStateServiceSaveResponse { + /** digest of the newly saved attestation state for Optimistic Concurrency Control */ + digest: string; } export interface AttestationStateServiceReadRequest { @@ -370,11 +372,14 @@ export const AttestationStateServiceSaveRequest = { }; function createBaseAttestationStateServiceSaveResponse(): AttestationStateServiceSaveResponse { - return {}; + return { digest: "" }; } export const AttestationStateServiceSaveResponse = { - encode(_: AttestationStateServiceSaveResponse, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer { + encode(message: AttestationStateServiceSaveResponse, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer { + if (message.digest !== "") { + writer.uint32(10).string(message.digest); + } return writer; }, @@ -385,6 +390,13 @@ export const AttestationStateServiceSaveResponse = { while (reader.pos < end) { const tag = reader.uint32(); switch (tag >>> 3) { + case 1: + if (tag !== 10) { + break; + } + + message.digest = reader.string(); + continue; } if ((tag & 7) === 4 || tag === 0) { break; @@ -394,12 +406,13 @@ export const AttestationStateServiceSaveResponse = { return message; }, - fromJSON(_: any): AttestationStateServiceSaveResponse { - return {}; + fromJSON(object: any): AttestationStateServiceSaveResponse { + return { digest: isSet(object.digest) ? String(object.digest) : "" }; }, - toJSON(_: AttestationStateServiceSaveResponse): unknown { + toJSON(message: AttestationStateServiceSaveResponse): unknown { const obj: any = {}; + message.digest !== undefined && (obj.digest = message.digest); return obj; }, @@ -410,9 +423,10 @@ export const AttestationStateServiceSaveResponse = { }, fromPartial, I>>( - _: I, + object: I, ): AttestationStateServiceSaveResponse { const message = createBaseAttestationStateServiceSaveResponse(); + message.digest = object.digest ?? ""; return message; }, }; diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationStateServiceSaveResponse.jsonschema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationStateServiceSaveResponse.jsonschema.json index 8baaf6f80..ccd716388 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationStateServiceSaveResponse.jsonschema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationStateServiceSaveResponse.jsonschema.json @@ -2,7 +2,12 @@ "$id": "controlplane.v1.AttestationStateServiceSaveResponse.jsonschema.json", "$schema": "https://json-schema.org/draft/2020-12/schema", "additionalProperties": false, - "properties": {}, + "properties": { + "digest": { + "description": "digest of the newly saved attestation state for Optimistic Concurrency Control", + "type": "string" + } + }, "title": "Attestation State Service Save Response", "type": "object" } diff --git a/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationStateServiceSaveResponse.schema.json b/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationStateServiceSaveResponse.schema.json index 5d3c1187e..57f930d72 100644 --- a/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationStateServiceSaveResponse.schema.json +++ b/app/controlplane/api/gen/jsonschema/controlplane.v1.AttestationStateServiceSaveResponse.schema.json @@ -2,7 +2,12 @@ "$id": "controlplane.v1.AttestationStateServiceSaveResponse.schema.json", "$schema": "https://json-schema.org/draft/2020-12/schema", "additionalProperties": false, - "properties": {}, + "properties": { + "digest": { + "description": "digest of the newly saved attestation state for Optimistic Concurrency Control", + "type": "string" + } + }, "title": "Attestation State Service Save Response", "type": "object" } diff --git a/app/controlplane/internal/service/attestationstate.go b/app/controlplane/internal/service/attestationstate.go index 4ff75cb51..3aaa52f0f 100644 --- a/app/controlplane/internal/service/attestationstate.go +++ b/app/controlplane/internal/service/attestationstate.go @@ -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. @@ -105,7 +105,7 @@ func (s *AttestationStateService) Save(ctx context.Context, req *cpAPI.Attestati return nil, errors.Forbidden("forbidden", "failed to authenticate request") } - err = s.attestationStateUseCase.Save(ctx, wf.ID.String(), req.WorkflowRunId, req.AttestationState, encryptionPassphrase, biz.WithAttStateBaseDigest(req.GetBaseDigest())) + digest, err := s.attestationStateUseCase.Save(ctx, wf.ID.String(), req.WorkflowRunId, req.AttestationState, encryptionPassphrase, biz.WithAttStateBaseDigest(req.GetBaseDigest())) if err != nil { if biz.IsErrAttestationStateConflict(err) { return nil, cpAPI.ErrorAttestationStateErrorConflict("saving attestation: %s", err.Error()) @@ -114,7 +114,7 @@ func (s *AttestationStateService) Save(ctx context.Context, req *cpAPI.Attestati return nil, handleUseCaseErr(err, s.log) } - return &cpAPI.AttestationStateServiceSaveResponse{}, nil + return &cpAPI.AttestationStateServiceSaveResponse{Digest: digest}, nil } func (s *AttestationStateService) Read(ctx context.Context, req *cpAPI.AttestationStateServiceReadRequest) (*cpAPI.AttestationStateServiceReadResponse, error) { diff --git a/app/controlplane/pkg/biz/attestationstate.go b/app/controlplane/pkg/biz/attestationstate.go index 074d53915..ee1e80bba 100644 --- a/app/controlplane/pkg/biz/attestationstate.go +++ b/app/controlplane/pkg/biz/attestationstate.go @@ -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. @@ -39,7 +39,7 @@ type AttestationState struct { type AttestationStateRepo interface { Initialized(ctx context.Context, workflowRunID uuid.UUID) (bool, error) - Save(ctx context.Context, workflowRunID uuid.UUID, state []byte, baseDigest string) error + Save(ctx context.Context, workflowRunID uuid.UUID, state []byte, baseDigest string) (string, error) Read(ctx context.Context, workflowRunID uuid.UUID) ([]byte, string, error) Reset(ctx context.Context, workflowRunID uuid.UUID) error } @@ -79,7 +79,7 @@ func WithAttStateBaseDigest(digest string) SaveOption { } } -func (uc *AttestationStateUseCase) Save(ctx context.Context, workflowID, runID string, state *v1.CraftingState, passphrase string, opts ...SaveOption) error { +func (uc *AttestationStateUseCase) Save(ctx context.Context, workflowID, runID string, state *v1.CraftingState, passphrase string, opts ...SaveOption) (string, error) { opt := &AttestationStateSaveOpts{} for _, o := range opts { @@ -88,28 +88,29 @@ func (uc *AttestationStateUseCase) Save(ctx context.Context, workflowID, runID s runUUID, err := uc.checkWorkflowRunInWorkflow(ctx, workflowID, runID) if err != nil { - return fmt.Errorf("failed to check workflow run: %w", err) + return "", fmt.Errorf("failed to check workflow run: %w", err) } rawState, err := proto.Marshal(state) if err != nil { - return fmt.Errorf("failed to marshal attestation state: %w", err) + return "", fmt.Errorf("failed to marshal attestation state: %w", err) } if passphrase == "" { - return NewErrValidationStr("passphrase is required") + return "", NewErrValidationStr("passphrase is required") } encryptedState, err := encrypt(rawState, passphrase) if err != nil { - return fmt.Errorf("failed to encrypt attestation state: %w", err) + return "", fmt.Errorf("failed to encrypt attestation state: %w", err) } - if err := uc.repo.Save(ctx, *runUUID, encryptedState, opt.BaseDigest); err != nil { - return fmt.Errorf("failed to save attestation state: %w", err) + digest, err := uc.repo.Save(ctx, *runUUID, encryptedState, opt.BaseDigest) + if err != nil { + return "", fmt.Errorf("failed to save attestation state: %w", err) } - return nil + return digest, nil } func (uc *AttestationStateUseCase) Read(ctx context.Context, workflowID, runID, passphrase string) (*AttestationState, error) { diff --git a/app/controlplane/pkg/biz/attestationstate_integration_test.go b/app/controlplane/pkg/biz/attestationstate_integration_test.go index e9a6c1e81..f8dd6e069 100644 --- a/app/controlplane/pkg/biz/attestationstate_integration_test.go +++ b/app/controlplane/pkg/biz/attestationstate_integration_test.go @@ -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. @@ -59,7 +59,7 @@ func (s *attestationStateTestSuite) TestInitialized() { }) s.T().Run("the run is initialized", func(t *testing.T) { - err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) + _, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) s.NoError(err) ok, err := s.AttestationState.Initialized(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String()) s.NoError(err) @@ -70,19 +70,19 @@ func (s *attestationStateTestSuite) TestInitialized() { func (s *attestationStateTestSuite) TestSave() { ctx := context.Background() s.Run("run in different workflow causes error", func() { - err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg2.ID.String(), s.testState, s.passphrase) + _, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg2.ID.String(), s.testState, s.passphrase) s.Error(err) s.True(biz.IsNotFound(err)) }) s.Run("the run doesn't exist", func() { - err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), uuid.NewString(), s.testState, s.passphrase) + _, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), uuid.NewString(), s.testState, s.passphrase) s.Error(err) s.True(biz.IsNotFound(err)) }) s.Run("the run exists", func() { - err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) + _, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) s.NoError(err) got, err := s.AttestationState.Read(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.passphrase) @@ -93,7 +93,7 @@ func (s *attestationStateTestSuite) TestSave() { }) s.Run("it can be overridden", func() { - err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) + _, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) s.NoError(err) got, err := s.AttestationState.Read(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.passphrase) @@ -103,7 +103,7 @@ func (s *attestationStateTestSuite) TestSave() { } newState := &v1.CraftingState{} - err = s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), newState, s.passphrase) + _, err = s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), newState, s.passphrase) s.NoError(err) got, err = s.AttestationState.Read(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.passphrase) @@ -120,23 +120,24 @@ func (s *attestationStateTestSuite) TestSave() { // Write again passing the base digest update := &v1.CraftingState{Attestation: &v1.Attestation{Annotations: map[string]string{"key": "updated"}}} - err = s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), update, s.passphrase, biz.WithAttStateBaseDigest(state.Digest)) + returnedDigest, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), update, s.passphrase, biz.WithAttStateBaseDigest(state.Digest)) s.NoError(err) + s.NotEmpty(returnedDigest) - // The digest now should be different + // The returned digest should match what Read returns updatedState, err := s.AttestationState.Read(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.passphrase) s.NoError(err) s.NotEmpty(updatedState.Digest) s.NotEqual(state.Digest, updatedState.Digest) + s.Equal(returnedDigest, updatedState.Digest) // trying to update passing the wrong base digest should fail with a conflict error - err = s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), update, s.passphrase, biz.WithAttStateBaseDigest(state.Digest)) + _, err = s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), update, s.passphrase, biz.WithAttStateBaseDigest(state.Digest)) s.Error(err) s.True(biz.IsErrAttestationStateConflict(err)) // but will not fail if base digest is not passed - // trying to update passing the wrong base digest should fail with a conflict error - err = s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), update, s.passphrase, biz.WithAttStateBaseDigest("")) + _, err = s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), update, s.passphrase, biz.WithAttStateBaseDigest("")) s.NoError(err) }) } @@ -144,7 +145,7 @@ func (s *attestationStateTestSuite) TestSave() { func (s *attestationStateTestSuite) TestRead() { ctx := context.Background() s.T().Run("can be retrieved with same passphrase", func(t *testing.T) { - err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) + _, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) s.NoError(err) got, err := s.AttestationState.Read(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.passphrase) @@ -155,7 +156,7 @@ func (s *attestationStateTestSuite) TestRead() { }) s.T().Run("it fails if they are different passphrases", func(t *testing.T) { - err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) + _, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) s.NoError(err) got, err := s.AttestationState.Read(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), "wrong-passphrase") @@ -164,11 +165,11 @@ func (s *attestationStateTestSuite) TestRead() { }) s.T().Run("it fails if the content has been tampered with", func(t *testing.T) { - err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) + _, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) s.NoError(err) // tamper directly with the database - err = s.Repos.AttestationState.Save(ctx, s.runOrg1.ID, []byte("tampered data modified directly in the DB"), "") + _, err = s.Repos.AttestationState.Save(ctx, s.runOrg1.ID, []byte("tampered data modified directly in the DB"), "") s.NoError(err) got, err := s.AttestationState.Read(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.passphrase) @@ -180,7 +181,7 @@ func (s *attestationStateTestSuite) TestRead() { func (s *attestationStateTestSuite) TestWorkflowRunLifecycle() { ctx := context.Background() s.T().Run("the state gets cleared when workflow run is set as finished", func(t *testing.T) { - err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) + _, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) s.NoError(err) err = s.WorkflowRun.MarkAsFinished(ctx, s.runOrg1.ID.String(), biz.WorkflowRunSuccess, "finished") @@ -192,7 +193,7 @@ func (s *attestationStateTestSuite) TestWorkflowRunLifecycle() { }) s.T().Run("or it expires", func(t *testing.T) { - err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) + _, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) s.NoError(err) err = s.Repos.WorkflowRunRepo.Expire(ctx, s.runOrg1.ID) @@ -208,7 +209,7 @@ func (s *attestationStateTestSuite) TestReset() { ctx := context.Background() s.T().Run("the run is initialized", func(t *testing.T) { - err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) + _, err := s.AttestationState.Save(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String(), s.testState, s.passphrase) s.NoError(err) ok, err := s.AttestationState.Initialized(ctx, s.workflowOrg1.ID.String(), s.runOrg1.ID.String()) diff --git a/app/controlplane/pkg/data/attestationstate.go b/app/controlplane/pkg/data/attestationstate.go index b667586eb..834e7e441 100644 --- a/app/controlplane/pkg/data/attestationstate.go +++ b/app/controlplane/pkg/data/attestationstate.go @@ -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. @@ -52,9 +52,10 @@ func (r *AttestationStateRepo) Initialized(ctx context.Context, runID uuid.UUID) } // baseDigest, when provided will be used to check that it matches the digest of the state currently in the DB -// if the digests do not match, the state has been modified and the caller should retry -func (r *AttestationStateRepo) Save(ctx context.Context, runID uuid.UUID, state []byte, baseDigest string) error { - return WithTx(ctx, r.data.DB, func(tx *ent.Tx) error { +// if the digests do not match, the state has been modified and the caller should retry. +// It returns the digest of the newly saved state. +func (r *AttestationStateRepo) Save(ctx context.Context, runID uuid.UUID, state []byte, baseDigest string) (string, error) { + err := WithTx(ctx, r.data.DB, func(tx *ent.Tx) error { // compared the provided digest with the digest of the state in the DB // TODO: make digest check mandatory on updates if baseDigest != "" { @@ -84,8 +85,19 @@ func (r *AttestationStateRepo) Save(ctx context.Context, runID uuid.UUID, state } else if err != nil { return biz.NewErrNotFound("workflow run") } + return nil }) + if err != nil { + return "", err + } + + newDigest, err := digest(state) + if err != nil { + return "", fmt.Errorf("failed to calculate new digest: %w", err) + } + + return newDigest, nil } func (r *AttestationStateRepo) Read(ctx context.Context, runID uuid.UUID) ([]byte, string, error) { diff --git a/pkg/attestation/crafter/.mockery.yml b/pkg/attestation/crafter/.mockery.yml new file mode 100644 index 000000000..9e3b17f8b --- /dev/null +++ b/pkg/attestation/crafter/.mockery.yml @@ -0,0 +1,18 @@ +all: false +formatter: goimports +include-auto-generated: false +log-level: info +recursive: false +require-template-schema-exists: true +template: testify +template-schema: "{{.Template}}.schema.json" +packages: + github.com/chainloop-dev/chainloop/pkg/attestation/crafter: + config: + dir: "{{.InterfaceDir}}/mocks" + filename: "{{.InterfaceName}}.go" + pkgname: mocks + structname: "{{.InterfaceName}}" + interfaces: + StateManager: + Collector: diff --git a/pkg/attestation/crafter/mocks/Collector.go b/pkg/attestation/crafter/mocks/Collector.go new file mode 100644 index 000000000..19aa36ee4 --- /dev/null +++ b/pkg/attestation/crafter/mocks/Collector.go @@ -0,0 +1,153 @@ +// Code generated by mockery; DO NOT EDIT. +// github.com/vektra/mockery +// template: testify + +package mocks + +import ( + "context" + + "github.com/chainloop-dev/chainloop/pkg/attestation/crafter" + "github.com/chainloop-dev/chainloop/pkg/casclient" + mock "github.com/stretchr/testify/mock" +) + +// NewCollector creates a new instance of Collector. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewCollector(t interface { + mock.TestingT + Cleanup(func()) +}) *Collector { + mock := &Collector{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} + +// Collector is an autogenerated mock type for the Collector type +type Collector struct { + mock.Mock +} + +type Collector_Expecter struct { + mock *mock.Mock +} + +func (_m *Collector) EXPECT() *Collector_Expecter { + return &Collector_Expecter{mock: &_m.Mock} +} + +// Collect provides a mock function for the type Collector +func (_mock *Collector) Collect(ctx context.Context, crafter1 *crafter.Crafter, attestationID string, casBackend *casclient.CASBackend) error { + ret := _mock.Called(ctx, crafter1, attestationID, casBackend) + + if len(ret) == 0 { + panic("no return value specified for Collect") + } + + var r0 error + if returnFunc, ok := ret.Get(0).(func(context.Context, *crafter.Crafter, string, *casclient.CASBackend) error); ok { + r0 = returnFunc(ctx, crafter1, attestationID, casBackend) + } else { + r0 = ret.Error(0) + } + return r0 +} + +// Collector_Collect_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Collect' +type Collector_Collect_Call struct { + *mock.Call +} + +// Collect is a helper method to define mock.On call +// - ctx context.Context +// - crafter1 *crafter.Crafter +// - attestationID string +// - casBackend *casclient.CASBackend +func (_e *Collector_Expecter) Collect(ctx interface{}, crafter1 interface{}, attestationID interface{}, casBackend interface{}) *Collector_Collect_Call { + return &Collector_Collect_Call{Call: _e.mock.On("Collect", ctx, crafter1, attestationID, casBackend)} +} + +func (_c *Collector_Collect_Call) Run(run func(ctx context.Context, crafter1 *crafter.Crafter, attestationID string, casBackend *casclient.CASBackend)) *Collector_Collect_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 *crafter.Crafter + if args[1] != nil { + arg1 = args[1].(*crafter.Crafter) + } + var arg2 string + if args[2] != nil { + arg2 = args[2].(string) + } + var arg3 *casclient.CASBackend + if args[3] != nil { + arg3 = args[3].(*casclient.CASBackend) + } + run( + arg0, + arg1, + arg2, + arg3, + ) + }) + return _c +} + +func (_c *Collector_Collect_Call) Return(err error) *Collector_Collect_Call { + _c.Call.Return(err) + return _c +} + +func (_c *Collector_Collect_Call) RunAndReturn(run func(ctx context.Context, crafter1 *crafter.Crafter, attestationID string, casBackend *casclient.CASBackend) error) *Collector_Collect_Call { + _c.Call.Return(run) + return _c +} + +// ID provides a mock function for the type Collector +func (_mock *Collector) ID() string { + ret := _mock.Called() + + if len(ret) == 0 { + panic("no return value specified for ID") + } + + var r0 string + if returnFunc, ok := ret.Get(0).(func() string); ok { + r0 = returnFunc() + } else { + r0 = ret.Get(0).(string) + } + return r0 +} + +// Collector_ID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ID' +type Collector_ID_Call struct { + *mock.Call +} + +// ID is a helper method to define mock.On call +func (_e *Collector_Expecter) ID() *Collector_ID_Call { + return &Collector_ID_Call{Call: _e.mock.On("ID")} +} + +func (_c *Collector_ID_Call) Run(run func()) *Collector_ID_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *Collector_ID_Call) Return(s string) *Collector_ID_Call { + _c.Call.Return(s) + return _c +} + +func (_c *Collector_ID_Call) RunAndReturn(run func() string) *Collector_ID_Call { + _c.Call.Return(run) + return _c +} diff --git a/pkg/attestation/crafter/mocks/StateManager.go b/pkg/attestation/crafter/mocks/StateManager.go new file mode 100644 index 000000000..3f87ac28d --- /dev/null +++ b/pkg/attestation/crafter/mocks/StateManager.go @@ -0,0 +1,345 @@ +// Code generated by mockery; DO NOT EDIT. +// github.com/vektra/mockery +// template: testify + +package mocks + +import ( + "context" + + "github.com/chainloop-dev/chainloop/pkg/attestation/crafter" + mock "github.com/stretchr/testify/mock" +) + +// NewStateManager creates a new instance of StateManager. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewStateManager(t interface { + mock.TestingT + Cleanup(func()) +}) *StateManager { + mock := &StateManager{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} + +// StateManager is an autogenerated mock type for the StateManager type +type StateManager struct { + mock.Mock +} + +type StateManager_Expecter struct { + mock *mock.Mock +} + +func (_m *StateManager) EXPECT() *StateManager_Expecter { + return &StateManager_Expecter{mock: &_m.Mock} +} + +// Info provides a mock function for the type StateManager +func (_mock *StateManager) Info(ctx context.Context, key string) string { + ret := _mock.Called(ctx, key) + + if len(ret) == 0 { + panic("no return value specified for Info") + } + + var r0 string + if returnFunc, ok := ret.Get(0).(func(context.Context, string) string); ok { + r0 = returnFunc(ctx, key) + } else { + r0 = ret.Get(0).(string) + } + return r0 +} + +// StateManager_Info_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Info' +type StateManager_Info_Call struct { + *mock.Call +} + +// Info is a helper method to define mock.On call +// - ctx context.Context +// - key string +func (_e *StateManager_Expecter) Info(ctx interface{}, key interface{}) *StateManager_Info_Call { + return &StateManager_Info_Call{Call: _e.mock.On("Info", ctx, key)} +} + +func (_c *StateManager_Info_Call) Run(run func(ctx context.Context, key string)) *StateManager_Info_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 string + if args[1] != nil { + arg1 = args[1].(string) + } + run( + arg0, + arg1, + ) + }) + return _c +} + +func (_c *StateManager_Info_Call) Return(s string) *StateManager_Info_Call { + _c.Call.Return(s) + return _c +} + +func (_c *StateManager_Info_Call) RunAndReturn(run func(ctx context.Context, key string) string) *StateManager_Info_Call { + _c.Call.Return(run) + return _c +} + +// Initialized provides a mock function for the type StateManager +func (_mock *StateManager) Initialized(ctx context.Context, key string) (bool, error) { + ret := _mock.Called(ctx, key) + + if len(ret) == 0 { + panic("no return value specified for Initialized") + } + + var r0 bool + var r1 error + if returnFunc, ok := ret.Get(0).(func(context.Context, string) (bool, error)); ok { + return returnFunc(ctx, key) + } + if returnFunc, ok := ret.Get(0).(func(context.Context, string) bool); ok { + r0 = returnFunc(ctx, key) + } else { + r0 = ret.Get(0).(bool) + } + if returnFunc, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = returnFunc(ctx, key) + } else { + r1 = ret.Error(1) + } + return r0, r1 +} + +// StateManager_Initialized_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Initialized' +type StateManager_Initialized_Call struct { + *mock.Call +} + +// Initialized is a helper method to define mock.On call +// - ctx context.Context +// - key string +func (_e *StateManager_Expecter) Initialized(ctx interface{}, key interface{}) *StateManager_Initialized_Call { + return &StateManager_Initialized_Call{Call: _e.mock.On("Initialized", ctx, key)} +} + +func (_c *StateManager_Initialized_Call) Run(run func(ctx context.Context, key string)) *StateManager_Initialized_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 string + if args[1] != nil { + arg1 = args[1].(string) + } + run( + arg0, + arg1, + ) + }) + return _c +} + +func (_c *StateManager_Initialized_Call) Return(b bool, err error) *StateManager_Initialized_Call { + _c.Call.Return(b, err) + return _c +} + +func (_c *StateManager_Initialized_Call) RunAndReturn(run func(ctx context.Context, key string) (bool, error)) *StateManager_Initialized_Call { + _c.Call.Return(run) + return _c +} + +// Read provides a mock function for the type StateManager +func (_mock *StateManager) Read(ctx context.Context, key string, state *crafter.VersionedCraftingState) error { + ret := _mock.Called(ctx, key, state) + + if len(ret) == 0 { + panic("no return value specified for Read") + } + + var r0 error + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *crafter.VersionedCraftingState) error); ok { + r0 = returnFunc(ctx, key, state) + } else { + r0 = ret.Error(0) + } + return r0 +} + +// StateManager_Read_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Read' +type StateManager_Read_Call struct { + *mock.Call +} + +// Read is a helper method to define mock.On call +// - ctx context.Context +// - key string +// - state *crafter.VersionedCraftingState +func (_e *StateManager_Expecter) Read(ctx interface{}, key interface{}, state interface{}) *StateManager_Read_Call { + return &StateManager_Read_Call{Call: _e.mock.On("Read", ctx, key, state)} +} + +func (_c *StateManager_Read_Call) Run(run func(ctx context.Context, key string, state *crafter.VersionedCraftingState)) *StateManager_Read_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 string + if args[1] != nil { + arg1 = args[1].(string) + } + var arg2 *crafter.VersionedCraftingState + if args[2] != nil { + arg2 = args[2].(*crafter.VersionedCraftingState) + } + run( + arg0, + arg1, + arg2, + ) + }) + return _c +} + +func (_c *StateManager_Read_Call) Return(err error) *StateManager_Read_Call { + _c.Call.Return(err) + return _c +} + +func (_c *StateManager_Read_Call) RunAndReturn(run func(ctx context.Context, key string, state *crafter.VersionedCraftingState) error) *StateManager_Read_Call { + _c.Call.Return(run) + return _c +} + +// Reset provides a mock function for the type StateManager +func (_mock *StateManager) Reset(ctx context.Context, key string) error { + ret := _mock.Called(ctx, key) + + if len(ret) == 0 { + panic("no return value specified for Reset") + } + + var r0 error + if returnFunc, ok := ret.Get(0).(func(context.Context, string) error); ok { + r0 = returnFunc(ctx, key) + } else { + r0 = ret.Error(0) + } + return r0 +} + +// StateManager_Reset_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Reset' +type StateManager_Reset_Call struct { + *mock.Call +} + +// Reset is a helper method to define mock.On call +// - ctx context.Context +// - key string +func (_e *StateManager_Expecter) Reset(ctx interface{}, key interface{}) *StateManager_Reset_Call { + return &StateManager_Reset_Call{Call: _e.mock.On("Reset", ctx, key)} +} + +func (_c *StateManager_Reset_Call) Run(run func(ctx context.Context, key string)) *StateManager_Reset_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 string + if args[1] != nil { + arg1 = args[1].(string) + } + run( + arg0, + arg1, + ) + }) + return _c +} + +func (_c *StateManager_Reset_Call) Return(err error) *StateManager_Reset_Call { + _c.Call.Return(err) + return _c +} + +func (_c *StateManager_Reset_Call) RunAndReturn(run func(ctx context.Context, key string) error) *StateManager_Reset_Call { + _c.Call.Return(run) + return _c +} + +// Write provides a mock function for the type StateManager +func (_mock *StateManager) Write(ctx context.Context, key string, state *crafter.VersionedCraftingState) error { + ret := _mock.Called(ctx, key, state) + + if len(ret) == 0 { + panic("no return value specified for Write") + } + + var r0 error + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *crafter.VersionedCraftingState) error); ok { + r0 = returnFunc(ctx, key, state) + } else { + r0 = ret.Error(0) + } + return r0 +} + +// StateManager_Write_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Write' +type StateManager_Write_Call struct { + *mock.Call +} + +// Write is a helper method to define mock.On call +// - ctx context.Context +// - key string +// - state *crafter.VersionedCraftingState +func (_e *StateManager_Expecter) Write(ctx interface{}, key interface{}, state interface{}) *StateManager_Write_Call { + return &StateManager_Write_Call{Call: _e.mock.On("Write", ctx, key, state)} +} + +func (_c *StateManager_Write_Call) Run(run func(ctx context.Context, key string, state *crafter.VersionedCraftingState)) *StateManager_Write_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 string + if args[1] != nil { + arg1 = args[1].(string) + } + var arg2 *crafter.VersionedCraftingState + if args[2] != nil { + arg2 = args[2].(*crafter.VersionedCraftingState) + } + run( + arg0, + arg1, + arg2, + ) + }) + return _c +} + +func (_c *StateManager_Write_Call) Return(err error) *StateManager_Write_Call { + _c.Call.Return(err) + return _c +} + +func (_c *StateManager_Write_Call) RunAndReturn(run func(ctx context.Context, key string, state *crafter.VersionedCraftingState) error) *StateManager_Write_Call { + _c.Call.Return(run) + return _c +} diff --git a/pkg/attestation/crafter/runcollectors_test.go b/pkg/attestation/crafter/runcollectors_test.go new file mode 100644 index 000000000..7103744a1 --- /dev/null +++ b/pkg/attestation/crafter/runcollectors_test.go @@ -0,0 +1,131 @@ +// +// 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 crafter_test + +import ( + "context" + "errors" + "testing" + + schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" + "github.com/chainloop-dev/chainloop/pkg/attestation/crafter" + api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" + craftermocks "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/mocks" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// validCraftingState returns a minimal crafting state that satisfies LoadCraftingState requirements. +func validCraftingState() *api.CraftingState { + return &api.CraftingState{ + Attestation: &api.Attestation{ + RunnerType: schemaapi.CraftingSchema_Runner_RUNNER_TYPE_UNSPECIFIED, + }, + } +} + +// setupReadExpectation configures the mock to populate state on Read calls. +func setupReadExpectation(sm *craftermocks.StateManager, digest string) *mock.Call { + return sm.On("Read", mock.Anything, mock.Anything, mock.Anything). + Run(func(args mock.Arguments) { + st := args.Get(2).(*crafter.VersionedCraftingState) + st.CraftingState = validCraftingState() + st.UpdateCheckSum = digest + }).Return(nil) +} + +// newSuccessCollector creates a mock collector that succeeds. +// ID() is marked Maybe() since it's only called on failure paths. +func newSuccessCollector(t *testing.T, id string) *craftermocks.Collector { + c := craftermocks.NewCollector(t) + c.On("ID").Maybe().Return(id) + c.On("Collect", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + return c +} + +func TestRunCollectors(t *testing.T) { + t.Run("loads state once before running collectors", func(t *testing.T) { + sm := craftermocks.NewStateManager(t) + setupReadExpectation(sm, "digest-1") + sm.On("Info", mock.Anything, mock.Anything).Return("mock://run-1") + + c1 := newSuccessCollector(t, "c1") + c2 := newSuccessCollector(t, "c2") + + cr, err := crafter.NewCrafter(sm, nil) + require.NoError(t, err) + cr.RegisterCollectors(c1, c2) + + cr.RunCollectors(context.Background(), "run-1", nil) + + // Single Read before the loop + sm.AssertNumberOfCalls(t, "Read", 1) + c1.AssertCalled(t, "Collect", mock.Anything, mock.Anything, "run-1", mock.Anything) + c2.AssertCalled(t, "Collect", mock.Anything, mock.Anything, "run-1", mock.Anything) + }) + + t.Run("collector failure does not stop other collectors", func(t *testing.T) { + sm := craftermocks.NewStateManager(t) + setupReadExpectation(sm, "d") + sm.On("Info", mock.Anything, mock.Anything).Return("mock://run-1") + + c1 := craftermocks.NewCollector(t) + c1.On("ID").Return("failing") + c1.On("Collect", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(errors.New("collector error")) + + c2 := newSuccessCollector(t, "ok") + + cr, err := crafter.NewCrafter(sm, nil) + require.NoError(t, err) + cr.RegisterCollectors(c1, c2) + + cr.RunCollectors(context.Background(), "run-1", nil) + + c1.AssertCalled(t, "Collect", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + c2.AssertCalled(t, "Collect", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) + + t.Run("no collectors only reloads state", func(t *testing.T) { + sm := craftermocks.NewStateManager(t) + setupReadExpectation(sm, "d") + sm.On("Info", mock.Anything, mock.Anything).Return("mock://run-1") + + cr, err := crafter.NewCrafter(sm, nil) + require.NoError(t, err) + + cr.RunCollectors(context.Background(), "run-1", nil) + + sm.AssertNumberOfCalls(t, "Read", 1) + }) + + t.Run("state load failure aborts before running collectors", func(t *testing.T) { + sm := craftermocks.NewStateManager(t) + sm.On("Read", mock.Anything, mock.Anything, mock.Anything).Return(errors.New("read error")) + sm.On("Info", mock.Anything, mock.Anything).Return("mock://run-1") + + c1 := craftermocks.NewCollector(t) + + cr, err := crafter.NewCrafter(sm, nil) + require.NoError(t, err) + cr.RegisterCollectors(c1) + + cr.RunCollectors(context.Background(), "run-1", nil) + + // Collector should never be called + c1.AssertNotCalled(t, "Collect") + }) +} diff --git a/pkg/attestation/crafter/statemanager/remote/remote.go b/pkg/attestation/crafter/statemanager/remote/remote.go index bd0b2beca..24f4deca9 100644 --- a/pkg/attestation/crafter/statemanager/remote/remote.go +++ b/pkg/attestation/crafter/statemanager/remote/remote.go @@ -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. @@ -67,10 +67,14 @@ func (r *Remote) Write(ctx context.Context, key string, state *crafter.Versioned } r.logger.Debug().Str("key", key).Str("baseDigest", state.UpdateCheckSum).Msg("Writing state to remote") - if _, err := r.client.Save(ctx, &pb.AttestationStateServiceSaveRequest{WorkflowRunId: key, AttestationState: state.CraftingState, BaseDigest: state.UpdateCheckSum}); err != nil { + resp, err := r.client.Save(ctx, &pb.AttestationStateServiceSaveRequest{WorkflowRunId: key, AttestationState: state.CraftingState, BaseDigest: state.UpdateCheckSum}) + if err != nil { return fmt.Errorf("failed to save state: %w", err) } + // Update the checksum so subsequent writes use the correct base digest + state.UpdateCheckSum = resp.GetDigest() + return nil } diff --git a/pkg/attestation/crafter/statemanager/remote/remote_test.go b/pkg/attestation/crafter/statemanager/remote/remote_test.go new file mode 100644 index 000000000..b6a196ef9 --- /dev/null +++ b/pkg/attestation/crafter/statemanager/remote/remote_test.go @@ -0,0 +1,129 @@ +// +// 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 remote + +import ( + "context" + "testing" + + pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" + pbmocks "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1/mocks" + v1 "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" + + "github.com/chainloop-dev/chainloop/pkg/attestation/crafter" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestWriteUpdatesChecksum(t *testing.T) { + testCases := []struct { + name string + initialChecksum string + responseDigest string + expectedChecksum string + }{ + { + name: "updates checksum from response digest", + initialChecksum: "old-digest", + responseDigest: "new-digest-from-server", + expectedChecksum: "new-digest-from-server", + }, + { + name: "clears checksum when response digest is empty", + initialChecksum: "old-digest", + responseDigest: "", + expectedChecksum: "", + }, + { + name: "first write with empty initial checksum", + initialChecksum: "", + responseDigest: "first-digest", + expectedChecksum: "first-digest", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + client := pbmocks.NewAttestationStateServiceClient(t) + client.On("Save", mock.Anything, mock.Anything, mock.Anything). + Return(&pb.AttestationStateServiceSaveResponse{Digest: tc.responseDigest}, nil) + + r, err := New(client, nil) + require.NoError(t, err) + + state := &crafter.VersionedCraftingState{ + CraftingState: &v1.CraftingState{}, + UpdateCheckSum: tc.initialChecksum, + } + + err = r.Write(context.Background(), "run-123", state) + require.NoError(t, err) + assert.Equal(t, tc.expectedChecksum, state.UpdateCheckSum) + }) + } +} + +func TestWriteSendsBaseDigest(t *testing.T) { + client := pbmocks.NewAttestationStateServiceClient(t) + client.On("Save", mock.Anything, mock.MatchedBy(func(req *pb.AttestationStateServiceSaveRequest) bool { + return req.BaseDigest == "current-digest" && req.WorkflowRunId == "run-456" + }), mock.Anything).Return(&pb.AttestationStateServiceSaveResponse{Digest: "next-digest"}, nil) + + r, err := New(client, nil) + require.NoError(t, err) + + state := &crafter.VersionedCraftingState{ + CraftingState: &v1.CraftingState{}, + UpdateCheckSum: "current-digest", + } + + err = r.Write(context.Background(), "run-456", state) + require.NoError(t, err) + assert.Equal(t, "next-digest", state.UpdateCheckSum) +} + +func TestConsecutiveWritesUseUpdatedDigest(t *testing.T) { + client := pbmocks.NewAttestationStateServiceClient(t) + + // First write: sends empty digest, gets "digest-1" back + client.On("Save", mock.Anything, mock.MatchedBy(func(req *pb.AttestationStateServiceSaveRequest) bool { + return req.BaseDigest == "" + }), mock.Anything).Return(&pb.AttestationStateServiceSaveResponse{Digest: "digest-1"}, nil).Once() + + // Second write: sends "digest-1", gets "digest-2" back + client.On("Save", mock.Anything, mock.MatchedBy(func(req *pb.AttestationStateServiceSaveRequest) bool { + return req.BaseDigest == "digest-1" + }), mock.Anything).Return(&pb.AttestationStateServiceSaveResponse{Digest: "digest-2"}, nil).Once() + + r, err := New(client, nil) + require.NoError(t, err) + + state := &crafter.VersionedCraftingState{ + CraftingState: &v1.CraftingState{}, + UpdateCheckSum: "", + } + + // First write + err = r.Write(context.Background(), "run-789", state) + require.NoError(t, err) + assert.Equal(t, "digest-1", state.UpdateCheckSum) + + // Second write uses the updated digest + err = r.Write(context.Background(), "run-789", state) + require.NoError(t, err) + assert.Equal(t, "digest-2", state.UpdateCheckSum) +} From 2baee0092e11bc3071ddfde52046bbccdd2ea1b1 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Sat, 21 Mar 2026 16:16:40 +0100 Subject: [PATCH 2/5] fix: reload state after collectors to sync digest for old servers Servers that don't return digests in Save responses leave the client with a stale UpdateCheckSum. Reloading state after all collectors run ensures the local digest matches the server, preventing OCC conflicts on subsequent writes. Signed-off-by: Miguel Martinez Trivino --- pkg/attestation/crafter/crafter.go | 6 ++++++ pkg/attestation/crafter/runcollectors_test.go | 10 +++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/pkg/attestation/crafter/crafter.go b/pkg/attestation/crafter/crafter.go index d4c9080ea..b75fddd40 100644 --- a/pkg/attestation/crafter/crafter.go +++ b/pkg/attestation/crafter/crafter.go @@ -152,6 +152,12 @@ func (c *Crafter) RunCollectors(ctx context.Context, attestationID string, casBa c.Logger.Warn().Err(err).Str("collector", collector.ID()).Msg("collector failed") } } + + // Reload state to sync the local digest with the server after collectors modified it. + // This is needed for backward compatibility with servers that don't return digests in Save responses. + if err := c.LoadCraftingState(ctx, attestationID); err != nil { + c.Logger.Warn().Err(err).Msg("failed to reload crafting state after running collectors") + } } // Create a completely new crafter diff --git a/pkg/attestation/crafter/runcollectors_test.go b/pkg/attestation/crafter/runcollectors_test.go index 7103744a1..081170f77 100644 --- a/pkg/attestation/crafter/runcollectors_test.go +++ b/pkg/attestation/crafter/runcollectors_test.go @@ -57,7 +57,7 @@ func newSuccessCollector(t *testing.T, id string) *craftermocks.Collector { } func TestRunCollectors(t *testing.T) { - t.Run("loads state once before running collectors", func(t *testing.T) { + t.Run("loads state before and after running collectors", func(t *testing.T) { sm := craftermocks.NewStateManager(t) setupReadExpectation(sm, "digest-1") sm.On("Info", mock.Anything, mock.Anything).Return("mock://run-1") @@ -71,8 +71,8 @@ func TestRunCollectors(t *testing.T) { cr.RunCollectors(context.Background(), "run-1", nil) - // Single Read before the loop - sm.AssertNumberOfCalls(t, "Read", 1) + // Read before the loop + Read after to sync digest + sm.AssertNumberOfCalls(t, "Read", 2) c1.AssertCalled(t, "Collect", mock.Anything, mock.Anything, "run-1", mock.Anything) c2.AssertCalled(t, "Collect", mock.Anything, mock.Anything, "run-1", mock.Anything) }) @@ -99,7 +99,7 @@ func TestRunCollectors(t *testing.T) { c2.AssertCalled(t, "Collect", mock.Anything, mock.Anything, mock.Anything, mock.Anything) }) - t.Run("no collectors only reloads state", func(t *testing.T) { + t.Run("no collectors reloads state before and after", func(t *testing.T) { sm := craftermocks.NewStateManager(t) setupReadExpectation(sm, "d") sm.On("Info", mock.Anything, mock.Anything).Return("mock://run-1") @@ -109,7 +109,7 @@ func TestRunCollectors(t *testing.T) { cr.RunCollectors(context.Background(), "run-1", nil) - sm.AssertNumberOfCalls(t, "Read", 1) + sm.AssertNumberOfCalls(t, "Read", 2) }) t.Run("state load failure aborts before running collectors", func(t *testing.T) { From d82817322e76de16bfb953c9c02434a4a6c2b239 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Sat, 21 Mar 2026 16:19:50 +0100 Subject: [PATCH 3/5] fix: only reload state after collectors when digest changed Compare the digest before and after collectors run. If it changed, writes happened and the local checksum may be stale, so reload from the server. If unchanged, no writes occurred and reload is skipped. Signed-off-by: Miguel Martinez Trivino --- pkg/attestation/crafter/crafter.go | 13 +++++++---- pkg/attestation/crafter/runcollectors_test.go | 22 ++++++++++++------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/pkg/attestation/crafter/crafter.go b/pkg/attestation/crafter/crafter.go index b75fddd40..1764a8744 100644 --- a/pkg/attestation/crafter/crafter.go +++ b/pkg/attestation/crafter/crafter.go @@ -147,16 +147,21 @@ func (c *Crafter) RunCollectors(ctx context.Context, attestationID string, casBa return } + digestBeforeCollectors := c.CraftingState.UpdateCheckSum + for _, collector := range c.collectors { if err := collector.Collect(ctx, c, attestationID, casBackend); err != nil { c.Logger.Warn().Err(err).Str("collector", collector.ID()).Msg("collector failed") } } - // Reload state to sync the local digest with the server after collectors modified it. - // This is needed for backward compatibility with servers that don't return digests in Save responses. - if err := c.LoadCraftingState(ctx, attestationID); err != nil { - c.Logger.Warn().Err(err).Msg("failed to reload crafting state after running collectors") + // Reload state if collectors modified it and the local digest may be stale. + // When the digest changed, writes happened but we can't be sure the checksum + // reflects the true server-side state (e.g. old servers don't return digests). + if c.CraftingState.UpdateCheckSum != digestBeforeCollectors { + if err := c.LoadCraftingState(ctx, attestationID); err != nil { + c.Logger.Warn().Err(err).Msg("failed to reload crafting state after running collectors") + } } } diff --git a/pkg/attestation/crafter/runcollectors_test.go b/pkg/attestation/crafter/runcollectors_test.go index 081170f77..8066e4e04 100644 --- a/pkg/attestation/crafter/runcollectors_test.go +++ b/pkg/attestation/crafter/runcollectors_test.go @@ -57,24 +57,29 @@ func newSuccessCollector(t *testing.T, id string) *craftermocks.Collector { } func TestRunCollectors(t *testing.T) { - t.Run("loads state before and after running collectors", func(t *testing.T) { + t.Run("reloads state after collectors when digest changes", func(t *testing.T) { sm := craftermocks.NewStateManager(t) setupReadExpectation(sm, "digest-1") sm.On("Info", mock.Anything, mock.Anything).Return("mock://run-1") - c1 := newSuccessCollector(t, "c1") - c2 := newSuccessCollector(t, "c2") + // Collector modifies the digest (simulates a Write that updates UpdateCheckSum) + c1 := craftermocks.NewCollector(t) + c1.On("ID").Maybe().Return("c1") + c1.On("Collect", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Run(func(args mock.Arguments) { + cr := args.Get(1).(*crafter.Crafter) + cr.CraftingState.UpdateCheckSum = "digest-after-write" + }).Return(nil) cr, err := crafter.NewCrafter(sm, nil) require.NoError(t, err) - cr.RegisterCollectors(c1, c2) + cr.RegisterCollectors(c1) cr.RunCollectors(context.Background(), "run-1", nil) - // Read before the loop + Read after to sync digest + // Read before + Read after (digest changed, so reload triggered) sm.AssertNumberOfCalls(t, "Read", 2) c1.AssertCalled(t, "Collect", mock.Anything, mock.Anything, "run-1", mock.Anything) - c2.AssertCalled(t, "Collect", mock.Anything, mock.Anything, "run-1", mock.Anything) }) t.Run("collector failure does not stop other collectors", func(t *testing.T) { @@ -99,7 +104,7 @@ func TestRunCollectors(t *testing.T) { c2.AssertCalled(t, "Collect", mock.Anything, mock.Anything, mock.Anything, mock.Anything) }) - t.Run("no collectors reloads state before and after", func(t *testing.T) { + t.Run("no collectors skips reload when digest unchanged", func(t *testing.T) { sm := craftermocks.NewStateManager(t) setupReadExpectation(sm, "d") sm.On("Info", mock.Anything, mock.Anything).Return("mock://run-1") @@ -109,7 +114,8 @@ func TestRunCollectors(t *testing.T) { cr.RunCollectors(context.Background(), "run-1", nil) - sm.AssertNumberOfCalls(t, "Read", 2) + // Only the initial Read, no reload since digest didn't change + sm.AssertNumberOfCalls(t, "Read", 1) }) t.Run("state load failure aborts before running collectors", func(t *testing.T) { From 807b76cd6ea1fe1a8f279068f1e42f251fd6b1ce Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Sat, 21 Mar 2026 16:27:13 +0100 Subject: [PATCH 4/5] fix: improve comment on stale digest reload workaround Signed-off-by: Miguel Martinez Trivino --- pkg/attestation/crafter/crafter.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/attestation/crafter/crafter.go b/pkg/attestation/crafter/crafter.go index 1764a8744..f7e1164c5 100644 --- a/pkg/attestation/crafter/crafter.go +++ b/pkg/attestation/crafter/crafter.go @@ -155,9 +155,10 @@ func (c *Crafter) RunCollectors(ctx context.Context, attestationID string, casBa } } - // Reload state if collectors modified it and the local digest may be stale. - // When the digest changed, writes happened but we can't be sure the checksum - // reflects the true server-side state (e.g. old servers don't return digests). + // NOTE: workaround for old servers that don't return digests in Save responses. + // not returning digest makes digests stale, and state save failing with conflict errors + // https://github.com/chainloop-dev/chainloop/issues/2908 + // this conditition will not apply to new servers that return digests in Save responses. if c.CraftingState.UpdateCheckSum != digestBeforeCollectors { if err := c.LoadCraftingState(ctx, attestationID); err != nil { c.Logger.Warn().Err(err).Msg("failed to reload crafting state after running collectors") From 16d97d50fde66101d49f11335992101b991d6e4a Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Sat, 21 Mar 2026 17:09:35 +0100 Subject: [PATCH 5/5] fix: preserve checksum when server returns empty digest Old servers don't return digests in Save responses. Guard against overwriting a valid checksum with an empty string, which would break OCC protection on subsequent writes. Signed-off-by: Miguel Martinez Trivino --- pkg/attestation/crafter/statemanager/remote/remote.go | 7 +++++-- pkg/attestation/crafter/statemanager/remote/remote_test.go | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/attestation/crafter/statemanager/remote/remote.go b/pkg/attestation/crafter/statemanager/remote/remote.go index 24f4deca9..0fae1837f 100644 --- a/pkg/attestation/crafter/statemanager/remote/remote.go +++ b/pkg/attestation/crafter/statemanager/remote/remote.go @@ -72,8 +72,11 @@ func (r *Remote) Write(ctx context.Context, key string, state *crafter.Versioned return fmt.Errorf("failed to save state: %w", err) } - // Update the checksum so subsequent writes use the correct base digest - state.UpdateCheckSum = resp.GetDigest() + // Update the checksum so subsequent writes use the correct base digest. + // Only update if the server returned a digest (old servers don't). + if d := resp.GetDigest(); d != "" { + state.UpdateCheckSum = d + } return nil } diff --git a/pkg/attestation/crafter/statemanager/remote/remote_test.go b/pkg/attestation/crafter/statemanager/remote/remote_test.go index b6a196ef9..7b2e4288f 100644 --- a/pkg/attestation/crafter/statemanager/remote/remote_test.go +++ b/pkg/attestation/crafter/statemanager/remote/remote_test.go @@ -43,10 +43,10 @@ func TestWriteUpdatesChecksum(t *testing.T) { expectedChecksum: "new-digest-from-server", }, { - name: "clears checksum when response digest is empty", + name: "keeps checksum when response digest is empty", initialChecksum: "old-digest", responseDigest: "", - expectedChecksum: "", + expectedChecksum: "old-digest", }, { name: "first write with empty initial checksum",