From 3fbf901739f7e7b933bae448ccb902013c673bc7 Mon Sep 17 00:00:00 2001 From: Todd Date: Fri, 20 May 2022 11:55:54 -0700 Subject: [PATCH] Add source to worker tags table (#2087) * Remove the server_id from session table, change it to worker_id on session_connection This is step 1 in updating the worker table. The worker is becoming a public resource so it must have a public id. We drop the server_id column on the session_connection table and add the worker_id column so we can use the wt_public_key type. Since db migrations are applied while the controller is offline, it should be safe to assume all session connections are not in a running state, thus allowing us to null out all the worker ids in the session_connection table. For now we don't have very good enforcement of the wt_public_id contraints, since the id is currently defined in the worker config, but this will soon change and the worker's id will be generated by the controller. * Adding worker tag db writing test. * Move the server table back to the top. * Update comment for the worker object. * Update comment for the worker table. * Add worker tag sources ("config" or "api") * Revert renaming of UpsertWorker until later. * Remove no longer needed files. * Moving the TestWorker into the servers package. Fix the worker tags test. * Update internal/proto/controller/storage/servers/store/v1/worker.proto Co-authored-by: Irena Rindos * Place the repos back together so they can be refactored in a different PR. Co-authored-by: Irena Rindos --- .../workers/worker_service_status_test.go | 6 +-- .../99/02_worker_controller_tables.up.sql | 20 ++++++++- .../storage/servers/store/v1/worker.proto | 4 ++ internal/servers/query.go | 2 +- internal/servers/repository.go | 8 ++-- internal/servers/repository_test.go | 2 +- internal/servers/store/worker.pb.go | 23 +++++++--- internal/servers/testing.go | 31 ++++++-------- internal/servers/testing_test.go | 25 +++++++++++ internal/servers/worker.go | 8 ---- internal/servers/worker_tag.go | 26 ++++++++++++ internal/servers/worker_tags_test.go | 42 ++++++++++++++++--- .../session/repository_connection_test.go | 5 ++- .../service_authorize_connection_test.go | 3 +- .../service_worker_status_report_test.go | 14 +++---- internal/session/session_cleanup_job_test.go | 16 +++---- internal/session/testing.go | 26 ------------ internal/session/testing_test.go | 3 +- 18 files changed, 171 insertions(+), 93 deletions(-) create mode 100644 internal/servers/testing_test.go create mode 100644 internal/servers/worker_tag.go diff --git a/internal/daemon/controller/handlers/workers/worker_service_status_test.go b/internal/daemon/controller/handlers/workers/worker_service_status_test.go index 9c8854fa2..bc3842897 100644 --- a/internal/daemon/controller/handlers/workers/worker_service_status_test.go +++ b/internal/daemon/controller/handlers/workers/worker_service_status_test.go @@ -71,7 +71,7 @@ func TestStatus(t *testing.T) { target.WithSessionConnectionLimit(-1), ) - worker1 := session.TestWorker(t, conn, wrapper) + worker1 := servers.TestWorker(t, conn, wrapper) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, @@ -239,7 +239,7 @@ func TestStatusSessionClosed(t *testing.T) { target.WithSessionConnectionLimit(-1), ) - worker1 := session.TestWorker(t, conn, wrapper) + worker1 := servers.TestWorker(t, conn, wrapper) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, @@ -421,7 +421,7 @@ func TestStatusDeadConnection(t *testing.T) { target.WithSessionConnectionLimit(-1), ) - worker1 := session.TestWorker(t, conn, wrapper) + worker1 := servers.TestWorker(t, conn, wrapper) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, diff --git a/internal/db/schema/migrations/oss/postgres/99/02_worker_controller_tables.up.sql b/internal/db/schema/migrations/oss/postgres/99/02_worker_controller_tables.up.sql index 88f97444b..aa1ed8142 100644 --- a/internal/db/schema/migrations/oss/postgres/99/02_worker_controller_tables.up.sql +++ b/internal/db/schema/migrations/oss/postgres/99/02_worker_controller_tables.up.sql @@ -59,6 +59,19 @@ create trigger worker_update_time_column before update on server_worker for each row execute procedure update_time_column(); -- Create table worker tag +create table server_worker_tag_enm ( + source text primary key + constraint only_predefined_server_worker_tag_sources_allowed + check ( + source in ('configuration', 'api') + ) +); + +insert into server_worker_tag_enm (source) +values + ('configuration'), + ('api'); + create table server_worker_tag ( worker_id wt_public_id constraint server_worker_fkey @@ -67,7 +80,12 @@ create table server_worker_tag ( on update cascade, key wt_tagpair, value wt_tagpair, - primary key(worker_id, key, value) + source text not null + constraint server_worker_tag_enm_fkey + references server_worker_tag_enm(source) + on delete restrict + on update cascade, + primary key(worker_id, key, value, source) ); -- Aaand drop server_tag diff --git a/internal/proto/controller/storage/servers/store/v1/worker.proto b/internal/proto/controller/storage/servers/store/v1/worker.proto index 3898131d2..aa743f32d 100644 --- a/internal/proto/controller/storage/servers/store/v1/worker.proto +++ b/internal/proto/controller/storage/servers/store/v1/worker.proto @@ -55,4 +55,8 @@ message WorkerTag { // value is the value // @inject_tag: `gorm:"default:null"` string value = 30; + + // source is the source of the tag. Either 'configuration' or 'api'. + // @inject_tag: `gorm:"default:not_null"` + string source = 40; } diff --git a/internal/servers/query.go b/internal/servers/query.go index 81d52677e..61e07b293 100644 --- a/internal/servers/query.go +++ b/internal/servers/query.go @@ -2,5 +2,5 @@ package servers const ( deleteWhereCreateTimeSql = `create_time < ?` - deleteTagsSql = `worker_id = ?` + deleteConfigTagsSql = `source = 'configuration' and worker_id = ?` ) diff --git a/internal/servers/repository.go b/internal/servers/repository.go index 908a6beee..f6609847e 100644 --- a/internal/servers/repository.go +++ b/internal/servers/repository.go @@ -119,7 +119,8 @@ func (r *Repository) listControllersWithReader(ctx context.Context, reader db.Re // ListTagsForWorkers returns a map from the worker's id to the list of Tags // that are for that worker. All options are ignored. -func (r *Repository) ListTagsForWorkers(ctx context.Context, workerIds []string, opt ...Option) (map[string][]*Tag, error) { +func (r *Repository) ListTagsForWorkers(ctx context.Context, workerIds []string, _ ...Option) (map[string][]*Tag, error) { + const op = "servers.ListTagsForWorkers" var workerTags []*store.WorkerTag if err := r.reader.SearchWhere( ctx, @@ -128,7 +129,7 @@ func (r *Repository) ListTagsForWorkers(ctx context.Context, workerIds []string, []interface{}{workerIds}, db.WithLimit(-1), ); err != nil { - return nil, errors.Wrap(ctx, err, "servers.ListTagsForWorkers", errors.WithMsg(fmt.Sprintf("worker IDs %v", workerIds))) + return nil, errors.Wrap(ctx, err, op, errors.WithMsg(fmt.Sprintf("worker IDs %v", workerIds))) } ret := make(map[string][]*Tag, len(workerIds)) @@ -185,7 +186,7 @@ func (r *Repository) UpsertWorker(ctx context.Context, worker *Worker, opt ...Op // delete all tags for the given worker, then add the new ones // we've been sent. if opts.withUpdateTags { - _, err = w.Delete(ctx, &store.WorkerTag{}, db.WithWhere(deleteTagsSql, worker.GetPublicId())) + _, err = w.Delete(ctx, &store.WorkerTag{}, db.WithWhere(deleteConfigTagsSql, worker.GetPublicId())) if err != nil { return errors.Wrap(ctx, err, op+":DeleteTags", errors.WithMsg(worker.GetPublicId())) } @@ -204,6 +205,7 @@ func (r *Repository) UpsertWorker(ctx context.Context, worker *Worker, opt ...Op WorkerId: worker.GetPublicId(), Key: v.Key, Value: v.Value, + Source: string(ConfigurationTagSource), }) } if err = w.CreateItems(ctx, tags); err != nil { diff --git a/internal/servers/repository_test.go b/internal/servers/repository_test.go index 8f46ebe7e..30519f577 100644 --- a/internal/servers/repository_test.go +++ b/internal/servers/repository_test.go @@ -194,7 +194,7 @@ func TestTagUpdatingListing(t *testing.T) { require.Equal(exp, tags) } -func TestListServersWithLiveness(t *testing.T) { +func TestListWorkersWithLiveness(t *testing.T) { t.Parallel() require := require.New(t) conn, _ := db.TestSetup(t, "postgres") diff --git a/internal/servers/store/worker.pb.go b/internal/servers/store/worker.pb.go index 3ad103fc6..1b1825ab0 100644 --- a/internal/servers/store/worker.pb.go +++ b/internal/servers/store/worker.pb.go @@ -157,6 +157,9 @@ type WorkerTag struct { // value is the value // @inject_tag: `gorm:"default:null"` Value string `protobuf:"bytes,30,opt,name=value,proto3" json:"value,omitempty" gorm:"default:null"` + // source is the source of the tag. Either 'configuration' or 'api'. + // @inject_tag: `gorm:"default:null"` + Source string `protobuf:"bytes,40,opt,name=source,proto3" json:"source,omitempty" gorm:"default:null"` } func (x *WorkerTag) Reset() { @@ -212,6 +215,13 @@ func (x *WorkerTag) GetValue() string { return "" } +func (x *WorkerTag) GetSource() string { + if x != nil { + return x.Source + } + return "" +} + var File_controller_storage_servers_store_v1_worker_proto protoreflect.FileDescriptor var file_controller_storage_servers_store_v1_worker_proto_rawDesc = []byte{ @@ -244,16 +254,17 @@ var file_controller_storage_servers_store_v1_worker_proto_rawDesc = []byte{ 0x6f, 0x70, 0x65, 0x5f, 0x69, 0x64, 0x18, 0x46, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x73, 0x63, 0x6f, 0x70, 0x65, 0x49, 0x64, 0x12, 0x18, 0x0a, 0x07, 0x76, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x18, 0x50, 0x20, 0x01, 0x28, 0x0d, 0x52, 0x07, 0x76, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x22, - 0x50, 0x0a, 0x09, 0x57, 0x6f, 0x72, 0x6b, 0x65, 0x72, 0x54, 0x61, 0x67, 0x12, 0x1b, 0x0a, 0x09, + 0x68, 0x0a, 0x09, 0x57, 0x6f, 0x72, 0x6b, 0x65, 0x72, 0x54, 0x61, 0x67, 0x12, 0x1b, 0x0a, 0x09, 0x77, 0x6f, 0x72, 0x6b, 0x65, 0x72, 0x5f, 0x69, 0x64, 0x18, 0x0a, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, 0x77, 0x6f, 0x72, 0x6b, 0x65, 0x72, 0x49, 0x64, 0x12, 0x10, 0x0a, 0x03, 0x6b, 0x65, 0x79, 0x18, 0x14, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x6b, 0x65, 0x79, 0x12, 0x14, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x1e, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x76, 0x61, 0x6c, 0x75, - 0x65, 0x42, 0x3c, 0x5a, 0x3a, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, - 0x68, 0x61, 0x73, 0x68, 0x69, 0x63, 0x6f, 0x72, 0x70, 0x2f, 0x62, 0x6f, 0x75, 0x6e, 0x64, 0x61, - 0x72, 0x79, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x73, 0x65, 0x72, 0x76, - 0x65, 0x72, 0x73, 0x2f, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x3b, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x62, - 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x65, 0x12, 0x16, 0x0a, 0x06, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x18, 0x28, 0x20, 0x01, 0x28, + 0x09, 0x52, 0x06, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x42, 0x3c, 0x5a, 0x3a, 0x67, 0x69, 0x74, + 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x68, 0x61, 0x73, 0x68, 0x69, 0x63, 0x6f, 0x72, + 0x70, 0x2f, 0x62, 0x6f, 0x75, 0x6e, 0x64, 0x61, 0x72, 0x79, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, + 0x6e, 0x61, 0x6c, 0x2f, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x73, 0x2f, 0x73, 0x74, 0x6f, 0x72, + 0x65, 0x3b, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( diff --git a/internal/servers/testing.go b/internal/servers/testing.go index def4ca5e6..79e5c42b1 100644 --- a/internal/servers/testing.go +++ b/internal/servers/testing.go @@ -10,10 +10,8 @@ import ( "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/hashicorp/boundary/internal/iam" "github.com/hashicorp/boundary/internal/kms" - "github.com/hashicorp/boundary/internal/servers/store" "github.com/hashicorp/boundary/internal/types/scope" wrapping "github.com/hashicorp/go-kms-wrapping/v2" - "github.com/hashicorp/go-uuid" "github.com/stretchr/testify/require" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -39,7 +37,7 @@ func TestKkmsKey(ctx context.Context, t *testing.T, conn *db.DB, wrapper wrappin return testKey } -func TestRootCertificate(ctx context.Context, t *testing.T, conn *db.DB, kmsKey string) *store.RootCertificate { +func TestRootCertificate(ctx context.Context, t *testing.T, conn *db.DB, kmsKey string) *RootCertificate { t.Helper() rw := db.New(conn) @@ -55,10 +53,10 @@ func TestRootCertificate(ctx context.Context, t *testing.T, conn *db.DB, kmsKey rootCertKeys, kmsKey, CurrentState) err = rw.Create(ctx, cert) require.NoError(t, err) - return cert.RootCertificate + return cert } -func TestWorkerAuth(ctx context.Context, t *testing.T, conn *db.DB, worker *store.Worker, kmsKey string) *store.WorkerAuth { +func TestWorkerAuth(ctx context.Context, t *testing.T, conn *db.DB, worker *Worker, kmsKey string) *WorkerAuth { t.Helper() rw := db.New(conn) wSignPubKey := populateBytes(defaultLength) @@ -78,30 +76,25 @@ func TestWorkerAuth(ctx context.Context, t *testing.T, conn *db.DB, worker *stor err = rw.Create(ctx, workerAuth) require.NoError(t, err) - return workerAuth.WorkerAuth + return workerAuth } -func TestWorker(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper) *store.Worker { +func TestWorker(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper) *Worker { t.Helper() rw := db.New(conn) kms := kms.TestKms(t, conn, wrapper) serversRepo, err := NewRepository(rw, rw, kms) require.NoError(t, err) - id, err := uuid.GenerateUUID() + id, err := newWorkerId(context.Background()) require.NoError(t, err) - id = "test-session-worker-" + id - name := "test-worker-" + id - worker := &Worker{ - Worker: &store.Worker{ - PublicId: id, - Name: name, - Address: "127.0.0.1", - ScopeId: scope.Global.String(), - }, - } + + worker := NewWorker(scope.Global.String(), + WithPublicId(id), + WithName(name), + WithAddress("127.0.0.1")) _, _, err = serversRepo.UpsertWorker(context.Background(), worker) require.NoError(t, err) - return worker.Worker + return worker } diff --git a/internal/servers/testing_test.go b/internal/servers/testing_test.go new file mode 100644 index 000000000..0764adf3e --- /dev/null +++ b/internal/servers/testing_test.go @@ -0,0 +1,25 @@ +package servers + +import ( + "context" + "strings" + "testing" + + "github.com/hashicorp/boundary/internal/db" + "github.com/hashicorp/boundary/internal/types/scope" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTestWorker(t *testing.T) { + conn, _ := db.TestSetup(t, "postgres") + wrapper := db.TestWrapper(t) + tWorker := TestWorker(t, conn, wrapper) + assert.NotNil(t, tWorker) + assert.True(t, strings.HasPrefix(tWorker.GetPublicId(), WorkerPrefix)) + + lkpWorker := NewWorker(scope.Global.String(), WithPublicId(tWorker.GetPublicId())) + rw := db.New(conn) + require.NoError(t, rw.LookupById(context.Background(), lkpWorker)) + assert.NotNil(t, lkpWorker) +} diff --git a/internal/servers/worker.go b/internal/servers/worker.go index 2b1a68af4..eb1d9b0eb 100644 --- a/internal/servers/worker.go +++ b/internal/servers/worker.go @@ -13,14 +13,6 @@ type Worker struct { Tags []*Tag `gorm:"-"` } -// A Tag is a custom key/value pair which can be attached to a Worker. -// Multiple Tags may contain the same key and different values in which -// case both key/value pairs are valid. -type Tag struct { - Key string - Value string -} - // NewWorker returns a new Worker. Valid options are WithName, WithDescription // WithAddress, and WithWorkerTags. All other options are ignored. func NewWorker(scopeId string, opt ...Option) *Worker { diff --git a/internal/servers/worker_tag.go b/internal/servers/worker_tag.go new file mode 100644 index 000000000..75afc0fe6 --- /dev/null +++ b/internal/servers/worker_tag.go @@ -0,0 +1,26 @@ +package servers + +// A Tag is a custom key/value pair which can be attached to a Worker. +// Multiple Tags may contain the same key and different values in which +// case both key/value pairs are valid. Tags can be sourced from either the +// worker's configuration or the api. key/value pairs can be the same from +// different sources. +type Tag struct { + Key string + Value string +} + +type TagSource string + +const ( + ConfigurationTagSource TagSource = "configuration" + ApiTagSource TagSource = "api" +) + +func (t TagSource) isValid() bool { + return t == ConfigurationTagSource || t == ApiTagSource +} + +func (t TagSource) String() string { + return string(t) +} diff --git a/internal/servers/worker_tags_test.go b/internal/servers/worker_tags_test.go index bd7e16019..679832e6a 100644 --- a/internal/servers/worker_tags_test.go +++ b/internal/servers/worker_tags_test.go @@ -6,19 +6,16 @@ import ( "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/servers/store" - "github.com/hashicorp/boundary/internal/types/scope" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestWorkerTags_Create(t *testing.T) { t.Parallel() conn, _ := db.TestSetup(t, "postgres") + wrapper := db.TestWrapper(t) rw := db.New(conn) - workerPublicid, err := newWorkerId(context.Background()) - require.NoError(t, err) - worker := NewWorker(scope.Global.String(), WithPublicId(workerPublicid)) + worker := TestWorker(t, conn, wrapper) tests := []struct { name string @@ -26,12 +23,41 @@ func TestWorkerTags_Create(t *testing.T) { wantCreateErr bool }{ { - name: "success", + name: "success api source", + want: &store.WorkerTag{ + WorkerId: worker.GetPublicId(), + Key: "key", + Value: "value", + Source: ApiTagSource.String(), + }, + }, + { + name: "success config source", + want: &store.WorkerTag{ + WorkerId: worker.GetPublicId(), + Key: "key", + Value: "value", + Source: ConfigurationTagSource.String(), + }, + }, + { + name: "unknown source", + want: &store.WorkerTag{ + WorkerId: worker.GetPublicId(), + Key: "key", + Value: "value", + Source: "unknown", + }, + wantCreateErr: true, + }, + { + name: "no source", want: &store.WorkerTag{ WorkerId: worker.GetPublicId(), Key: "key", Value: "value", }, + wantCreateErr: true, }, { name: "bad worker id", @@ -56,6 +82,7 @@ func TestWorkerTags_Create(t *testing.T) { WorkerId: worker.GetPublicId(), Value: "value", }, + wantCreateErr: true, }, { name: "missing value", @@ -63,6 +90,7 @@ func TestWorkerTags_Create(t *testing.T) { WorkerId: worker.GetPublicId(), Key: "key", }, + wantCreateErr: true, }, } @@ -72,6 +100,8 @@ func TestWorkerTags_Create(t *testing.T) { err := rw.Create(context.Background(), tt.want) if tt.wantCreateErr { assert.Error(t, err) + } else { + assert.NoError(t, err) } }) } diff --git a/internal/session/repository_connection_test.go b/internal/session/repository_connection_test.go index ba5d38029..200df8272 100644 --- a/internal/session/repository_connection_test.go +++ b/internal/session/repository_connection_test.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/boundary/internal/iam" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" + "github.com/hashicorp/boundary/internal/servers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -358,8 +359,8 @@ func TestRepository_orphanedConnections(t *testing.T) { // Create two "workers". One will remain untouched while the other "goes // away and comes back" (worker 2). - worker1 := TestWorker(t, conn, wrapper) - worker2 := TestWorker(t, conn, wrapper) + worker1 := servers.TestWorker(t, conn, wrapper) + worker2 := servers.TestWorker(t, conn, wrapper) // Create a few sessions on each, activate, and authorize a connection var connIds []string diff --git a/internal/session/service_authorize_connection_test.go b/internal/session/service_authorize_connection_test.go index b6d88580c..6261bfb06 100644 --- a/internal/session/service_authorize_connection_test.go +++ b/internal/session/service_authorize_connection_test.go @@ -5,6 +5,7 @@ import ( "testing" "time" + "github.com/hashicorp/boundary/internal/servers" "google.golang.org/protobuf/types/known/timestamppb" "github.com/hashicorp/boundary/internal/db" @@ -35,7 +36,7 @@ func TestService_AuthorizeConnection(t *testing.T) { composedOf.ExpirationTime = exp } s := TestSession(t, conn, wrapper, composedOf) - srv := TestWorker(t, conn, wrapper) + srv := servers.TestWorker(t, conn, wrapper) testServer = srv.PublicId tofu := TestTofu(t) _, _, err := repo.ActivateSession(context.Background(), s.PublicId, s.Version, tofu) diff --git a/internal/session/service_worker_status_report_test.go b/internal/session/service_worker_status_report_test.go index ff5b2b6d7..330da5202 100644 --- a/internal/session/service_worker_status_report_test.go +++ b/internal/session/service_worker_status_report_test.go @@ -68,7 +68,7 @@ func TestWorkerStatusReport(t *testing.T) { { name: "No Sessions", caseFn: func(t *testing.T) testCase { - worker := session.TestWorker(t, conn, wrapper) + worker := servers.TestWorker(t, conn, wrapper) return testCase{ worker: worker, req: []session.StateReport{}, @@ -79,7 +79,7 @@ func TestWorkerStatusReport(t *testing.T) { { name: "No Sessions already canceled", caseFn: func(t *testing.T) testCase { - worker := session.TestWorker(t, conn, wrapper) + worker := servers.TestWorker(t, conn, wrapper) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, HostId: h.GetPublicId(), @@ -111,7 +111,7 @@ func TestWorkerStatusReport(t *testing.T) { { name: "Still Active", caseFn: func(t *testing.T) testCase { - worker := session.TestWorker(t, conn, wrapper) + worker := servers.TestWorker(t, conn, wrapper) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, HostId: h.GetPublicId(), @@ -145,7 +145,7 @@ func TestWorkerStatusReport(t *testing.T) { { name: "SessionClosed", caseFn: func(t *testing.T) testCase { - worker := session.TestWorker(t, conn, wrapper) + worker := servers.TestWorker(t, conn, wrapper) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, HostId: h.GetPublicId(), @@ -185,7 +185,7 @@ func TestWorkerStatusReport(t *testing.T) { { name: "MultipleSessionsClosed", caseFn: func(t *testing.T) testCase { - worker := session.TestWorker(t, conn, wrapper) + worker := servers.TestWorker(t, conn, wrapper) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, HostId: h.GetPublicId(), @@ -252,7 +252,7 @@ func TestWorkerStatusReport(t *testing.T) { { name: "OrphanedConnection", caseFn: func(t *testing.T) testCase { - worker := session.TestWorker(t, conn, wrapper) + worker := servers.TestWorker(t, conn, wrapper) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, HostId: h.GetPublicId(), @@ -303,7 +303,7 @@ func TestWorkerStatusReport(t *testing.T) { { name: "MultipleSessionsAndOrphanedConnections", caseFn: func(t *testing.T) testCase { - worker := session.TestWorker(t, conn, wrapper) + worker := servers.TestWorker(t, conn, wrapper) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, HostId: h.GetPublicId(), diff --git a/internal/session/session_cleanup_job_test.go b/internal/session/session_cleanup_job_test.go index fb8d819b3..75a75fd03 100644 --- a/internal/session/session_cleanup_job_test.go +++ b/internal/session/session_cleanup_job_test.go @@ -44,8 +44,8 @@ func TestSessionConnectionCleanupJob(t *testing.T) { // Create two "workers". One will remain untouched while the other "goes // away and comes back" (worker 2). - worker1 := TestWorker(t, conn, wrapper) - worker2 := TestWorker(t, conn, wrapper) + worker1 := servers.TestWorker(t, conn, wrapper) + worker2 := servers.TestWorker(t, conn, wrapper) // Create a few sessions on each, activate, and authorize a connection var connIds []string @@ -196,10 +196,10 @@ func TestCloseConnectionsForDeadWorkers(t *testing.T) { // firstly, the last worker will have no connections at all, and we will be // closing the others in stages to test multiple servers being closed at // once. - worker1 := TestWorker(t, conn, wrapper) - worker2 := TestWorker(t, conn, wrapper) - worker3 := TestWorker(t, conn, wrapper) - worker4 := TestWorker(t, conn, wrapper) + worker1 := servers.TestWorker(t, conn, wrapper) + worker2 := servers.TestWorker(t, conn, wrapper) + worker3 := servers.TestWorker(t, conn, wrapper) + worker4 := servers.TestWorker(t, conn, wrapper) // Create sessions on the first three, activate, and authorize connections var worker1ConnIds, worker2ConnIds, worker3ConnIds []string @@ -471,7 +471,7 @@ func TestCloseWorkerlessConnections(t *testing.T) { } // Setup deleted worker connections - deletedWorker := TestWorker(t, conn, wrapper) + deletedWorker := servers.TestWorker(t, conn, wrapper) dActiveConn := createConnection(deletedWorker.GetPublicId()) dClosedConn := createConnection(deletedWorker.GetPublicId()) connRepo.closeConnections(ctx, []CloseWith{{ @@ -484,7 +484,7 @@ func TestCloseWorkerlessConnections(t *testing.T) { require.NoError(err) // Non deleted worker case - activeWorker := TestWorker(t, conn, wrapper) + activeWorker := servers.TestWorker(t, conn, wrapper) activeConn := createConnection(activeWorker.GetPublicId()) closedConn := createConnection(activeWorker.GetPublicId()) connRepo.closeConnections(ctx, []CloseWith{{ diff --git a/internal/session/testing.go b/internal/session/testing.go index dfc783025..4d7b9aecf 100644 --- a/internal/session/testing.go +++ b/internal/session/testing.go @@ -6,8 +6,6 @@ import ( "testing" "time" - "github.com/hashicorp/boundary/internal/types/scope" - "github.com/hashicorp/boundary/internal/auth/password" "github.com/hashicorp/boundary/internal/authtoken" "github.com/hashicorp/boundary/internal/db" @@ -16,12 +14,10 @@ import ( "github.com/hashicorp/boundary/internal/host/static" "github.com/hashicorp/boundary/internal/iam" "github.com/hashicorp/boundary/internal/kms" - "github.com/hashicorp/boundary/internal/servers" "github.com/hashicorp/boundary/internal/target" "github.com/hashicorp/boundary/internal/target/tcp" wrapping "github.com/hashicorp/go-kms-wrapping/v2" "github.com/hashicorp/go-secure-stdlib/base62" - "github.com/hashicorp/go-uuid" "github.com/stretchr/testify/require" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -180,28 +176,6 @@ func TestTofu(t testing.TB) []byte { return []byte(tofu) } -// TestWorker inserts a worker into the db to satisfy foreign key constraints. -func TestWorker(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, opt ...Option) *servers.Worker { - t.Helper() - rw := db.New(conn) - kms := kms.TestKms(t, conn, wrapper) - serversRepo, err := servers.NewRepository(rw, rw, kms) - require.NoError(t, err) - - id, err := uuid.GenerateUUID() - require.NoError(t, err) - id = "test_session_worker-" + id - - name := "test-worker-" + id - worker := servers.NewWorker(scope.Global.String(), - servers.WithPublicId(id), - servers.WithName(name), - servers.WithAddress("127.0.0.1")) - _, _, err = serversRepo.UpsertWorker(context.Background(), worker) - require.NoError(t, err) - return worker -} - // TestCert is a temporary test func that intentionally doesn't take testing.T // as a parameter. It's currently used in controller.jobTestingHandler() and // should be deprecated once that function is refactored to use sessions properly. diff --git a/internal/session/testing_test.go b/internal/session/testing_test.go index 8e18df86a..f344ae896 100644 --- a/internal/session/testing_test.go +++ b/internal/session/testing_test.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/iam" + "github.com/hashicorp/boundary/internal/servers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -73,7 +74,7 @@ func Test_TestWorker(t *testing.T) { require := require.New(t) conn, _ := db.TestSetup(t, "postgres") wrapper := db.TestWrapper(t) - w := TestWorker(t, conn, wrapper) + w := servers.TestWorker(t, conn, wrapper) require.NotNil(w) }