fix: immutable receivers, cleanups

block · Aug 19, 2024 · 8233d17 · 8233d17
1 parent fbaeb39
commit 8233d17
Show file tree

Hide file tree

Showing 9 changed files with 20 additions and 25 deletions.
diff --git a/backend/controller/controller.go b/backend/controller/controller.go
@@ -230,7 +230,7 @@ func New(ctx context.Context, conn *sql.DB, config Config, runnerScaling scaling
 		config.ControllerTimeout = time.Second * 5
 	}
 
-	db, err := dal.New(ctx, conn, *encryption.NewBuilder().WithKMSURI(config.KMSURI))
+	db, err := dal.New(ctx, conn, encryption.NewBuilder().WithKMSURI(optional.Ptr(config.KMSURI)))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create DAL: %w", err)
 	}

diff --git a/backend/controller/cronjobs/cronjobs_test.go b/backend/controller/cronjobs/cronjobs_test.go
@@ -38,7 +38,7 @@ func TestServiceWithMockDal(t *testing.T) {
 		attemptCountMap: map[string]int{},
 	}
 	conn := sqltest.OpenForTesting(ctx, t)
-	parentDAL, err := db.New(ctx, conn, *encryption.NewBuilder())
+	parentDAL, err := db.New(ctx, conn, encryption.NewBuilder())
 	assert.NoError(t, err)
 
 	testServiceWithDal(ctx, t, mockDal, parentDAL, clk)

