diff --git a/internal/attestation/crafter/crafter.go b/internal/attestation/crafter/crafter.go index edb3062bb..a2a7fd17c 100644 --- a/internal/attestation/crafter/crafter.go +++ b/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 diff --git a/internal/attestation/crafter/crafter_test.go b/internal/attestation/crafter/crafter_test.go index d8d5efde6..48b37bb35 100644 --- a/internal/attestation/crafter/crafter_test.go +++ b/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()) }) } diff --git a/internal/attestation/crafter/materials/materials.go b/internal/attestation/crafter/materials/materials.go index 809be8a29..426d0bef2 100644 --- a/internal/attestation/crafter/materials/materials.go +++ b/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