feat(referrer): support same digest for two different kinds (#431)

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

Author: migmartri

app/controlplane/internal/biz/errors.go

@@ -91,3 +91,19 @@ func (e ErrUnauthorized) Error() string {
 func IsErrUnauthorized(err error) bool {
 	return errors.As(err, &ErrUnauthorized{})
 }
+
+// A referrer with the same digest points to two different artifact types
+// and we require filtering out which one
+type ErrAmbiguousReferrer struct {
+	digest string
+	// what kinds contain duplicates
+	kinds []string
+}
+
+func NewErrReferrerAmbiguous(digest string, kinds []string) error {
+	return ErrAmbiguousReferrer{digest, kinds}
+}
+
+func (e ErrAmbiguousReferrer) Error() string {
+	return fmt.Sprintf("digest %s present in %d kinds %q", e.digest, len(e.kinds), e.kinds)
+}

app/controlplane/internal/biz/referrer.go

@@ -19,8 +19,10 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
+	"sort"
 	"time"
 
 	"github.com/chainloop-dev/chainloop/internal/attestation/renderer/chainloop"
@@ -37,27 +39,21 @@ type Referrer struct {
 	Kind   string
 	// Wether the item is downloadable from CAS or not
 	Downloadable bool
-	// points to other digests
-	References []string
+	References   []*Referrer
 }
 
 // Actual referrer stored in the DB which includes a nested list of storedReferences
 type StoredReferrer struct {
-	ID     uuid.UUID
-	Digest string
-	Kind   string
-	// Wether the item is downloadable from CAS or not
-	Downloadable bool
-	CreatedAt    *time.Time
+	*Referrer
+	ID        uuid.UUID
+	CreatedAt *time.Time
 	// Fully expanded list of 1-level off references
 	References []*StoredReferrer
 	OrgIDs     []uuid.UUID
 }
 
-type ReferrerMap map[string]*Referrer
-
 type ReferrerRepo interface {
-	Save(ctx context.Context, input ReferrerMap, orgID uuid.UUID) error
+	Save(ctx context.Context, input []*Referrer, orgID uuid.UUID) error
 	// GetFromRoot returns the referrer identified by the provided content digest, including its first-level references
 	// For example if sha:deadbeef represents an attestation, the result will contain the attestation + materials associated to it
 	// OrgIDs represent an allowList of organizations where the referrers should be looked for
@@ -129,6 +125,10 @@ func (s *ReferrerUseCase) GetFromRoot(ctx context.Context, digest string, userID
 
 	ref, err := s.repo.GetFromRoot(ctx, digest, orgIDs)
 	if err != nil {
+		if errors.As(err, &ErrAmbiguousReferrer{}) {
+			return nil, NewErrValidation(fmt.Errorf("please provide the referrer kind: %w", err))
+		}
+
 		return nil, fmt.Errorf("getting referrer from root: %w", err)
 	} else if ref == nil {
 		return nil, NewErrNotFound("referrer")
@@ -142,14 +142,22 @@ const (
 	referrerGitHeadType     = "GIT_HEAD_COMMIT"
 )
 
+func newRef(digest, kind string) string {
+	return fmt.Sprintf("%s-%s", kind, digest)
+}
+
+func (r *Referrer) MapID() string {
+	return newRef(r.Digest, r.Kind)
+}
+
 // ExtractReferrers extracts the referrers from the given attestation
 // this means
 // 1 - write an entry for the attestation itself
 // 2 - then to all the materials contained in the predicate
 // 3 - and the subjects (some of them)
 // 4 - creating link between the attestation and the materials/subjects as needed
 // see tests for examples
-func extractReferrers(att *dsse.Envelope) (ReferrerMap, error) {
+func extractReferrers(att *dsse.Envelope) ([]*Referrer, error) {
 	// Calculate the attestation hash
 	jsonAtt, err := json.Marshal(att)
 	if err != nil {
@@ -162,16 +170,18 @@ func extractReferrers(att *dsse.Envelope) (ReferrerMap, error) {
 		return nil, fmt.Errorf("calculating attestation hash: %w", err)
 	}
 
-	referrers := make(ReferrerMap)
+	referrersMap := make(map[string]*Referrer)
 	// 1 - Attestation referrer
 	// Add the attestation itself as a referrer to the map without references yet
 	attestationHash := h.String()
-	referrers[attestationHash] = &Referrer{
+	attestationReferrer := &Referrer{
 		Digest:       attestationHash,
 		Kind:         referrerAttestationType,
 		Downloadable: true,
 	}
 
+	referrersMap[newRef(attestationHash, referrerAttestationType)] = attestationReferrer
+
 	// 2 - Predicate that's referenced from the attestation
 	predicate, err := chainloop.ExtractPredicate(att)
 	if err != nil {
@@ -189,23 +199,23 @@ func extractReferrers(att *dsse.Envelope) (ReferrerMap, error) {
 		// Create its referrer entry if it doesn't exist yet
 		// the reason it might exist is because you might be attaching the same material twice
 		// i.e the same SBOM twice, in that case we don't want to create a new referrer
-		// If we are providing different types for the same digest, we should error out
-		if r, ok := referrers[material.Hash.String()]; ok {
-			if r.Kind != material.Type {
-				return nil, fmt.Errorf("material %s has different types: %s and %s", material.Hash.String(), r.Kind, material.Type)
-			}
-
+		materialRef := newRef(material.Hash.String(), material.Type)
+		if _, ok := referrersMap[materialRef]; ok {
 			continue
 		}
 
-		referrers[material.Hash.String()] = &Referrer{
+		referrersMap[materialRef] = &Referrer{
 			Digest:       material.Hash.String(),
 			Kind:         material.Type,
 			Downloadable: material.UploadedToCAS,
 		}
 
+		materialReferrer := referrersMap[materialRef]
+
 		// Add the reference to the attestation
-		referrers[attestationHash].References = append(referrers[attestationHash].References, material.Hash.String())
+		attestationReferrer.References = append(attestationReferrer.References, &Referrer{
+			Digest: materialReferrer.Digest, Kind: materialReferrer.Kind,
+		})
 	}
 
 	// 3 - Subject that points to the attestation
@@ -215,25 +225,42 @@ func extractReferrers(att *dsse.Envelope) (ReferrerMap, error) {
 	}
 
 	for _, subject := range statement.Subject {
-		subjectRef, err := intotoSubjectToReferrer(subject)
+		subjectReferrer, err := intotoSubjectToReferrer(subject)
 		if err != nil {
 			return nil, fmt.Errorf("transforming subject to referrer: %w", err)
 		}
 
-		if subjectRef == nil {
+		if subjectReferrer == nil {
 			continue
 		}
 
+		subjectRef := newRef(subjectReferrer.Digest, subjectReferrer.Kind)
+
 		// check if we already have a referrer for this digest and set it otherwise
 		// this is the case for example for git.Head ones
-		if _, ok := referrers[subjectRef.Digest]; !ok {
-			referrers[subjectRef.Digest] = subjectRef
+		if _, ok := referrersMap[subjectRef]; !ok {
+			referrersMap[subjectRef] = subjectReferrer
 			// add it to the list of of attestation-referenced digests
-			referrers[attestationHash].References = append(referrers[attestationHash].References, subjectRef.Digest)
+			attestationReferrer.References = append(attestationReferrer.References,
+				&Referrer{
+					Digest: subjectReferrer.Digest, Kind: subjectReferrer.Kind,
+				})
 		}
 
 		// Update referrer to point to the attestation
-		referrers[subjectRef.Digest].References = []string{attestationHash}
+		referrersMap[subjectRef].References = []*Referrer{{Digest: attestationReferrer.Digest, Kind: attestationReferrer.Kind}}
+	}
+
+	// Return a sorted list of referrers
+	mapKeys := make([]string, 0, len(referrersMap))
+	for k := range referrersMap {
+		mapKeys = append(mapKeys, k)
+	}
+	sort.Strings(mapKeys)
+
+	referrers := make([]*Referrer, 0, len(referrersMap))
+	for _, k := range mapKeys {
+		referrers = append(referrers, referrersMap[k])
 	}
 
 	return referrers, nil

app/controlplane/internal/biz/referrer_integration_test.go

@@ -36,42 +36,42 @@ func (s *referrerIntegrationTestSuite) TestExtractAndPersists() {
 	var envelope *dsse.Envelope
 	require.NoError(s.T(), json.Unmarshal(attJSON, &envelope))
 
-	wantReferrerAtt := &biz.StoredReferrer{
+	wantReferrerAtt := &biz.Referrer{
 		Digest:       "sha256:ad704d286bcad6e155e71c33d48247931231338396acbcd9769087530085b2a2",
 		Kind:         "ATTESTATION",
 		Downloadable: true,
 	}
 
-	wantReferrerCommit := &biz.StoredReferrer{
+	wantReferrerCommit := &biz.Referrer{
 		Digest: "sha1:78ac366c9e8a300d51808d581422ca61f7b5b721",
 		Kind:   "GIT_HEAD_COMMIT",
 	}
 
-	wantReferrerSBOM := &biz.StoredReferrer{
+	wantReferrerSBOM := &biz.Referrer{
 		Digest:       "sha256:16159bb881eb4ab7eb5d8afc5350b0feeed1e31c0a268e355e74f9ccbe885e0c",
 		Kind:         "SBOM_CYCLONEDX_JSON",
 		Downloadable: true,
 	}
 
-	wantReferrerArtifact := &biz.StoredReferrer{
+	wantReferrerArtifact := &biz.Referrer{
 		Digest:       "sha256:385c4188b9c080499413f2e0fa0b3951ed107b5f0cb35c2f2b1f07a7be9a7512",
 		Kind:         "ARTIFACT",
 		Downloadable: true,
 	}
 
-	wantReferrerOpenVEX := &biz.StoredReferrer{
+	wantReferrerOpenVEX := &biz.Referrer{
 		Digest:       "sha256:b4bd86d5855f94bcac0a92d3100ae7b85d050bd2e5fb9037a200e5f5f0b073a2",
 		Kind:         "OPENVEX",
 		Downloadable: true,
 	}
 
-	wantReferrerSarif := &biz.StoredReferrer{
+	wantReferrerSarif := &biz.Referrer{
 		Digest:       "sha256:c4a63494f9289dd9fd44f841efb4f5b52765c2de6332f2d86e5f6c0340b40a95",
 		Kind:         "SARIF",
 		Downloadable: true,
 	}
 
-	wantReferrerContainerImage := &biz.StoredReferrer{
+	wantReferrerContainerImage := &biz.Referrer{
 		Digest: "sha256:fbd9335f55d83d8aaf9ab1a539b0f2a87b444e8c54f34c9a1ca9d7df15605db4",
 		Kind:   "CONTAINER_IMAGE",
 	}
@@ -109,12 +109,10 @@ func (s *referrerIntegrationTestSuite) TestExtractAndPersists() {
 		// it has all the references
 		require.Len(t, got.References, 6)
 
-		for i, want := range []*biz.StoredReferrer{
+		for i, want := range []*biz.Referrer{
 			wantReferrerCommit, wantReferrerSBOM, wantReferrerArtifact, wantReferrerOpenVEX, wantReferrerSarif, wantReferrerContainerImage} {
 			gotR := got.References[i]
-			s.Equal(want.Digest, gotR.Digest)
-			s.Equal(want.Kind, gotR.Kind)
-			s.Equal(want.Downloadable, gotR.Downloadable)
+			s.Equal(want, gotR.Referrer)
 		}
 		s.Equal([]uuid.UUID{s.org1UUID}, got.OrgIDs)
 	})
@@ -181,13 +179,13 @@ func (s *referrerIntegrationTestSuite) TestExtractAndPersists() {
 		s.Nil(got)
 	})
 
-	s.T().Run("it should fail if the attestation has the same material twice with different types", func(t *testing.T) {
+	s.T().Run("it should NOT fail storing the attestation with the same material twice with different types", func(t *testing.T) {
 		attJSON, err = os.ReadFile("testdata/attestations/with-duplicated-sha.json")
 		require.NoError(s.T(), err)
 		require.NoError(s.T(), json.Unmarshal(attJSON, &envelope))
 
 		err := s.Referrer.ExtractAndPersist(ctx, envelope, s.org1.ID)
-		s.ErrorContains(err, "has different types")
+		s.NoError(err)
 	})
 
 	s.T().Run("it should fail on retrieval if we have stored two referrers with same digest (for two different types)", func(t *testing.T) {
@@ -203,7 +201,7 @@ func (s *referrerIntegrationTestSuite) TestExtractAndPersists() {
 		// but retrieval should fail. In the future we will ask the user to provide the artifact type in these cases of ambiguity
 		got, err := s.Referrer.GetFromRoot(ctx, wantReferrerSarif.Digest, s.user.ID)
 		s.Nil(got)
-		s.ErrorContains(err, "found more than one referrer with digest")
+		s.ErrorContains(err, "present in 2 kinds")
 	})
 
 	s.T().Run("now there should a container image pointing to two attestations", func(t *testing.T) {