Skip to content

Commit

Permalink
fix test and lint
Browse files Browse the repository at this point in the history
  • Loading branch information
matt2e committed Jul 12, 2024
1 parent 92b3109 commit 40cc1df
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 19 deletions.
4 changes: 2 additions & 2 deletions common/configuration/asm.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@ func newASMForTesting(ctx context.Context, secretsClient *secretsmanager.Client,
if override, ok := override.Get(); ok {
return override, nil
}
return newASMLeader(ctx, secretsClient), nil
return newASMLeader(secretsClient), nil
}
followerFactory := func(ctx context.Context, url *url.URL) (client asmClient, err error) {
if override, ok := override.Get(); ok {
return override, nil
}
rpcClient := rpc.Dial(ftlv1connect.NewAdminServiceClient, url.String(), log.Error)
return newASMFollower(ctx, rpcClient, url.String()), nil
return newASMFollower(rpcClient, url.String()), nil
}
return &ASM{
coordinator: leader.NewCoordinator[asmClient](
Expand Down
2 changes: 1 addition & 1 deletion common/configuration/asm_follower.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type asmFollower struct {

var _ asmClient = &asmFollower{}

func newASMFollower(ctx context.Context, rpcClient ftlv1connect.AdminServiceClient, leaderName string) *asmFollower {
func newASMFollower(rpcClient ftlv1connect.AdminServiceClient, leaderName string) *asmFollower {
f := &asmFollower{
leaderName: leaderName,
client: rpcClient,
Expand Down
4 changes: 2 additions & 2 deletions common/configuration/asm_leader.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ type asmLeader struct {

var _ asmClient = &asmLeader{}

func newASMLeader(ctx context.Context, client *secretsmanager.Client) *asmLeader {
func newASMLeader(client *secretsmanager.Client) *asmLeader {
l := &asmLeader{
client: client,
}
return l
}

func (l *asmLeader) name() string {
return fmt.Sprintf("asm/leader")
return "asm/leader"
}

func (l *asmLeader) syncInterval() time.Duration {
Expand Down
27 changes: 14 additions & 13 deletions common/configuration/asm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func TestASMPagination(t *testing.T) {
func TestLeaderSync(t *testing.T) {
ctx := log.ContextWithNewDefaultLogger(context.Background())
sm, _, _, externalClient, clock, _ := setUp(ctx, t)
testClientSync(ctx, t, sm, externalClient, func(percentage float64) {
testClientSync(ctx, t, sm, externalClient, true, func(percentage float64) {
clock.Add(time.Duration(percentage) * asmLeaderSyncInterval)
if percentage == 1.0 {
time.Sleep(time.Second * 2)
Expand All @@ -186,7 +186,7 @@ func TestFollowerSync(t *testing.T) {
// fakeRPCClient connects the follower to the leader
fakeRPCClient := &fakeAdminClient{sm: leaderManager}
followerClock := clock.NewMock()
follower := newASMFollower(ctx, fakeRPCClient)
follower := newASMFollower(fakeRPCClient, "fake")

followerASM := newASMForTesting(ctx, externalClient, URL("http://localhost:1235"), leaser, optional.Some[asmClient](follower))
asmClient, err := followerASM.coordinator.Get()
Expand All @@ -197,7 +197,7 @@ func TestFollowerSync(t *testing.T) {
sm, err := newForTesting(ctx, leaderManager.router, []Provider[Secrets]{followerASM}, followerClock)
assert.NoError(t, err)

testClientSync(ctx, t, sm, externalClient, func(percentage float64) {
testClientSync(ctx, t, sm, externalClient, false, func(percentage float64) {
// sync leader
leaderClock.Add(time.Duration(percentage) * asmLeaderSyncInterval)
if percentage == 1.0 {
Expand All @@ -217,6 +217,7 @@ func testClientSync(ctx context.Context,
t *testing.T,
sm *Manager[Secrets],
externalClient *secretsmanager.Client,
isLeader bool,
progressByIntervalPercentage func(percentage float64)) {
t.Helper()

Expand Down Expand Up @@ -293,6 +294,12 @@ func testClientSync(ctx context.Context,
assert.Equal(t, expectedValue, string(value), "unexpected secret value for %s", entry.Ref)
}

if !isLeader {
// only test deleting secrets causing errors in ASM Leader.
// when leader starts returning errors for list, follower will not get updates
return
}

// delete 2 secrets without client knowing
tr := true
_, err = externalClient.DeleteSecret(ctx, &secretsmanager.DeleteSecretInput{
Expand All @@ -308,17 +315,11 @@ func testClientSync(ctx context.Context,

// give client a change to sync
progressByIntervalPercentage(1.0)
time.Sleep(time.Second * 5)

if _, hadSyncError := sm.cache.providers["asm"].currentBackoff.Get(); hadSyncError {
// expect follower to have a sync error because leader can't return list of secret values
} else {
// otherwise confirm secrets were deleted
_, err = sm.getData(ctx, smRef)
assert.Error(t, err, "expected to fail because secret was deleted")
_, err = sm.getData(ctx, smClientRef)
assert.Error(t, err, "expected to fail because secret was deleted")
}
_, err = sm.getData(ctx, smRef)
assert.Error(t, err, "expected to fail because secret was deleted")
_, err = sm.getData(ctx, smClientRef)
assert.Error(t, err, "expected to fail because secret was deleted")
}

func storeUnobfuscatedValueInASM(ctx context.Context, sm *Manager[Secrets], externalClient *secretsmanager.Client, ref Ref, value []byte, isNew bool) error {
Expand Down
20 changes: 19 additions & 1 deletion common/configuration/db_secret_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"net/url"
"sync"
"testing"

"github.com/TBD54566975/ftl/common/configuration/dal"
Expand All @@ -12,6 +13,7 @@ import (
)

type mockDBSecretResolverDAL struct {
lock sync.Mutex
entries []dal.ModuleSecret
}

Expand All @@ -25,6 +27,9 @@ func (d *mockDBSecretResolverDAL) findEntry(module Option[string], name string)
}

func (d *mockDBSecretResolverDAL) GetModuleSecretURL(ctx context.Context, module Option[string], name string) (string, error) {
d.lock.Lock()
defer d.lock.Unlock()

entry, _ := d.findEntry(module, name)
if e, ok := entry.Get(); ok {
return e.Url, nil
Expand All @@ -33,11 +38,17 @@ func (d *mockDBSecretResolverDAL) GetModuleSecretURL(ctx context.Context, module
}

func (d *mockDBSecretResolverDAL) ListModuleSecrets(ctx context.Context) ([]dal.ModuleSecret, error) {
d.lock.Lock()
defer d.lock.Unlock()

return d.entries, nil
}

func (d *mockDBSecretResolverDAL) SetModuleSecretURL(ctx context.Context, module Option[string], name string, url string) error {
err := d.UnsetModuleSecret(ctx, module, name)
d.lock.Lock()
defer d.lock.Unlock()

err := d.remove(ctx, module, name)
if err != nil {
return fmt.Errorf("could not unset secret %w", err)
}
Expand All @@ -46,6 +57,13 @@ func (d *mockDBSecretResolverDAL) SetModuleSecretURL(ctx context.Context, module
}

func (d *mockDBSecretResolverDAL) UnsetModuleSecret(ctx context.Context, module Option[string], name string) error {
d.lock.Lock()
defer d.lock.Unlock()

return d.remove(ctx, module, name)
}

func (d *mockDBSecretResolverDAL) remove(ctx context.Context, module Option[string], name string) error {

Check failure on line 66 in common/configuration/db_secret_resolver_test.go

View workflow job for this annotation

GitHub Actions / Lint

`(*mockDBSecretResolverDAL).remove` - `ctx` is unused (unparam)
entry, i := d.findEntry(module, name)
if _, ok := entry.Get(); ok {
d.entries = append(d.entries[:i], d.entries[i+1:]...)
Expand Down

0 comments on commit 40cc1df

Please sign in to comment.