refactor(cli): store material info in crafting state (#1714)

Signed-off-by: Miguel Martinez <[email protected]>

Author: migmartri

app/cli/cmd/attestation_add.go

@@ -1,5 +1,5 @@
 //
-// Copyright 2023 The Chainloop Authors.
+// Copyright 2024-2025 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.
@@ -62,7 +62,7 @@ func newAttestationAddCmd() *cobra.Command {
 
 			return nil
 		},
-		RunE: func(cmd *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, _ []string) error {
 			a, err := action.NewAttestationAdd(
 				&action.AttestationAddOpts{
 					ActionsOpts:        actionOpts,
@@ -89,7 +89,9 @@ func newAttestationAddCmd() *cobra.Command {
 			// optimistic locking. We retry the operation if the state has changed since we last read it.
 			return runWithBackoffRetry(
 				func() error {
-					if err := a.Run(cmd.Context(), attestationID, name, value, kind, annotations); err != nil {
+					// TODO: take the material output and show render it
+					_, err := a.Run(cmd.Context(), attestationID, name, value, kind, annotations)
+					if err != nil {
 						return err
 					}
 

app/cli/internal/action/attestation_add.go

@@ -1,5 +1,5 @@
 //
-// Copyright 2024 The Chainloop Authors.
+// Copyright 2024-2025 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.
@@ -21,10 +21,10 @@ import (
 	"fmt"
 
 	pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1"
-	schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1"
 	"github.com/chainloop-dev/chainloop/internal/casclient"
 	"github.com/chainloop-dev/chainloop/internal/grpcconn"
 	"github.com/chainloop-dev/chainloop/pkg/attestation/crafter"
+	api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1"
 	"google.golang.org/grpc"
 )
 
@@ -73,22 +73,22 @@ func NewAttestationAdd(cfg *AttestationAddOpts) (*AttestationAdd, error) {
 
 var ErrAttestationNotInitialized = errors.New("attestation not yet initialized")
 
-func (action *AttestationAdd) Run(ctx context.Context, attestationID, materialName, materialValue, materialType string, annotations map[string]string) error {
+func (action *AttestationAdd) Run(ctx context.Context, attestationID, materialName, materialValue, materialType string, annotations map[string]string) (*AttestationStatusMaterial, error) {
 	// initialize the crafter. If attestation-id is provided we assume the attestation is performed using remote state
 	crafter, err := newCrafter(&newCrafterStateOpts{enableRemoteState: (attestationID != ""), localStatePath: action.localStatePath}, action.CPConnection, action.newCrafterOpts.opts...)
 	if err != nil {
-		return fmt.Errorf("failed to load crafter: %w", err)
+		return nil, fmt.Errorf("failed to load crafter: %w", err)
 	}
 
 	if initialized, err := crafter.AlreadyInitialized(ctx, attestationID); err != nil {
-		return fmt.Errorf("checking if attestation is already initialized: %w", err)
+		return nil, fmt.Errorf("checking if attestation is already initialized: %w", err)
 	} else if !initialized {
-		return ErrAttestationNotInitialized
+		return nil, ErrAttestationNotInitialized
 	}
 
 	if err := crafter.LoadCraftingState(ctx, attestationID); err != nil {
 		action.Logger.Err(err).Msg("loading existing attestation")
-		return err
+		return nil, err
 	}
 
 	// Default to inline CASBackend and override if we are not in dry-run mode
@@ -106,11 +106,11 @@ func (action *AttestationAdd) Run(ctx context.Context, attestationID, materialNa
 			},
 		)
 		if err != nil {
-			return err
+			return nil, fmt.Errorf("getting upload creds: %w", err)
 		}
 		b := creds.GetResult().GetBackend()
 		if b == nil {
-			return fmt.Errorf("no backend found in upload creds")
+			return nil, fmt.Errorf("no backend found in upload creds")
 		}
 		casBackend.Name = b.Provider
 		casBackend.MaxSize = b.GetLimits().MaxBytes
@@ -127,7 +127,7 @@ func (action *AttestationAdd) Run(ctx context.Context, attestationID, materialNa
 
 			artifactCASConn, err := grpcconn.New(action.casURI, creds.Result.Token, opts...)
 			if err != nil {
-				return err
+				return nil, fmt.Errorf("creating CAS connection: %w", err)
 			}
 			defer artifactCASConn.Close()
 
@@ -141,37 +141,42 @@ func (action *AttestationAdd) Run(ctx context.Context, attestationID, materialNa
 	// 2. If materialName is not empty, check if the material is in the contract. If it is, add material from contract
 	// 2.1. If materialType is empty, try to guess the material kind with auto-detected kind and materialName
 	// 3. If materialType is not empty, add material contract free with materialType and materialName
-	var kind schemaapi.CraftingSchema_Material_MaterialType
+	var mt *api.Attestation_Material
 	switch {
 	case materialName == "" && materialType == "":
-		kind, err = crafter.AddMaterialContactFreeWithAutoDetectedKind(ctx, attestationID, "", materialValue, casBackend, annotations)
+		mt, err = crafter.AddMaterialContactFreeWithAutoDetectedKind(ctx, attestationID, "", materialValue, casBackend, annotations)
 		if err != nil {
-			return fmt.Errorf("adding material: %w", err)
+			return nil, fmt.Errorf("adding material: %w", err)
 		}
-		action.Logger.Info().Str("kind", kind.String()).Msg("material kind detected")
+		action.Logger.Info().Str("kind", mt.MaterialType.String()).Msg("material kind detected")
 	case materialName != "":
 		switch {
 		// If the material is in the contract, add it from the contract
 		case crafter.IsMaterialInContract(materialName):
-			err = crafter.AddMaterialFromContract(ctx, attestationID, materialName, materialValue, casBackend, annotations)
+			mt, err = crafter.AddMaterialFromContract(ctx, attestationID, materialName, materialValue, casBackend, annotations)
 		// If the material is not in the contract and the materialType is not provided, add material contract free with auto-detected kind, guessing the kind
 		case materialType == "":
-			kind, err = crafter.AddMaterialContactFreeWithAutoDetectedKind(ctx, attestationID, materialName, materialValue, casBackend, annotations)
+			mt, err = crafter.AddMaterialContactFreeWithAutoDetectedKind(ctx, attestationID, materialName, materialValue, casBackend, annotations)
 			if err != nil {
-				return fmt.Errorf("adding material: %w", err)
+				return nil, fmt.Errorf("adding material: %w", err)
 			}
-			action.Logger.Info().Str("kind", kind.String()).Msg("material kind detected")
+			action.Logger.Info().Str("kind", mt.MaterialType.String()).Msg("material kind detected")
 		// If the material is not in the contract and has a materialType, add material contract free with the provided materialType
 		default:
-			err = crafter.AddMaterialContractFree(ctx, attestationID, materialType, materialName, materialValue, casBackend, annotations)
+			mt, err = crafter.AddMaterialContractFree(ctx, attestationID, materialType, materialName, materialValue, casBackend, annotations)
 		}
 	default:
-		err = crafter.AddMaterialContractFree(ctx, attestationID, materialType, materialName, materialValue, casBackend, annotations)
+		mt, err = crafter.AddMaterialContractFree(ctx, attestationID, materialType, materialName, materialValue, casBackend, annotations)
 	}
 
 	if err != nil {
-		return fmt.Errorf("adding material: %w", err)
+		return nil, fmt.Errorf("adding material: %w", err)
 	}
 
-	return nil
+	materialResult, err := attMaterialToAction(mt)
+	if err != nil {
+		return nil, fmt.Errorf("converting material to action: %w", err)
+	}
+
+	return materialResult, nil
 }

app/cli/internal/action/attestation_status.go

@@ -45,18 +45,18 @@ type AttestationStatus struct {
 }
 
 type AttestationStatusResult struct {
-	AttestationID               string                            `json:"attestationID"`
-	InitializedAt               *time.Time                        `json:"initializedAt"`
-	WorkflowMeta                *AttestationStatusWorkflowMeta    `json:"workflowMeta"`
-	Materials                   []AttestationStatusResultMaterial `json:"materials"`
-	EnvVars                     map[string]string                 `json:"envVars"`
-	RunnerContext               *AttestationResultRunnerContext   `json:"runnerContext"`
-	DryRun                      bool                              `json:"dryRun"`
-	Annotations                 []*Annotation                     `json:"annotations"`
-	IsPushed                    bool                              `json:"isPushed"`
-	PolicyEvaluations           map[string][]*PolicyEvaluation    `json:"policy_evaluations,omitempty"`
-	HasPolicyViolations         bool                              `json:"has_policy_violations"`
-	MustBlockOnPolicyViolations bool                              `json:"must_block_on_policy_violations"`
+	AttestationID               string                          `json:"attestationID"`
+	InitializedAt               *time.Time                      `json:"initializedAt"`
+	WorkflowMeta                *AttestationStatusWorkflowMeta  `json:"workflowMeta"`
+	Materials                   []AttestationStatusMaterial     `json:"materials"`
+	EnvVars                     map[string]string               `json:"envVars"`
+	RunnerContext               *AttestationResultRunnerContext `json:"runnerContext"`
+	DryRun                      bool                            `json:"dryRun"`
+	Annotations                 []*Annotation                   `json:"annotations"`
+	IsPushed                    bool                            `json:"isPushed"`
+	PolicyEvaluations           map[string][]*PolicyEvaluation  `json:"policy_evaluations,omitempty"`
+	HasPolicyViolations         bool                            `json:"has_policy_violations"`
+	MustBlockOnPolicyViolations bool                            `json:"must_block_on_policy_violations"`
 	// This might only be set if the attestation is pushed
 	Digest string `json:"digest"`
 }
@@ -71,7 +71,7 @@ type AttestationStatusWorkflowMeta struct {
 	ProjectVersion                                                                *ProjectVersion
 }
 
-type AttestationStatusResultMaterial struct {
+type AttestationStatusMaterial struct {
 	*Material
 	Set, IsOutput, Required bool
 }
@@ -255,7 +255,8 @@ func populateMaterials(craftingState *v1.CraftingState, res *AttestationStatusRe
 // populateContractMaterials populates the materials that are defined in the contract schema
 func populateContractMaterials(inputSchemaMaterials []*pbc.CraftingSchema_Material, attsMaterial map[string]*v1.Attestation_Material, res *AttestationStatusResult, visitedMaterials map[string]struct{}) error {
 	for _, m := range inputSchemaMaterials {
-		materialResult := &AttestationStatusResultMaterial{
+		// This one need to be crafter manually because it might not be in the attestation yet
+		materialResult := &AttestationStatusMaterial{
 			Material: &Material{
 				Name: m.Name, Type: m.Type.String(),
 				Annotations: pbAnnotationsToAction(m.Annotations),
@@ -284,26 +285,37 @@ func populateAdditionalMaterials(attsMaterials map[string]*v1.Attestation_Materi
 
 		// No need to check for name collisions, as it is not defined in the contract schema and it's
 		// autogenerated by the crafter
-		materialResult := &AttestationStatusResultMaterial{
-			Material: &Material{
-				Name:        name,
-				Type:        m.GetMaterialType().String(),
-				Annotations: stateAnnotationToAction(m.Annotations),
-			},
-			// No need to check if the material is optional or not, as it is not defined in the contract schema
-			// TODO: Make IsOutput configurable
-			IsOutput: false, Required: false,
-		}
-
-		if err := setMaterialValue(m, materialResult); err != nil {
+		materialResult, err := attMaterialToAction(m)
+		if err != nil {
 			return fmt.Errorf("setting material value: %w", err)
 		}
 
 		res.Materials = append(res.Materials, *materialResult)
+		visitedMaterials[name] = struct{}{}
 	}
+
 	return nil
 }
 
+// attMaterialToAction converts the attestation material to the action material
+func attMaterialToAction(m *v1.Attestation_Material) (*AttestationStatusMaterial, error) {
+	res := &AttestationStatusMaterial{
+		Material: &Material{
+			Name:        m.GetId(),
+			Type:        m.GetMaterialType().String(),
+			Annotations: stateAnnotationToAction(m.Annotations),
+		},
+		IsOutput: m.Output,
+		Required: m.Required,
+	}
+
+	if err := setMaterialValue(m, res); err != nil {
+		return nil, fmt.Errorf("setting material value: %w", err)
+	}
+
+	return res, nil
+}
+
 func pbAnnotationsToAction(in []*pbc.Annotation) []*Annotation {
 	res := make([]*Annotation, 0, len(in))
 
@@ -331,7 +343,7 @@ func stateAnnotationToAction(in map[string]string) []*Annotation {
 	return res
 }
 
-func setMaterialValue(w *v1.Attestation_Material, o *AttestationStatusResultMaterial) error {
+func setMaterialValue(w *v1.Attestation_Material, o *AttestationStatusMaterial) error {
 	switch m := w.GetM().(type) {
 	case *v1.Attestation_Material_String_:
 		o.Value = m.String_.GetValue()

app/cli/internal/action/attestation_status_test.go

@@ -1,5 +1,5 @@
 //
-// Copyright 2024 The Chainloop Authors.
+// Copyright 2024-2025 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,9 +67,9 @@ func TestPopulateContractMaterials(t *testing.T) {
 				Attestation: &v1.Attestation{
 					Materials: map[string]*v1.Attestation_Material{
 						"vex-file": {
+							Id: "random",
 							M: &v1.Attestation_Material_Artifact_{
 								Artifact: &v1.Attestation_Material_Artifact{
-									Id:      "random",
 									Name:    "vex-file",
 									Digest:  "random-digest",
 									Content: []byte("random-content"),
@@ -79,9 +79,9 @@ func TestPopulateContractMaterials(t *testing.T) {
 							UploadedToCas: true,
 						},
 						"other-file": {
+							Id: "random",
 							M: &v1.Attestation_Material_Artifact_{
 								Artifact: &v1.Attestation_Material_Artifact{
-									Id:      "random",
 									Name:    "other-file",
 									Digest:  "random-digest-2",
 									Content: []byte("random-content-2"),