Use provider ID instead of name when sending events

internal/artifacts/db.go

@@ -0,0 +1,81 @@
+// Copyright 2024 Stacklok, Inc
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package artifacts stores logic relating to the artifact entity type
+package artifacts
+
+import (
+	"context"
+	"database/sql"
+	"errors"
+	"fmt"
+
+	"github.com/google/uuid"
+	"google.golang.org/protobuf/types/known/timestamppb"
+
+	"github.com/stacklok/minder/internal/db"
+	"github.com/stacklok/minder/internal/util/ptr"
+	minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
+)
+
+// GetArtifact retrieves an artifact and its versions from the database
+// Returns provider ID alongside the artifact
+func GetArtifact(
+	ctx context.Context,
+	store db.Querier,
+	projectID uuid.UUID,
+	artifactID uuid.UUID,
+) (uuid.UUID, *minderv1.Artifact, error) {
+	// Retrieve artifact details
+	artifact, err := store.GetArtifactByID(ctx, db.GetArtifactByIDParams{
+		ID:        artifactID,
+		ProjectID: projectID,
+	})
+	if errors.Is(err, sql.ErrNoRows) {
+		return uuid.Nil, nil, fmt.Errorf("artifact not found")
+	} else if err != nil {
+		return uuid.Nil, nil, fmt.Errorf("failed to get artifact: %v", err)
+	}
+
+	var repoOwner, repoName string
+	if artifact.RepositoryID.Valid {
+		dbrepo, err := store.GetRepositoryByIDAndProject(ctx, db.GetRepositoryByIDAndProjectParams{
+			ID:        artifact.RepositoryID.UUID,
+			ProjectID: projectID,
+		})
+		if errors.Is(err, sql.ErrNoRows) {
+			return uuid.Nil, nil, fmt.Errorf("repository not found")
+		} else if err != nil {
+			return uuid.Nil, nil, fmt.Errorf("cannot read repository: %v", err)
+		}
+
+		repoOwner = dbrepo.RepoOwner
+		repoName = dbrepo.RepoName
+	}
+
+	// Build the artifact protobuf
+	return artifact.ProviderID, &minderv1.Artifact{
+		ArtifactPk: artifact.ID.String(),
+		Context: &minderv1.Context{
+			Project:  ptr.Ptr(projectID.String()),
+			Provider: ptr.Ptr(artifact.ProviderName),
+		},
+		Owner:      repoOwner,
+		Name:       artifact.ArtifactName,
+		Type:       artifact.ArtifactType,
+		Visibility: artifact.ArtifactVisibility,
+		Repository: repoName,
+		CreatedAt:  timestamppb.New(artifact.CreatedAt),
+	}, nil
+}

internal/controlplane/handlers_githubwebhooks.go

