Skip to content

Commit

Permalink
feat(server): refactor LookupWorkerByName to be a test method
Browse files Browse the repository at this point in the history
  • Loading branch information
irenarindos committed Oct 23, 2024
1 parent dfde64e commit 8ee8e80
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ pollFirstController:
case <-timeout.C:
t.Fatalf("timeout wait for worker to connect to first controller")
case <-poll.C:
w, err = serversRepo.LookupWorkerByName(testController.Context(), "test")
w, err = server.TestLookupWorkerByName(testController.Context(), t, "test", serversRepo)
require.NoError(err)
if w != nil {
switch {
Expand Down Expand Up @@ -135,7 +135,7 @@ pollForNoStatus:
poll.Stop()
break pollForNoStatus
case <-poll.C:
w, err = serversRepo.LookupWorkerByName(testController2.Context(), "test")
w, err = server.TestLookupWorkerByName(testController2.Context(), t, "test", serversRepo)
require.NoError(err)
if w != nil {
switch {
Expand Down Expand Up @@ -170,7 +170,7 @@ pollSecondController:
case <-timeout.C:
t.Fatalf("timeout wait for worker to connect to second controller")
case <-poll.C:
w, err = serversRepo.LookupWorkerByName(testController2.Context(), "test")
w, err = server.TestLookupWorkerByName(testController2.Context(), t, "test", serversRepo)
require.NoError(err)
if w != nil {
switch {
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/commands/server/worker_tags_reload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"
"time"

"github.com/hashicorp/boundary/internal/server"
"github.com/hashicorp/boundary/testing/controller"
wrapping "github.com/hashicorp/go-kms-wrapping/v2"
"github.com/hashicorp/go-kms-wrapping/v2/aead"
Expand Down Expand Up @@ -101,7 +102,7 @@ func TestServer_ReloadWorkerTags(t *testing.T) {
t.Helper()
serversRepo, err := testController.Controller().ServersRepoFn()
require.NoError(err)
w, err := serversRepo.LookupWorkerByName(testController.Context(), name)
w, err := server.TestLookupWorkerByName(testController.Context(), t, name, serversRepo)
require.NoError(err)
require.NotNil(w)
v, ok := w.CanonicalTags()[key]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1368,7 +1368,7 @@ func TestWorkerOperationalStatus(t *testing.T) {
}
require.NoError(err)
require.NotNil(got)
repoWorker, err := serverRepo.LookupWorkerByName(ctx, worker1.Name)
repoWorker, err := server.TestLookupWorkerByName(ctx, t, worker1.Name, serverRepo)
require.NoError(err)
assert.Equal(tc.wantState, repoWorker.OperationalState)
})
Expand Down Expand Up @@ -1537,7 +1537,7 @@ func TestWorkerLocalStorageStateStatus(t *testing.T) {
}
require.NoError(err)
require.NotNil(got)
repoWorker, err := serverRepo.LookupWorkerByName(ctx, worker1.Name)
repoWorker, err := server.TestLookupWorkerByName(ctx, t, worker1.Name, serverRepo)
require.NoError(err)
assert.Equal(tc.wantState, repoWorker.LocalStorageState)
})
Expand Down
46 changes: 0 additions & 46 deletions internal/server/repository_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,52 +65,6 @@ func (r *Repository) DeleteWorker(ctx context.Context, publicId string, _ ...Opt
return rowsDeleted, nil
}

// LookupWorkerByName returns the worker with the provided name. In the event
// that no worker is found that matches then nil, nil will be returned.
func (r *Repository) LookupWorkerByName(ctx context.Context, name string) (*Worker, error) {
const op = "server.(Repository).LookupWorkerByName"
switch {
case name == "":
return nil, errors.New(ctx, errors.InvalidParameter, op, "name is empty")
}
w, err := lookupWorkerByName(ctx, r.reader, name)
if err != nil {
return nil, errors.Wrap(ctx, err, op)
}
if w == nil {
return nil, nil
}
w.RemoteStorageStates, err = r.ListWorkerStorageBucketCredentialState(ctx, w.GetPublicId())
if err != nil {
return nil, errors.Wrap(ctx, err, op)
}
return w, nil
}

func lookupWorkerByName(ctx context.Context, reader db.Reader, name string) (*Worker, error) {
const op = "server.lookupWorkerByName"
switch {
case isNil(reader):
return nil, errors.New(ctx, errors.InvalidParameter, op, "reader is nil")
case name == "":
return nil, errors.New(ctx, errors.InvalidParameter, op, "name is empty")
}

wAgg := &workerAggregate{}
err := reader.LookupWhere(ctx, &wAgg, "name = ?", []any{name})
if err != nil {
if errors.IsNotFoundError(err) {
return nil, nil
}
return nil, errors.Wrap(ctx, err, op)
}
w, err := wAgg.toWorker(ctx)
if err != nil {
return nil, errors.Wrap(ctx, err, op)
}
return w, nil
}

func (r *Repository) LookupWorkerIdByKeyId(ctx context.Context, keyId string) (string, error) {
const op = "server.(Repository).LookupWorkerIdByKeyId"
switch {
Expand Down
33 changes: 0 additions & 33 deletions internal/server/repository_worker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,39 +110,6 @@ func TestDeleteWorker(t *testing.T) {
}
}

func TestLookupWorkerByName(t *testing.T) {
ctx := context.Background()
conn, _ := db.TestSetup(t, "postgres")
rw := db.New(conn)
wrapper := db.TestWrapper(t)
kms := kms.TestKms(t, conn, wrapper)
repo, err := server.NewRepository(ctx, rw, rw, kms)
require.NoError(t, err)

w := server.TestKmsWorker(t, conn, wrapper)
t.Run("success", func(t *testing.T) {
got, err := repo.LookupWorkerByName(ctx, w.GetName())
require.NoError(t, err)
assert.Empty(t, cmp.Diff(w.Worker, got.Worker, protocmp.Transform()))
})
t.Run("not found", func(t *testing.T) {
got, err := repo.LookupWorkerByName(ctx, "unknown_name")
require.NoError(t, err)
assert.Nil(t, got)
})
t.Run("db error", func(t *testing.T) {
conn, mock := db.TestSetupWithMock(t)
rw := db.New(conn)
mock.ExpectQuery(`SELECT`).WillReturnError(errors.New(ctx, errors.Internal, "test", "lookup-error"))
r, err := server.NewRepository(ctx, rw, rw, kms)
require.NoError(t, err)
got, err := r.LookupWorkerByName(ctx, w.GetName())
assert.NoError(t, mock.ExpectationsWereMet())
assert.Truef(t, errors.Match(errors.T(errors.Op("server.(Repository).LookupWorkerByName")), err), "got error %v", err)
assert.Nil(t, got)
})
}

func TestLookupWorkerIdByKeyId(t *testing.T) {
ctx := context.Background()
conn, _ := db.TestSetup(t, "postgres")
Expand Down
12 changes: 12 additions & 0 deletions internal/server/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,15 @@ func TestPkiWorker(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, opt ...O
require.NoError(t, err)
return wrk
}

// TestLookupWorkerByName looks up a worker by name
func TestLookupWorkerByName(ctx context.Context, t *testing.T, name string, serversRepo *Repository) (*Worker, error) {
workers, err := serversRepo.ListWorkers(ctx, []string{"global"})
require.NoError(t, err)
for _, w := range workers {
if w.GetName() == name {
return w, nil
}
}
return nil, nil
}
24 changes: 24 additions & 0 deletions internal/server/testing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ import (
"strings"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/boundary/globals"
"github.com/hashicorp/boundary/internal/db"
"github.com/hashicorp/boundary/internal/kms"
"github.com/hashicorp/boundary/internal/types/scope"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/testing/protocmp"
)

func TestTestKmsWorker(t *testing.T) {
Expand Down Expand Up @@ -71,3 +73,25 @@ func TestTestPkiWorker(t *testing.T) {
assert.True(t, strings.HasPrefix(authorizedWorker.GetPublicId(), globals.WorkerPrefix))
assert.NotEmpty(t, keyId)
}

func TestTestLookupWorkerByName(t *testing.T) {
ctx := context.Background()
conn, _ := db.TestSetup(t, "postgres")
rw := db.New(conn)
wrapper := db.TestWrapper(t)
kms := kms.TestKms(t, conn, wrapper)
repo, err := NewRepository(ctx, rw, rw, kms)
require.NoError(t, err)

w := TestKmsWorker(t, conn, wrapper)
t.Run("success", func(t *testing.T) {
got, err := TestLookupWorkerByName(ctx, t, w.GetName(), repo)
require.NoError(t, err)
assert.Empty(t, cmp.Diff(w.Worker, got.Worker, protocmp.Transform()))
})
t.Run("not found", func(t *testing.T) {
got, err := TestLookupWorkerByName(ctx, t, "unknown_name", repo)
require.NoError(t, err)
assert.Nil(t, got)
})
}

0 comments on commit 8ee8e80

Please sign in to comment.