From 8ee8e8060ead0248ff15e286c8af36292baaffd0 Mon Sep 17 00:00:00 2001 From: irenarindos Date: Wed, 23 Oct 2024 12:31:00 -0400 Subject: [PATCH] feat(server): refactor LookupWorkerByName to be a test method --- .../worker_initial_upstreams_reload_test.go | 6 +-- .../server/worker_tags_reload_test.go | 3 +- .../handlers/worker_service_status_test.go | 4 +- internal/server/repository_worker.go | 46 ------------------- internal/server/repository_worker_test.go | 33 ------------- internal/server/testing.go | 12 +++++ internal/server/testing_test.go | 24 ++++++++++ 7 files changed, 43 insertions(+), 85 deletions(-) diff --git a/internal/cmd/commands/server/worker_initial_upstreams_reload_test.go b/internal/cmd/commands/server/worker_initial_upstreams_reload_test.go index 24b7b36fb1..b268c83b00 100644 --- a/internal/cmd/commands/server/worker_initial_upstreams_reload_test.go +++ b/internal/cmd/commands/server/worker_initial_upstreams_reload_test.go @@ -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 { @@ -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 { @@ -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 { diff --git a/internal/cmd/commands/server/worker_tags_reload_test.go b/internal/cmd/commands/server/worker_tags_reload_test.go index 4159b9f181..2f99048be4 100644 --- a/internal/cmd/commands/server/worker_tags_reload_test.go +++ b/internal/cmd/commands/server/worker_tags_reload_test.go @@ -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" @@ -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] diff --git a/internal/daemon/cluster/handlers/worker_service_status_test.go b/internal/daemon/cluster/handlers/worker_service_status_test.go index 1183372751..1dc27f1a7c 100644 --- a/internal/daemon/cluster/handlers/worker_service_status_test.go +++ b/internal/daemon/cluster/handlers/worker_service_status_test.go @@ -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) }) @@ -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) }) diff --git a/internal/server/repository_worker.go b/internal/server/repository_worker.go index 339ed77925..b51dbefcf1 100644 --- a/internal/server/repository_worker.go +++ b/internal/server/repository_worker.go @@ -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 { diff --git a/internal/server/repository_worker_test.go b/internal/server/repository_worker_test.go index 1295535167..c86ce5ced8 100644 --- a/internal/server/repository_worker_test.go +++ b/internal/server/repository_worker_test.go @@ -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") diff --git a/internal/server/testing.go b/internal/server/testing.go index 8070d9bff3..ae90e56898 100644 --- a/internal/server/testing.go +++ b/internal/server/testing.go @@ -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 +} diff --git a/internal/server/testing_test.go b/internal/server/testing_test.go index 8775e2b06a..19f3b20027 100644 --- a/internal/server/testing_test.go +++ b/internal/server/testing_test.go @@ -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) { @@ -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) + }) +}