@@ -38,6 +38,7 @@ import (
 	"golang.org/x/exp/slices"
 	"google.golang.org/protobuf/types/known/timestamppb"
 
+	"github.com/stacklok/minder/internal/artifacts"
 	"github.com/stacklok/minder/internal/config/server"
 	"github.com/stacklok/minder/internal/controlplane/metrics"
 	"github.com/stacklok/minder/internal/db"
@@ -47,6 +48,7 @@ import (
 	"github.com/stacklok/minder/internal/providers"
 	"github.com/stacklok/minder/internal/providers/github/installations"
 	ghprov "github.com/stacklok/minder/internal/providers/github/service"
+	"github.com/stacklok/minder/internal/repositories"
 	"github.com/stacklok/minder/internal/util"
 	"github.com/stacklok/minder/internal/verifier/verifyif"
 	pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
@@ -494,7 +496,7 @@ func (s *Server) parseGithubEventForProcessing(
 	}
 
 	// get the provider for the repository
-	prov, err := s.providerStore.GetByName(ctx, dbRepo.ProjectID, dbRepo.Provider)
+	prov, err := s.providerStore.GetByID(ctx, dbRepo.ProviderID)
 	if err != nil {
 		return fmt.Errorf("error getting provider: %w", err)
 	}
@@ -522,7 +524,7 @@ func (s *Server) parseGithubEventForProcessing(
 	} else if ent == pb.Entity_ENTITY_PULL_REQUESTS {
 		return parsePullRequestModEvent(ctx, payload, msg, dbRepo, s.store, provBuilder, action)
 	} else if ent == pb.Entity_ENTITY_REPOSITORIES {
-		return parseRepoEvent(payload, msg, dbRepo, provBuilder.GetName(), action)
+		return parseRepoEvent(payload, msg, dbRepo, action)
 	}
 
 	return newErrNotHandled("event %s with action %s not handled",
@@ -533,7 +535,6 @@ func parseRepoEvent(
 	whPayload map[string]any,
 	msg *message.Message,
 	dbrepo db.Repository,
-	providerName string,
 	action string,
 ) error {
 	if action == WebhookActionEventDeleted {
@@ -558,9 +559,9 @@ func parseRepoEvent(
 	}
 
 	// protobufs are our API, so we always execute on these instead of the DB directly.
-	repo := util.PBRepositoryFromDB(dbrepo)
+	repo := repositories.PBRepositoryFromDB(dbrepo)
 	eiw := entities.NewEntityInfoWrapper().
-		WithProvider(providerName).
+		WithProviderID(dbrepo.ProviderID).
 		WithRepository(repo).
 		WithProjectID(dbrepo.ProjectID).
 		WithRepositoryID(dbrepo.ID).
@@ -616,7 +617,7 @@ func (s *Server) parseArtifactPublishedEvent(
 		return fmt.Errorf("error upserting artifact: %w", err)
 	}
 
-	pbArtifact, err := util.GetArtifact(ctx, s.store, dbrepo.ProjectID, dbArtifact.ID)
+	_, pbArtifact, err := artifacts.GetArtifact(ctx, s.store, dbrepo.ProjectID, dbArtifact.ID)
 	if err != nil {
 		return fmt.Errorf("error getting artifact with versions: %w", err)
 	}
@@ -625,7 +626,7 @@ func (s *Server) parseArtifactPublishedEvent(
 
 	eiw := entities.NewEntityInfoWrapper().
 		WithArtifact(pbArtifact).
-		WithProvider(prov.GetName()).
+		WithProviderID(dbrepo.ProviderID).
 		WithProjectID(dbrepo.ProjectID).
 		WithRepositoryID(dbrepo.ID).
 		WithArtifactID(dbArtifact.ID).
@@ -677,7 +678,7 @@ func parsePullRequestModEvent(
 	eiw := entities.NewEntityInfoWrapper().
 		WithPullRequest(prEvalInfo).
 		WithPullRequestID(dbPr.ID).
-		WithProvider(prov.GetName()).
+		WithProviderID(dbrepo.ProviderID).
 		WithProjectID(dbrepo.ProjectID).
 		WithRepositoryID(dbrepo.ID).
 		WithActionEvent(action)

internal/controlplane/handlers_githubwebhooks_test.go

@@ -238,6 +238,7 @@ func (s *UnitTestSuite) TestHandleWebHookRepository() {
 	providerName := "github"
 	repositoryID := uuid.New()
 	projectID := uuid.New()
+	providerID := uuid.New()
 
 	// create a test oauth2 token
 	token := oauth2.Token{
@@ -263,30 +264,20 @@ func (s *UnitTestSuite) TestHandleWebHookRepository() {
 	mockStore.EXPECT().
 		GetRepositoryByRepoID(gomock.Any(), gomock.Any()).
 		Return(db.Repository{
-			ID:        repositoryID,
-			ProjectID: projectID,
-			RepoID:    12345,
-			Provider:  providerName,
+			ID:         repositoryID,
+			ProjectID:  projectID,
+			RepoID:     12345,
+			Provider:   providerName,
+			ProviderID: providerID,
 		}, nil)
 
 	mockStore.EXPECT().
-		GetParentProjects(gomock.Any(), projectID).
-		Return([]uuid.UUID{
-			projectID,
-		}, nil)
-
-	mockStore.EXPECT().
-		FindProviders(gomock.Any(), db.FindProvidersParams{
-			Name: sql.NullString{String: providerName, Valid: true},
-			Projects: []uuid.UUID{
-				projectID,
-			},
-			Trait: db.NullProviderType{},
-		}).
-		Return([]db.Provider{{
+		GetProviderByID(gomock.Any(), gomock.Eq(providerID)).
+		Return(db.Provider{
+			ID:        providerID,
 			ProjectID: projectID,
 			Name:      providerName,
-		}}, nil)
+		}, nil)
 
 	hook := srv.HandleGitHubWebHook()
 	port, err := rand.GetRandomPort()
@@ -331,7 +322,7 @@ func (s *UnitTestSuite) TestHandleWebHookRepository() {
 	assert.Equal(t, "12345", received.Metadata["id"])
 	assert.Equal(t, "meta", received.Metadata["type"])
 	assert.Equal(t, "https://api.github.com/", received.Metadata["source"])
-	assert.Equal(t, "github", received.Metadata["provider"])
+	assert.Equal(t, providerID.String(), received.Metadata["provider_id"])
 	assert.Equal(t, projectID.String(), received.Metadata[entities.ProjectIDEventKey])
 	assert.Equal(t, repositoryID.String(), received.Metadata["repository_id"])
 

internal/controlplane/handlers_reconciliationtasks.go

@@ -99,7 +99,7 @@ func getRepositoryReconciliationMessage(ctx context.Context, store db.Store,
 		return nil, status.Errorf(codes.NotFound, "repository not found")
 	}
 
-	msg, err := reconcilers.NewRepoReconcilerMessage(repo.Provider, repo.RepoID, repo.ProjectID)
+	msg, err := reconcilers.NewRepoReconcilerMessage(repo.ProviderID, repo.RepoID, repo.ProjectID)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "error getting reconciler message: %v", err)
 	}

internal/controlplane/handlers_repositories.go

@@ -33,6 +33,7 @@ import (
 	"github.com/stacklok/minder/internal/projects/features"
 	"github.com/stacklok/minder/internal/providers"
 	"github.com/stacklok/minder/internal/providers/github"
+	"github.com/stacklok/minder/internal/repositories"
 	ghrepo "github.com/stacklok/minder/internal/repositories/github"
 	"github.com/stacklok/minder/internal/util"
 	cursorutil "github.com/stacklok/minder/internal/util/cursor"
@@ -112,6 +113,9 @@ func (s *Server) ListRepositories(ctx context.Context,
 	projectID := entityCtx.Project.ID
 	providerName := entityCtx.Provider.Name
 
+	logger.BusinessRecord(ctx).Provider = providerName
+	logger.BusinessRecord(ctx).Project = projectID
+
 	providerFilter := getNameFilterParam(providerName)
 
 	reqRepoCursor, err := cursorutil.NewRepoCursor(in.GetCursor())
@@ -149,7 +153,7 @@ func (s *Server) ListRepositories(ctx context.Context,
 
 	for _, repo := range repos {
 		projID := repo.ProjectID.String()
-		r := util.PBRepositoryFromDB(repo)
+		r := repositories.PBRepositoryFromDB(repo)
 		r.Context = &pb.Context{
 			Project:  &projID,
 			Provider: &repo.Provider,
@@ -173,11 +177,6 @@ func (s *Server) ListRepositories(ctx context.Context,
 	resp.Results = results
 	resp.Cursor = respRepoCursor.String()
 
-	// Telemetry logging
-	// TODO: Change to ProviderID
-	logger.BusinessRecord(ctx).Provider = providerName
-	logger.BusinessRecord(ctx).Project = projectID
-
 	return &resp, nil
 }
 
@@ -202,7 +201,7 @@ func (s *Server) GetRepositoryById(ctx context.Context,
 	}
 
 	projID := repo.ProjectID.String()
-	r := util.PBRepositoryFromDB(repo)
+	r := repositories.PBRepositoryFromDB(repo)
 	r.Context = &pb.Context{
 		Project:  &projID,
 		Provider: &repo.Provider,
@@ -247,7 +246,7 @@ func (s *Server) GetRepositoryByName(ctx context.Context,
 	}
 
 	projID := repo.ProjectID.String()
-	r := util.PBRepositoryFromDB(repo)
+	r := repositories.PBRepositoryFromDB(repo)
 	r.Context = &pb.Context{
 		Project:  &projID,
 		Provider: &repo.Provider,
@@ -435,16 +434,14 @@ func (s *Server) deleteRepository(
 	ctx context.Context,
 	repoQueryMethod func() (db.Repository, error),
 ) error {
-	projectID := getProjectID(ctx)
-
 	repo, err := repoQueryMethod()
 	if errors.Is(err, sql.ErrNoRows) {
 		return status.Errorf(codes.NotFound, "repository not found")
 	} else if err != nil {
 		return status.Errorf(codes.Internal, "unexpected error fetching repo: %v", err)
 	}
 
-	provider, err := s.providerStore.GetByName(ctx, projectID, repo.Provider)
+	provider, err := s.providerStore.GetByID(ctx, repo.ID)
 	if err != nil {
 		return status.Errorf(codes.Internal, "cannot get provider: %v", err)
 	}

internal/controlplane/handlers_repositories_test.go

@@ -58,7 +58,7 @@ func TestServer_RegisterRepository(t *testing.T) {
 			RepoOwner:     repoOwner,
 			RepoName:      repoName,
 			ProviderFails: true,
-			ExpectedError: "cannot retrieve provider",
+			ExpectedError: "cannot get provider",
 		},
 		{
 			Name:          "Repo creation fails when repo name is missing",
@@ -222,7 +222,7 @@ func TestServer_DeleteRepository(t *testing.T) {
 			RepoName:         repoOwnerAndName,
 			RepoServiceSetup: newRepoService(withSuccessfulGetRepoByName),
 			ProviderFails:    true,
-			ExpectedError:    "cannot retrieve provider",
+			ExpectedError:    "cannot get provider",
 		},
 		{
 			Name:          "delete by name fails when name is malformed",
@@ -463,9 +463,12 @@ func createServer(ctrl *gomock.Controller, repoServiceSetup repoMockBuilder, git
 		AnyTimes()
 
 	if providerFails {
+		store.EXPECT().
+			GetProviderByID(gomock.Any(), gomock.Any()).
+			Return(db.Provider{}, errDefault).AnyTimes()
 		store.EXPECT().
 			FindProviders(gomock.Any(), gomock.Any()).
-			Return([]db.Provider{}, errDefault)
+			Return([]db.Provider{}, errDefault).AnyTimes()
 	} else {
 		store.EXPECT().
 			FindProviders(gomock.Any(), gomock.Any()).
@@ -475,6 +478,9 @@ func createServer(ctrl *gomock.Controller, repoServiceSetup repoMockBuilder, git
 			Return(db.ProviderAccessToken{
 				EncryptedToken: "encryptedToken",
 			}, nil).AnyTimes()
+		store.EXPECT().
+			GetProviderByID(gomock.Any(), gomock.Any()).
+			Return(provider, nil).AnyTimes()
 	}
 
 	return &Server{