Refactor repo connections (#333)

app/app.go

@@ -11,10 +11,9 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/leg100/otf"
 	"github.com/leg100/otf/cloud"
-	"github.com/leg100/otf/hooks"
 	"github.com/leg100/otf/inmem"
 	"github.com/leg100/otf/module"
-	"github.com/leg100/otf/workspace"
+	"github.com/leg100/otf/repo"
 )
 
 var _ otf.Application = (*Application)(nil)
@@ -31,8 +30,7 @@ type Application struct {
 
 	*otf.RunFactory
 	*otf.VCSProviderFactory
-	otf.WorkspaceConnector
-	otf.HookService
+	otf.RepoService
 	*otf.RunStarter
 	*module.Publisher
 	*otf.ModuleVersionUploader
@@ -91,16 +89,13 @@ func newChildApp(parent *Application, opts Options, db otf.DB) *Application {
 		WorkspaceService:            child,
 		ConfigurationVersionService: child,
 	}
-	child.HookService = hooks.NewService(hooks.NewServiceOptions{
-		Database:        db,
-		CloudService:    child.Service,
-		HostnameService: child,
-	})
-	child.WorkspaceConnector = &workspace.Connector{
-		HookService:        child,
-		WorkspaceService:   child,
+	child.RepoService = repo.NewService(repo.NewServiceOptions{
+		Logger:             opts.Logger,
+		Database:           db,
+		CloudService:       child.Service,
+		HostnameService:    child,
 		VCSProviderService: child,
-	}
+	})
 	child.Publisher = module.NewPublisher(child)
 	child.RunStarter = &otf.RunStarter{child}
 	child.ModuleVersionUploader = &otf.ModuleVersionUploader{child}

app/module.go

@@ -125,9 +125,7 @@ func (a *Application) DeleteModule(ctx context.Context, id string) (*otf.Module,
 		return nil, err
 	}
 
-	// TODO: delete webhook
-
-	if err = a.db.DeleteModule(ctx, id); err != nil {
+	if err = a.ModuleDeleter.Delete(ctx, module); err != nil {
 		a.Error(err, "deleting module", "subject", subject, "module", module)
 		return nil, err
 	}

app/workspace.go

@@ -2,6 +2,7 @@ package app
 
 import (
 	"context"
+	"errors"
 
 	"github.com/google/uuid"
 	"github.com/leg100/otf"
@@ -68,7 +69,13 @@ func (a *Application) ConnectWorkspace(ctx context.Context, workspaceID string,
 		return err
 	}
 
-	err = a.Connect(ctx, workspaceID, opts)
+	_, err = a.Connect(ctx, otf.ConnectOptions{
+		ConnectionType: otf.WorkspaceConnection,
+		ResourceID:     workspaceID,
+		VCSProviderID:  opts.ProviderID,
+		Identifier:     opts.Identifier,
+	})
+
 	if err != nil {
 		a.Error(err, "connecting workspace", "workspace", workspaceID, "subject", subject, "repo", opts.Identifier)
 		return err
@@ -79,38 +86,25 @@ func (a *Application) ConnectWorkspace(ctx context.Context, workspaceID string,
 	return nil
 }
 
-func (a *Application) UpdateWorkspaceRepo(ctx context.Context, workspaceID string, repo otf.WorkspaceRepo) (*otf.Workspace, error) {
+func (a *Application) DisconnectWorkspace(ctx context.Context, workspaceID string) error {
 	subject, err := a.CanAccessWorkspaceByID(ctx, rbac.UpdateWorkspaceAction, workspaceID)
 	if err != nil {
-		return nil, err
-	}
-
-	ws, err := a.db.UpdateWorkspaceRepo(ctx, workspaceID, repo)
-	if err != nil {
-		a.Error(err, "updating workspace repo connection", "workspace", workspaceID, "subject", subject, "repo", repo)
-		return nil, err
-	}
-
-	a.V(0).Info("updated workspace repo connection", "workspace", workspaceID, "subject", subject, "repo", repo)
-
-	return ws, nil
-}
-
-func (a *Application) DisconnectWorkspace(ctx context.Context, workspaceID string) (*otf.Workspace, error) {
-	subject, err := a.CanAccessWorkspaceByID(ctx, rbac.UpdateWorkspaceAction, workspaceID)
-	if err != nil {
-		return nil, err
+		return err
 	}
 
-	ws, err := a.Disconnect(ctx, workspaceID)
-	if err != nil {
+	err = a.RepoService.Disconnect(ctx, otf.DisconnectOptions{
+		ConnectionType: otf.WorkspaceConnection,
+		ResourceID:     workspaceID,
+	})
+	// ignore warnings; the repo is still disconnected successfully
+	if err != nil && !errors.Is(err, otf.ErrWarning) {
 		a.Error(err, "disconnecting workspace", "workspace", workspaceID, "subject", subject)
-		return nil, err
+		return err
 	}
 
 	a.V(0).Info("disconnected workspace", "workspace", workspaceID, "subject", subject)
 
-	return ws, nil
+	return nil
 }
 
 func (a *Application) ListWorkspaces(ctx context.Context, opts otf.WorkspaceListOptions) (*otf.WorkspaceList, error) {
@@ -186,6 +180,15 @@ func (a *Application) DeleteWorkspace(ctx context.Context, workspaceID string) (
 		return nil, err
 	}
 
+	// disconnect repo before deleting
+	if ws.Repo() != nil {
+		err = a.DisconnectWorkspace(ctx, ws.ID())
+		// ignore warnings; the repo is still disconnected successfully
+		if err != nil && !errors.Is(err, otf.ErrWarning) {
+			return nil, err
+		}
+	}
+
 	if err := a.db.DeleteWorkspace(ctx, ws.ID()); err != nil {
 		a.Error(err, "deleting workspace", "id", ws.ID(), "name", ws.Name(), "subject", subject)
 		return nil, err

cloud/client.go

@@ -32,8 +32,8 @@ type ClientOptions struct {
 }
 
 type GetRepoTarballOptions struct {
-	Identifier string // repo identifier, <owner>/<repo>
-	Ref        string // branch/tag/SHA ref
+	Identifier string  // repo identifier, <owner>/<repo>
+	Ref        *string // branch/tag/SHA ref, nil means default branch
 }
 
 type ListRepositoriesOptions struct {

cloud/vcs_event.go

@@ -17,11 +17,12 @@ type VCSEvent any
 
 // VCSPullEvent occurs when an action is carried out on a pull request
 type VCSPullEvent struct {
-	WebhookID  uuid.UUID
-	Action     VCSPullEventAction
-	Identifier string // repo identifier, <owner>/<repo>
-	CommitSHA  string
-	Branch     string // head branch
+	WebhookID     uuid.UUID
+	Action        VCSPullEventAction
+	Identifier    string // repo identifier, <owner>/<repo>
+	CommitSHA     string
+	Branch        string // head branch
+	DefaultBranch string
 }
 
 type VCSPullEventAction string
@@ -35,19 +36,21 @@ const (
 
 // VCSPushEvent occurs when a commit is pushed to a repo.
 type VCSPushEvent struct {
-	WebhookID  uuid.UUID
-	Identifier string // repo identifier, <owner>/<repo>
-	CommitSHA  string
-	Branch     string
+	WebhookID     uuid.UUID
+	Identifier    string // repo identifier, <owner>/<repo>
+	CommitSHA     string
+	Branch        string
+	DefaultBranch string
 }
 
 // VCSTagEvent occurs when a tag is created or deleted on a repo.
 type VCSTagEvent struct {
-	WebhookID  uuid.UUID
-	Identifier string // repo identifier, <owner>/<repo>
-	CommitSHA  string
-	Tag        string
-	Action     VCSTagEventAction
+	WebhookID     uuid.UUID
+	Identifier    string // repo identifier, <owner>/<repo>
+	CommitSHA     string
+	Tag           string
+	Action        VCSTagEventAction
+	DefaultBranch string
 }
 
 type VCSTagEventAction string

github/client.go

@@ -171,16 +171,18 @@ func (g *Client) ListTags(ctx context.Context, opts cloud.ListTagsOptions) ([]st
 	return tags, nil
 }
 
-func (g *Client) GetRepoTarball(ctx context.Context, topts cloud.GetRepoTarballOptions) ([]byte, error) {
-	owner, name, found := strings.Cut(topts.Identifier, "/")
+func (g *Client) GetRepoTarball(ctx context.Context, opts cloud.GetRepoTarballOptions) ([]byte, error) {
+	owner, name, found := strings.Cut(opts.Identifier, "/")
 	if !found {
-		return nil, fmt.Errorf("malformed identifier: %s", topts.Identifier)
+		return nil, fmt.Errorf("malformed identifier: %s", opts.Identifier)
 	}
 
-	opts := github.RepositoryContentGetOptions{
-		Ref: topts.Ref,
+	var gopts github.RepositoryContentGetOptions
+	if opts.Ref != nil {
+		gopts.Ref = *opts.Ref
 	}
-	link, _, err := g.client.Repositories.GetArchiveLink(ctx, owner, name, github.Tarball, &opts, true)
+
+	link, _, err := g.client.Repositories.GetArchiveLink(ctx, owner, name, github.Tarball, &gopts, true)
 	if err != nil {
 		return nil, err
 	}

github/client_test.go

@@ -52,7 +52,6 @@ func TestGetRepoTarball(t *testing.T) {
 
 	got, err := client.GetRepoTarball(ctx, cloud.GetRepoTarballOptions{
 		Identifier: "acme/terraform",
-		Ref:        "master",
 	})
 	require.NoError(t, err)
 

github/event_handler.go

@@ -52,18 +52,20 @@ func handle(r *http.Request, opts cloud.HandleEventOptions) (cloud.VCSEvent, err
 			}
 
 			return cloud.VCSTagEvent{
-				WebhookID:  opts.WebhookID,
-				Tag:        parts[2],
-				Action:     action,
-				Identifier: event.GetRepo().GetFullName(),
-				CommitSHA:  event.GetAfter(),
+				WebhookID:     opts.WebhookID,
+				Tag:           parts[2],
+				Action:        action,
+				Identifier:    event.GetRepo().GetFullName(),
+				CommitSHA:     event.GetAfter(),
+				DefaultBranch: event.GetRepo().GetDefaultBranch(),
 			}, nil
 		case "heads":
 			return cloud.VCSPushEvent{
-				WebhookID:  opts.WebhookID,
-				Branch:     parts[2],
-				Identifier: event.GetRepo().GetFullName(),
-				CommitSHA:  event.GetAfter(),
+				WebhookID:     opts.WebhookID,
+				Branch:        parts[2],
+				Identifier:    event.GetRepo().GetFullName(),
+				CommitSHA:     event.GetAfter(),
+				DefaultBranch: event.GetRepo().GetDefaultBranch(),
 			}, nil
 		default:
 			return nil, fmt.Errorf("malformed ref: %s", event.GetRef())
@@ -84,11 +86,12 @@ func handle(r *http.Request, opts cloud.HandleEventOptions) (cloud.VCSEvent, err
 		}
 
 		return cloud.VCSPullEvent{
-			WebhookID:  opts.WebhookID,
-			Action:     action,
-			Identifier: event.GetRepo().GetFullName(),
-			Branch:     event.PullRequest.Head.GetRef(),
-			CommitSHA:  event.GetPullRequest().GetHead().GetSHA(),
+			WebhookID:     opts.WebhookID,
+			Action:        action,
+			Identifier:    event.GetRepo().GetFullName(),
+			Branch:        event.PullRequest.Head.GetRef(),
+			CommitSHA:     event.GetPullRequest().GetHead().GetSHA(),
+			DefaultBranch: event.GetRepo().GetDefaultBranch(),
 		}, nil
 	default:
 		return nil, nil

gitlab/client.go

@@ -155,7 +155,7 @@ func (g *Client) GetRepoTarball(ctx context.Context, opts cloud.GetRepoTarballOp
 
 	tarball, _, err := g.client.Repositories.Archive(opts.Identifier, &gitlab.ArchiveOptions{
 		Format: otf.String("tar.gz"),
-		SHA:    otf.String(opts.Ref),
+		SHA:    opts.Ref,
 	})
 	if err != nil {
 		return nil, err

gitlab/client_test.go

@@ -75,7 +75,6 @@ func TestClient(t *testing.T) {
 
 		got, err := client.GetRepoTarball(ctx, cloud.GetRepoTarballOptions{
 			Identifier: "acme/terraform",
-			Ref:        "master",
 		})
 		require.NoError(t, err)
 

gitlab/event_handler.go

@@ -43,10 +43,11 @@ func handle(r *http.Request, opts cloud.HandleEventOptions) (cloud.VCSEvent, err
 			return nil, fmt.Errorf("malformed ref: %s", event.Ref)
 		}
 		return cloud.VCSPushEvent{
-			WebhookID:  opts.WebhookID,
-			Branch:     refParts[2],
-			Identifier: event.Project.PathWithNamespace,
-			CommitSHA:  event.After,
+			WebhookID:     opts.WebhookID,
+			Branch:        refParts[2],
+			Identifier:    event.Project.PathWithNamespace,
+			CommitSHA:     event.After,
+			DefaultBranch: event.Project.DefaultBranch,
 		}, nil
 	case *gitlab.TagEvent:
 		refParts := strings.Split(event.Ref, "/")
@@ -57,8 +58,9 @@ func handle(r *http.Request, opts cloud.HandleEventOptions) (cloud.VCSEvent, err
 			WebhookID: opts.WebhookID,
 			Tag:       refParts[2],
 			// Action:     action,
-			Identifier: event.Project.PathWithNamespace,
-			CommitSHA:  event.After,
+			Identifier:    event.Project.PathWithNamespace,
+			CommitSHA:     event.After,
+			DefaultBranch: event.Project.DefaultBranch,
 		}, nil
 	case *gitlab.MergeEvent:
 	}