Big refactor (#294)

Makefile

@@ -3,6 +3,7 @@ GIT_COMMIT = $(shell git rev-parse HEAD)
 RANDOM_SUFFIX := $(shell cat /dev/urandom | tr -dc 'a-z0-9' | head -c5)
 IMAGE_NAME = leg100/otfd
 IMAGE_TAG ?= $(VERSION)-$(RANDOM_SUFFIX)
+GOOSE_DBSTRING=postgres:///otf
 LD_FLAGS = " \
     -s -w \
 	-X 'github.com/leg100/otf.Version=$(VERSION)' \
@@ -78,10 +79,15 @@ image: build
 load: image
 	kind load docker-image $(IMAGE_NAME):$(IMAGE_TAG)
 
+# Install sql code generator
+.PHONY: install-pggen
+install-pggen:
+	@sh -c "which pggen > /dev/null || go install github.com/leg100/pggen/cmd/pggen@latest"
+
 # Generate sql code
 .PHONY: sql
-sql:
-	../pggen/dist/pggen-linux-amd64 gen go \
+sql: install-pggen
+	pggen gen go \
 		--postgres-connection "dbname=otf" \
 		--query-glob 'sql/queries/*.sql' \
 		--output-dir sql/pggen \
@@ -99,15 +105,30 @@ sql:
 	goimports -w ./sql/pggen
 	go fmt ./sql/pggen
 
-# Migrate SQL schema
+# Install DB migration tool
+.PHONY: install-goose
+install-goose:
+	@sh -c "which goose > /dev/null || go install github.com/pressly/goose/v3/cmd/goose@latest"
+
+# Migrate SQL schema to latest version
 .PHONY: migrate
-migrate:
-	GOOSE_DRIVER=postgres goose -dir ./sql/migrations up
+migrate: install-goose
+	GOOSE_DBSTRING=$(GOOSE_DBSTRING) GOOSE_DRIVER=postgres goose -dir ./sql/migrations up
 
 # Redo SQL schema migration
 .PHONY: migrate-redo
-migrate-redo:
-	GOOSE_DRIVER=postgres goose -dir ./sql/migrations redo
+migrate-redo: install-goose
+	GOOSE_DBSTRING=$(GOOSE_DBSTRING) GOOSE_DRIVER=postgres goose -dir ./sql/migrations redo
+
+# Rollback SQL schema by one version
+.PHONY: migrate-rollback
+migrate-rollback: install-goose
+	GOOSE_DBSTRING=$(GOOSE_DBSTRING) GOOSE_DRIVER=postgres goose -dir ./sql/migrations down
+
+# Get SQL schema migration status
+.PHONY: migrate-status
+migrate-status: install-goose
+	GOOSE_DBSTRING=$(GOOSE_DBSTRING) GOOSE_DRIVER=postgres goose -dir ./sql/migrations status
 
 # Run docs server with live reload
 .PHONY: serve-docs
@@ -126,6 +147,11 @@ paths:
 	go generate ./http/html/paths
 	goimports -w ./http/html/paths
 
+# Re-generate RBAC action strings
+.PHONY: actions
+actions:
+	stringer -type Action ./rbac
+
 # Install staticcheck linter
 .PHONY: install-linter
 install-linter:

agent/agent.go

@@ -12,7 +12,7 @@ import (
 	"path/filepath"
 
 	"github.com/go-logr/logr"
-	"github.com/leg100/otf"
+	"github.com/leg100/otf/client"
 	"golang.org/x/sync/errgroup"
 )
 
@@ -29,29 +29,29 @@ var (
 	}
 )
 
