fix(craft): Wrap upload and craft errors to raise errors

internal/attestation/crafter/crafter.go

@@ -517,26 +517,23 @@ func (c *Crafter) AddMaterialFromContract(ctx context.Context, attestationID, ke
 // AddMaterialContactFreeAutomatic adds a material to the crafting state checking the incoming material matches any of the
 // supported types in validation order. If the material is not found it will return an error.
 func (c *Crafter) AddMaterialContactFreeAutomatic(ctx context.Context, attestationID, value string, casBackend *casclient.CASBackend, runtimeAnnotations map[string]string) (schemaapi.CraftingSchema_Material_MaterialType, error) {
-	var kind schemaapi.CraftingSchema_Material_MaterialType
-	var found bool
-
-	// We want to run the material validation in a specific order
-	for _, kind = range schemaapi.CraftingMaterialInValidationOrder {
-		if err := c.AddMaterialContractFree(ctx, attestationID, kind.String(), value, casBackend, runtimeAnnotations); err != nil {
-			c.logger.Debug().Err(err).Str("kind", kind.String()).Msg("failed to add material")
-			continue
+	for _, kind := range schemaapi.CraftingMaterialInValidationOrder {
+		err := c.AddMaterialContractFree(ctx, attestationID, kind.String(), value, casBackend, runtimeAnnotations)
+		if err == nil {
+			// Successfully added material, return the kind
+			return kind, nil
 		}
-		// If we found a match we break the loop and stop looking
-		found = true
-		break
-	}
 
-	// Return an error if the material could not be added
-	if !found {
-		return kind, fmt.Errorf("failed to add material with attestationID: %s", attestationID)
+		c.logger.Debug().Err(err).Str("kind", kind.String()).Msg("failed to add material")
+
+		// Handle base error for upload and craft errors except the opening file error
+		if errors.Is(err, materials.ErrBaseUploadAndCraft) {
+			return kind, err
+		}
 	}
 
-	return kind, nil
+	// Return an error if no material could be added
+	return schemaapi.CraftingSchema_Material_MATERIAL_TYPE_UNSPECIFIED, fmt.Errorf("failed to auto-discover material kind for value: %v", value)
 }
 
 // addMaterials adds the incoming material m to the crafting state

internal/attestation/crafter/crafter_test.go

@@ -26,6 +26,7 @@ import (
 	schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1"
 	"github.com/chainloop-dev/chainloop/internal/attestation/crafter"
 	v1 "github.com/chainloop-dev/chainloop/internal/attestation/crafter/api/attestation/v1"
+	"github.com/chainloop-dev/chainloop/internal/attestation/crafter/materials"
 	"github.com/chainloop-dev/chainloop/internal/attestation/crafter/runners"
 	"github.com/chainloop-dev/chainloop/internal/attestation/crafter/statemanager/filesystem"
 	"github.com/chainloop-dev/chainloop/internal/casclient"
@@ -423,10 +424,11 @@ func TestSuite(t *testing.T) {
 
 func (s *crafterSuite) TestAddMaterialsAutomatic() {
 	testCases := []struct {
-		name         string
-		materialPath string
-		expectedType schemaapi.CraftingSchema_Material_MaterialType
-		wantErr      bool
+		name           string
+		materialPath   string
+		expectedType   schemaapi.CraftingSchema_Material_MaterialType
+		uploadArtifact bool
+		wantErr        bool
 	}{
 		{
 			name:         "sarif",
@@ -464,10 +466,17 @@ func (s *crafterSuite) TestAddMaterialsAutomatic() {
 			expectedType: schemaapi.CraftingSchema_Material_ARTIFACT,
 		},
 		{
-			name:         "random string",
-			materialPath: "random-string",
-			expectedType: schemaapi.CraftingSchema_Material_STRING,
-			wantErr:      true,
+			name:           "random string",
+			materialPath:   "random-string",
+			expectedType:   schemaapi.CraftingSchema_Material_STRING,
+			uploadArtifact: true,
+		},
+		{
+			name:           "file too large",
+			materialPath:   "./materials/testdata/sbom.cyclonedx.json",
+			expectedType:   schemaapi.CraftingSchema_Material_SBOM_CYCLONEDX_JSON,
+			wantErr:        true,
+			uploadArtifact: true,
 		},
 	}
 
@@ -477,7 +486,7 @@ func (s *crafterSuite) TestAddMaterialsAutomatic() {
 			contract := "testdata/contracts/empty_generic.yaml"
 			uploader := mUploader.NewUploader(s.T())
 
-			if !tc.wantErr {
+			if !tc.uploadArtifact {
 				uploader.On("UploadFile", context.Background(), tc.materialPath).
 					Return(&casclient.UpDownStatus{
 						Digest:   "deadbeef",
@@ -487,11 +496,21 @@ func (s *crafterSuite) TestAddMaterialsAutomatic() {
 
 			backend := &casclient.CASBackend{Uploader: uploader}
 
+			// Establishing a maximum size for the artifact to be uploaded to the CAS causes an error
+			// if the value is exceeded
+			if tc.wantErr {
+				backend.MaxSize = 1
+			}
+
 			c, err := newInitializedCrafter(s.T(), contract, &v1.WorkflowMetadata{}, false, "", runner)
 			require.NoError(s.T(), err)
 
 			kind, err := c.AddMaterialContactFreeAutomatic(context.Background(), "random-id", tc.materialPath, backend, nil)
-			require.NoError(s.T(), err)
+			if tc.wantErr {
+				assert.ErrorIs(s.T(), err, materials.ErrBaseUploadAndCraft)
+			} else {
+				require.NoError(s.T(), err)
+			}
 			assert.Equal(s.T(), tc.expectedType.String(), kind.String())
 		})
 	}

internal/attestation/crafter/materials/materials.go

@@ -17,6 +17,7 @@ package materials
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -33,9 +34,13 @@ import (
 	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
-// ErrInvalidMaterialType is returned when the provided material type
-// is not from the kind we are expecting
-var ErrInvalidMaterialType = fmt.Errorf("unexpected material type")
+var (
+	// ErrInvalidMaterialType is returned when the provided material type
+	// is not from the kind we are expecting
+	ErrInvalidMaterialType = fmt.Errorf("unexpected material type")
+	// ErrBaseUploadAndCraft is returned as a base error when the upload and craft of a material fails
+	ErrBaseUploadAndCraft = errors.New("upload and craft error")
+)
 
 type crafterCommon struct {
 	logger *zerolog.Logger
@@ -59,7 +64,7 @@ func uploadAndCraft(ctx context.Context, input *schemaapi.CraftingSchema_Materia
 
 	// If there is a max size set and the file is bigger than that, return an error
 	if backend.MaxSize > 0 && result.size > backend.MaxSize {
-		return nil, fmt.Errorf("this file is too big for the %s CAS backend, please contact your administrator: fileSize=%s, maxSize=%s", backend.Name, bytefmt.ByteSize(uint64(result.size)), bytefmt.ByteSize(uint64(backend.MaxSize)))
+		return nil, fmt.Errorf("%w: %w", ErrBaseUploadAndCraft, fmt.Errorf("this file is too big for the %s CAS backend, please contact your administrator: fileSize=%s, maxSize=%s", backend.Name, bytefmt.ByteSize(uint64(result.size)), bytefmt.ByteSize(uint64(backend.MaxSize))))
 	}
 
 	material := &api.Attestation_Material{
@@ -80,7 +85,7 @@ func uploadAndCraft(ctx context.Context, input *schemaapi.CraftingSchema_Materia
 
 		_, err = backend.Uploader.UploadFile(ctx, artifactPath)
 		if err != nil {
-			return nil, fmt.Errorf("uploading material: %w", err)
+			return nil, fmt.Errorf("%w: %w", ErrBaseUploadAndCraft, fmt.Errorf("uploading material: %w", err))
 		}
 
 		material.UploadedToCas = true
@@ -89,7 +94,7 @@ func uploadAndCraft(ctx context.Context, input *schemaapi.CraftingSchema_Materia
 		// or store it inline if no uploader is provided
 		content, err := io.ReadAll(result.r)
 		if err != nil {
-			return nil, fmt.Errorf("reading file: %w", err)
+			return nil, fmt.Errorf("%w: %w", ErrBaseUploadAndCraft, fmt.Errorf("reading file: %w", err))
 		}
 
 		material.InlineCas = true