diff --git a/internal/daemon/cluster/handlers/worker_service_status_test.go b/internal/daemon/cluster/handlers/worker_service_status_test.go index 8c7abb54f3..9a9c77d4f2 100644 --- a/internal/daemon/cluster/handlers/worker_service_status_test.go +++ b/internal/daemon/cluster/handlers/worker_service_status_test.go @@ -108,7 +108,7 @@ func TestStatus(t *testing.T) { WorkerStatus: &servers.ServerWorkerStatus{ PublicId: worker1.GetPublicId(), Name: worker1.GetName(), - Address: worker1.CanonicalAddress(), + Address: worker1.GetAddress(), }, }, want: &pbs.StatusResponse{ @@ -128,7 +128,7 @@ func TestStatus(t *testing.T) { WorkerStatus: &servers.ServerWorkerStatus{ PublicId: worker1.GetPublicId(), Name: worker1.GetName(), - Address: worker1.CanonicalAddress(), + Address: worker1.GetAddress(), }, Jobs: []*pbs.JobStatus{ { @@ -166,7 +166,7 @@ func TestStatus(t *testing.T) { req: &pbs.StatusRequest{ WorkerStatus: &servers.ServerWorkerStatus{ PublicId: worker1.GetPublicId(), - Address: worker1.CanonicalAddress(), + Address: worker1.GetAddress(), }, }, wantErrMsg: status.Error(codes.InvalidArgument, "Name and keyId are not set in the request; one is required.").Error(), @@ -311,7 +311,7 @@ func TestStatusSessionClosed(t *testing.T) { WorkerStatus: &servers.ServerWorkerStatus{ PublicId: worker1.GetPublicId(), Name: worker1.GetName(), - Address: worker1.CanonicalAddress(), + Address: worker1.GetAddress(), }, Jobs: []*pbs.JobStatus{ { @@ -478,7 +478,7 @@ func TestStatusDeadConnection(t *testing.T) { WorkerStatus: &servers.ServerWorkerStatus{ PublicId: worker1.GetPublicId(), Name: worker1.GetName(), - Address: worker1.CanonicalAddress(), + Address: worker1.GetAddress(), }, Jobs: []*pbs.JobStatus{ { diff --git a/internal/daemon/controller/handlers/targets/target_service.go b/internal/daemon/controller/handlers/targets/target_service.go index 787e50cd75..164a5fafe1 100644 --- a/internal/daemon/controller/handlers/targets/target_service.go +++ b/internal/daemon/controller/handlers/targets/target_service.go @@ -2140,7 +2140,7 @@ type workerList []*servers.Worker func (w workerList) addresses() []string { ret := make([]string, 0, len(w)) for _, worker := range w { - ret = append(ret, worker.CanonicalAddress()) + ret = append(ret, worker.GetAddress()) } return ret } @@ -2149,7 +2149,7 @@ func (w workerList) addresses() []string { func (w workerList) workerInfos() []*pb.WorkerInfo { ret := make([]*pb.WorkerInfo, 0, len(w)) for _, worker := range w { - ret = append(ret, &pb.WorkerInfo{Address: worker.CanonicalAddress()}) + ret = append(ret, &pb.WorkerInfo{Address: worker.GetAddress()}) } return ret } diff --git a/internal/daemon/controller/handlers/workers/worker_service_test.go b/internal/daemon/controller/handlers/workers/worker_service_test.go index 80e4532298..5705e4d467 100644 --- a/internal/daemon/controller/handlers/workers/worker_service_test.go +++ b/internal/daemon/controller/handlers/workers/worker_service_test.go @@ -70,18 +70,6 @@ func TestGet(t *testing.T) { servers.WithDescription("test kms worker description"), servers.WithAddress("test kms worker address"), servers.WithWorkerTags(&servers.Tag{Key: "key", Value: "val"})) - // Add config tags to the created worker - kmsWorker, err = repo.UpsertWorkerStatus(context.Background(), - servers.NewWorkerForStatus(kmsWorker.GetScopeId(), - servers.WithName(kmsWorker.GetName()), - servers.WithAddress(kmsWorker.GetAddress()), - servers.WithWorkerTags(&servers.Tag{ - Key: "config", - Value: "test", - })), - servers.WithUpdateTags(true), - servers.WithPublicId(kmsWorker.GetPublicId())) - require.NoError(t, err) kmsAuthzActions := make([]string, len(testAuthorizedActions)) copy(kmsAuthzActions, testAuthorizedActions) @@ -99,11 +87,10 @@ func TestGet(t *testing.T) { AuthorizedActions: strutil.StrListDelete(kmsAuthzActions, action.Update.String()), LastStatusTime: kmsWorker.GetLastStatusTime().GetTimestamp(), CanonicalTags: map[string]*structpb.ListValue{ - "key": structListValue(t, "val"), - "config": structListValue(t, "test"), + "key": structListValue(t, "val"), }, ConfigTags: map[string]*structpb.ListValue{ - "config": structListValue(t, "test"), + "key": structListValue(t, "val"), }, Type: KmsWorkerType, } @@ -113,7 +100,7 @@ func TestGet(t *testing.T) { servers.WithDescription("test pki worker description")) // Add config tags to the created worker pkiWorker, err = repo.UpsertWorkerStatus(context.Background(), - servers.NewWorkerForStatus(pkiWorker.GetScopeId(), + servers.NewWorker(pkiWorker.GetScopeId(), servers.WithName(pkiWorker.GetName()), servers.WithAddress("test kms worker address"), servers.WithWorkerTags(&servers.Tag{ diff --git a/internal/servers/repository_worker_test.go b/internal/servers/repository_worker_test.go index 133ace9467..2dd49af609 100644 --- a/internal/servers/repository_worker_test.go +++ b/internal/servers/repository_worker_test.go @@ -251,7 +251,6 @@ func TestLookupWorker(t *testing.T) { require.NoError(t, err) assert.Empty(t, cmp.Diff(w, got, protocmp.Transform())) assert.Equal(t, uint32(3), got.ActiveConnectionCount()) - assert.Empty(t, got.GetApiTags()) assert.Equal(t, map[string][]string{ "key": {"val"}, }, got.CanonicalTags()) @@ -289,13 +288,13 @@ func TestUpsertWorkerStatus(t *testing.T) { ctx := context.Background() t.Run("create an initial kms worker", func(t *testing.T) { - wStatus1 := servers.NewWorkerForStatus(scope.Global.String(), + wStatus1 := servers.NewWorker(scope.Global.String(), servers.WithAddress("address"), servers.WithName("config_name1")) worker, err := repo.UpsertWorkerStatus(ctx, wStatus1) require.NoError(t, err) assert.True(t, strings.HasPrefix(worker.GetPublicId(), "w_")) - assert.Equal(t, wStatus1.GetAddress(), worker.CanonicalAddress()) + assert.Equal(t, wStatus1.GetAddress(), worker.GetAddress()) assert.Equal(t, "config_name1", worker.Name) assert.Equal(t, worker.GetLastStatusTime().AsTime(), worker.GetUpdateTime().AsTime()) assert.Equal(t, uint32(1), worker.Version) @@ -332,13 +331,13 @@ func TestUpsertWorkerStatus(t *testing.T) { pkiWorkerKeyId := registeredNode.GetId() t.Run("update status for pki worker", func(t *testing.T) { - wStatus1 := servers.NewWorkerForStatus(scope.Global.String(), + wStatus1 := servers.NewWorker(scope.Global.String(), servers.WithAddress("pki_address")) worker, err := repo.UpsertWorkerStatus(ctx, wStatus1, servers.WithKeyId(pkiWorkerKeyId)) require.NoError(t, err) assert.True(t, strings.HasPrefix(worker.GetPublicId(), "w_")) - assert.Equal(t, wStatus1.GetAddress(), worker.CanonicalAddress()) + assert.Equal(t, wStatus1.GetAddress(), worker.GetAddress()) assert.Equal(t, "pki", worker.Name) assert.Greater(t, worker.GetLastStatusTime().AsTime(), worker.GetCreateTime().AsTime()) assert.Equal(t, worker.GetLastStatusTime().AsTime(), worker.GetUpdateTime().AsTime()) @@ -357,7 +356,7 @@ func TestUpsertWorkerStatus(t *testing.T) { { name: "conflicting name with pki", repo: repo, - status: servers.NewWorkerForStatus(scope.Global.String(), + status: servers.NewWorker(scope.Global.String(), servers.WithName(pkiWorker.GetName()), servers.WithAddress("someaddress")), errAssert: func(t *testing.T, err error) { @@ -368,7 +367,7 @@ func TestUpsertWorkerStatus(t *testing.T) { { name: "no address", repo: repo, - status: servers.NewWorkerForStatus(scope.Global.String(), + status: servers.NewWorker(scope.Global.String(), servers.WithName("worker_with_no_address")), errAssert: func(t *testing.T, err error) { t.Helper() @@ -379,7 +378,7 @@ func TestUpsertWorkerStatus(t *testing.T) { name: "cant specifying public id", repo: repo, status: func() *servers.Worker { - w := servers.NewWorkerForStatus(scope.Global.String(), + w := servers.NewWorker(scope.Global.String(), servers.WithName("worker_with_no_address"), servers.WithAddress("workeraddress")) w.PublicId = "w_specified" @@ -393,7 +392,7 @@ func TestUpsertWorkerStatus(t *testing.T) { { name: "name and key id provided", repo: repo, - status: servers.NewWorkerForStatus(scope.Global.String(), + status: servers.NewWorker(scope.Global.String(), servers.WithName("name and key id provided"), servers.WithAddress("someaddress")), options: []servers.Option{servers.WithKeyId(pkiWorkerKeyId)}, @@ -405,7 +404,7 @@ func TestUpsertWorkerStatus(t *testing.T) { { name: "no name or key id", repo: repo, - status: servers.NewWorkerForStatus(scope.Global.String(), + status: servers.NewWorker(scope.Global.String(), servers.WithAddress("no_name_address")), errAssert: func(t *testing.T, err error) { t.Helper() @@ -424,7 +423,7 @@ func TestUpsertWorkerStatus(t *testing.T) { { name: "empty scope", repo: repo, - status: servers.NewWorkerForStatus("", + status: servers.NewWorker("", servers.WithAddress("address"), servers.WithName("config_name1")), errAssert: func(t *testing.T, err error) { @@ -444,7 +443,7 @@ func TestUpsertWorkerStatus(t *testing.T) { require.NoError(t, err) return r }(), - status: servers.NewWorkerForStatus(scope.Global.String(), + status: servers.NewWorker(scope.Global.String(), servers.WithName("database failure"), servers.WithAddress("address")), errAssert: func(t *testing.T, err error) { @@ -467,7 +466,7 @@ func TestUpsertWorkerStatus(t *testing.T) { } t.Run("add another status", func(t *testing.T) { - anotherStatus := servers.NewWorkerForStatus(scope.Global.String(), + anotherStatus := servers.NewWorker(scope.Global.String(), servers.WithName("another_test_worker"), servers.WithAddress("address")) _, err = repo.UpsertWorkerStatus(ctx, anotherStatus) @@ -490,7 +489,7 @@ func TestTagUpdatingListing(t *testing.T) { ctx := context.Background() worker1 := servers.TestKmsWorker(t, conn, wrapper) - wStatus := servers.NewWorkerForStatus(scope.Global.String(), + wStatus := servers.NewWorker(scope.Global.String(), servers.WithName(worker1.GetName()), servers.WithAddress("somethingnew"), servers.WithWorkerTags( @@ -510,7 +509,7 @@ func TestTagUpdatingListing(t *testing.T) { assert.ElementsMatch(t, []string{"value1", "value2"}, worker1.CanonicalTags()["tag1"]) // Update without saying to update tags - wStatus = servers.NewWorkerForStatus(scope.Global.String(), + wStatus = servers.NewWorker(scope.Global.String(), servers.WithName(worker1.GetName()), servers.WithAddress(worker1.GetAddress()), servers.WithWorkerTags( @@ -625,7 +624,7 @@ func TestListWorkers_WithLiveness(t *testing.T) { // Push an upsert to the first worker so that its status has been // updated. _, err = serversRepo.UpsertWorkerStatus(ctx, - servers.NewWorkerForStatus(scope.Global.String(), + servers.NewWorker(scope.Global.String(), servers.WithName(worker1.GetName()), servers.WithAddress(worker1.GetAddress())), servers.WithPublicId(worker1.GetPublicId())) @@ -654,7 +653,7 @@ func TestListWorkers_WithLiveness(t *testing.T) { // Upsert second server. _, err = serversRepo.UpsertWorkerStatus(ctx, - servers.NewWorkerForStatus(scope.Global.String(), + servers.NewWorker(scope.Global.String(), servers.WithName(worker2.GetName()), servers.WithAddress(worker2.GetAddress())), servers.WithPublicId(worker2.GetPublicId())) diff --git a/internal/servers/worker.go b/internal/servers/worker.go index fb2e7f665b..8f7986e19d 100644 --- a/internal/servers/worker.go +++ b/internal/servers/worker.go @@ -85,14 +85,6 @@ func NewWorker(scopeId string, opt ...Option) *Worker { } } -// NewWorkerForStatus returns a new Worker usable for status updates. -// Valid options are WithName, WithAddress, and WithWorkerTags, all of which -// are assigned to the worker reported variations of these fields. -// All other options are ignored. -func NewWorkerForStatus(scopeId string, opt ...Option) *Worker { - return NewWorker(scopeId, opt...) -} - // allocWorker will allocate a Worker func allocWorker() Worker { return Worker{Worker: &store.Worker{}} @@ -127,15 +119,6 @@ func (w *Worker) clone() *Worker { return cWorker } -// CanonicalAddress returns the actual address boundary believes should be used -// to communicate with this worker. This will be the worker resource's address -// unless it is not set in which case it will use address the worker provides -// in its connection status updates. If neither is available, an empty string -// is returned. -func (w *Worker) CanonicalAddress() string { - return w.GetAddress() -} - // ActiveConnectionCount is the current number of sessions this worker is handling // according to the controllers. func (w *Worker) ActiveConnectionCount() uint32 { @@ -159,15 +142,6 @@ func (w *Worker) CanonicalTags() map[string][]string { return tags } -// GetApiTags returns the tags for this worker which has been set by the api. -func (w *Worker) GetApiTags() map[string][]string { - tags := make(map[string][]string) - for _, t := range w.apiTags { - tags[t.Key] = append(tags[t.Key], t.Value) - } - return tags -} - // GetConfigTags returns the tags for this worker which has been set through // the worker daemon's configuration file. func (w *Worker) GetConfigTags() map[string][]string { diff --git a/internal/servers/worker_test.go b/internal/servers/worker_test.go index 4b514091d0..7c80297f5f 100644 --- a/internal/servers/worker_test.go +++ b/internal/servers/worker_test.go @@ -19,13 +19,6 @@ import ( "google.golang.org/protobuf/types/known/timestamppb" ) -func TestWorkerCanonicalAddress(t *testing.T) { - worker := NewWorkerForStatus(scope.Global.String(), WithAddress("status")) - assert.Equal(t, "status", worker.CanonicalAddress()) - worker.Address = "worker" - assert.Equal(t, "worker", worker.CanonicalAddress()) -} - func TestWorkerCanonicalTags(t *testing.T) { w := NewWorker(scope.Global.String()) w.apiTags = []*Tag{ @@ -86,7 +79,7 @@ func TestWorkerAggregate(t *testing.T) { t.Run("Worker with status", func(t *testing.T) { id, err := newWorkerId(ctx) require.NoError(t, err) - w := NewWorkerForStatus(scope.Global.String(), + w := NewWorker(scope.Global.String(), WithAddress("address"), WithName(strings.ToLower(id))) w.Type = KmsWorkerType.String() @@ -104,7 +97,7 @@ func TestWorkerAggregate(t *testing.T) { t.Run("Worker with a config tag", func(t *testing.T) { id, err := newWorkerId(ctx) require.NoError(t, err) - w := NewWorkerForStatus(scope.Global.String(), + w := NewWorker(scope.Global.String(), WithAddress("address"), WithName(strings.ToLower(id))) w.Type = KmsWorkerType.String() @@ -129,7 +122,7 @@ func TestWorkerAggregate(t *testing.T) { t.Run("Worker with many config tag", func(t *testing.T) { id, err := newWorkerId(ctx) require.NoError(t, err) - w := NewWorkerForStatus(scope.Global.String(), + w := NewWorker(scope.Global.String(), WithAddress("address"), WithName(strings.ToLower(id))) w.Type = KmsWorkerType.String() @@ -167,7 +160,7 @@ func TestWorkerAggregate(t *testing.T) { t.Run("Worker with an api tag", func(t *testing.T) { id, err := newWorkerId(ctx) require.NoError(t, err) - w := NewWorkerForStatus(scope.Global.String(), + w := NewWorker(scope.Global.String(), WithAddress("address"), WithName(strings.ToLower(id))) w.Type = KmsWorkerType.String() @@ -193,7 +186,7 @@ func TestWorkerAggregate(t *testing.T) { t.Run("Worker with mix of tag sources", func(t *testing.T) { id, err := newWorkerId(ctx) require.NoError(t, err) - w := NewWorkerForStatus(scope.Global.String(), + w := NewWorker(scope.Global.String(), WithAddress("address"), WithName(strings.ToLower(id))) w.Type = KmsWorkerType.String() diff --git a/internal/session/job_session_cleanup_test.go b/internal/session/job_session_cleanup_test.go index 126d76411e..837567c749 100644 --- a/internal/session/job_session_cleanup_test.go +++ b/internal/session/job_session_cleanup_test.go @@ -109,7 +109,7 @@ func TestSessionConnectionCleanupJob(t *testing.T) { // Push an upsert to the first worker so that its status has been // updated. - _, err = serversRepo.UpsertWorkerStatus(ctx, servers.NewWorkerForStatus(scope.Global.String(), + _, err = serversRepo.UpsertWorkerStatus(ctx, servers.NewWorker(scope.Global.String(), servers.WithName(worker1.GetName()), servers.WithAddress(worker1.GetAddress())), servers.WithPublicId(worker1.GetPublicId())) diff --git a/internal/session/service_worker_status_report_test.go b/internal/session/service_worker_status_report_test.go index d9956ccfd0..4cf5a608b8 100644 --- a/internal/session/service_worker_status_report_test.go +++ b/internal/session/service_worker_status_report_test.go @@ -14,7 +14,6 @@ import ( "github.com/hashicorp/boundary/internal/session" "github.com/hashicorp/boundary/internal/target" "github.com/hashicorp/boundary/internal/target/tcp" - "github.com/hashicorp/boundary/internal/types/scope" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -28,12 +27,11 @@ func TestWorkerStatusReport(t *testing.T) { org, prj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) serverRepo, _ := servers.NewRepository(rw, rw, kms) - serverRepo.UpsertController(ctx, &store.Controller{ + _, err := serverRepo.UpsertController(ctx, &store.Controller{ PrivateId: "test_controller1", Address: "127.0.0.1", }) - serverRepo.UpsertWorkerStatus(ctx, servers.NewWorkerForStatus(scope.Global.String(), - servers.WithAddress("127.0.0.1"))) + require.NoError(t, err) repo, err := session.NewRepository(rw, rw, kms) require.NoError(t, err) diff --git a/testing/dbtest/session_list_benchmarks_dump_generation_test.go b/testing/dbtest/session_list_benchmarks_dump_generation_test.go index daccd59456..2b7beb0326 100644 --- a/testing/dbtest/session_list_benchmarks_dump_generation_test.go +++ b/testing/dbtest/session_list_benchmarks_dump_generation_test.go @@ -22,7 +22,6 @@ import ( "github.com/hashicorp/boundary/internal/session" "github.com/hashicorp/boundary/internal/target" "github.com/hashicorp/boundary/internal/target/tcp" - "github.com/hashicorp/boundary/internal/types/scope" "github.com/hashicorp/boundary/testing/dbtest" "github.com/stretchr/testify/require" "golang.org/x/sync/errgroup" @@ -130,11 +129,7 @@ func TestGenerateSessionBenchmarkTemplateDumps(t *testing.T) { require.NoError(err) connRepo, err := session.NewConnectionRepository(ctx, rw, rw, kms) require.NoError(err) - serversRepo, err := servers.NewRepository(rw, rw, kms) - require.NoError(err) - _, err = serversRepo.UpsertWorkerStatus(ctx, servers.NewWorkerForStatus(scope.Global.String(), - servers.WithAddress("127.0.0.1"))) - require.NoError(err) + _ = servers.TestKmsWorker(t, conn, wrap) usersStart := time.Now() t.Logf("Populating %d users", scenario.users)