-// Agent processes runs.
-type Agent struct {
+// agent processes runs.
+type agent struct {
 	Config
-	otf.Client
+	client.Client
 	logr.Logger
 
-	Spooler        // spools new run events
-	*Terminator    // terminates runs
-	otf.Downloader // terraform cli downloader
+	spooler     // spools new run events
+	*terminator // terminates runs
+	Downloader  // terraform cli downloader
 
 	envs []string // terraform environment variables
 }
 
-// NewAgent is the constructor for an Agent
-func NewAgent(logger logr.Logger, app otf.Client, cfg Config) (*Agent, error) {
-	agent := &Agent{
-		Client:   app,
+// NewAgent is the constructor for an agent
+func NewAgent(logger logr.Logger, app client.Client, cfg Config) (*agent, error) {
+	agent := &agent{
+		Client:     app,
 		Config:     cfg,
 		Logger:     logger,
 		envs:       DefaultEnvs,
-		Spooler:    NewSpooler(app, logger, cfg),
-		Terminator: NewTerminator(),
-		Downloader: NewTerraformDownloader(),
+		spooler:    newSpooler(app, logger, cfg),
+		terminator: newTerminator(),
+		Downloader: newTerraformDownloader(),
 	}
 
 	if cfg.Sandbox {
@@ -76,26 +76,26 @@ func NewAgent(logger logr.Logger, app otf.Client, cfg Config) (*Agent, error) {
 }
 
 // Start starts the agent daemon and its workers
-func (a *Agent) Start(ctx context.Context) error {
+func (a *agent) Start(ctx context.Context) error {
 	g, ctx := errgroup.WithContext(ctx)
 
 	g.Go(func() error {
-		if err := a.Spooler.Start(ctx); err != nil {
+		if err := a.spooler.start(ctx); err != nil {
 			return fmt.Errorf("spooler terminated: %w", err)
 		}
 		return nil
 	})
 
 	g.Go(func() error {
 		for i := 0; i < a.Concurrency; i++ {
-			w := &Worker{a}
+			w := &worker{a}
 			go w.Start(ctx)
 		}
 
 		for {
 			select {
-			case cancelation := <-a.GetCancelation():
-				a.Cancel(cancelation.Run.ID(), cancelation.Forceful)
+			case cancelation := <-a.getCancelation():
+				a.cancel(cancelation.Run.ID, cancelation.Forceful)
 			case <-ctx.Done():
 				return nil
 			}

agent/download.go

@@ -13,21 +13,21 @@ import (
 	"github.com/natefinch/atomic"
 )
 
-// Download represents a current download of a version of terraform
-type Download struct {
+// download represents a current download of a version of terraform
+type download struct {
 	// for outputting progress updates
 	io.Writer
 	version   string
 	src, dest string
 	client    *http.Client
 }
 
-func (d *Download) Download() error {
+func (d *download) download() error {
 	if otf.Exists(d.dest) {
 		return nil
 	}
 
-	zipfile, err := d.download()
+	zipfile, err := d.getZipfile()
 	if err != nil {
 		return fmt.Errorf("downloading zipfile: %w", err)
 	}
@@ -44,7 +44,8 @@ func (d *Download) Download() error {
 	return nil
 }
 
-func (d *Download) download() (string, error) {
+func (d *download) getZipfile() (string, error) {
+	// TODO: why no context?
 	req, err := http.NewRequestWithContext(context.Background(), "GET", d.src, nil)
 	if err != nil {
 		return "", fmt.Errorf("building request: %w", err)
@@ -76,7 +77,7 @@ func (d *Download) download() (string, error) {
 	return tmp.Name(), nil
 }
 
-func (d *Download) unzip(zipfile string) error {
+func (d *download) unzip(zipfile string) error {
 	zr, err := zip.OpenReader(zipfile)
 	if err != nil {
 		return fmt.Errorf("opening archive: %s: %w", zipfile, err)

agent/downloader.go

@@ -13,25 +13,32 @@ import (
 
 const HashicorpReleasesHost = "releases.hashicorp.com"
 
-// TerraformDownloader downloads terraform binaries
-type TerraformDownloader struct {
-	// server hosting binaries
-	host string
-	// used to lookup destination path for saving download
-	Terraform
-	// client for downloading from server via http
-	client *http.Client
-	// mutex channel
-	mu chan struct{}
-}
+type (
+	// terraformDownloader downloads terraform binaries
+	terraformDownloader struct {
+		// server hosting binaries
+		host string
+		// used to lookup destination path for saving download
+		terraform
+		// client for downloading from server via http
+		client *http.Client
+		// mutex channel
+		mu chan struct{}
+	}
+
+	// Downloader downloads a specific version of a binary and returns its path
+	Downloader interface {
+		download(ctx context.Context, version string, w io.Writer) (string, error)
+	}
+)
 
-func NewTerraformDownloader() *TerraformDownloader {
+func newTerraformDownloader() *terraformDownloader {
 	mu := make(chan struct{}, 1)
 	mu <- struct{}{}
 
-	return &TerraformDownloader{
+	return &terraformDownloader{
 		host:      HashicorpReleasesHost,
-		Terraform: &TerraformPathFinder{},
+		terraform: &terraformPathFinder{},
 		client:    &http.Client{},
 		mu:        mu,
 	}
@@ -41,7 +48,7 @@ func NewTerraformDownloader() *TerraformDownloader {
 // filesystem and returns its path. Thread-safe: if a download is in-flight and
 // another download is requested then it'll be made to wait until the
 // former has finished.
-func (d *TerraformDownloader) Download(ctx context.Context, version string, w io.Writer) (string, error) {
+func (d *terraformDownloader) download(ctx context.Context, version string, w io.Writer) (string, error) {
 	if otf.Exists(d.dest(version)) {
 		return d.dest(version), nil
 	}
@@ -52,20 +59,20 @@ func (d *TerraformDownloader) Download(ctx context.Context, version string, w io
 		return "", ctx.Err()
 	}
 
-	err := (&Download{
+	err := (&download{
 		Writer:  w,
 		version: version,
 		src:     d.src(version),
 		dest:    d.dest(version),
 		client:  d.client,
-	}).Download()
+	}).download()
 
 	d.mu <- struct{}{}
 
 	return d.dest(version), err
 }
 
-func (d *TerraformDownloader) src(version string) string {
+func (d *terraformDownloader) src(version string) string {
 	return (&url.URL{
 		Scheme: "https",
 		Host:   d.host,
@@ -76,6 +83,6 @@ func (d *TerraformDownloader) src(version string) string {
 	}).String()
 }
 
-func (d *TerraformDownloader) dest(version string) string {
+func (d *terraformDownloader) dest(version string) string {
 	return d.TerraformPath(version)
 }

agent/downloader_test.go

@@ -25,17 +25,17 @@ func TestDownloader(t *testing.T) {
 	u, err := url.Parse(srv.URL)
 	require.NoError(t, err)
 
-	dl := NewTerraformDownloader()
+	dl := newTerraformDownloader()
 	dl.host = u.Host
-	dl.Terraform = &fakeTerraform{t.TempDir()}
+	dl.terraform = &fakeTerraform{t.TempDir()}
 	dl.client = &http.Client{
 		Transport: &http.Transport{
 			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
 		},
 	}
 
 	buf := new(bytes.Buffer)
-	tfpath, err := dl.Download(context.Background(), "1.2.3", buf)
+	tfpath, err := dl.download(context.Background(), "1.2.3", buf)
 	require.NoError(t, err)
 	require.FileExists(t, tfpath)
 	tfbin, err := os.ReadFile(tfpath)