Make Project Create/Delete a service (#3227)

Relates to #2845

This is part of a change which allows me to avoid some ugly hackery in
my draft PR (#3221). Break Project creation/deletion out into a pair of
dedicated structs/interfaces. Change the `ProjectFactory` function used
in the GitHubProviderService from a method on the controlplane to a
function which returns a function. This is needed for some additional
cleanup introduced in a future PR.

internal/controlplane/handlers_oauth.go

@@ -42,7 +42,6 @@ import (
 	"github.com/stacklok/minder/internal/db"
 	"github.com/stacklok/minder/internal/engine"
 	"github.com/stacklok/minder/internal/logger"
-	"github.com/stacklok/minder/internal/projects"
 	"github.com/stacklok/minder/internal/providers"
 	"github.com/stacklok/minder/internal/providers/github/service"
 	"github.com/stacklok/minder/internal/util"
@@ -615,42 +614,6 @@ func (s *Server) VerifyProviderCredential(ctx context.Context,
 	return &pb.VerifyProviderCredentialResponse{Created: false}, nil
 }
 
-// makeProjectForGitHubApp constructs a project for a GitHub App installation which was
-// created by a user through the app installation flow on GitHub.  The flow on GitHub
-// cannot be tied back to a specific project, so we create a new project for the provider.
-//
-// This is a callback because we want to encapsulate components like s.cfg.Identity,
-// s.marketplace, s.authzClient and the like from the providers implementation.
-func (s *Server) makeProjectForGitHubApp(
-	ctx context.Context,
-	qtx db.Querier,
-	name string,
-	ghUser int64,
-) (*db.Project, error) {
-	user, err := auth.GetUserForGitHubId(ctx, s.cfg.Identity, ghUser)
-	if err != nil {
-		return nil, fmt.Errorf("error getting user for GitHub ID: %w", err)
-	}
-	// Ensure the user already exists in the database
-	if _, err := qtx.GetUserBySubject(ctx, user); err != nil {
-		return nil, fmt.Errorf("error getting user %s from database: %w", user, err)
-	}
-
-	topLevelProject, err := projects.ProvisionSelfEnrolledProject(
-		ctx,
-		s.authzClient,
-		qtx,
-		name,
-		user,
-		s.marketplace,
-		s.cfg.DefaultProfiles,
-	)
-	if err != nil {
-		return nil, fmt.Errorf("error creating project: %w", err)
-	}
-	return topLevelProject, nil
-}
-
 type httpResponseError struct {
 	statusCode   int
 	short        string

internal/controlplane/handlers_projects.go

@@ -21,7 +21,6 @@ import (
 	"errors"
 
 	"github.com/google/uuid"
-	"github.com/rs/zerolog"
 	"github.com/rs/zerolog/log"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -189,12 +188,7 @@ func (s *Server) DeleteProject(
 			"project does not allow project hierarchy operations")
 	}
 
-	l := zerolog.Ctx(ctx).With().
-		Str("component", "controlplane").
-		Str("operation", "delete").
-		Str("project", projectID.String()).
-		Logger()
-	if err := projects.DeleteProject(ctx, projectID, qtx, s.authzClient, s.providerManager, l); err != nil {
+	if err := s.projectDeleter.DeleteProject(ctx, projectID, qtx); err != nil {
 		return nil, status.Errorf(codes.Internal, "error deleting project: %v", err)
 	}
 

internal/controlplane/handlers_user.go

@@ -77,14 +77,11 @@ func (s *Server) CreateUser(ctx context.Context,
 			baseName = token.PreferredUsername()
 		}
 
-		project, err := projects.ProvisionSelfEnrolledOAuthProject(
+		project, err := s.projectCreator.ProvisionSelfEnrolledOAuthProject(
 			ctx,
-			s.authzClient,
 			qtx,
 			baseName,
 			subject,
-			s.marketplace,
-			s.cfg.DefaultProfiles,
 		)
 		if err != nil {
 			if errors.Is(err, projects.ErrProjectAlreadyExists) {
@@ -166,7 +163,7 @@ func (s *Server) DeleteUser(ctx context.Context,
 
 	subject := token.Subject()
 
-	err = DeleteUser(ctx, s.store, s.authzClient, s.providerManager, subject)
+	err = DeleteUser(ctx, s.store, s.authzClient, s.projectDeleter, subject)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to delete user from database: %v", err)
 	}

internal/controlplane/handlers_user_test.go

@@ -41,6 +41,7 @@ import (
 	"github.com/stacklok/minder/internal/db"
 	"github.com/stacklok/minder/internal/events"
 	"github.com/stacklok/minder/internal/marketplaces"
+	"github.com/stacklok/minder/internal/projects"
 	"github.com/stacklok/minder/internal/providers"
 	mockprov "github.com/stacklok/minder/internal/providers/github/service/mock"
 	"github.com/stacklok/minder/internal/providers/ratecache"
@@ -259,14 +260,19 @@ func TestCreateUser_gRPC(t *testing.T) {
 			reqCtx := tc.buildStubs(ctx, mockStore, mockJwtValidator, mockProviders)
 			crypeng := mockcrypto.NewMockEngine(ctrl)
 
+			authz := &mock.NoopClient{Authorized: true}
 			server := &Server{
 				store:        mockStore,
 				cfg:          &serverconfig.Config{},
 				cryptoEngine: crypeng,
 				vldtr:        mockJwtValidator,
 				ghProviders:  mockProviders,
-				authzClient:  &mock.NoopClient{Authorized: true},
-				marketplace:  marketplaces.NewNoopMarketplace(),
+				authzClient:  authz,
+				projectCreator: projects.NewProjectCreator(
+					authz,
+					marketplaces.NewNoopMarketplace(),
+					&serverconfig.DefaultProfilesConfig{},
+				),
 			}
 
 			// server, err := NewServer(mockStore, evt, &serverconfig.Config{

internal/controlplane/identity_events.go

@@ -31,7 +31,6 @@ import (
 	serverconfig "github.com/stacklok/minder/internal/config/server"
 	"github.com/stacklok/minder/internal/db"
 	"github.com/stacklok/minder/internal/projects"
-	"github.com/stacklok/minder/internal/providers/manager"
 )
 
 const (
@@ -54,11 +53,11 @@ func SubscribeToIdentityEvents(
 	store db.Store,
 	authzClient authz.Client,
 	cfg *serverconfig.Config,
-	providerManager manager.ProviderManager,
+	projectDeleter projects.ProjectDeleter,
 ) error {
 	c := cron.New()
 	_, err := c.AddFunc(eventFetchInterval, func() {
-		HandleEvents(ctx, store, authzClient, cfg, providerManager)
+		HandleEvents(ctx, store, authzClient, cfg, projectDeleter)
 	})
 	if err != nil {
 		return err
@@ -73,7 +72,7 @@ func HandleEvents(
 	store db.Store,
 	authzClient authz.Client,
 	cfg *serverconfig.Config,
-	providerManager manager.ProviderManager,
+	projectDeleter projects.ProjectDeleter,
 ) {
 	d := time.Now().Add(time.Duration(10) * time.Minute)
 	ctx, cancel := context.WithDeadline(ctx, d)
@@ -100,7 +99,7 @@ func HandleEvents(
 	}
 	for _, event := range events {
 		if event.Type == deleteAccountEventType {
-			err := DeleteUser(ctx, store, authzClient, providerManager, event.UserId)
+			err := DeleteUser(ctx, store, authzClient, projectDeleter, event.UserId)
 			if err != nil {
 				zerolog.Ctx(ctx).Error().Msgf("events chron: error deleting user account: %v", err)
 			}
@@ -113,7 +112,7 @@ func DeleteUser(
 	ctx context.Context,
 	store db.Store,
 	authzClient authz.Client,
-	providerManager manager.ProviderManager,
+	projectDeleter projects.ProjectDeleter,
 	userId string,
 ) error {
 	l := zerolog.Ctx(ctx).With().
@@ -140,7 +139,7 @@ func DeleteUser(
 
 		for _, proj := range projs {
 			l.Debug().Str("project_id", proj.String()).Msg("cleaning up project")
-			if err := projects.CleanUpUnmanagedProjects(l.WithContext(ctx), userId, proj, qtx, authzClient, providerManager); err != nil {
+			if err := projectDeleter.CleanUpUnmanagedProjects(l.WithContext(ctx), userId, proj, qtx); err != nil {
 				return db.User{}, fmt.Errorf("error deleting project %s: %v", proj.String(), err)
 			}
 		}

internal/controlplane/identity_events_test.go

@@ -35,6 +35,7 @@ import (
 	serverconfig "github.com/stacklok/minder/internal/config/server"
 	"github.com/stacklok/minder/internal/db"
 	"github.com/stacklok/minder/internal/db/embedded"
+	"github.com/stacklok/minder/internal/projects"
 	mockmanager "github.com/stacklok/minder/internal/providers/manager/mock"
 	minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
 )
@@ -91,8 +92,10 @@ func TestHandleEvents(t *testing.T) {
 			},
 		},
 	}
+	authzClient := &mock.NoopClient{Authorized: true}
 	mockProviderManager := mockmanager.NewMockProviderManager(ctrl)
-	HandleEvents(context.Background(), mockStore, &mock.NoopClient{Authorized: true}, &c, mockProviderManager)
+	deleter := projects.NewProjectDeleter(authzClient, mockProviderManager)
+	HandleEvents(context.Background(), mockStore, authzClient, &c, deleter)
 }
 
 func TestDeleteUserOneProject(t *testing.T) {
@@ -133,9 +136,10 @@ func TestDeleteUserOneProject(t *testing.T) {
 
 	ctrl := gomock.NewController(t)
 	mockProviderManager := mockmanager.NewMockProviderManager(ctrl)
+	deleter := projects.NewProjectDeleter(&authzClient, mockProviderManager)
 
 	t.Log("Deleting user")
-	err = DeleteUser(ctx, store, &authzClient, mockProviderManager, u1.IdentitySubject)
+	err = DeleteUser(ctx, store, &authzClient, deleter, u1.IdentitySubject)
 	assert.NoError(t, err, "DeleteUser failed")
 
 	t.Log("Checking if user is removed from DB")
@@ -213,9 +217,10 @@ func TestDeleteUserMultiProjectMembership(t *testing.T) {
 
 	ctrl := gomock.NewController(t)
 	mockProviderManager := mockmanager.NewMockProviderManager(ctrl)
+	deleter := projects.NewProjectDeleter(&authzClient, mockProviderManager)
 
 	t.Log("Deleting user")
-	err = DeleteUser(ctx, store, &authzClient, mockProviderManager, u1.IdentitySubject)
+	err = DeleteUser(ctx, store, &authzClient, deleter, u1.IdentitySubject)
 	assert.NoError(t, err, "DeleteUser failed")
 
 	t.Log("Checking if user1 is removed from DB")

internal/controlplane/server.go

@@ -58,6 +58,7 @@ import (
 	"github.com/stacklok/minder/internal/logger"
 	"github.com/stacklok/minder/internal/marketplaces"
 	"github.com/stacklok/minder/internal/profiles"
+	"github.com/stacklok/minder/internal/projects"
 	"github.com/stacklok/minder/internal/providers"
 	ghprov "github.com/stacklok/minder/internal/providers/github"
 	"github.com/stacklok/minder/internal/providers/github/clients"
@@ -101,11 +102,12 @@ type Server struct {
 	repos               github.RepositoryService
 	profiles            profiles.ProfileService
 	ghProviders         service.GitHubProviderService
-	marketplace         marketplaces.Marketplace
 	providerStore       providers.ProviderStore
 	ghClient            ghprov.ClientService
 	fallbackTokenClient *gh.Client
 	providerManager     manager.ProviderManager
+	projectCreator      projects.ProjectCreator
+	projectDeleter      projects.ProjectDeleter
 
 	// Implementations for service registration
 	pb.UnimplementedHealthServiceServer
@@ -196,7 +198,6 @@ func NewServer(
 		provMt:              provMt,
 		profiles:            profileSvc,
 		ruleTypes:           ruleSvc,
-		marketplace:         marketplace,
 		providerStore:       providerStore,
 		featureFlags:        openfeature.NewClient(cfg.Flags.AppName),
 		ghClient:            &ghprov.ClientServiceImplementation{},
@@ -215,11 +216,24 @@ func NewServer(
 		opt(s)
 	}
 
+	// Moved after opts because it depends on the authz client
+	// This will be cleaned up in the next PR
+	s.projectCreator = projects.NewProjectCreator(s.authzClient, marketplace, &cfg.DefaultProfiles)
+
+	projectFactory := makeProjectFactory(s.projectCreator, cfg.Identity)
 	// TODO: Now that RestCacheClient is passed around directly, this will be
 	// cleaned up in a future PR.
 	// Moved here because we have a dependency on s.restClientCache
 	s.ghProviders = service.NewGithubProviderService(
-		store, eng, mt, provMt, &cfg.Provider, s.makeProjectForGitHubApp, s.restClientCache, s.fallbackTokenClient)
+		store,
+		eng,
+		mt,
+		provMt,
+		&cfg.Provider,
+		projectFactory,
+		s.restClientCache,
+		s.fallbackTokenClient,
+	)
 
 	githubProviderManager := ghmanager.NewGitHubProviderClassManager(
 		s.restClientCache,
@@ -236,14 +250,16 @@ func NewServer(
 		return nil, fmt.Errorf("failed to create provider manager: %w", err)
 	}
 	s.providerManager = providerManager
+	s.projectDeleter = projects.NewProjectDeleter(s.authzClient, providerManager)
 	s.repos = github.NewRepositoryService(whManager, store, evt, providerManager)
 
 	return s, nil
 }
 
-// GetProviderManager returns the provider manager
-func (s *Server) GetProviderManager() manager.ProviderManager {
-	return s.providerManager
+// GetProjectDeleter returns the project deleter
+// TODO: this will be removed in the next PR
+func (s *Server) GetProjectDeleter() projects.ProjectDeleter {
+	return s.projectDeleter
 }
 
 // GetProviderService returns the provider service
@@ -554,3 +570,42 @@ func shutdownHandler(component string, sdf shutdowner) {
 		log.Fatal().Msgf("error shutting down '%s': %+v", component, err)
 	}
 }
+
+// makeProjectFactory creates a callback used for GitHub project creation.
+// The callback is used to construct a project for a GitHub App installation which was
+// created by a user through the app installation flow on GitHub.  The flow on GitHub
+// cannot be tied back to a specific project, so we create a new project for the provider.
+//
+// This is a callback because we want to encapsulate components like identity,
+// projectCreator and the like from the providers implementation.
+func makeProjectFactory(
+	projectCreator projects.ProjectCreator,
+	identity serverconfig.IdentityConfigWrapper,
+) service.ProjectFactory {
+	return func(
+		ctx context.Context,
+		qtx db.Querier,
+		name string,
+		ghUser int64,
+	) (*db.Project, error) {
+		user, err := auth.GetUserForGitHubId(ctx, identity, ghUser)
+		if err != nil {
+			return nil, fmt.Errorf("error getting user for GitHub ID: %w", err)
+		}
+		// Ensure the user already exists in the database
+		if _, err := qtx.GetUserBySubject(ctx, user); err != nil {
+			return nil, fmt.Errorf("error getting user %s from database: %w", user, err)
+		}
+
+		topLevelProject, err := projectCreator.ProvisionSelfEnrolledProject(
+			ctx,
+			qtx,
+			name,
+			user,
+		)
+		if err != nil {
+			return nil, fmt.Errorf("error creating project: %w", err)
+		}
+		return topLevelProject, nil
+	}
+}