diff --git a/backend/controller/dal/async_calls_test.go b/backend/controller/dal/async_calls_test.go
@@ -15,7 +15,7 @@ import (
 func TestNoCallToAcquire(t *testing.T) {
 	ctx := log.ContextWithNewDefaultLogger(context.Background())
 	conn := sqltest.OpenForTesting(ctx, t)
-	dal, err := New(ctx, conn, *encryption.NewBuilder())
+	dal, err := New(ctx, conn, encryption.NewBuilder())
 	assert.NoError(t, err)
 
 	_, err = dal.AcquireAsyncCall(ctx)

diff --git a/backend/controller/dal/dal_test.go b/backend/controller/dal/dal_test.go
@@ -27,7 +27,7 @@ import (
 func TestDAL(t *testing.T) {
 	ctx := log.ContextWithNewDefaultLogger(context.Background())
 	conn := sqltest.OpenForTesting(ctx, t)
-	dal, err := New(ctx, conn, *encryption.NewBuilder())
+	dal, err := New(ctx, conn, encryption.NewBuilder())
 	assert.NoError(t, err)
 	assert.NotZero(t, dal)
 	var testContent = bytes.Repeat([]byte("sometestcontentthatislongerthanthereadbuffer"), 100)
@@ -374,7 +374,7 @@ func TestDAL(t *testing.T) {
 func TestCreateArtefactConflict(t *testing.T) {
 	ctx := log.ContextWithNewDefaultLogger(context.Background())
 	conn := sqltest.OpenForTesting(ctx, t)
-	dal, err := New(ctx, conn, *encryption.NewBuilder())
+	dal, err := New(ctx, conn, encryption.NewBuilder())
 	assert.NoError(t, err)
 
 	idch := make(chan sha256.SHA256, 2)
@@ -451,7 +451,7 @@ func assertEventsEqual(t *testing.T, expected, actual []TimelineEvent) {
 func TestDeleteOldEvents(t *testing.T) {
 	ctx := log.ContextWithNewDefaultLogger(context.Background())
 	conn := sqltest.OpenForTesting(ctx, t)
-	dal, err := New(ctx, conn, *encryption.NewBuilder())
+	dal, err := New(ctx, conn, encryption.NewBuilder())
 	assert.NoError(t, err)
 
 	var testContent = bytes.Repeat([]byte("sometestcontentthatislongerthanthereadbuffer"), 100)

diff --git a/backend/controller/dal/encryption.go b/backend/controller/dal/encryption.go
@@ -68,7 +68,7 @@ func (d *DAL) EnsureKey(ctx context.Context, generateKey func() ([]byte, error))
 
 	encryptedKey, err = tx.db.GetOnlyEncryptionKey(ctx)
 	if err != nil && dal.IsNotFound(err) {
-		logger.Infof("No encryption key found, generating a new one")
+		logger.Debugf("No encryption key found, generating a new one")
 		key, err := generateKey()
 		if err != nil {
 			return nil, fmt.Errorf("failed to generate key: %w", err)

diff --git a/backend/controller/dal/fsm_test.go b/backend/controller/dal/fsm_test.go
@@ -18,7 +18,7 @@ import (
 func TestSendFSMEvent(t *testing.T) {
 	ctx := log.ContextWithNewDefaultLogger(context.Background())
 	conn := sqltest.OpenForTesting(ctx, t)
-	dal, err := New(ctx, conn, *encryption.NewBuilder())
+	dal, err := New(ctx, conn, encryption.NewBuilder())
 	assert.NoError(t, err)
 
 	_, err = dal.AcquireAsyncCall(ctx)

diff --git a/backend/controller/dal/lease_test.go b/backend/controller/dal/lease_test.go
@@ -37,7 +37,7 @@ func TestLease(t *testing.T) {
 	}
 	ctx := log.ContextWithNewDefaultLogger(context.Background())
 	conn := sqltest.OpenForTesting(ctx, t)
-	dal, err := New(ctx, conn, *encryption.NewBuilder())
+	dal, err := New(ctx, conn, encryption.NewBuilder())
 	assert.NoError(t, err)
 
 	// TTL is too short, expect an error
@@ -72,7 +72,7 @@ func TestExpireLeases(t *testing.T) {
 	}
 	ctx := log.ContextWithNewDefaultLogger(context.Background())
 	conn := sqltest.OpenForTesting(ctx, t)
-	dal, err := New(ctx, conn, *encryption.NewBuilder())
+	dal, err := New(ctx, conn, encryption.NewBuilder())
 	assert.NoError(t, err)
 
 	leasei, _, err := dal.AcquireLease(ctx, leases.SystemKey("test"), time.Second*5, optional.None[any]())

diff --git a/cmd/ftl-controller/main.go b/cmd/ftl-controller/main.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"database/sql"
 	"fmt"
+	"github.com/alecthomas/types/optional"
 	"os"
 	"strconv"
 	"time"
@@ -56,9 +57,9 @@ func main() {
 	conn, err := sql.Open("pgx", cli.ControllerConfig.DSN)
 	kctx.FatalIfErrorf(err)
 
-	encryptionBuilder := encryption.NewBuilder().WithKMSURI(cli.ControllerConfig.KMSURI)
+	encryptionBuilder := encryption.NewBuilder().WithKMSURI(optional.Ptr(cli.ControllerConfig.KMSURI))
 	kctx.FatalIfErrorf(err)
-	dal, err := dal.New(ctx, conn, *encryptionBuilder)
+	dal, err := dal.New(ctx, conn, encryptionBuilder)
 	kctx.FatalIfErrorf(err)
 
 	configDal, err := cfdal.New(ctx, conn)

diff --git a/internal/encryption/encryption.go b/internal/encryption/encryption.go
@@ -44,28 +44,22 @@ type KeyStoreProvider interface {
 // Builder constructs a DataEncryptor when used with a provider.
 // Use a chain of With* methods to configure the builder.
 type Builder struct {
-	kmsURI      optional.Option[string]
-	awsV1Client *awsv1kms.KMS
+	kmsURI optional.Option[string]
 }
 
-func NewBuilder() *Builder {
-	return &Builder{
+func NewBuilder() Builder {
+	return Builder{
 		kmsURI: optional.None[string](),
 	}
 }
 
-// WithKMSURI sets the URI for the KMS key to use. Omitting this call or using nil will create a NoOpEncryptor.
-func (b *Builder) WithKMSURI(kmsURI *string) *Builder {
-	b.kmsURI = optional.Ptr[string](kmsURI)
+// WithKMSURI sets the URI for the KMS key to use. Omitting this call or using None will create a NoOpEncryptor.
+func (b Builder) WithKMSURI(kmsURI optional.Option[string]) Builder {
+	b.kmsURI = kmsURI
 	return b
 }
 
-func (b *Builder) WithAWSV1Client(kms *awsv1kms.KMS) *Builder {
-	b.awsV1Client = kms
-	return b
-}
-
-func (b *Builder) Build(ctx context.Context, provider KeyStoreProvider) (DataEncryptor, error) {
+func (b Builder) Build(ctx context.Context, provider KeyStoreProvider) (DataEncryptor, error) {
 	kmsURI, ok := b.kmsURI.Get()
 	if !ok {
 		return NewNoOpEncryptor(), nil