From 77d1e6c2efd67a377d52a87b2b734103937d74ee Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Thu, 15 Aug 2024 14:10:29 +0300 Subject: [PATCH] Persist artifacts in central entity table (#4161) This ensures that artifacts are tracked in the central entity table. Signed-off-by: Juan Antonio Osorio --- database/mock/fixtures/store.go | 3 + .../controlplane/handlers_githubwebhooks.go | 78 +++++++++++++----- .../handlers_githubwebhooks_test.go | 2 + internal/reconcilers/repository.go | 80 +++++++++++++------ 4 files changed, 120 insertions(+), 43 deletions(-) diff --git a/database/mock/fixtures/store.go b/database/mock/fixtures/store.go index 410053c086..bc73d3495d 100644 --- a/database/mock/fixtures/store.go +++ b/database/mock/fixtures/store.go @@ -158,6 +158,9 @@ func WithSuccessfulUpsertArtifact( mockStore.EXPECT(). UpsertArtifact(gomock.Any(), gomock.Any()). Return(artifact, nil) + mockStore.EXPECT(). + CreateOrEnsureEntityByID(gomock.Any(), gomock.Any()). + Return(db.EntityInstance{}, nil) } } diff --git a/internal/controlplane/handlers_githubwebhooks.go b/internal/controlplane/handlers_githubwebhooks.go index 95abab0a73..dd49df10c9 100644 --- a/internal/controlplane/handlers_githubwebhooks.go +++ b/internal/controlplane/handlers_githubwebhooks.go @@ -659,6 +659,7 @@ func (_ *Server) processPingEvent( l.Debug().Msg("ping received") } +//nolint:gocyclo // This function will be re-simplified later on func (s *Server) processPackageEvent( ctx context.Context, payload []byte, @@ -710,33 +711,70 @@ func (s *Server) processPackageEvent( return nil, fmt.Errorf("error gathering versioned artifact: %w", err) } - dbArtifact, err := s.store.UpsertArtifact(ctx, db.UpsertArtifactParams{ - RepositoryID: uuid.NullUUID{ - UUID: dbrepo.ID, - Valid: true, - }, - ArtifactName: tempArtifact.GetName(), - ArtifactType: tempArtifact.GetTypeLower(), - ArtifactVisibility: tempArtifact.Visibility, - ProjectID: dbrepo.ProjectID, - ProviderID: dbrepo.ProviderID, - ProviderName: dbrepo.Provider, - }) - if err != nil { - return nil, fmt.Errorf("error upserting artifact: %w", err) - } + var artifactID uuid.UUID + pbArtifact, err := db.WithTransaction(s.store, func(tx db.ExtendQuerier) (*pb.Artifact, error) { + dbArtifact, err := tx.UpsertArtifact(ctx, db.UpsertArtifactParams{ + RepositoryID: uuid.NullUUID{ + UUID: dbrepo.ID, + Valid: true, + }, + ArtifactName: tempArtifact.GetName(), + ArtifactType: tempArtifact.GetTypeLower(), + ArtifactVisibility: tempArtifact.Visibility, + ProjectID: dbrepo.ProjectID, + ProviderID: dbrepo.ProviderID, + ProviderName: dbrepo.Provider, + }) + if err != nil { + return nil, fmt.Errorf("error upserting artifact: %w", err) + } - _, pbArtifact, err := artifacts.GetArtifact(ctx, s.store, dbrepo.ProjectID, dbArtifact.ID) + _, pba, err := artifacts.GetArtifact(ctx, tx, dbrepo.ProjectID, dbArtifact.ID) + if err != nil { + return nil, fmt.Errorf("error getting artifact with versions: %w", err) + } + // TODO: wrap in a function + pba.Versions = tempArtifact.Versions + + artifactID = dbArtifact.ID + + // name is provider specific and should be based on properties. + // In github's case it's lowercase owner / artifact name + // TODO: Replace with a provider call to get + // a name based on properties. + var prefix string + if tempArtifact.GetOwner() != "" { + prefix = tempArtifact.GetOwner() + "/" + } + + // At this point the package name has been checked. + artName := prefix + *event.Package.Name + + _, err = tx.CreateOrEnsureEntityByID(ctx, db.CreateOrEnsureEntityByIDParams{ + ID: dbArtifact.ID, + EntityType: db.EntitiesArtifact, + Name: artName, + ProjectID: dbrepo.ProjectID, + ProviderID: dbrepo.ProviderID, + OriginatedFrom: uuid.NullUUID{ + UUID: dbrepo.ID, + Valid: true, + }, + }) + if err != nil { + return nil, fmt.Errorf("error creating or ensuring entity: %w", err) + } + + return pba, nil + }) if err != nil { - return nil, fmt.Errorf("error getting artifact with versions: %w", err) + return nil, err } - // TODO: wrap in a function - pbArtifact.Versions = tempArtifact.Versions eiw := entities.NewEntityInfoWrapper(). WithActionEvent(*event.Action). WithArtifact(pbArtifact). - WithArtifactID(dbArtifact.ID). + WithArtifactID(artifactID). WithProjectID(dbrepo.ProjectID). WithProviderID(dbrepo.ProviderID). WithRepositoryID(dbrepo.ID) diff --git a/internal/controlplane/handlers_githubwebhooks_test.go b/internal/controlplane/handlers_githubwebhooks_test.go index fb12ba08c1..463e906d03 100644 --- a/internal/controlplane/handlers_githubwebhooks_test.go +++ b/internal/controlplane/handlers_githubwebhooks_test.go @@ -586,6 +586,7 @@ func (s *UnitTestSuite) TestHandleGitHubWebHook() { ID: uuid.New(), }, ), + df.WithTransaction(), ), topic: events.TopicQueueEntityEvaluate, statusCode: http.StatusOK, @@ -628,6 +629,7 @@ func (s *UnitTestSuite) TestHandleGitHubWebHook() { ID: uuid.New(), }, ), + df.WithTransaction(), ), topic: events.TopicQueueEntityEvaluate, statusCode: http.StatusOK, diff --git a/internal/reconcilers/repository.go b/internal/reconcilers/repository.go index c12d42eb10..dfe17a74de 100644 --- a/internal/reconcilers/repository.go +++ b/internal/reconcilers/repository.go @@ -119,42 +119,76 @@ func (r *Reconciler) handleArtifactsReconcilerEvent(ctx context.Context, evt *me for _, artifact := range artifacts { // store information if we do not have it typeLower := strings.ToLower(artifact.GetPackageType()) - newArtifact, err := r.store.UpsertArtifact(ctx, - db.UpsertArtifactParams{ - RepositoryID: uuid.NullUUID{ + var newArtifactID uuid.UUID + pbArtifact, err := db.WithTransaction(r.store, func(tx db.ExtendQuerier) (*pb.Artifact, error) { + newArtifact, err := tx.UpsertArtifact(ctx, + db.UpsertArtifactParams{ + RepositoryID: uuid.NullUUID{ + UUID: repository.ID, + Valid: true, + }, + ArtifactName: artifact.GetName(), + ArtifactType: typeLower, + ArtifactVisibility: artifact.GetVisibility(), + ProjectID: evt.Project, + ProviderName: repository.Provider, + ProviderID: providerID, + }) + + if err != nil { + return nil, err + } + + newArtifactID = newArtifact.ID + + // name is provider specific and should be based on properties. + // In github's case it's lowercase owner / artifact name + // TODO: Replace with a provider call to get + // a name based on properties. + var prefix string + if artifact.GetOwner().GetLogin() != "" { + prefix = artifact.GetOwner().GetLogin() + "/" + } + + artName := prefix + artifact.GetName() + + _, err = tx.CreateOrEnsureEntityByID(ctx, db.CreateOrEnsureEntityByIDParams{ + ID: newArtifact.ID, + EntityType: db.EntitiesArtifact, + Name: artName, + ProjectID: evt.Project, + ProviderID: providerID, + OriginatedFrom: uuid.NullUUID{ UUID: repository.ID, Valid: true, }, - ArtifactName: artifact.GetName(), - ArtifactType: typeLower, - ArtifactVisibility: artifact.GetVisibility(), - ProjectID: evt.Project, - ProviderName: repository.Provider, - ProviderID: providerID, }) - + if err != nil { + return nil, err + } + + // publish event for artifact + return &pb.Artifact{ + ArtifactPk: newArtifact.ID.String(), + Owner: *artifact.GetOwner().Login, + Name: artifact.GetName(), + Type: artifact.GetPackageType(), + Visibility: artifact.GetVisibility(), + Repository: repository.RepoName, + Versions: nil, // explicitly nil, will be filled by the ingester + CreatedAt: timestamppb.New(artifact.GetCreatedAt().Time), + }, nil + }) if err != nil { // just log error and continue log.Printf("error storing artifact: %v", err) continue } - - // publish event for artifact - pbArtifact := &pb.Artifact{ - ArtifactPk: newArtifact.ID.String(), - Owner: *artifact.GetOwner().Login, - Name: artifact.GetName(), - Type: artifact.GetPackageType(), - Visibility: artifact.GetVisibility(), - Repository: repository.RepoName, - Versions: nil, // explicitly nil, will be filled by the ingester - CreatedAt: timestamppb.New(artifact.GetCreatedAt().Time), - } err = entities.NewEntityInfoWrapper(). WithProviderID(providerID). WithArtifact(pbArtifact). WithProjectID(evt.Project). - WithArtifactID(newArtifact.ID). + WithArtifactID(newArtifactID). WithRepositoryID(repository.ID). Publish(r.evt) if err != nil {