From 0965e1091f6e9e98157fb99ad595a8f6cc7c3581 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Tue, 23 Jun 2026 17:04:49 +0200 Subject: [PATCH 1/4] feat(contract): support "at least one of" material choke groups Add a per-material `group` field to workflow contracts. Materials sharing the same non-empty group value form a choke group: at least one member must be present in the attestation, otherwise it cannot be pushed. Materials without a group keep their existing required/optional behavior. Enforcement happens at attestation completion with a clear error listing the group and its members. The CLI surfaces the group in `attestation status` and `attestation add`. Contract parsing now ignores unknown fields so contracts authored with fields from newer releases do not break older clients. Assisted-by: Claude Code Signed-off-by: Jose I. Paris --- app/cli/cmd/attestation_add.go | 4 + app/cli/cmd/attestation_status.go | 3 + app/cli/pkg/action/attestation_status.go | 8 +- app/cli/pkg/action/attestation_status_test.go | 41 +++++- .../workflowcontract/v1/crafting_schema.ts | 21 ++- ...v1.CraftingSchema.Material.jsonschema.json | 4 + ...act.v1.CraftingSchema.Material.schema.json | 4 + .../workflowcontract/v1/crafting_schema.pb.go | 22 +++- .../workflowcontract/v1/crafting_schema.proto | 8 ++ app/controlplane/pkg/unmarshal/unmarshal.go | 11 +- .../pkg/unmarshal/unmarshal_test.go | 96 +++++++++++++- .../v1/crafting_state_validations.go | 38 +++++- .../v1/crafting_state_validations_test.go | 124 +++++++++++++++++- 13 files changed, 369 insertions(+), 15 deletions(-) diff --git a/app/cli/cmd/attestation_add.go b/app/cli/cmd/attestation_add.go index a94859c1c..5c7009cf5 100644 --- a/app/cli/cmd/attestation_add.go +++ b/app/cli/cmd/attestation_add.go @@ -179,6 +179,10 @@ func displayMaterialInfo(status *action.AttestationStatusMaterial, policyEvaluat mt.AppendRow(table.Row{"Type", status.Type}) mt.AppendRow(table.Row{"Required", hBool(status.Required)}) + if status.Group != "" { + mt.AppendRow(table.Row{"Group", fmt.Sprintf("%s (at least one of the group required)", status.Group)}) + } + if status.IsOutput { mt.AppendRow(table.Row{"Is output", "Yes"}) } diff --git a/app/cli/cmd/attestation_status.go b/app/cli/cmd/attestation_status.go index 3920bb13f..1968525ed 100644 --- a/app/cli/cmd/attestation_status.go +++ b/app/cli/cmd/attestation_status.go @@ -203,6 +203,9 @@ func materialsTable(status *action.AttestationStatusResult, w io.Writer, full bo mt.AppendRow(table.Row{"Type", m.Type}) mt.AppendRow(table.Row{"Set", hBool(m.Set)}) mt.AppendRow(table.Row{"Required", hBool(m.Required)}) + if m.Group != "" { + mt.AppendRow(table.Row{"Group", fmt.Sprintf("%s (at least one of the group required)", m.Group)}) + } if m.IsOutput { mt.AppendRow(table.Row{"Is output", "Yes"}) } diff --git a/app/cli/pkg/action/attestation_status.go b/app/cli/pkg/action/attestation_status.go index 581898d9e..7c240af45 100644 --- a/app/cli/pkg/action/attestation_status.go +++ b/app/cli/pkg/action/attestation_status.go @@ -75,6 +75,9 @@ type AttestationStatusWorkflowMeta struct { type AttestationStatusMaterial struct { *Material Set, IsOutput, Required, SkipUpload bool + // Group is the choke group this material belongs to. Materials sharing a + // non-empty group form an "at least one of" set. + Group string `json:"group,omitempty"` } func NewAttestationStatus(cfg *AttestationStatusOpts) (*AttestationStatus, error) { @@ -225,12 +228,15 @@ func populateMaterials(craftingState *v1.CraftingState, res *AttestationStatusRe func populateContractMaterials(inputSchemaMaterials []*pbc.CraftingSchema_Material, attsMaterial map[string]*v1.Attestation_Material, res *AttestationStatusResult, visitedMaterials map[string]struct{}) error { for _, m := range inputSchemaMaterials { // This one need to be crafter manually because it might not be in the attestation yet + // Grouped materials are enforced at the group level ("at least one of"), + // so they are not individually required. materialResult := &AttestationStatusMaterial{ Material: &Material{ Name: m.Name, Type: m.Type.String(), Annotations: pbAnnotationsToAction(m.Annotations), }, - IsOutput: m.Output, Required: !m.Optional, SkipUpload: m.SkipUpload, + IsOutput: m.Output, Required: !m.Optional && m.Group == "", SkipUpload: m.SkipUpload, + Group: m.Group, } if cm, found := attsMaterial[m.Name]; found { diff --git a/app/cli/pkg/action/attestation_status_test.go b/app/cli/pkg/action/attestation_status_test.go index 974076f97..2a662bf58 100644 --- a/app/cli/pkg/action/attestation_status_test.go +++ b/app/cli/pkg/action/attestation_status_test.go @@ -1,5 +1,5 @@ // -// Copyright 2024-2025 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. @@ -115,3 +115,42 @@ func TestPopulateContractMaterials(t *testing.T) { }) } } + +func TestPopulateContractMaterialsGroups(t *testing.T) { + state := &v1.CraftingState{ + Schema: &v1.CraftingState_InputSchema{ + InputSchema: &craftingpb.CraftingSchema{ + SchemaVersion: "v1", + Materials: []*craftingpb.CraftingSchema_Material{ + {Type: craftingpb.CraftingSchema_Material_ARTIFACT, Name: "required-one"}, + {Type: craftingpb.CraftingSchema_Material_ARTIFACT, Name: "optional-one", Optional: true}, + {Type: craftingpb.CraftingSchema_Material_SBOM_CYCLONEDX_JSON, Name: "sbom-cyclonedx", Group: "sbom"}, + {Type: craftingpb.CraftingSchema_Material_SBOM_SPDX_JSON, Name: "sbom-spdx", Group: "sbom"}, + }, + }, + }, + } + + res := &AttestationStatusResult{} + err := populateMaterials(state, res) + assert.NoError(t, err) + + byName := make(map[string]AttestationStatusMaterial, len(res.Materials)) + for _, m := range res.Materials { + byName[m.Name] = m + } + + // Ungrouped required material: Required, no group + assert.True(t, byName["required-one"].Required) + assert.Empty(t, byName["required-one"].Group) + + // Ungrouped optional material: not required, no group + assert.False(t, byName["optional-one"].Required) + assert.Empty(t, byName["optional-one"].Group) + + // Grouped materials: carry the group and are NOT individually required + assert.Equal(t, "sbom", byName["sbom-cyclonedx"].Group) + assert.False(t, byName["sbom-cyclonedx"].Required) + assert.Equal(t, "sbom", byName["sbom-spdx"].Group) + assert.False(t, byName["sbom-spdx"].Required) +} diff --git a/app/controlplane/api/gen/frontend/workflowcontract/v1/crafting_schema.ts b/app/controlplane/api/gen/frontend/workflowcontract/v1/crafting_schema.ts index a0a2524b2..2be5f6737 100644 --- a/app/controlplane/api/gen/frontend/workflowcontract/v1/crafting_schema.ts +++ b/app/controlplane/api/gen/frontend/workflowcontract/v1/crafting_schema.ts @@ -200,6 +200,12 @@ export interface CraftingSchema_Material { * Defaults to false (material will be uploaded) */ skipUpload: boolean; + /** + * Choke group: materials sharing the same non-empty group value form an + * "at least one of" set, meaning at least one member must be present in the + * attestation. Materials without a group keep their individual required/optional behavior. + */ + group: string; } export enum CraftingSchema_Material_MaterialType { @@ -1010,7 +1016,7 @@ export const CraftingSchema_Runner = { }; function createBaseCraftingSchema_Material(): CraftingSchema_Material { - return { type: 0, name: "", optional: false, output: false, annotations: [], skipUpload: false }; + return { type: 0, name: "", optional: false, output: false, annotations: [], skipUpload: false, group: "" }; } export const CraftingSchema_Material = { @@ -1033,6 +1039,9 @@ export const CraftingSchema_Material = { if (message.skipUpload === true) { writer.uint32(48).bool(message.skipUpload); } + if (message.group !== "") { + writer.uint32(58).string(message.group); + } return writer; }, @@ -1085,6 +1094,13 @@ export const CraftingSchema_Material = { message.skipUpload = reader.bool(); continue; + case 7: + if (tag !== 58) { + break; + } + + message.group = reader.string(); + continue; } if ((tag & 7) === 4 || tag === 0) { break; @@ -1102,6 +1118,7 @@ export const CraftingSchema_Material = { output: isSet(object.output) ? Boolean(object.output) : false, annotations: Array.isArray(object?.annotations) ? object.annotations.map((e: any) => Annotation.fromJSON(e)) : [], skipUpload: isSet(object.skipUpload) ? Boolean(object.skipUpload) : false, + group: isSet(object.group) ? String(object.group) : "", }; }, @@ -1117,6 +1134,7 @@ export const CraftingSchema_Material = { obj.annotations = []; } message.skipUpload !== undefined && (obj.skipUpload = message.skipUpload); + message.group !== undefined && (obj.group = message.group); return obj; }, @@ -1132,6 +1150,7 @@ export const CraftingSchema_Material = { message.output = object.output ?? false; message.annotations = object.annotations?.map((e) => Annotation.fromPartial(e)) || []; message.skipUpload = object.skipUpload ?? false; + message.group = object.group ?? ""; return message; }, }; diff --git a/app/controlplane/api/gen/jsonschema/workflowcontract.v1.CraftingSchema.Material.jsonschema.json b/app/controlplane/api/gen/jsonschema/workflowcontract.v1.CraftingSchema.Material.jsonschema.json index 7cc29ba71..089902ead 100644 --- a/app/controlplane/api/gen/jsonschema/workflowcontract.v1.CraftingSchema.Material.jsonschema.json +++ b/app/controlplane/api/gen/jsonschema/workflowcontract.v1.CraftingSchema.Material.jsonschema.json @@ -16,6 +16,10 @@ }, "type": "array" }, + "group": { + "description": "Choke group: materials sharing the same non-empty group value form an\n \"at least one of\" set, meaning at least one member must be present in the\n attestation. Materials without a group keep their individual required/optional behavior.", + "type": "string" + }, "name": { "type": "string" }, diff --git a/app/controlplane/api/gen/jsonschema/workflowcontract.v1.CraftingSchema.Material.schema.json b/app/controlplane/api/gen/jsonschema/workflowcontract.v1.CraftingSchema.Material.schema.json index 973a4c934..8546a2610 100644 --- a/app/controlplane/api/gen/jsonschema/workflowcontract.v1.CraftingSchema.Material.schema.json +++ b/app/controlplane/api/gen/jsonschema/workflowcontract.v1.CraftingSchema.Material.schema.json @@ -16,6 +16,10 @@ }, "type": "array" }, + "group": { + "description": "Choke group: materials sharing the same non-empty group value form an\n \"at least one of\" set, meaning at least one member must be present in the\n attestation. Materials without a group keep their individual required/optional behavior.", + "type": "string" + }, "name": { "type": "string" }, diff --git a/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go b/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go index a6a87cb57..b2b648761 100644 --- a/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go +++ b/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go @@ -1680,7 +1680,11 @@ type CraftingSchema_Material struct { Annotations []*Annotation `protobuf:"bytes,5,rep,name=annotations,proto3" json:"annotations,omitempty"` // If true, skip uploading the material to CAS (only record metadata) // Defaults to false (material will be uploaded) - SkipUpload bool `protobuf:"varint,6,opt,name=skip_upload,json=skipUpload,proto3" json:"skip_upload,omitempty"` + SkipUpload bool `protobuf:"varint,6,opt,name=skip_upload,json=skipUpload,proto3" json:"skip_upload,omitempty"` + // Choke group: materials sharing the same non-empty group value form an + // "at least one of" set, meaning at least one member must be present in the + // attestation. Materials without a group keep their individual required/optional behavior. + Group string `protobuf:"bytes,7,opt,name=group,proto3" json:"group,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -1762,6 +1766,13 @@ func (x *CraftingSchema_Material) GetSkipUpload() bool { return false } +func (x *CraftingSchema_Material) GetGroup() string { + if x != nil { + return x.Group + } + return "" +} + type PolicyAttachment_MaterialSelector struct { state protoimpl.MessageState `protogen:"open.v1"` // material name @@ -1987,7 +1998,7 @@ var File_workflowcontract_v1_crafting_schema_proto protoreflect.FileDescriptor const file_workflowcontract_v1_crafting_schema_proto_rawDesc = "" + "\n" + - ")workflowcontract/v1/crafting_schema.proto\x12\x13workflowcontract.v1\x1a\x1bbuf/validate/validate.proto\"\xe7\x10\n" + + ")workflowcontract/v1/crafting_schema.proto\x12\x13workflowcontract.v1\x1a\x1bbuf/validate/validate.proto\"\x94\x12\n" + "\x0eCraftingSchema\x122\n" + "\x0eschema_version\x18\x01 \x01(\tB\v\xbaH\x06r\x04\n" + "\x02v1\x18\x01R\rschemaVersion\x12N\n" + @@ -2010,8 +2021,7 @@ const file_workflowcontract_v1_crafting_schema_proto_rawDesc = "" + "\x0fDAGGER_PIPELINE\x10\x06\x12\x15\n" + "\x11TEAMCITY_PIPELINE\x10\a\x12\x13\n" + "\x0fTEKTON_PIPELINE\x10\b\x12\x15\n" + - "\x11CHAINLOOP_SANDBOX\x10\t:\x02\x18\x01\x1a\xb2\n" + - "\n" + + "\x11CHAINLOOP_SANDBOX\x10\t:\x02\x18\x01\x1a\xdf\v\n" + "\bMaterial\x12[\n" + "\x04type\x18\x01 \x01(\x0e29.workflowcontract.v1.CraftingSchema.Material.MaterialTypeB\f\xbaH\a\x82\x01\x04\x10\x01 \x00\x18\x01R\x04type\x12\x99\x01\n" + "\x04name\x18\x02 \x01(\tB\x84\x01\xbaH\x7f\xba\x01|\n" + @@ -2020,7 +2030,9 @@ const file_workflowcontract_v1_crafting_schema_proto_rawDesc = "" + "\x06output\x18\x04 \x01(\bB\x02\x18\x01R\x06output\x12E\n" + "\vannotations\x18\x05 \x03(\v2\x1f.workflowcontract.v1.AnnotationB\x02\x18\x01R\vannotations\x12\x1f\n" + "\vskip_upload\x18\x06 \x01(\bR\n" + - "skipUpload\"\x84\a\n" + + "skipUpload\x12\xaa\x01\n" + + "\x05group\x18\a \x01(\tB\x93\x01\xbaH\x8f\x01\xba\x01\x8b\x01\n" + + "\x0egroup.dns-1123\x12:must contain only lowercase letters, numbers, and hyphens.\x1a=this == '' || this.matches('^[a-z0-9]([-a-z0-9]*[a-z0-9])?$')R\x05group\"\x84\a\n" + "\fMaterialType\x12\x1d\n" + "\x19MATERIAL_TYPE_UNSPECIFIED\x10\x00\x12\n" + "\n" + diff --git a/app/controlplane/api/workflowcontract/v1/crafting_schema.proto b/app/controlplane/api/workflowcontract/v1/crafting_schema.proto index f2455d2bb..0d326fc7c 100644 --- a/app/controlplane/api/workflowcontract/v1/crafting_schema.proto +++ b/app/controlplane/api/workflowcontract/v1/crafting_schema.proto @@ -97,6 +97,14 @@ message CraftingSchema { // If true, skip uploading the material to CAS (only record metadata) // Defaults to false (material will be uploaded) bool skip_upload = 6; + // Choke group: materials sharing the same non-empty group value form an + // "at least one of" set, meaning at least one member must be present in the + // attestation. Materials without a group keep their individual required/optional behavior. + string group = 7 [(buf.validate.field).cel = { + message: "must contain only lowercase letters, numbers, and hyphens." + expression: "this == '' || this.matches('^[a-z0-9]([-a-z0-9]*[a-z0-9])?$')" + id: "group.dns-1123" + }]; enum MaterialType { MATERIAL_TYPE_UNSPECIFIED = 0; diff --git a/app/controlplane/pkg/unmarshal/unmarshal.go b/app/controlplane/pkg/unmarshal/unmarshal.go index d9960975c..cea11baf6 100644 --- a/app/controlplane/pkg/unmarshal/unmarshal.go +++ b/app/controlplane/pkg/unmarshal/unmarshal.go @@ -61,14 +61,19 @@ func (v *validatorAdapter) Validate(msg proto.Message) error { var yamlValidator = &validatorAdapter{validator: protovalidate.GlobalValidator} func FromRaw(body []byte, format RawFormat, out proto.Message, doValidate bool) error { + // DiscardUnknown allows contracts to include fields added in newer proto + // versions without breaking older CLIs that haven't been updated yet. Unlike + // the binary wire format, protojson/protoyaml error on unknown fields by default. + jsonOpts := protojson.UnmarshalOptions{DiscardUnknown: true} + switch format { case RawFormatJSON: - if err := protojson.Unmarshal(body, out); err != nil { + if err := jsonOpts.Unmarshal(body, out); err != nil { return fmt.Errorf("error unmarshalling raw message: %w", err) } case RawFormatYAML: // protoyaml allows validating the contract while unmarshalling - yamlOpts := protoyaml.UnmarshalOptions{} + yamlOpts := protoyaml.UnmarshalOptions{DiscardUnknown: true} if doValidate { yamlOpts.Validator = yamlValidator } @@ -84,7 +89,7 @@ func FromRaw(body []byte, format RawFormat, out proto.Message, doValidate bool) return fmt.Errorf("error unmarshalling raw message: %w", err) } - if err := protojson.Unmarshal(jsonRawData, out); err != nil { + if err := jsonOpts.Unmarshal(jsonRawData, out); err != nil { return fmt.Errorf("error unmarshalling raw message: %w", err) } default: diff --git a/app/controlplane/pkg/unmarshal/unmarshal_test.go b/app/controlplane/pkg/unmarshal/unmarshal_test.go index 1fa541082..8d986f0dc 100644 --- a/app/controlplane/pkg/unmarshal/unmarshal_test.go +++ b/app/controlplane/pkg/unmarshal/unmarshal_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. @@ -19,10 +19,104 @@ import ( "os" "testing" + schemav1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestFromRawGroupAndUnknownFields(t *testing.T) { + // v2 contract with a choke group, expressed in the three supported formats + yamlContract := []byte(`apiVersion: chainloop.dev/v1 +kind: Contract +metadata: + name: test-contract +spec: + materials: + - name: a + type: ARTIFACT + group: choice + - name: b + type: ARTIFACT + group: choice +`) + jsonContract := []byte(`{ + "apiVersion": "chainloop.dev/v1", + "kind": "Contract", + "metadata": {"name": "test-contract"}, + "spec": {"materials": [ + {"name": "a", "type": "ARTIFACT", "group": "choice"}, + {"name": "b", "type": "ARTIFACT", "group": "choice"} + ]} +}`) + cueContract := []byte(`apiVersion: "chainloop.dev/v1" +kind: "Contract" +metadata: name: "test-contract" +spec: materials: [ + {name: "a", type: "ARTIFACT", group: "choice"}, + {name: "b", type: "ARTIFACT", group: "choice"}, +] +`) + + formats := []struct { + name string + format RawFormat + body []byte + }{ + {"yaml", RawFormatYAML, yamlContract}, + {"json", RawFormatJSON, jsonContract}, + {"cue", RawFormatCUE, cueContract}, + } + + t.Run("group round-trips", func(t *testing.T) { + for _, f := range formats { + t.Run(f.name, func(t *testing.T) { + out := &schemav1.CraftingSchemaV2{} + require.NoError(t, FromRaw(f.body, f.format, out, true)) + materials := out.GetSpec().GetMaterials() + require.Len(t, materials, 2) + assert.Equal(t, "choice", materials[0].GetGroup()) + assert.Equal(t, "choice", materials[1].GetGroup()) + }) + } + }) + + // An unknown field (e.g. one added by a newer CLI) must not break parsing. + t.Run("unknown fields are discarded", func(t *testing.T) { + yamlUnknown := []byte(`apiVersion: chainloop.dev/v1 +kind: Contract +metadata: + name: test-contract +spec: + materials: + - name: a + type: ARTIFACT + somethingFromTheFuture: true +`) + jsonUnknown := []byte(`{ + "apiVersion": "chainloop.dev/v1", + "kind": "Contract", + "metadata": {"name": "test-contract"}, + "spec": {"materials": [{"name": "a", "type": "ARTIFACT", "somethingFromTheFuture": true}]} +}`) + + for _, f := range []struct { + name string + format RawFormat + body []byte + }{ + {"yaml", RawFormatYAML, yamlUnknown}, + {"json", RawFormatJSON, jsonUnknown}, + } { + t.Run(f.name, func(t *testing.T) { + out := &schemav1.CraftingSchemaV2{} + require.NoError(t, FromRaw(f.body, f.format, out, true)) + require.Len(t, out.GetSpec().GetMaterials(), 1) + assert.Equal(t, "a", out.GetSpec().GetMaterials()[0].GetName()) + }) + } + }) +} + func TestIdentifyFormat(t *testing.T) { testData := []struct { filename string diff --git a/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations.go b/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations.go index c7478f56b..9cf22a389 100644 --- a/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations.go +++ b/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations.go @@ -16,6 +16,7 @@ package v1 import ( + "errors" "fmt" "strings" @@ -40,15 +41,48 @@ func (state *CraftingState) ValidateComplete(dryRun bool) error { var missing []string expectedMaterials := state.GetMaterials() craftedMaterials := state.GetAttestation().GetMaterials() + + // Choke groups: materials sharing the same non-empty group form an + // "at least one of" set. We track the members of each group and whether + // at least one of them has been crafted. + groupMembers := make(map[string][]string) + groupSatisfied := make(map[string]bool) + // Preserve a stable order of groups for deterministic error messages + var groupOrder []string + // Iterate on the expected materials for _, m := range expectedMaterials { - if _, ok := craftedMaterials[m.Name]; !ok && !m.Optional { + _, crafted := craftedMaterials[m.Name] + + // Grouped materials are enforced at the group level, not individually. + if m.Group != "" { + if _, seen := groupMembers[m.Group]; !seen { + groupOrder = append(groupOrder, m.Group) + } + groupMembers[m.Group] = append(groupMembers[m.Group], m.Name) + if crafted { + groupSatisfied[m.Group] = true + } + continue + } + + if !crafted && !m.Optional { missing = append(missing, m.Name) } } + var errs []string if len(missing) > 0 { - return fmt.Errorf("some materials have not been crafted yet: %s", strings.Join(missing, ", ")) + errs = append(errs, fmt.Sprintf("some materials have not been crafted yet: %s", strings.Join(missing, ", "))) + } + for _, group := range groupOrder { + if !groupSatisfied[group] { + errs = append(errs, fmt.Sprintf("at least one material from group %q is required: %s", group, strings.Join(groupMembers[group], ", "))) + } + } + + if len(errs) > 0 { + return errors.New(strings.Join(errs, "; ")) } return nil diff --git a/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations_test.go b/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations_test.go index 9e5d7b510..1fe93a032 100644 --- a/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations_test.go +++ b/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations_test.go @@ -1,5 +1,5 @@ // -// Copyright 2024-2025 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. @@ -23,6 +23,128 @@ import ( "github.com/stretchr/testify/require" ) +func TestCraftingStateValidateComplete(t *testing.T) { + // helper to build a state from a list of contract materials and the set of crafted material names + newState := func(materials []*workflowcontract.CraftingSchema_Material, crafted ...string) *CraftingState { + craftedMap := make(map[string]*Attestation_Material, len(crafted)) + for _, name := range crafted { + craftedMap[name] = &Attestation_Material{} + } + return &CraftingState{ + Schema: &CraftingState_InputSchema{ + InputSchema: &workflowcontract.CraftingSchema{Materials: materials}, + }, + Attestation: &Attestation{Materials: craftedMap}, + } + } + + art := workflowcontract.CraftingSchema_Material_ARTIFACT + + testCases := []struct { + name string + materials []*workflowcontract.CraftingSchema_Material + crafted []string + wantErr bool + // substrings expected in the error message (when wantErr) + errContains []string + }{ + { + name: "required material present", + materials: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "a"}, + }, + crafted: []string{"a"}, + }, + { + name: "required material missing", + materials: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "a"}, + }, + wantErr: true, + errContains: []string{"a"}, + }, + { + name: "optional material missing is allowed", + materials: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "a", Optional: true}, + }, + }, + { + name: "group satisfied with one member", + materials: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "a", Group: "choice"}, + {Type: art, Name: "b", Group: "choice"}, + }, + crafted: []string{"b"}, + }, + { + name: "group unsatisfied when no member present", + materials: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "a", Group: "choice"}, + {Type: art, Name: "b", Group: "choice"}, + }, + wantErr: true, + errContains: []string{"choice", "a", "b"}, + }, + { + name: "group enforced even if members are marked optional", + materials: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "a", Group: "choice", Optional: true}, + {Type: art, Name: "b", Group: "choice", Optional: true}, + }, + wantErr: true, + errContains: []string{"choice"}, + }, + { + name: "multiple groups: only the unsatisfied one is reported", + materials: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "a", Group: "g1"}, + {Type: art, Name: "b", Group: "g1"}, + {Type: art, Name: "c", Group: "g2"}, + {Type: art, Name: "d", Group: "g2"}, + }, + crafted: []string{"a"}, + wantErr: true, + errContains: []string{"g2", "c", "d"}, + }, + { + name: "mixed grouped and ungrouped, all satisfied", + materials: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "req"}, + {Type: art, Name: "opt", Optional: true}, + {Type: art, Name: "a", Group: "choice"}, + {Type: art, Name: "b", Group: "choice"}, + }, + crafted: []string{"req", "a"}, + }, + { + name: "mixed grouped and ungrouped, ungrouped required missing", + materials: []*workflowcontract.CraftingSchema_Material{ + {Type: art, Name: "req"}, + {Type: art, Name: "a", Group: "choice"}, + {Type: art, Name: "b", Group: "choice"}, + }, + crafted: []string{"a"}, + wantErr: true, + errContains: []string{"req"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := newState(tc.materials, tc.crafted...).ValidateComplete(true) + if !tc.wantErr { + require.NoError(t, err) + return + } + require.Error(t, err) + for _, sub := range tc.errContains { + assert.ErrorContains(t, err, sub) + } + }) + } +} + func TestCraftingStateGetEnvAllowList(t *testing.T) { testCases := []struct { name string From 891c1c95e326a3beb2a28274a0bd57c42480d272 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Tue, 23 Jun 2026 17:30:11 +0200 Subject: [PATCH 2/4] docs: add material choke group contract example Assisted-by: Claude Code Signed-off-by: Jose I. Paris Chainloop-Trace-Sessions: ec69dde4-acef-48c8-b319-76774e77c877 --- .../contracts/material-groups/contract.yaml | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 docs/examples/contracts/material-groups/contract.yaml diff --git a/docs/examples/contracts/material-groups/contract.yaml b/docs/examples/contracts/material-groups/contract.yaml new file mode 100644 index 000000000..f9961935f --- /dev/null +++ b/docs/examples/contracts/material-groups/contract.yaml @@ -0,0 +1,27 @@ +# This contract demonstrates "at least one of" material semantics using choke groups. +# +# Materials that share the same non-empty `group` value form a choke group: +# at least one member of the group must be present in the attestation, otherwise +# the attestation cannot be pushed. This makes the "one or another" intent explicit, +# instead of marking every material as `optional` and enforcing presence with a +# separate gated policy. +# +# In the example below the attestation must contain an SBOM in EITHER CycloneDX +# OR SPDX format (at least one of them), plus a mandatory container image. +apiVersion: chainloop.dev/v1 +kind: Contract +metadata: + name: material-groups-contract + description: Example contract requiring at least one SBOM format via a choke group +spec: + materials: + # Regular required material (no group): must always be present. + - name: container + type: CONTAINER_IMAGE + # Choke group "sbom": at least one of the following must be present. + - name: sbom-cyclonedx + type: SBOM_CYCLONEDX_JSON + group: sbom + - name: sbom-spdx + type: SBOM_SPDX_JSON + group: sbom From ccade4ddedfb3fea4dcd5ac6158d626bfa3d3b07 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Tue, 23 Jun 2026 18:13:05 +0200 Subject: [PATCH 3/4] test(controlplane): make invalid contract fixture fail on value not unknown field With unknown-field tolerance (DiscardUnknown) in contract parsing, a contract whose only problem was an unknown field is now valid. Update the invalid YAML fixture to fail validation for a real reason (invalid material name) so TestIdentifyAndValidateRawContract keeps asserting a validation error. Assisted-by: Claude Code Signed-off-by: Jose I. Paris Chainloop-Trace-Sessions: ec69dde4-acef-48c8-b319-76774e77c877 --- .../pkg/biz/testdata/contracts/invalid_contract.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controlplane/pkg/biz/testdata/contracts/invalid_contract.yaml b/app/controlplane/pkg/biz/testdata/contracts/invalid_contract.yaml index b6f401dfe..730c5177d 100644 --- a/app/controlplane/pkg/biz/testdata/contracts/invalid_contract.yaml +++ b/app/controlplane/pkg/biz/testdata/contracts/invalid_contract.yaml @@ -1,2 +1,4 @@ schemaVersion: v1 -notAContract: true \ No newline at end of file +materials: + - type: CONTAINER_IMAGE + name: INVALID_NAME From 1ed4be480aebc3623484cf4d87031fa55a1e35f9 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Wed, 24 Jun 2026 14:40:36 +0200 Subject: [PATCH 4/4] feat(cli): render choke-group materials under a group header Group attestation materials together in "attestation status" under a header that states the "at least one of" rule and whether the group is satisfied, with members indented beneath it, instead of repeating the requirement on each material as if they were independent. Apply the same group/rule wording to "attestation add". Assisted-by: Claude Code Signed-off-by: Jose I. Paris Chainloop-Trace-Sessions: ec69dde4-acef-48c8-b319-76774e77c877 --- app/cli/cmd/attestation_add.go | 3 +- app/cli/cmd/attestation_status.go | 137 ++++++++++++++++++++---------- 2 files changed, 96 insertions(+), 44 deletions(-) diff --git a/app/cli/cmd/attestation_add.go b/app/cli/cmd/attestation_add.go index 5c7009cf5..f41c73acd 100644 --- a/app/cli/cmd/attestation_add.go +++ b/app/cli/cmd/attestation_add.go @@ -180,7 +180,8 @@ func displayMaterialInfo(status *action.AttestationStatusMaterial, policyEvaluat mt.AppendRow(table.Row{"Required", hBool(status.Required)}) if status.Group != "" { - mt.AppendRow(table.Row{"Group", fmt.Sprintf("%s (at least one of the group required)", status.Group)}) + mt.AppendRow(table.Row{"Group", status.Group}) + mt.AppendRow(table.Row{"Rule", "at least one of the group required"}) } if status.IsOutput { diff --git a/app/cli/cmd/attestation_status.go b/app/cli/cmd/attestation_status.go index 1968525ed..7ac1dd5f1 100644 --- a/app/cli/cmd/attestation_status.go +++ b/app/cli/cmd/attestation_status.go @@ -190,66 +190,117 @@ func materialsTable(status *action.AttestationStatusResult, w io.Writer, full bo return nil } - // Sort materials by name for consistent output - slices.SortFunc(status.Materials, func(a, b action.AttestationStatusMaterial) int { - return strings.Compare(a.Name, b.Name) - }) + // Partition materials into standalone (ungrouped) ones and choke groups. + // Grouped materials are rendered together under a group header so it is + // clear they form an "at least one of" set rather than independent materials. + var ungrouped []action.AttestationStatusMaterial + groupedBy := make(map[string][]action.AttestationStatusMaterial) + var groupOrder []string + for _, m := range status.Materials { + if m.Group == "" { + ungrouped = append(ungrouped, m) + continue + } + if _, ok := groupedBy[m.Group]; !ok { + groupOrder = append(groupOrder, m.Group) + } + groupedBy[m.Group] = append(groupedBy[m.Group], m) + } + + byName := func(a, b action.AttestationStatusMaterial) int { return strings.Compare(a.Name, b.Name) } + slices.SortFunc(ungrouped, byName) + slices.Sort(groupOrder) mt := output.NewTableWriterWithWriter(w) mt.SetTitle("Materials") - for _, m := range status.Materials { - mt.AppendRow(table.Row{"Name", m.Name}) - mt.AppendRow(table.Row{"Type", m.Type}) - mt.AppendRow(table.Row{"Set", hBool(m.Set)}) - mt.AppendRow(table.Row{"Required", hBool(m.Required)}) - if m.Group != "" { - mt.AppendRow(table.Row{"Group", fmt.Sprintf("%s (at least one of the group required)", m.Group)}) - } - if m.IsOutput { - mt.AppendRow(table.Row{"Is output", "Yes"}) - } - if m.SkipUpload { - mt.AppendRow(table.Row{"Skip upload", "Yes"}) - } + for _, m := range ungrouped { + appendMaterialRows(mt, m, status, full, false) + mt.AppendSeparator() + } - if full { - if m.Value != "" { - v := m.Value - if m.Tag != "" { - v = fmt.Sprintf("%s:%s", v, m.Tag) - } - mt.AppendRow(table.Row{"Value", wrap.String(v, 100)}) - } + for _, g := range groupOrder { + members := groupedBy[g] + slices.SortFunc(members, byName) - if m.Hash != "" { - mt.AppendRow(table.Row{"Digest", m.Hash}) + // A choke group is satisfied as soon as one of its members is set. + satisfied := false + for _, m := range members { + if m.Set { + satisfied = true + break } } - if len(m.Annotations) > 0 { - mt.AppendRow(table.Row{"Annotations", "------"}) - for _, a := range m.Annotations { - value := a.Value - if value == "" { - value = NotSet - } + mt.AppendRow(table.Row{"Group", g}) + mt.AppendRow(table.Row{"Rule", fmt.Sprintf("at least one of %d required", len(members))}) + mt.AppendRow(table.Row{"Satisfied", hBool(satisfied)}) + mt.AppendSeparator() + + for _, m := range members { + appendMaterialRows(mt, m, status, full, true) + mt.AppendSeparator() + } + } + + mt.Render() + + return nil +} + +// appendMaterialRows renders a single material as a block of rows. When the +// material belongs to a choke group, its name is indented under the group +// header and the per-material "Required" row is omitted (the group header +// carries the "at least one of" requirement instead). +func appendMaterialRows(mt table.Writer, m action.AttestationStatusMaterial, status *action.AttestationStatusResult, full, grouped bool) { + name := m.Name + if grouped { + name = "↳ " + name + } + mt.AppendRow(table.Row{"Name", name}) + mt.AppendRow(table.Row{"Type", m.Type}) + mt.AppendRow(table.Row{"Set", hBool(m.Set)}) + if !grouped { + mt.AppendRow(table.Row{"Required", hBool(m.Required)}) + } + if m.IsOutput { + mt.AppendRow(table.Row{"Is output", "Yes"}) + } + if m.SkipUpload { + mt.AppendRow(table.Row{"Skip upload", "Yes"}) + } - mt.AppendRow(table.Row{"", fmt.Sprintf("%s: %s", a.Name, value)}) + if full { + if m.Value != "" { + v := m.Value + if m.Tag != "" { + v = fmt.Sprintf("%s:%s", v, m.Tag) } + mt.AppendRow(table.Row{"Value", wrap.String(v, 100)}) } - evs := status.PolicyEvaluations[m.Name] - if len(evs) > 0 { - mt.AppendRow(table.Row{"Policies", "------"}) - policiesTable(evs, mt, flagDebug) + if m.Hash != "" { + mt.AppendRow(table.Row{"Digest", m.Hash}) } + } - mt.AppendSeparator() + if len(m.Annotations) > 0 { + mt.AppendRow(table.Row{"Annotations", "------"}) + for _, a := range m.Annotations { + value := a.Value + if value == "" { + value = NotSet + } + + mt.AppendRow(table.Row{"", fmt.Sprintf("%s: %s", a.Name, value)}) + } } - mt.Render() - return nil + evs := status.PolicyEvaluations[m.Name] + if len(evs) > 0 { + mt.AppendRow(table.Row{"Policies", "------"}) + policiesTable(evs, mt, flagDebug) + } } func hBool(b bool) string {