From 1ae388438ff5a5e6918da923a3d4b7aee21381de Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Thu, 4 Nov 2021 11:35:15 -0400 Subject: [PATCH 1/4] feat(subtypes): Add function to provide all known prefixes --- internal/types/subtypes/registry.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/internal/types/subtypes/registry.go b/internal/types/subtypes/registry.go index 10462cbd5..a6a956357 100644 --- a/internal/types/subtypes/registry.go +++ b/internal/types/subtypes/registry.go @@ -70,6 +70,18 @@ func (r *Registry) SubtypeFromId(id string) Subtype { return subtype } +// Prefixes returns the list of all known Prefixes. +func (r *Registry) Prefixes() []string { + r.RLock() + defer r.RUnlock() + + ret := make([]string, 0, len(r.subtypesPrefixes)) + for p := range r.subtypesPrefixes { + ret = append(ret, p) + } + return ret +} + // Register registers all the prefixes for a provided Subtype. Register returns // an error if the subtype has already been registered or if any of the // prefixes are associated with another subtype. From c9b99306e17f89352ef301996ddcdbe559d43cd0 Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Thu, 4 Nov 2021 11:37:32 -0400 Subject: [PATCH 2/4] refact(target): Add NewFunc to registry This adds a new requirement onto subtypes to register a NewFunc. Previously the subtypes where implementing this function anyway, but the target package was not including them in the registry. This resulted in packages that use the target package needing to import the subtypes directly, which we want to avoid. This refactor will require the subtypes to register their NewFunc which can then be used by `target.New` so packages that need to create targets can do so by only using the target package. This also updates the tcp subpackage to register a NewFunc. --- internal/cmd/base/initial_resources.go | 2 +- internal/credential/vault/jobs_test.go | 18 +-- .../vault/repository_credentials_test.go | 6 +- .../credential/vault/testing_external_test.go | 3 +- .../handlers/sessions/session_service_test.go | 10 +- internal/target/registry.go | 31 ++++- .../repository_credential_source_test.go | 11 +- internal/target/targettest/target.go | 10 +- internal/target/tcp/exports_test.go | 6 +- internal/target/tcp/immutable_fields_test.go | 17 +-- internal/target/tcp/register.go | 2 +- .../tcp/repository_credential_source_test.go | 63 +++++----- .../target/tcp/repository_host_source_test.go | 63 +++++----- .../target/tcp/repository_tcp_target_test.go | 47 ++++---- internal/target/tcp/repository_test.go | 56 +++++---- internal/target/tcp/target.go | 4 +- internal/target/tcp/target_test.go | 112 ++++++++++-------- internal/target/tcp/testing.go | 12 +- internal/target/tcp/testing_test.go | 14 ++- 19 files changed, 279 insertions(+), 208 deletions(-) diff --git a/internal/cmd/base/initial_resources.go b/internal/cmd/base/initial_resources.go index fb19631fc..bd18a07b3 100644 --- a/internal/cmd/base/initial_resources.go +++ b/internal/cmd/base/initial_resources.go @@ -514,7 +514,7 @@ func (b *Server) CreateInitialTarget(ctx context.Context) (target.Target, error) target.WithSessionConnectionLimit(int32(b.DevTargetSessionConnectionLimit)), target.WithPublicId(b.DevTargetId), } - t, err := tcp.New(b.DevProjectId, opts...) + t, err := target.New(ctx, tcp.Subtype, b.DevProjectId, opts...) if err != nil { return nil, fmt.Errorf("error creating in memory target: %w", err) } diff --git a/internal/credential/vault/jobs_test.go b/internal/credential/vault/jobs_test.go index e739d3dfb..816c2f821 100644 --- a/internal/credential/vault/jobs_test.go +++ b/internal/credential/vault/jobs_test.go @@ -829,7 +829,7 @@ func TestTokenRevocationJob_Run(t *testing.T) { hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) target.TestCredentialLibrary(t, conn, tar.GetPublicId(), cl.GetPublicId()) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, @@ -1028,7 +1028,7 @@ func TestCredentialRenewalJob_RunLimits(t *testing.T) { hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) target.TestCredentialLibrary(t, conn, tar.GetPublicId(), cl.GetPublicId()) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, @@ -1141,7 +1141,7 @@ func TestCredentialRenewalJob_Run(t *testing.T) { hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, HostId: h.GetPublicId(), @@ -1251,7 +1251,7 @@ func TestCredentialRenewalJob_RunExpired(t *testing.T) { hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, HostId: h.GetPublicId(), @@ -1328,7 +1328,7 @@ func TestCredentialRenewalJob_NextRunIn(t *testing.T) { hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) target.TestCredentialLibrary(t, conn, tar.GetPublicId(), cl.GetPublicId()) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, @@ -1571,7 +1571,7 @@ func TestCredentialRevocationJob_RunLimits(t *testing.T) { hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) target.TestCredentialLibrary(t, conn, tar.GetPublicId(), cl.GetPublicId()) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, @@ -1686,7 +1686,7 @@ func TestCredentialRevocationJob_Run(t *testing.T) { hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, HostId: h.GetPublicId(), @@ -1780,7 +1780,7 @@ func TestCredentialRevocationJob_RunDeleted(t *testing.T) { hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) sess := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, HostId: h.GetPublicId(), @@ -2172,7 +2172,7 @@ func TestCredentialCleanupJob_Run(t *testing.T) { hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) sess1 := session.TestSession(t, conn, wrapper, session.ComposedOf{ UserId: uId, HostId: h.GetPublicId(), diff --git a/internal/credential/vault/repository_credentials_test.go b/internal/credential/vault/repository_credentials_test.go index 91f8245bb..14038f00f 100644 --- a/internal/credential/vault/repository_credentials_test.go +++ b/internal/credential/vault/repository_credentials_test.go @@ -106,7 +106,7 @@ func TestRepository_IssueCredentials(t *testing.T) { h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) rc2dc := func(rcs []credential.Request) []*session.DynamicCredential { var dcs []*session.DynamicCredential @@ -224,7 +224,7 @@ func TestRepository_Revoke(t *testing.T) { h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) target.TestCredentialLibrary(t, conn, tar.GetPublicId(), cl.GetPublicId()) const ( @@ -342,7 +342,7 @@ func Test_TerminateSession(t *testing.T) { h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) target.TestCredentialLibrary(t, conn, tar.GetPublicId(), cl.GetPublicId()) const ( diff --git a/internal/credential/vault/testing_external_test.go b/internal/credential/vault/testing_external_test.go index 4b24273eb..c70822b58 100644 --- a/internal/credential/vault/testing_external_test.go +++ b/internal/credential/vault/testing_external_test.go @@ -1,6 +1,7 @@ package vault_test import ( + "context" "testing" "github.com/hashicorp/boundary/internal/authtoken" @@ -35,7 +36,7 @@ func Test_TestCredentials(t *testing.T) { h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) target.TestCredentialLibrary(t, conn, tar.GetPublicId(), cl.GetPublicId()) at := authtoken.TestAuthToken(t, conn, kms, org.GetPublicId()) diff --git a/internal/servers/controller/handlers/sessions/session_service_test.go b/internal/servers/controller/handlers/sessions/session_service_test.go index 490d84e08..c55400fd9 100644 --- a/internal/servers/controller/handlers/sessions/session_service_test.go +++ b/internal/servers/controller/handlers/sessions/session_service_test.go @@ -59,7 +59,7 @@ func TestGetSession(t *testing.T) { hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, p.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, p.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) sess := session.TestSession(t, conn, wrap, session.ComposedOf{ UserId: uId, @@ -176,7 +176,7 @@ func TestList_Self(t *testing.T) { hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, pWithSessions.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, pWithSessions.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) // By default a user can read/cancel their own sessions. session.TestSession(t, conn, wrap, session.ComposedOf{ @@ -260,13 +260,13 @@ func TestList(t *testing.T) { hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, pWithSessions.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, pWithSessions.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) hcOther := static.TestCatalogs(t, conn, pWithOtherSessions.GetPublicId(), 1)[0] hsOther := static.TestSets(t, conn, hcOther.GetPublicId(), 1)[0] hOther := static.TestHosts(t, conn, hcOther.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hsOther.GetPublicId(), []*static.Host{hOther}) - tarOther := tcp.TestTarget(t, conn, pWithOtherSessions.GetPublicId(), "test", target.WithHostSources([]string{hsOther.GetPublicId()})) + tarOther := tcp.TestTarget(context.Background(), t, conn, pWithOtherSessions.GetPublicId(), "test", target.WithHostSources([]string{hsOther.GetPublicId()})) var wantSession []*pb.Session var totalSession []*pb.Session @@ -502,7 +502,7 @@ func TestCancel(t *testing.T) { hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h}) - tar := tcp.TestTarget(t, conn, p.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) + tar := tcp.TestTarget(context.Background(), t, conn, p.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()})) sess := session.TestSession(t, conn, wrap, session.ComposedOf{ UserId: uId, diff --git a/internal/target/registry.go b/internal/target/registry.go index 333cdd725..226025e83 100644 --- a/internal/target/registry.go +++ b/internal/target/registry.go @@ -5,9 +5,13 @@ import ( "fmt" "sync" + "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/types/subtypes" ) +// NewFunc is a function that creates a Target with the provided scope and options. +type NewFunc func(scopeId string, opt ...Option) (Target, error) + // AllocFunc is a function that creates an in-memory Target. type AllocFunc func() Target @@ -20,6 +24,7 @@ type VetFunc func(context.Context, Target) error type VetCredentialLibrariesFunc func(context.Context, []*CredentialLibrary) error type registryEntry struct { + newFunc NewFunc alloc AllocFunc vet VetFunc vetCredentialLibraries VetCredentialLibrariesFunc @@ -60,6 +65,14 @@ func (r *registry) get(s subtypes.Subtype) (*registryEntry, bool) { return nil, ok } +func (r *registry) newFunc(s subtypes.Subtype) (NewFunc, bool) { + entry, ok := r.get(s) + if !ok { + return nil, ok + } + return entry.newFunc, ok +} + func (r *registry) allocFunc(s subtypes.Subtype) (AllocFunc, bool) { entry, ok := r.get(s) if !ok { @@ -113,11 +126,27 @@ func SubtypeFromId(id string) subtypes.Subtype { return subtypeRegistry.subtypes.SubtypeFromId(id) } +// Prefixes returns the list of all known target Prefixes. +func Prefixes() []string { + return subtypeRegistry.subtypes.Prefixes() +} + +// New creates a Target of the given subtype and scopeId. +func New(ctx context.Context, subtype subtypes.Subtype, scopeId string, opt ...Option) (Target, error) { + const op = "target.New" + nf, ok := subtypeRegistry.newFunc(subtype) + if !ok { + return nil, errors.New(ctx, errors.InvalidParameter, op, "unsupported subtype") + } + return nf(scopeId, opt...) +} + // Register registers repository hooks and the prefixes for a provided Subtype. Register // panics if the subtype has already been registered or if any of the // prefixes are associated with another subtype. -func Register(s subtypes.Subtype, af AllocFunc, vf VetFunc, vclf VetCredentialLibrariesFunc, prefix string) { +func Register(s subtypes.Subtype, nf NewFunc, af AllocFunc, vf VetFunc, vclf VetCredentialLibrariesFunc, prefix string) { subtypeRegistry.set(s, ®istryEntry{ + newFunc: nf, alloc: af, vet: vf, vetCredentialLibraries: vclf, diff --git a/internal/target/repository_credential_source_test.go b/internal/target/repository_credential_source_test.go index d03c438f8..f1afe7072 100644 --- a/internal/target/repository_credential_source_test.go +++ b/internal/target/repository_credential_source_test.go @@ -17,7 +17,7 @@ import ( ) func TestRepository_SetTargetCredentialSources(t *testing.T) { - target.Register(targettest.Subtype, targettest.Alloc, targettest.Vet, targettest.VetCredentialLibraries, targettest.TargetPrefix) + target.Register(targettest.Subtype, targettest.New, targettest.Alloc, targettest.Vet, targettest.VetCredentialLibraries, targettest.TargetPrefix) t.Parallel() conn, _ := db.TestSetup(t, "postgres") @@ -181,7 +181,8 @@ func TestRepository_SetTargetCredentialSources(t *testing.T) { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - tar := targettest.TestNewTestTarget(t, conn, proj.PublicId, tt.name) + ctx := context.Background() + tar := targettest.TestNewTestTarget(ctx, t, conn, proj.PublicId, tt.name) var origCredSources []target.CredentialSource var origCredLibraries []*target.CredentialLibrary @@ -205,11 +206,11 @@ func TestRepository_SetTargetCredentialSources(t *testing.T) { } } - origTarget, _, lookupCredSources, err := repo.LookupTarget(context.Background(), tar.GetPublicId()) + origTarget, _, lookupCredSources, err := repo.LookupTarget(ctx, tar.GetPublicId()) require.NoError(err) assert.Equal(origCredSources, lookupCredSources) - _, got, affectedRows, err := repo.SetTargetCredentialSources(context.Background(), tar.GetPublicId(), tt.args.targetVersion, tt.args.cls) + _, got, affectedRows, err := repo.SetTargetCredentialSources(ctx, tar.GetPublicId(), tt.args.targetVersion, tt.args.cls) if tt.wantErr { require.Error(err) assert.Equal(0, affectedRows) @@ -229,7 +230,7 @@ func TestRepository_SetTargetCredentialSources(t *testing.T) { assert.Equal(w.CredentialPurpose(), cs.CredentialPurpose()) } - foundTarget, _, _, err := repo.LookupTarget(context.Background(), tar.GetPublicId()) + foundTarget, _, _, err := repo.LookupTarget(ctx, tar.GetPublicId()) require.NoError(err) if tt.name != "no-change" { assert.Equalf(tt.args.targetVersion+1, foundTarget.GetVersion(), "%s unexpected version: %d/%d", tt.name, tt.args.targetVersion+1, foundTarget.GetVersion()) diff --git a/internal/target/targettest/target.go b/internal/target/targettest/target.go index 4d9b3e31b..101d2b60f 100644 --- a/internal/target/targettest/target.go +++ b/internal/target/targettest/target.go @@ -209,7 +209,7 @@ func VetCredentialLibraries(_ context.Context, _ []*target.CredentialLibrary) er } // New creates a targettest.Target. -func New(scopeId string, opt ...target.Option) (*Target, error) { +func New(scopeId string, opt ...target.Option) (target.Target, error) { const op = "target_test.New" opts := target.GetOpts(opt...) if scopeId == "" { @@ -230,7 +230,7 @@ func New(scopeId string, opt ...target.Option) (*Target, error) { } // TestNewTestTarget is a test helper for creating a targettest.Target. -func TestNewTestTarget(t *testing.T, conn *db.DB, scopeId, name string, opt ...target.Option) *Target { +func TestNewTestTarget(ctx context.Context, t *testing.T, conn *db.DB, scopeId, name string, opt ...target.Option) target.Target { t.Helper() opt = append(opt, target.WithName(name)) opts := target.GetOpts(opt...) @@ -240,14 +240,14 @@ func TestNewTestTarget(t *testing.T, conn *db.DB, scopeId, name string, opt ...t require.NoError(err) id, err := db.NewPublicId(TargetPrefix) require.NoError(err) - tar.PublicId = id + tar.SetPublicId(ctx, id) err = rw.Create(context.Background(), tar) require.NoError(err) if len(opts.WithHostSources) > 0 { newHostSets := make([]interface{}, 0, len(opts.WithHostSources)) for _, s := range opts.WithHostSources { - hostSet, err := target.NewTargetHostSet(tar.PublicId, s) + hostSet, err := target.NewTargetHostSet(tar.GetPublicId(), s) require.NoError(err) newHostSets = append(newHostSets, hostSet) } @@ -257,7 +257,7 @@ func TestNewTestTarget(t *testing.T, conn *db.DB, scopeId, name string, opt ...t if len(opts.WithCredentialLibraries) > 0 { newCredLibs := make([]interface{}, 0, len(opts.WithCredentialLibraries)) for _, cl := range opts.WithCredentialLibraries { - cl.TargetId = tar.PublicId + cl.TargetId = tar.GetPublicId() newCredLibs = append(newCredLibs, cl) } err := rw.CreateItems(context.Background(), newCredLibs) diff --git a/internal/target/tcp/exports_test.go b/internal/target/tcp/exports_test.go index 023d2683c..d1cb5cdfe 100644 --- a/internal/target/tcp/exports_test.go +++ b/internal/target/tcp/exports_test.go @@ -12,8 +12,8 @@ var ( // NewTestTarget is a test helper that bypasses the scopeId checks // performed by NewTarget, allowing tests to create Targets with // nil scopeIds for more robust testing. -func NewTestTarget(scopeId string, opt ...target.Option) *Target { - t, _ := New("testScope", opt...) - t.ScopeId = scopeId +func NewTestTarget(scopeId string, opt ...target.Option) target.Target { + t, _ := newTarget("testScope", opt...) + t.SetScopeId(scopeId) return t } diff --git a/internal/target/tcp/immutable_fields_test.go b/internal/target/tcp/immutable_fields_test.go index f268806e1..2b7082968 100644 --- a/internal/target/tcp/immutable_fields_test.go +++ b/internal/target/tcp/immutable_fields_test.go @@ -26,7 +26,8 @@ func TestTarget_ImmutableFields(t *testing.T) { ts := timestamp.Timestamp{Timestamp: ×tamppb.Timestamp{Seconds: 0, Nanos: 0}} _, proj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) - new := tcp.TestTarget(t, conn, proj.PublicId, tcp.TestId(t)) + ctx := context.Background() + new := tcp.TestTarget(ctx, t, conn, proj.PublicId, tcp.TestId(t)) tests := []struct { name string @@ -95,7 +96,8 @@ func TestTcpTarget_ImmutableFields(t *testing.T) { ts := timestamp.Timestamp{Timestamp: ×tamppb.Timestamp{Seconds: 0, Nanos: 0}} _, proj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) - new := tcp.TestTarget(t, conn, proj.PublicId, tcp.TestId(t)) + ctx := context.Background() + new := tcp.TestTarget(ctx, t, conn, proj.PublicId, tcp.TestId(t)) tests := []struct { name string @@ -163,18 +165,19 @@ func TestTargetHostSet_ImmutableFields(t *testing.T) { ts := timestamp.Timestamp{Timestamp: ×tamppb.Timestamp{Seconds: 0, Nanos: 0}} _, proj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) - projTarget := tcp.TestTarget(t, conn, proj.PublicId, tcp.TestId(t)) + ctx := context.Background() + projTarget := tcp.TestTarget(ctx, t, conn, proj.PublicId, tcp.TestId(t)) testCats := static.TestCatalogs(t, conn, proj.PublicId, 1) hsets := static.TestSets(t, conn, testCats[0].GetPublicId(), 2) require.Equal(t, 2, len(hsets)) - updateTarget := tcp.TestTarget(t, conn, proj.PublicId, tcp.TestId(t)) + updateTarget := tcp.TestTarget(ctx, t, conn, proj.PublicId, tcp.TestId(t)) updateHset := hsets[1] - _, gotHostSources, _, err := repo.AddTargetHostSources(context.Background(), projTarget.PublicId, 1, []string{hsets[0].PublicId}) + _, gotHostSources, _, err := repo.AddTargetHostSources(ctx, projTarget.GetPublicId(), 1, []string{hsets[0].PublicId}) require.NoError(t, err) require.Equal(t, 1, len(gotHostSources)) - new, err := target.NewTargetHostSet(projTarget.PublicId, gotHostSources[0].Id()) + new, err := target.NewTargetHostSet(projTarget.GetPublicId(), gotHostSources[0].Id()) require.NoError(t, err) tests := []struct { @@ -186,7 +189,7 @@ func TestTargetHostSet_ImmutableFields(t *testing.T) { name: "target_id", update: func() *target.TargetHostSet { target := new.Clone().(*target.TargetHostSet) - target.TargetId = updateTarget.PublicId + target.TargetId = updateTarget.GetPublicId() return target }(), fieldMask: []string{"TargetId"}, diff --git a/internal/target/tcp/register.go b/internal/target/tcp/register.go index 943ff8ac8..df0acb464 100644 --- a/internal/target/tcp/register.go +++ b/internal/target/tcp/register.go @@ -10,7 +10,7 @@ import ( ) func init() { - target.Register(Subtype, allocTarget, vet, vetCredentialLibraries, TargetPrefix) + target.Register(Subtype, newTarget, allocTarget, vet, vetCredentialLibraries, TargetPrefix) } const ( diff --git a/internal/target/tcp/repository_credential_source_test.go b/internal/target/tcp/repository_credential_source_test.go index 1b0c80b28..0380bfb76 100644 --- a/internal/target/tcp/repository_credential_source_test.go +++ b/internal/target/tcp/repository_credential_source_test.go @@ -167,13 +167,14 @@ func TestRepository_AddTargetCredentialSources(t *testing.T) { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - projTarget := tcp.TestTarget(t, conn, staticProj.PublicId, tt.name) + ctx := context.Background() + projTarget := tcp.TestTarget(ctx, t, conn, staticProj.PublicId, tt.name) for _, cl := range tt.args.cls { - cl.TargetId = projTarget.PublicId + cl.TargetId = projTarget.GetPublicId() } - gotTarget, _, gotCredSources, err := repo.AddTargetCredentialSources(context.Background(), projTarget.PublicId, tt.args.targetVersion, tt.args.cls) + gotTarget, _, gotCredSources, err := repo.AddTargetCredentialSources(context.Background(), projTarget.GetPublicId(), tt.args.targetVersion, tt.args.cls) if tt.wantErr { require.Error(err) assert.Truef(errors.Match(errors.T(tt.wantErrCode), err), "unexpected error %s", err.Error()) @@ -190,10 +191,10 @@ func TestRepository_AddTargetCredentialSources(t *testing.T) { } // test to see of the target version update oplog was created - err = db.TestVerifyOplog(t, rw, projTarget.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) + err = db.TestVerifyOplog(t, rw, projTarget.GetPublicId(), db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) assert.NoError(err) - tar, _, lookupCredSources, err := repo.LookupTarget(context.Background(), projTarget.PublicId) + tar, _, lookupCredSources, err := repo.LookupTarget(context.Background(), projTarget.GetPublicId()) require.NoError(err) assert.Equal(tt.args.targetVersion+1, tar.GetVersion()) assert.Equal(projTarget.GetVersion(), tar.GetVersion()-1) @@ -205,35 +206,36 @@ func TestRepository_AddTargetCredentialSources(t *testing.T) { assert.True(ok, "got unexpected credentialsource %v", cs) assert.Equal(w.Id(), cs.Id()) assert.Equal(w.CredentialPurpose(), cs.CredentialPurpose()) - assert.Equal(projTarget.PublicId, cs.TargetId()) + assert.Equal(projTarget.GetPublicId(), cs.TargetId()) } }) } t.Run("add-existing", func(t *testing.T) { assert, require := assert.New(t), require.New(t) - projTarget := tcp.TestTarget(t, conn, staticProj.PublicId, "add-existing") - cl1 := target.TestNewCredentialLibrary(projTarget.PublicId, lib1.PublicId, credential.ApplicationPurpose) - cl2 := target.TestNewCredentialLibrary(projTarget.PublicId, lib2.PublicId, credential.ApplicationPurpose) - cl3 := target.TestNewCredentialLibrary(projTarget.PublicId, lib3.PublicId, credential.ApplicationPurpose) + ctx := context.Background() + projTarget := tcp.TestTarget(ctx, t, conn, staticProj.PublicId, "add-existing") + cl1 := target.TestNewCredentialLibrary(projTarget.GetPublicId(), lib1.PublicId, credential.ApplicationPurpose) + cl2 := target.TestNewCredentialLibrary(projTarget.GetPublicId(), lib2.PublicId, credential.ApplicationPurpose) + cl3 := target.TestNewCredentialLibrary(projTarget.GetPublicId(), lib3.PublicId, credential.ApplicationPurpose) - _, _, gotCredSources, err := repo.AddTargetCredentialSources(context.Background(), projTarget.PublicId, 1, []*target.CredentialLibrary{cl1}) + _, _, gotCredSources, err := repo.AddTargetCredentialSources(ctx, projTarget.GetPublicId(), 1, []*target.CredentialLibrary{cl1}) require.NoError(err) assert.Len(gotCredSources, 1) assert.Equal(lib1.PublicId, gotCredSources[0].Id()) // Adding lib1 again should error - _, _, _, err = repo.AddTargetCredentialSources(context.Background(), projTarget.PublicId, 2, []*target.CredentialLibrary{cl1}) + _, _, _, err = repo.AddTargetCredentialSources(ctx, projTarget.GetPublicId(), 2, []*target.CredentialLibrary{cl1}) require.Error(err) assert.True(errors.Match(errors.T(errors.NotUnique), err)) // Adding multiple with lib1 in set should error - _, _, _, err = repo.AddTargetCredentialSources(context.Background(), projTarget.PublicId, 2, []*target.CredentialLibrary{cl3, cl2, cl1}) + _, _, _, err = repo.AddTargetCredentialSources(ctx, projTarget.GetPublicId(), 2, []*target.CredentialLibrary{cl3, cl2, cl1}) require.Error(err) assert.True(errors.Match(errors.T(errors.NotUnique), err)) // Previous transactions should have been rolled back and only lib1 should be associated - _, _, lookupCredSources, err := repo.LookupTarget(context.Background(), projTarget.PublicId) + _, _, lookupCredSources, err := repo.LookupTarget(ctx, projTarget.GetPublicId()) require.NoError(err) assert.Len(lookupCredSources, 1) assert.Equal(lib1.PublicId, lookupCredSources[0].Id()) @@ -351,7 +353,8 @@ func TestRepository_DeleteTargetCredentialSources(t *testing.T) { assert, require := assert.New(t), require.New(t) cs := css[i] - tar := tcp.TestTarget(t, conn, proj.PublicId, tt.name) + ctx := context.Background() + tar := tcp.TestTarget(ctx, t, conn, proj.PublicId, tt.name) cls := make([]*target.CredentialLibrary, 0, tt.args.createCnt) if tt.args.createCnt > 0 { @@ -360,7 +363,7 @@ func TestRepository_DeleteTargetCredentialSources(t *testing.T) { cls = append(cls, target.TestNewCredentialLibrary(tar.GetPublicId(), cl.PublicId, credential.ApplicationPurpose)) } } - _, _, addedCredSources, err := repo.AddTargetCredentialSources(context.Background(), tar.GetPublicId(), 1, cls) + _, _, addedCredSources, err := repo.AddTargetCredentialSources(ctx, tar.GetPublicId(), 1, cls) require.NoError(err) assert.Equal(tt.args.createCnt, len(addedCredSources)) @@ -382,7 +385,7 @@ func TestRepository_DeleteTargetCredentialSources(t *testing.T) { default: targetVersion = 2 } - deletedRows, err := repo.DeleteTargetCredentialSources(context.Background(), targetId, targetVersion, deleteCredSources) + deletedRows, err := repo.DeleteTargetCredentialSources(ctx, targetId, targetVersion, deleteCredSources) if tt.wantErr { require.Error(err) assert.Equal(0, deletedRows) @@ -408,29 +411,30 @@ func TestRepository_DeleteTargetCredentialSources(t *testing.T) { lib2 := libs[1] lib3 := libs[2] - projTarget := tcp.TestTarget(t, conn, proj.PublicId, "add-existing") - cl1 := target.TestNewCredentialLibrary(projTarget.PublicId, lib1.PublicId, credential.ApplicationPurpose) - cl2 := target.TestNewCredentialLibrary(projTarget.PublicId, lib2.PublicId, credential.ApplicationPurpose) - cl3 := target.TestNewCredentialLibrary(projTarget.PublicId, lib3.PublicId, credential.ApplicationPurpose) + ctx := context.Background() + projTarget := tcp.TestTarget(ctx, t, conn, proj.PublicId, "add-existing") + cl1 := target.TestNewCredentialLibrary(projTarget.GetPublicId(), lib1.PublicId, credential.ApplicationPurpose) + cl2 := target.TestNewCredentialLibrary(projTarget.GetPublicId(), lib2.PublicId, credential.ApplicationPurpose) + cl3 := target.TestNewCredentialLibrary(projTarget.GetPublicId(), lib3.PublicId, credential.ApplicationPurpose) - _, _, gotCredSources, err := repo.AddTargetCredentialSources(context.Background(), projTarget.PublicId, 1, []*target.CredentialLibrary{cl1, cl2}) + _, _, gotCredSources, err := repo.AddTargetCredentialSources(ctx, projTarget.GetPublicId(), 1, []*target.CredentialLibrary{cl1, cl2}) require.NoError(err) assert.Len(gotCredSources, 2) // Deleting an unassociated source should return an error - delCount, err := repo.DeleteTargetCredentialSources(context.Background(), projTarget.PublicId, 2, []*target.CredentialLibrary{cl3}) + delCount, err := repo.DeleteTargetCredentialSources(ctx, projTarget.GetPublicId(), 2, []*target.CredentialLibrary{cl3}) require.Error(err) assert.True(errors.Match(errors.T(errors.MultipleRecords), err)) assert.Equal(0, delCount) // Deleting sources which includes an unassociated source should return an error - delCount, err = repo.DeleteTargetCredentialSources(context.Background(), projTarget.PublicId, 2, []*target.CredentialLibrary{cl1, cl2, cl3}) + delCount, err = repo.DeleteTargetCredentialSources(ctx, projTarget.GetPublicId(), 2, []*target.CredentialLibrary{cl1, cl2, cl3}) require.Error(err) assert.True(errors.Match(errors.T(errors.MultipleRecords), err)) assert.Equal(0, delCount) // Previous transactions should have been rolled back and only lib1 should be associated - _, _, lookupCredSources, err := repo.LookupTarget(context.Background(), projTarget.PublicId) + _, _, lookupCredSources, err := repo.LookupTarget(ctx, projTarget.GetPublicId()) require.NoError(err) assert.Len(lookupCredSources, 2) }) @@ -584,7 +588,8 @@ func TestRepository_SetTargetCredentialSources(t *testing.T) { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - tar := tcp.TestTarget(t, conn, proj.PublicId, tt.name) + ctx := context.Background() + tar := tcp.TestTarget(ctx, t, conn, proj.PublicId, tt.name) var origCredSources []target.CredentialSource var origCredLibraries []*target.CredentialLibrary @@ -608,11 +613,11 @@ func TestRepository_SetTargetCredentialSources(t *testing.T) { } } - origTarget, _, lookupCredSources, err := repo.LookupTarget(context.Background(), tar.GetPublicId()) + origTarget, _, lookupCredSources, err := repo.LookupTarget(ctx, tar.GetPublicId()) require.NoError(err) assert.Equal(origCredSources, lookupCredSources) - _, got, affectedRows, err := repo.SetTargetCredentialSources(context.Background(), tar.GetPublicId(), tt.args.targetVersion, tt.args.cls) + _, got, affectedRows, err := repo.SetTargetCredentialSources(ctx, tar.GetPublicId(), tt.args.targetVersion, tt.args.cls) if tt.wantErr { require.Error(err) assert.Equal(0, affectedRows) @@ -632,7 +637,7 @@ func TestRepository_SetTargetCredentialSources(t *testing.T) { assert.Equal(w.CredentialPurpose(), cs.CredentialPurpose()) } - foundTarget, _, _, err := repo.LookupTarget(context.Background(), tar.GetPublicId()) + foundTarget, _, _, err := repo.LookupTarget(ctx, tar.GetPublicId()) require.NoError(err) if tt.name != "no-change" { assert.Equalf(tt.args.targetVersion+1, foundTarget.GetVersion(), "%s unexpected version: %d/%d", tt.name, tt.args.targetVersion+1, foundTarget.GetVersion()) diff --git a/internal/target/tcp/repository_host_source_test.go b/internal/target/tcp/repository_host_source_test.go index d0a2b3fa9..acaa1aa49 100644 --- a/internal/target/tcp/repository_host_source_test.go +++ b/internal/target/tcp/repository_host_source_test.go @@ -101,10 +101,11 @@ func TestRepository_AddTargetHostSets(t *testing.T) { require.NoError(conn.Where("1=1").Delete(allocTargetHostSet()).Error) require.NoError(conn.Where("1=1").Delete(tcp.NewTestTarget("")).Error) - projTarget := tcp.TestTarget(t, conn, staticProj.PublicId, "static-proj") + ctx := context.Background() + projTarget := tcp.TestTarget(ctx, t, conn, staticProj.PublicId, "static-proj") var hostSourceIds []string - origTarget, origHostSet, _, err := repo.LookupTarget(context.Background(), projTarget.PublicId) + origTarget, origHostSet, _, err := repo.LookupTarget(ctx, projTarget.GetPublicId()) require.NoError(err) require.Equal(0, len(origHostSet)) @@ -112,14 +113,14 @@ func TestRepository_AddTargetHostSets(t *testing.T) { hostSourceIds = createHostSetsFn([]string{staticOrg.PublicId}, []string{staticProj.PublicId}) } - gotTarget, gotHostSources, _, err := repo.AddTargetHostSources(context.Background(), projTarget.PublicId, tt.args.targetVersion, hostSourceIds, tt.args.opt...) + gotTarget, gotHostSources, _, err := repo.AddTargetHostSources(ctx, projTarget.GetPublicId(), tt.args.targetVersion, hostSourceIds, tt.args.opt...) if tt.wantErr { require.Error(err) if tt.wantErrIs != nil { assert.Truef(errors.Is(err, tt.wantErrIs), "unexpected error %s", err.Error()) } // test to see of the target version update oplog was not created - err = db.TestVerifyOplog(t, rw, projTarget.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) + err = db.TestVerifyOplog(t, rw, projTarget.GetPublicId(), db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) assert.Error(err) // TODO (jimlambrt 9/2020) - unfortunately, we can currently @@ -147,10 +148,10 @@ func TestRepository_AddTargetHostSets(t *testing.T) { // appropriate granularity, we should add an appropriate assert. // test to see of the target version update oplog was created - err = db.TestVerifyOplog(t, rw, projTarget.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) + err = db.TestVerifyOplog(t, rw, projTarget.GetPublicId(), db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) assert.NoError(err) - tar, ths, _, err := repo.LookupTarget(context.Background(), projTarget.PublicId) + tar, ths, _, err := repo.LookupTarget(ctx, projTarget.GetPublicId()) require.NoError(err) assert.Equal(tt.args.targetVersion+1, tar.GetVersion()) assert.Equal(origTarget.GetVersion(), tar.GetVersion()-1) @@ -170,24 +171,25 @@ func TestRepository_AddTargetHostSets(t *testing.T) { hs2 := hsets[1] hs3 := hsets[2] - projTarget := tcp.TestTarget(t, conn, staticProj.PublicId, "add-existing") - _, gotHostSources, _, err := repo.AddTargetHostSources(context.Background(), projTarget.PublicId, 1, []string{hs1.PublicId}) + ctx := context.Background() + projTarget := tcp.TestTarget(ctx, t, conn, staticProj.PublicId, "add-existing") + _, gotHostSources, _, err := repo.AddTargetHostSources(ctx, projTarget.GetPublicId(), 1, []string{hs1.PublicId}) require.NoError(err) assert.Len(gotHostSources, 1) assert.Equal(hs1.PublicId, gotHostSources[0].Id()) // Adding hs1 again should error - _, _, _, err = repo.AddTargetHostSources(context.Background(), projTarget.PublicId, 2, []string{hs1.PublicId}) + _, _, _, err = repo.AddTargetHostSources(ctx, projTarget.GetPublicId(), 2, []string{hs1.PublicId}) require.Error(err) assert.True(errors.Match(errors.T(errors.NotUnique), err)) // Adding multiple with hs1 in set should error - _, _, _, err = repo.AddTargetHostSources(context.Background(), projTarget.PublicId, 2, []string{hs3.PublicId, hs2.PublicId, hs1.PublicId}) + _, _, _, err = repo.AddTargetHostSources(ctx, projTarget.GetPublicId(), 2, []string{hs3.PublicId, hs2.PublicId, hs1.PublicId}) require.Error(err) assert.True(errors.Match(errors.T(errors.NotUnique), err)) // Previous transactions should have been rolled back and only hs1 should be associated - _, gotHostSources, _, err = repo.LookupTarget(context.Background(), projTarget.PublicId) + _, gotHostSources, _, err = repo.LookupTarget(ctx, projTarget.GetPublicId()) require.NoError(err) assert.Len(gotHostSources, 1) assert.Equal(hs1.PublicId, gotHostSources[0].Id()) @@ -205,6 +207,8 @@ func TestRepository_DeleteTargetHosts(t *testing.T) { repo, err := target.NewRepository(rw, rw, testKms) require.NoError(t, err) + ctx := context.Background() + type args struct { target target.Target targetIdOverride *string @@ -223,7 +227,7 @@ func TestRepository_DeleteTargetHosts(t *testing.T) { { name: "valid", args: args{ - target: tcp.TestTarget(t, conn, proj.PublicId, "valid"), + target: tcp.TestTarget(ctx, t, conn, proj.PublicId, "valid"), createCnt: 5, deleteCnt: 5, }, @@ -233,7 +237,7 @@ func TestRepository_DeleteTargetHosts(t *testing.T) { { name: "valid-keeping-some", args: args{ - target: tcp.TestTarget(t, conn, proj.PublicId, "valid-keeping-some"), + target: tcp.TestTarget(ctx, t, conn, proj.PublicId, "valid-keeping-some"), createCnt: 5, deleteCnt: 2, }, @@ -243,7 +247,7 @@ func TestRepository_DeleteTargetHosts(t *testing.T) { { name: "no-deletes", args: args{ - target: tcp.TestTarget(t, conn, proj.PublicId, "no-deletes"), + target: tcp.TestTarget(ctx, t, conn, proj.PublicId, "no-deletes"), createCnt: 5, }, wantRowsDeleted: 0, @@ -253,7 +257,7 @@ func TestRepository_DeleteTargetHosts(t *testing.T) { { name: "not-found", args: args{ - target: tcp.TestTarget(t, conn, proj.PublicId, "not-found"), + target: tcp.TestTarget(ctx, t, conn, proj.PublicId, "not-found"), targetIdOverride: func() *string { id := tcp.TestId(t); return &id }(), createCnt: 5, deleteCnt: 5, @@ -264,7 +268,7 @@ func TestRepository_DeleteTargetHosts(t *testing.T) { { name: "missing-target-id", args: args{ - target: tcp.TestTarget(t, conn, proj.PublicId, "missing-target-id"), + target: tcp.TestTarget(ctx, t, conn, proj.PublicId, "missing-target-id"), targetIdOverride: func() *string { id := ""; return &id }(), createCnt: 5, deleteCnt: 5, @@ -276,7 +280,7 @@ func TestRepository_DeleteTargetHosts(t *testing.T) { { name: "zero-version", args: args{ - target: tcp.TestTarget(t, conn, proj.PublicId, "zero-version"), + target: tcp.TestTarget(ctx, t, conn, proj.PublicId, "zero-version"), targetVersionOverride: func() *uint32 { v := uint32(0); return &v }(), createCnt: 5, deleteCnt: 5, @@ -288,7 +292,7 @@ func TestRepository_DeleteTargetHosts(t *testing.T) { { name: "bad-version", args: args{ - target: tcp.TestTarget(t, conn, proj.PublicId, "bad-version"), + target: tcp.TestTarget(ctx, t, conn, proj.PublicId, "bad-version"), targetVersionOverride: func() *uint32 { v := uint32(1000); return &v }(), createCnt: 5, deleteCnt: 5, @@ -373,26 +377,27 @@ func TestRepository_DeleteTargetHosts(t *testing.T) { hs2 := hsets[1] hs3 := hsets[2] - projTarget := tcp.TestTarget(t, conn, proj.PublicId, "delete-unassociated") - _, gotHostSources, _, err := repo.AddTargetHostSources(context.Background(), projTarget.PublicId, 1, []string{hs1.PublicId, hs2.PublicId}) + ctx := context.Background() + projTarget := tcp.TestTarget(ctx, t, conn, proj.PublicId, "delete-unassociated") + _, gotHostSources, _, err := repo.AddTargetHostSources(ctx, projTarget.GetPublicId(), 1, []string{hs1.PublicId, hs2.PublicId}) require.NoError(err) assert.Len(gotHostSources, 2) assert.Equal(hs1.PublicId, gotHostSources[0].Id()) // Deleting an unassociated host set should return an error - delCount, err := repo.DeleteTargetHostSources(context.Background(), projTarget.PublicId, 2, []string{hs3.PublicId}) + delCount, err := repo.DeleteTargetHostSources(ctx, projTarget.GetPublicId(), 2, []string{hs3.PublicId}) require.Error(err) assert.True(errors.Match(errors.T(errors.MultipleRecords), err)) assert.Equal(0, delCount) // Deleting host sets which includes an unassociated host set should return an error - delCount, err = repo.DeleteTargetHostSources(context.Background(), projTarget.PublicId, 2, []string{hs1.PublicId, hs2.PublicId, hs3.PublicId}) + delCount, err = repo.DeleteTargetHostSources(ctx, projTarget.GetPublicId(), 2, []string{hs1.PublicId, hs2.PublicId, hs3.PublicId}) require.Error(err) assert.True(errors.Match(errors.T(errors.MultipleRecords), err)) assert.Equal(0, delCount) // Previous transactions should have been rolled back - _, gotHostSources, _, err = repo.LookupTarget(context.Background(), projTarget.PublicId) + _, gotHostSources, _, err = repo.LookupTarget(ctx, projTarget.GetPublicId()) require.NoError(err) assert.Len(gotHostSources, 2) }) @@ -427,6 +432,8 @@ func TestRepository_SetTargetHostSets(t *testing.T) { return results } + ctx := context.Background() + setupFn := func(target target.Target) []target.HostSource { hs := createHostSetsFn() _, created, _, err := repo.AddTargetHostSources(context.Background(), target.GetPublicId(), 1, hs) @@ -452,7 +459,7 @@ func TestRepository_SetTargetHostSets(t *testing.T) { name: "clear", setup: setupFn, args: args{ - target: tcp.TestTarget(t, conn, proj.PublicId, "clear"), + target: tcp.TestTarget(ctx, t, conn, proj.PublicId, "clear"), targetVersion: 2, // yep, since setupFn will increment it to 2 hostSourceIds: []string{}, }, @@ -463,7 +470,7 @@ func TestRepository_SetTargetHostSets(t *testing.T) { name: "no-change", setup: setupFn, args: args{ - target: tcp.TestTarget(t, conn, proj.PublicId, "no-change"), + target: tcp.TestTarget(ctx, t, conn, proj.PublicId, "no-change"), targetVersion: 2, // yep, since setupFn will increment it to 2 hostSourceIds: []string{}, addToOrigHostSources: true, @@ -475,7 +482,7 @@ func TestRepository_SetTargetHostSets(t *testing.T) { name: "add-sets", setup: setupFn, args: args{ - target: tcp.TestTarget(t, conn, proj.PublicId, "add-sets"), + target: tcp.TestTarget(ctx, t, conn, proj.PublicId, "add-sets"), targetVersion: 2, // yep, since setupFn will increment it to 2 hostSourceIds: []string{testHostSetIds[0], testHostSetIds[1]}, addToOrigHostSources: true, @@ -487,7 +494,7 @@ func TestRepository_SetTargetHostSets(t *testing.T) { name: "add host sets with zero version", setup: setupFn, args: args{ - target: tcp.TestTarget(t, conn, proj.PublicId, "add host sets with zero version"), + target: tcp.TestTarget(ctx, t, conn, proj.PublicId, "add host sets with zero version"), targetVersion: 0, hostSourceIds: []string{testHostSetIds[0], testHostSetIds[1]}, addToOrigHostSources: true, @@ -498,7 +505,7 @@ func TestRepository_SetTargetHostSets(t *testing.T) { name: "remove existing and add users and grps", setup: setupFn, args: args{ - target: tcp.TestTarget(t, conn, proj.PublicId, "remove existing and add host sets"), + target: tcp.TestTarget(ctx, t, conn, proj.PublicId, "remove existing and add host sets"), targetVersion: 2, // yep, since setupFn will increment it to 2 hostSourceIds: []string{testHostSetIds[0], testHostSetIds[1]}, addToOrigHostSources: false, diff --git a/internal/target/tcp/repository_tcp_target_test.go b/internal/target/tcp/repository_tcp_target_test.go index 9b2338a67..e522eae06 100644 --- a/internal/target/tcp/repository_tcp_target_test.go +++ b/internal/target/tcp/repository_tcp_target_test.go @@ -53,8 +53,10 @@ func TestRepository_CreateTarget(t *testing.T) { credLibIds = append(credLibIds, cl.PublicId) } + ctx := context.Background() + type args struct { - target *tcp.Target + target target.Target opt []target.Option } tests := []struct { @@ -68,8 +70,8 @@ func TestRepository_CreateTarget(t *testing.T) { { name: "valid-org", args: args{ - target: func() *tcp.Target { - target, err := tcp.New(proj.PublicId, + target: func() target.Target { + target, err := target.New(ctx, tcp.Subtype, proj.PublicId, target.WithName("valid-org"), target.WithDescription("valid-org"), target.WithDefaultPort(uint32(22))) @@ -103,8 +105,10 @@ func TestRepository_CreateTarget(t *testing.T) { { name: "public-id-not-empty", args: args{ - target: func() *tcp.Target { - tar, err := tcp.New( + target: func() target.Target { + tar, err := target.New( + ctx, + tcp.Subtype, proj.PublicId, target.WithName("valid-org"), target.WithDescription("valid-org"), @@ -113,7 +117,7 @@ func TestRepository_CreateTarget(t *testing.T) { require.NoError(t, err) id, err := db.NewPublicId(tcp.TargetPrefix) require.NoError(t, err) - tar.PublicId = id + tar.SetPublicId(ctx, id) return tar }(), }, @@ -123,13 +127,15 @@ func TestRepository_CreateTarget(t *testing.T) { { name: "empty-scope-id", args: args{ - target: func() *tcp.Target { - tar, err := tcp.New( + target: func() target.Target { + tar, err := target.New( + ctx, + tcp.Subtype, proj.PublicId, target.WithName("empty-scope-id"), ) require.NoError(t, err) - tar.ScopeId = "" + tar.SetScopeId("") return tar }(), }, @@ -184,7 +190,6 @@ func TestRepository_CreateTarget(t *testing.T) { func TestRepository_UpdateTcpTarget(t *testing.T) { t.Parallel() - ctx := context.Background() conn, _ := db.TestSetup(t, "postgres") rw := db.New(conn) wrapper := db.TestWrapper(t) @@ -388,8 +393,9 @@ func TestRepository_UpdateTcpTarget(t *testing.T) { t.Run(tt.name, func(t *testing.T) { cs := css[i] require, assert := require.New(t), assert.New(t) + ctx := context.Background() if tt.wantDup { - _ = tcp.TestTarget(t, conn, proj.PublicId, tt.args.name) + _ = tcp.TestTarget(ctx, t, conn, proj.PublicId, tt.args.name) } testCats := static.TestCatalogs(t, conn, proj.PublicId, 1) @@ -421,26 +427,27 @@ func TestRepository_UpdateTcpTarget(t *testing.T) { if name == "" { name = tcp.TestId(t) } - tar := tcp.TestTarget(t, conn, tt.newScopeId, name, tt.newTargetOpts...) + tar := tcp.TestTarget(ctx, t, conn, tt.newScopeId, name, tt.newTargetOpts...) updateTarget := tcp.NewTestTarget( tt.args.ScopeId, target.WithName(tt.args.name), target.WithDescription(tt.args.description), target.WithDefaultPort(tt.args.port), ) - updateTarget.PublicId = tar.PublicId + updateTarget.SetPublicId(ctx, tar.GetPublicId()) if tt.args.PublicId != nil { - updateTarget.PublicId = *tt.args.PublicId + ut := updateTarget.(*tcp.Target) + ut.PublicId = *tt.args.PublicId } - targetAfterUpdate, hostSources, credSources, updatedRows, err := repo.UpdateTarget(context.Background(), updateTarget, tar.Version, tt.args.fieldMaskPaths, tt.args.opt...) + targetAfterUpdate, hostSources, credSources, updatedRows, err := repo.UpdateTarget(ctx, updateTarget, tar.GetVersion(), tt.args.fieldMaskPaths, tt.args.opt...) if tt.wantErr { assert.Error(err) assert.True(errors.Match(errors.T(tt.wantIsError), err)) assert.Nil(targetAfterUpdate) assert.Equal(0, updatedRows) assert.Contains(err.Error(), tt.wantErrMsg) - err = db.TestVerifyOplog(t, rw, tar.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) + err = db.TestVerifyOplog(t, rw, tar.GetPublicId(), db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) assert.Error(err) assert.True(errors.IsNotFoundError(err)) return @@ -462,11 +469,11 @@ func TestRepository_UpdateTcpTarget(t *testing.T) { switch tt.name { case "valid-no-op": - assert.Equal(tar.UpdateTime, targetAfterUpdate.(*tcp.Target).UpdateTime) + assert.Equal(tar.GetUpdateTime(), targetAfterUpdate.GetUpdateTime()) default: - assert.NotEqual(tar.UpdateTime, targetAfterUpdate.(*tcp.Target).UpdateTime) + assert.NotEqual(tar.GetUpdateTime(), targetAfterUpdate.GetUpdateTime()) } - foundTarget, _, _, err := repo.LookupTarget(context.Background(), tar.PublicId) + foundTarget, _, _, err := repo.LookupTarget(context.Background(), tar.GetPublicId()) assert.NoError(err) assert.True(proto.Equal(targetAfterUpdate.((*tcp.Target)), foundTarget.((*tcp.Target)))) underlyingDB, err := conn.SqlDB(ctx) @@ -476,7 +483,7 @@ func TestRepository_UpdateTcpTarget(t *testing.T) { assert.Equal(foundTarget.GetDescription(), "") dbassert.IsNull(foundTarget, "description") } - err = db.TestVerifyOplog(t, rw, tar.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) + err = db.TestVerifyOplog(t, rw, tar.GetPublicId(), db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) assert.NoError(err) }) } diff --git a/internal/target/tcp/repository_test.go b/internal/target/tcp/repository_test.go index 32501047b..e7474b271 100644 --- a/internal/target/tcp/repository_test.go +++ b/internal/target/tcp/repository_test.go @@ -26,12 +26,13 @@ func TestRepository_LookupTarget(t *testing.T) { iamRepo := iam.TestRepo(t, conn, wrapper) _, proj := iam.TestScopes(t, iamRepo) proj.Name = "project-name" - _, _, err := iamRepo.UpdateScope(context.Background(), proj, 1, []string{"name"}) + ctx := context.Background() + _, _, err := iamRepo.UpdateScope(ctx, proj, 1, []string{"name"}) require.NoError(t, err) rw := db.New(conn) repo, err := target.NewRepository(rw, rw, testKms) require.NoError(t, err) - tgt := tcp.TestTarget(t, conn, proj.PublicId, "target-name") + tgt := tcp.TestTarget(ctx, t, conn, proj.PublicId, "target-name") tests := []struct { testName string @@ -43,12 +44,12 @@ func TestRepository_LookupTarget(t *testing.T) { }{ { testName: "id", - id: tgt.PublicId, + id: tgt.GetPublicId(), wantErr: false, }, { testName: "name only", - name: tgt.Name, + name: tgt.GetName(), wantErr: true, }, { @@ -69,39 +70,39 @@ func TestRepository_LookupTarget(t *testing.T) { }, { testName: "everything", - name: tgt.Name, + name: tgt.GetName(), scopeId: proj.PublicId, scopeName: proj.Name, wantErr: true, }, { testName: "name and scope name", - name: tgt.Name, + name: tgt.GetName(), scopeName: proj.Name, wantErr: false, }, { testName: "name and scope id", - name: tgt.Name, + name: tgt.GetName(), scopeId: proj.PublicId, wantErr: false, }, { testName: "id and name", - id: tgt.PublicId, - name: tgt.Name, + id: tgt.GetPublicId(), + name: tgt.GetName(), scopeId: proj.PublicId, wantErr: true, }, { testName: "id and scope name", - id: tgt.PublicId, + id: tgt.GetPublicId(), scopeName: proj.Name, wantErr: true, }, { testName: "id and scope id", - id: tgt.PublicId, + id: tgt.GetPublicId(), scopeId: proj.PublicId, wantErr: true, }, @@ -123,13 +124,13 @@ func TestRepository_LookupTarget(t *testing.T) { if tt.scopeName != "" { opts = append(opts, target.WithScopeName(tt.scopeName)) } - got, _, _, err := repo.LookupTarget(context.Background(), id, opts...) + got, _, _, err := repo.LookupTarget(ctx, id, opts...) if tt.wantErr { require.Error(err) return } require.NoError(err) - assert.Equal(tgt.PublicId, got.GetPublicId()) + assert.Equal(tgt.GetPublicId(), got.GetPublicId()) }) } } @@ -213,18 +214,19 @@ func TestRepository_ListTargets(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) + ctx := context.Background() require.NoError(conn.Where("1=1").Delete(tcp.NewTestTarget("")).Error) - testGroups := []*tcp.Target{} + testGroups := []target.Target{} for i := 0; i < tt.createCnt; i++ { switch { case tt.createScopeId2 != "" && i%2 == 0: - testGroups = append(testGroups, tcp.TestTarget(t, conn, tt.createScopeId2, strconv.Itoa(i))) + testGroups = append(testGroups, tcp.TestTarget(ctx, t, conn, tt.createScopeId2, strconv.Itoa(i))) default: - testGroups = append(testGroups, tcp.TestTarget(t, conn, tt.createScopeId, strconv.Itoa(i))) + testGroups = append(testGroups, tcp.TestTarget(ctx, t, conn, tt.createScopeId, strconv.Itoa(i))) } } assert.Equal(tt.createCnt, len(testGroups)) - got, err := repo.ListTargets(context.Background(), tt.args.opt...) + got, err := repo.ListTargets(ctx, tt.args.opt...) if tt.wantErr { require.Error(err) return @@ -249,16 +251,17 @@ func TestRepository_ListRoles_Multiple_Scopes(t *testing.T) { require.NoError(t, conn.Where("1=1").Delete(tcp.NewTestTarget("")).Error) + ctx := context.Background() const numPerScope = 10 var total int for i := 0; i < numPerScope; i++ { - tcp.TestTarget(t, conn, proj1.GetPublicId(), fmt.Sprintf("proj1-%d", i)) + tcp.TestTarget(ctx, t, conn, proj1.GetPublicId(), fmt.Sprintf("proj1-%d", i)) total++ - tcp.TestTarget(t, conn, proj2.GetPublicId(), fmt.Sprintf("proj2-%d", i)) + tcp.TestTarget(ctx, t, conn, proj2.GetPublicId(), fmt.Sprintf("proj2-%d", i)) total++ } - got, err := repo.ListTargets(context.Background(), target.WithScopeIds([]string{"global", proj1.GetPublicId(), proj2.GetPublicId()})) + got, err := repo.ListTargets(ctx, target.WithScopeIds([]string{"global", proj1.GetPublicId(), proj2.GetPublicId()})) require.NoError(t, err) assert.Equal(t, total, len(got)) } @@ -274,6 +277,7 @@ func TestRepository_DeleteTarget(t *testing.T) { repo, err := target.NewRepository(rw, rw, testKms) require.NoError(t, err) + ctx := context.Background() type args struct { target target.Target opt []target.Option @@ -288,7 +292,7 @@ func TestRepository_DeleteTarget(t *testing.T) { { name: "valid", args: args{ - target: tcp.TestTarget(t, conn, proj.PublicId, "valid"), + target: tcp.TestTarget(ctx, t, conn, proj.PublicId, "valid"), }, wantRowsDeleted: 1, wantErr: false, @@ -297,7 +301,7 @@ func TestRepository_DeleteTarget(t *testing.T) { name: "no-public-id", args: args{ target: func() target.Target { - tar, _ := tcp.New(proj.PublicId) + tar, _ := target.New(ctx, tcp.Subtype, proj.PublicId) return tar }(), }, @@ -311,8 +315,8 @@ func TestRepository_DeleteTarget(t *testing.T) { target: func() target.Target { id, err := db.NewPublicId(tcp.TargetPrefix) require.NoError(t, err) - tar, _ := tcp.New(proj.PublicId) - tar.PublicId = id + tar, _ := target.New(ctx, tcp.Subtype, proj.PublicId) + tar.SetPublicId(ctx, id) return tar }(), }, @@ -324,7 +328,7 @@ func TestRepository_DeleteTarget(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert := assert.New(t) - deletedRows, err := repo.DeleteTarget(context.Background(), tt.args.target.GetPublicId(), tt.args.opt...) + deletedRows, err := repo.DeleteTarget(ctx, tt.args.target.GetPublicId(), tt.args.opt...) if tt.wantErr { assert.Error(err) assert.Equal(0, deletedRows) @@ -336,7 +340,7 @@ func TestRepository_DeleteTarget(t *testing.T) { } assert.NoError(err) assert.Equal(tt.wantRowsDeleted, deletedRows) - foundGroup, _, _, err := repo.LookupTarget(context.Background(), tt.args.target.GetPublicId()) + foundGroup, _, _, err := repo.LookupTarget(ctx, tt.args.target.GetPublicId()) assert.NoError(err) assert.Nil(foundGroup) diff --git a/internal/target/tcp/target.go b/internal/target/tcp/target.go index e98a7f329..bcbe4d471 100644 --- a/internal/target/tcp/target.go +++ b/internal/target/tcp/target.go @@ -37,9 +37,9 @@ var ( _ oplog.ReplayableMessage = (*Target)(nil) ) -// New creates a new in memory tcp target. WithName, WithDescription and +// newTarget creates a new in memory tcp target. WithName, WithDescription and // WithDefaultPort options are supported -func New(scopeId string, opt ...target.Option) (*Target, error) { +func newTarget(scopeId string, opt ...target.Option) (target.Target, error) { const op = "tcp.NewTarget" opts := target.GetOpts(opt...) if scopeId == "" { diff --git a/internal/target/tcp/target_test.go b/internal/target/tcp/target_test.go index 82ec6b46c..b53447390 100644 --- a/internal/target/tcp/target_test.go +++ b/internal/target/tcp/target_test.go @@ -22,6 +22,7 @@ func TestTarget_Create(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") wrapper := db.TestWrapper(t) _, prj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) + ctx := context.Background() type args struct { scopeId string opt []target.Option @@ -29,7 +30,7 @@ func TestTarget_Create(t *testing.T) { tests := []struct { name string args args - want *tcp.Target + want target.Target wantErr bool wantIsErr errors.Code create bool @@ -47,8 +48,10 @@ func TestTarget_Create(t *testing.T) { scopeId: prj.PublicId, opt: []target.Option{target.WithName("valid-proj-scope")}, }, - want: func() *tcp.Target { - t, _ := tcp.New( + want: func() target.Target { + t, _ := target.New( + ctx, + tcp.Subtype, prj.PublicId, target.WithName("valid-proj-scope"), target.WithSessionMaxSeconds(uint32((8 * time.Hour).Seconds())), @@ -62,7 +65,7 @@ func TestTarget_Create(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - got, err := tcp.New(tt.args.scopeId, tt.args.opt...) + got, err := target.New(ctx, tcp.Subtype, tt.args.scopeId, tt.args.opt...) if tt.wantErr { require.Error(err) assert.True(errors.Match(errors.T(tt.wantIsErr), err)) @@ -73,8 +76,8 @@ func TestTarget_Create(t *testing.T) { if tt.create { id, err := db.NewPublicId(tcp.TargetPrefix) require.NoError(err) - got.PublicId = id - err = db.New(conn).Create(context.Background(), got) + got.SetPublicId(ctx, id) + err = db.New(conn).Create(ctx, got) if tt.wantCreateErr { assert.Error(err) return @@ -92,29 +95,30 @@ func TestTarget_Delete(t *testing.T) { rw := db.New(conn) wrapper := db.TestWrapper(t) _, proj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) + ctx := context.Background() tests := []struct { name string - target *tcp.Target + target target.Target wantRowsDeleted int wantErr bool wantErrMsg string }{ { name: "valid", - target: tcp.TestTarget(t, conn, proj.PublicId, tcp.TestTargetName(t, proj.PublicId)), + target: tcp.TestTarget(ctx, t, conn, proj.PublicId, tcp.TestTargetName(t, proj.PublicId)), wantErr: false, wantRowsDeleted: 1, }, { name: "bad-id", - target: func() *tcp.Target { - tar, _ := tcp.New(proj.PublicId) + target: func() target.Target { + tar, _ := target.New(ctx, tcp.Subtype, proj.PublicId) id, err := db.NewPublicId(tcp.TargetPrefix) require.NoError(t, err) - tar.PublicId = id - tar.Name = tcp.TestTargetName(t, proj.PublicId) + tar.SetPublicId(ctx, id) + tar.SetName(tcp.TestTargetName(t, proj.PublicId)) return tar }(), wantErr: false, @@ -125,7 +129,7 @@ func TestTarget_Delete(t *testing.T) { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) deleteTarget := tcp.NewTestTarget("") - deleteTarget.PublicId = tt.target.PublicId + deleteTarget.SetPublicId(ctx, tt.target.GetPublicId()) deletedRows, err := rw.Delete(context.Background(), deleteTarget) if tt.wantErr { require.Error(err) @@ -138,7 +142,7 @@ func TestTarget_Delete(t *testing.T) { } assert.Equal(tt.wantRowsDeleted, deletedRows) foundTarget := tcp.NewTestTarget("") - foundTarget.PublicId = tt.target.PublicId + foundTarget.SetPublicId(ctx, tt.target.GetPublicId()) err = rw.LookupById(context.Background(), foundTarget) require.Error(err) assert.True(errors.IsNotFoundError(err)) @@ -149,7 +153,6 @@ func TestTarget_Delete(t *testing.T) { func TestTarget_Update(t *testing.T) { t.Parallel() id := tcp.TestId(t) - ctx := context.Background() conn, _ := db.TestSetup(t, "postgres") rw := db.New(conn) wrapper := db.TestWrapper(t) @@ -248,94 +251,99 @@ func TestTarget_Update(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) + ctx := context.Background() if tt.wantDup { - target := tcp.TestTarget(t, conn, proj.PublicId, tcp.TestTargetName(t, proj.PublicId)) - target.Name = tt.args.name - _, err := rw.Update(context.Background(), target, tt.args.fieldMaskPaths, tt.args.nullPaths) + target := tcp.TestTarget(ctx, t, conn, proj.PublicId, tcp.TestTargetName(t, proj.PublicId)) + target.SetName(tt.args.name) + _, err := rw.Update(ctx, target, tt.args.fieldMaskPaths, tt.args.nullPaths) require.NoError(err) } id := tcp.TestId(t) - tar := tcp.TestTarget(t, conn, proj.PublicId, id, target.WithDescription(id)) + tar := tcp.TestTarget(ctx, t, conn, proj.PublicId, id, target.WithDescription(id)) updateTarget := tcp.NewTestTarget(tt.args.ScopeId) - updateTarget.PublicId = tar.PublicId - updateTarget.Name = tt.args.name - updateTarget.Description = tt.args.description + updateTarget.SetPublicId(ctx, tar.GetPublicId()) + updateTarget.SetName(tt.args.name) + updateTarget.SetDescription(tt.args.description) - updatedRows, err := rw.Update(context.Background(), updateTarget, tt.args.fieldMaskPaths, tt.args.nullPaths) + updatedRows, err := rw.Update(ctx, updateTarget, tt.args.fieldMaskPaths, tt.args.nullPaths) if tt.wantErr { require.Error(err) assert.Equal(0, updatedRows) assert.Equal(tt.wantErrMsg, err.Error()) - err = db.TestVerifyOplog(t, rw, tar.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) + err = db.TestVerifyOplog(t, rw, tar.GetPublicId(), db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) require.Error(err) assert.Contains(err.Error(), "record not found") return } require.NoError(err) assert.Equal(tt.wantRowsUpdate, updatedRows) - assert.NotEqual(tar.UpdateTime, updateTarget.UpdateTime) + assert.NotEqual(tar.GetUpdateTime(), updateTarget.GetUpdateTime()) foundTarget := tcp.NewTestTarget(tt.args.ScopeId) - foundTarget.PublicId = tar.GetPublicId() - err = rw.LookupByPublicId(context.Background(), foundTarget) + foundTarget.SetPublicId(ctx, tar.GetPublicId()) + err = rw.LookupByPublicId(ctx, foundTarget) require.NoError(err) - assert.True(proto.Equal(updateTarget, foundTarget)) + assert.True(proto.Equal(updateTarget.(*tcp.Target).Target, foundTarget.(*tcp.Target).Target)) if len(tt.args.nullPaths) != 0 { underlyingDB, err := conn.SqlDB(ctx) require.NoError(err) dbassert := dbassert.New(t, underlyingDB) for _, f := range tt.args.nullPaths { - dbassert.IsNull(&foundTarget, f) + ft := foundTarget.(*tcp.Target) + dbassert.IsNull(&ft, f) } } }) } t.Run("update dup names in diff scopes", func(t *testing.T) { + ctx := context.Background() assert, require := assert.New(t), require.New(t) id := tcp.TestId(t) _, proj2 := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) - _ = tcp.TestTarget(t, conn, proj2.PublicId, id, target.WithDescription(id)) - projTarget := tcp.TestTarget(t, conn, proj.PublicId, id) - projTarget.Name = id - updatedRows, err := rw.Update(context.Background(), projTarget, []string{"Name"}, nil) + _ = tcp.TestTarget(ctx, t, conn, proj2.PublicId, id, target.WithDescription(id)) + projTarget := tcp.TestTarget(ctx, t, conn, proj.PublicId, id) + projTarget.SetName(id) + updatedRows, err := rw.Update(ctx, projTarget, []string{"Name"}, nil) require.NoError(err) assert.Equal(1, updatedRows) - foundTarget, _ := tcp.New(proj2.PublicId) - foundTarget.PublicId = projTarget.GetPublicId() - err = rw.LookupByPublicId(context.Background(), foundTarget) + foundTarget, _ := target.New(ctx, tcp.Subtype, proj2.PublicId) + foundTarget.SetPublicId(ctx, projTarget.GetPublicId()) + err = rw.LookupByPublicId(ctx, foundTarget) require.NoError(err) - assert.Equal(id, projTarget.Name) + assert.Equal(id, projTarget.GetName()) }) } func TestTarget_Clone(t *testing.T) { t.Parallel() + ctx := context.Background() conn, _ := db.TestSetup(t, "postgres") wrapper := db.TestWrapper(t) t.Run("valid", func(t *testing.T) { assert := assert.New(t) _, proj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) - tar := tcp.TestTarget(t, conn, proj.PublicId, tcp.TestTargetName(t, proj.PublicId)) + tar := tcp.TestTarget(ctx, t, conn, proj.PublicId, tcp.TestTargetName(t, proj.PublicId)) cp := tar.Clone() - assert.True(proto.Equal(cp.(*tcp.Target).Target, tar.Target)) + assert.True(proto.Equal(cp.(*tcp.Target).Target, tar.(*tcp.Target).Target)) }) t.Run("not-equal", func(t *testing.T) { assert := assert.New(t) _, proj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) _, proj2 := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) - target1 := tcp.TestTarget(t, conn, proj.PublicId, tcp.TestTargetName(t, proj.PublicId)) - target2 := tcp.TestTarget(t, conn, proj2.PublicId, tcp.TestTargetName(t, proj2.PublicId)) + target1 := tcp.TestTarget(ctx, t, conn, proj.PublicId, tcp.TestTargetName(t, proj.PublicId)) + target2 := tcp.TestTarget(ctx, t, conn, proj2.PublicId, tcp.TestTargetName(t, proj2.PublicId)) cp := target1.Clone() - assert.True(!proto.Equal(cp.(*tcp.Target).Target, target2.Target)) + assert.True(!proto.Equal(cp.(*tcp.Target).Target, target2.(*tcp.Target).Target)) }) } func TestTable_SetTableName(t *testing.T) { t.Parallel() defaultTableName := tcp.DefaultTableName + ctx := context.Background() tests := []struct { name string setNameTo string @@ -355,9 +363,10 @@ func TestTable_SetTableName(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - def, _ := tcp.New("testScope") - require.Equal(defaultTableName, def.TableName()) - s, _ := tcp.New("testScope") + def, _ := target.New(ctx, tcp.Subtype, "testScope") + require.Equal(defaultTableName, def.(*tcp.Target).TableName()) + ss, _ := target.New(ctx, tcp.Subtype, "testScope") + s := ss.(*tcp.Target) s.SetTableName(tt.setNameTo) assert.Equal(tt.want, s.TableName()) }) @@ -365,19 +374,22 @@ func TestTable_SetTableName(t *testing.T) { } func TestTarget_oplog(t *testing.T) { + ctx := context.Background() id := tcp.TestId(t) tests := []struct { name string - target *tcp.Target + target target.Target op oplog.OpType want oplog.Metadata }{ { name: "simple", - target: func() *tcp.Target { - t, _ := tcp.New(id) - t.PublicId = id - return t + target: func() target.Target { + tar, _ := target.New(ctx, tcp.Subtype, id) + if err := tar.SetPublicId(ctx, id); err != nil { + t.Fatalf("failed to set public id: %s", err) + } + return tar }(), op: oplog.OpType_OP_TYPE_CREATE, want: oplog.Metadata{ diff --git a/internal/target/tcp/testing.go b/internal/target/tcp/testing.go index 6e917abd4..6a8e310db 100644 --- a/internal/target/tcp/testing.go +++ b/internal/target/tcp/testing.go @@ -12,24 +12,24 @@ import ( ) // TestTarget is used to create a Target that can be used by tests in other packages. -func TestTarget(t *testing.T, conn *db.DB, scopeId, name string, opt ...target.Option) *Target { +func TestTarget(ctx context.Context, t *testing.T, conn *db.DB, scopeId, name string, opt ...target.Option) target.Target { t.Helper() opt = append(opt, target.WithName(name)) opts := target.GetOpts(opt...) require := require.New(t) rw := db.New(conn) - tar, err := New(scopeId, opt...) + tar, err := target.New(ctx, Subtype, scopeId, opt...) require.NoError(err) id, err := db.NewPublicId(TargetPrefix) require.NoError(err) - tar.PublicId = id + tar.SetPublicId(ctx, id) err = rw.Create(context.Background(), tar) require.NoError(err) if len(opts.WithHostSources) > 0 { newHostSets := make([]interface{}, 0, len(opts.WithHostSources)) for _, s := range opts.WithHostSources { - hostSet, err := target.NewTargetHostSet(tar.PublicId, s) + hostSet, err := target.NewTargetHostSet(tar.GetPublicId(), s) require.NoError(err) newHostSets = append(newHostSets, hostSet) } @@ -39,7 +39,7 @@ func TestTarget(t *testing.T, conn *db.DB, scopeId, name string, opt ...target.O if len(opts.WithCredentialLibraries) > 0 { newCredLibs := make([]interface{}, 0, len(opts.WithCredentialLibraries)) for _, cl := range opts.WithCredentialLibraries { - cl.TargetId = tar.PublicId + cl.TargetId = tar.GetPublicId() newCredLibs = append(newCredLibs, cl) } err := rw.CreateItems(context.Background(), newCredLibs) @@ -57,5 +57,5 @@ func testId(t *testing.T) string { t.Helper() id, err := uuid.GenerateUUID() require.NoError(t, err) - return id + return fmt.Sprintf("%s_%s", TargetPrefix, id) } diff --git a/internal/target/tcp/testing_test.go b/internal/target/tcp/testing_test.go index 7faeb2c14..9716a3f52 100644 --- a/internal/target/tcp/testing_test.go +++ b/internal/target/tcp/testing_test.go @@ -25,6 +25,7 @@ func Test_TestTcpTarget(t *testing.T) { _, proj := iam.TestScopes(t, iamRepo) repo, err := target.NewRepository(rw, rw, testKms) require.NoError(err) + ctx := context.Background() cats := static.TestCatalogs(t, conn, proj.PublicId, 1) hsets := static.TestSets(t, conn, cats[0].GetPublicId(), 2) @@ -33,12 +34,12 @@ func Test_TestTcpTarget(t *testing.T) { sets = append(sets, s.PublicId) } name := tcp.TestTargetName(t, proj.PublicId) - tar := tcp.TestTarget(t, conn, proj.PublicId, name, target.WithHostSources(sets)) + tar := tcp.TestTarget(ctx, t, conn, proj.PublicId, name, target.WithHostSources(sets)) require.NotNil(t) - require.NotEmpty(tar.PublicId) - require.Equal(name, tar.Name) + require.NotEmpty(tar.GetPublicId()) + require.Equal(name, tar.GetName()) - _, foundSources, _, err := repo.LookupTarget(context.Background(), tar.PublicId) + _, foundSources, _, err := repo.LookupTarget(context.Background(), tar.GetPublicId()) require.NoError(err) foundIds := make([]string, 0, len(foundSources)) for _, s := range foundSources { @@ -57,8 +58,9 @@ func Test_TestCredentialLibrary(t *testing.T) { _, proj := iam.TestScopes(t, iamRepo) repo, err := target.NewRepository(rw, rw, testKms) require.NoError(err) + ctx := context.Background() - tar := tcp.TestTarget(t, conn, proj.PublicId, t.Name()) + tar := tcp.TestTarget(ctx, t, conn, proj.PublicId, t.Name()) store := vault.TestCredentialStores(t, conn, wrapper, proj.GetPublicId(), 1)[0] vlibs := vault.TestCredentialLibraries(t, conn, wrapper, store.GetPublicId(), 2) var libIds []string @@ -72,7 +74,7 @@ func Test_TestCredentialLibrary(t *testing.T) { assert.Len(libs, 2) - _, _, foundSources, err := repo.LookupTarget(context.Background(), tar.PublicId) + _, _, foundSources, err := repo.LookupTarget(context.Background(), tar.GetPublicId()) require.NoError(err) foundIds := make([]string, 0, len(foundSources)) for _, s := range foundSources { From 533b434d1a7d8256a93e3bf5061cb476747e26ec Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Thu, 4 Nov 2021 11:44:22 -0400 Subject: [PATCH 3/4] refact(controller/targets): Move tcp functionality into subpackage This follows a similar pattern as the target domain package, providing a subpackage for the tcp target functionality, allowing this to be pluggable. --- cmd/boundary/package_registry_oss.go | 1 + .../server/worker_shutdown_reload_test.go | 2 + .../controller/handlers/targets/options.go | 44 ++++ .../controller/handlers/targets/registry.go | 101 ++++++++ .../handlers/targets/target_service.go | 151 ++++++------ .../targets/{ => tcp}/target_service_test.go | 225 ++++++++++-------- .../controller/handlers/targets/tcp/tcp.go | 57 +++++ internal/session/testing.go | 6 +- internal/tests/api/targets/target_test.go | 2 + .../tests/cluster/package_registry_test.go | 7 + 10 files changed, 419 insertions(+), 177 deletions(-) create mode 100644 internal/servers/controller/handlers/targets/options.go create mode 100644 internal/servers/controller/handlers/targets/registry.go rename internal/servers/controller/handlers/targets/{ => tcp}/target_service_test.go (88%) create mode 100644 internal/servers/controller/handlers/targets/tcp/tcp.go create mode 100644 internal/tests/cluster/package_registry_test.go diff --git a/cmd/boundary/package_registry_oss.go b/cmd/boundary/package_registry_oss.go index 65616ff18..b43025f1c 100644 --- a/cmd/boundary/package_registry_oss.go +++ b/cmd/boundary/package_registry_oss.go @@ -2,5 +2,6 @@ package main import ( // Enable tcp target support. + _ "github.com/hashicorp/boundary/internal/servers/controller/handlers/targets/tcp" _ "github.com/hashicorp/boundary/internal/target/tcp" ) diff --git a/internal/cmd/commands/server/worker_shutdown_reload_test.go b/internal/cmd/commands/server/worker_shutdown_reload_test.go index 3699e693a..5c0b05e01 100644 --- a/internal/cmd/commands/server/worker_shutdown_reload_test.go +++ b/internal/cmd/commands/server/worker_shutdown_reload_test.go @@ -20,6 +20,8 @@ import ( "github.com/mitchellh/cli" "github.com/stretchr/testify/require" "go.uber.org/atomic" + + _ "github.com/hashicorp/boundary/internal/servers/controller/handlers/targets/tcp" ) const shutdownReloadApiAddr = "http://127.0.0.1:9203" diff --git a/internal/servers/controller/handlers/targets/options.go b/internal/servers/controller/handlers/targets/options.go new file mode 100644 index 000000000..887a2cff3 --- /dev/null +++ b/internal/servers/controller/handlers/targets/options.go @@ -0,0 +1,44 @@ +package targets + +import ( + "github.com/hashicorp/boundary/internal/target" + "google.golang.org/protobuf/types/known/structpb" +) + +func getOpts(opt ...option) options { + opts := getDefaultOptions() + for _, o := range opt { + o(&opts) + } + return opts +} + +// Option - how Options are passed as arguments +type option func(*options) + +// options = how options are represented +type options struct { + withTarget target.Target + withStruct *structpb.Struct +} + +func getDefaultOptions() options { + return options{ + withTarget: nil, + withStruct: nil, + } +} + +// withTarget provides an option to provide a target.Target. +func withTarget(t target.Target) option { + return func(o *options) { + o.withTarget = t + } +} + +// withStruct provides an option to provide a structpb.Struct. +func withStruct(s *structpb.Struct) option { + return func(o *options) { + o.withStruct = s + } +} diff --git a/internal/servers/controller/handlers/targets/registry.go b/internal/servers/controller/handlers/targets/registry.go new file mode 100644 index 000000000..61f279f31 --- /dev/null +++ b/internal/servers/controller/handlers/targets/registry.go @@ -0,0 +1,101 @@ +package targets + +import ( + "fmt" + "sync" + + "github.com/hashicorp/boundary/internal/servers/controller/handlers" + "github.com/hashicorp/boundary/internal/target" + "github.com/hashicorp/boundary/internal/types/subtypes" + "google.golang.org/protobuf/proto" +) + +// Attributes represent the subtype specific request/response attributes. +type Attributes interface { + proto.Message + + // Options create target.Options to be used to create a target.Target. + Options() []target.Option + // Vet validates the Attributes and returns a map of fields to error messages + // if any fields are invalid. + Vet() map[string]string +} + +type attributeFunc func(target.Target) Attributes + +type registryEntry struct { + maskManager handlers.MaskManager + attrFunc attributeFunc +} + +type registry struct { + sync.Map +} + +func (r registry) get(s subtypes.Subtype) (*registryEntry, error) { + v, ok := r.Load(s) + if !ok { + return nil, fmt.Errorf("subtype %q not registered", s) + } + + re, ok := v.(*registryEntry) + if !ok { + return nil, fmt.Errorf("malformed registry subtypye %q registered as incorrect type %T", s, v) + } + return re, nil +} + +func (r registry) maskManager(s subtypes.Subtype) (handlers.MaskManager, error) { + re, err := r.get(s) + if err != nil { + return nil, err + } + + return re.maskManager, nil +} + +// newAttribute creates an Attribute for the given subtype. It delegates the +// allocation of the Attribute to the registered attrFunc for the given +// subtype. An error is returned if the provided subtype is not registered. It +// supports the options of: +// +// - withTarget: The Attribute will be initialized based on the given +// target.Target. This initialization is delegated to the attrFunc. +// +// - withStruct: The Attribute will be initialized based on the given +// structpb.Struct. This is *not* delegated. +// +// These two options are mutually exclusive. If both are provided, and error is returned. +// If no options are provided an empty Attribute will be returned. +func (r registry) newAttribute(s subtypes.Subtype, opt ...option) (Attributes, error) { + re, err := r.get(s) + if err != nil { + return nil, err + } + + opts := getOpts(opt...) + if opts.withTarget != nil && opts.withStruct != nil { + return nil, fmt.Errorf("cannot use both withTarget and withStruct") + } + + attr := re.attrFunc(opts.withTarget) + if opts.withStruct != nil { + if err := handlers.StructToProto(opts.withStruct, attr); err != nil { + return nil, fmt.Errorf("Provided attributes don't match expected format.") + } + } + + return attr, nil +} + +var subtypeRegistry = registry{} + +// Register registers a subtype for used by the service handler. +func Register(s subtypes.Subtype, maskManager handlers.MaskManager, af attributeFunc) { + if _, existed := subtypeRegistry.LoadOrStore(s, ®istryEntry{ + maskManager: maskManager, + attrFunc: af, + }); existed { + panic(fmt.Sprintf("subtype %s already registered", s)) + } +} diff --git a/internal/servers/controller/handlers/targets/target_service.go b/internal/servers/controller/handlers/targets/target_service.go index ee01be7b3..2d9da18b5 100644 --- a/internal/servers/controller/handlers/targets/target_service.go +++ b/internal/servers/controller/handlers/targets/target_service.go @@ -10,7 +10,6 @@ import ( "net/url" "strings" - "github.com/golang/protobuf/ptypes/wrappers" "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/credential" "github.com/hashicorp/boundary/internal/credential/vault" @@ -30,8 +29,6 @@ import ( "github.com/hashicorp/boundary/internal/servers/controller/handlers" "github.com/hashicorp/boundary/internal/session" "github.com/hashicorp/boundary/internal/target" - "github.com/hashicorp/boundary/internal/target/tcp" - tcpStore "github.com/hashicorp/boundary/internal/target/tcp/store" "github.com/hashicorp/boundary/internal/types/action" "github.com/hashicorp/boundary/internal/types/resource" "github.com/hashicorp/boundary/internal/types/scope" @@ -48,8 +45,6 @@ import ( ) var ( - maskManager handlers.MaskManager - // IdActions contains the set of actions that can be performed on // individual resources IdActions = action.ActionSet{ @@ -77,16 +72,6 @@ var ( } ) -func init() { - var err error - if maskManager, err = handlers.NewMaskManager( - handlers.MaskDestination{&tcpStore.Target{}}, - handlers.MaskSource{&pb.Target{}, &pb.TcpTargetAttributes{}}, - ); err != nil { - panic(err) - } -} - // Service handles request as described by the pbs.TargetServiceServer interface. type Service struct { pbs.UnimplementedTargetServiceServer @@ -1191,14 +1176,14 @@ func (s Service) createInRepo(ctx context.Context, item *pb.Target) (target.Targ if item.GetWorkerFilter() != nil { opts = append(opts, target.WithWorkerFilter(item.GetWorkerFilter().GetValue())) } - tcpAttrs := &pb.TcpTargetAttributes{} - if err := handlers.StructToProto(item.GetAttributes(), tcpAttrs); err != nil { - return nil, nil, nil, handlers.ApiErrorWithCodeAndMessage(codes.InvalidArgument, "Provided attributes don't match expected format.") + + attr, err := subtypeRegistry.newAttribute(target.SubtypeFromType(item.GetType()), withStruct(item.GetAttributes())) + if err != nil { + return nil, nil, nil, handlers.ApiErrorWithCodeAndMessage(codes.InvalidArgument, err.Error()) } - if tcpAttrs.GetDefaultPort().GetValue() != 0 { - opts = append(opts, target.WithDefaultPort(tcpAttrs.GetDefaultPort().GetValue())) - } - u, err := tcp.New(item.GetScopeId(), opts...) + opts = append(opts, attr.Options()...) + + u, err := target.New(ctx, target.SubtypeFromType(item.GetType()), item.GetScopeId(), opts...) if err != nil { return nil, nil, nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to build target for creation: %v.", err) } @@ -1234,19 +1219,28 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []st if filter := item.GetWorkerFilter(); filter != nil { opts = append(opts, target.WithWorkerFilter(item.GetWorkerFilter().GetValue())) } - tcpAttrs := &pb.TcpTargetAttributes{} - if err := handlers.StructToProto(item.GetAttributes(), tcpAttrs); err != nil { - return nil, nil, nil, handlers.ApiErrorWithCodeAndMessage(codes.InvalidArgument, "Provided attributes don't match expected format.") - } - if tcpAttrs.GetDefaultPort().GetValue() != 0 { - opts = append(opts, target.WithDefaultPort(tcpAttrs.GetDefaultPort().GetValue())) + subtype := target.SubtypeFromId(id) + + attr, err := subtypeRegistry.newAttribute(subtype, withStruct(item.GetAttributes())) + if err != nil { + return nil, nil, nil, handlers.ApiErrorWithCodeAndMessage(codes.InvalidArgument, err.Error()) } + + opts = append(opts, attr.Options()...) + version := item.GetVersion() - u, err := tcp.New(scopeId, opts...) + + u, err := target.New(ctx, subtype, scopeId, opts...) if err != nil { return nil, nil, nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to build target for update: %v.", err) } - u.PublicId = id + u.SetPublicId(ctx, id) + + maskManager, err := subtypeRegistry.maskManager(subtype) + if err != nil { + return nil, nil, nil, errors.Wrap(ctx, err, op, errors.WithMsg("unable to update target")) + } + dbMask := maskManager.Translate(mask) if len(dbMask) == 0 { return nil, nil, nil, handlers.InvalidArgumentErrorf("No valid fields included in the update mask.", map[string]string{"update_mask": "No valid paths provided in the update mask."}) @@ -1595,11 +1589,12 @@ func toProto(ctx context.Context, in target.Target, hostSources []target.HostSou } } if outputFields.Has(globals.AttributesField) { - attrs := &pb.TcpTargetAttributes{} - if in.GetDefaultPort() > 0 { - attrs.DefaultPort = &wrappers.UInt32Value{Value: in.GetDefaultPort()} + attr, err := subtypeRegistry.newAttribute(in.GetType(), withTarget(in)) + if err != nil { + return nil, errors.Wrap(ctx, err, op) } - st, err := handlers.ProtoToStruct(attrs) + + st, err := handlers.ProtoToStruct(attr) if err != nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "failed building password attribute struct: %v", err) } @@ -1614,7 +1609,7 @@ func toProto(ctx context.Context, in target.Target, hostSources []target.HostSou // * All required parameters are set // * There are no conflicting parameters provided func validateGetRequest(req *pbs.GetTargetRequest) error { - return handlers.ValidateGetRequest(handlers.NoopValidatorFn, req, tcp.TargetPrefix) + return handlers.ValidateGetRequest(handlers.NoopValidatorFn, req, target.Prefixes()...) } func validateCreateRequest(req *pbs.CreateTargetRequest) error { @@ -1638,16 +1633,6 @@ func validateCreateRequest(req *pbs.CreateTargetRequest) error { if req.GetItem().GetSessionMaxSeconds() != nil && req.GetItem().GetSessionMaxSeconds().GetValue() == 0 { badFields[globals.SessionMaxSecondsField] = "This must be greater than zero." } - switch target.SubtypeFromType(req.GetItem().GetType()) { - case tcp.Subtype: - tcpAttrs := &pb.TcpTargetAttributes{} - if err := handlers.StructToProto(req.GetItem().GetAttributes(), tcpAttrs); err != nil { - badFields[globals.AttributesField] = "Attribute fields do not match the expected format." - } - if tcpAttrs.GetDefaultPort() != nil && tcpAttrs.GetDefaultPort().GetValue() == 0 { - badFields["attributes.default_port"] = "This optional field cannot be set to 0." - } - } if req.GetItem().GetType() == "" { badFields[globals.TypeField] = "This is a required field." } else if target.SubtypeFromType(req.GetItem().GetType()) == "" { @@ -1658,6 +1643,21 @@ func validateCreateRequest(req *pbs.CreateTargetRequest) error { badFields[globals.WorkerFilterField] = "Unable to successfully parse filter expression." } } + + subtype := target.SubtypeFromType(req.GetItem().GetType()) + _, err := subtypeRegistry.get(subtype) + if err != nil { + badFields[globals.TypeField] = "Unknown type provided." + } else { + a, err := subtypeRegistry.newAttribute(subtype, withStruct(req.GetItem().GetAttributes())) + if err != nil { + badFields[globals.AttributesField] = "Attribute fields do not match the expected format." + } else { + for k, v := range a.Vet() { + badFields[k] = v + } + } + } return badFields }) } @@ -1680,30 +1680,35 @@ func validateUpdateRequest(req *pbs.UpdateTargetRequest) error { if req.GetItem().GetSessionMaxSeconds() != nil && req.GetItem().GetSessionMaxSeconds().GetValue() == 0 { badFields[globals.SessionMaxSecondsField] = "This must be greater than zero." } - switch target.SubtypeFromId(req.GetItem().GetType()) { - case tcp.Subtype: - if req.GetItem().GetType() != "" && target.SubtypeFromType(req.GetItem().GetType()) != tcp.Subtype { - badFields[globals.TypeField] = "Cannot modify the resource type." - } - tcpAttrs := &pb.TcpTargetAttributes{} - if err := handlers.StructToProto(req.GetItem().GetAttributes(), tcpAttrs); err != nil { - badFields[globals.AttributesField] = "Attribute fields do not match the expected format." - } - if tcpAttrs.GetDefaultPort() != nil && tcpAttrs.GetDefaultPort().GetValue() == 0 { - badFields["attributes.default_port"] = "This optional field cannot be set to 0." - } - } if filter := req.GetItem().GetWorkerFilter(); filter != nil { if _, err := bexpr.CreateEvaluator(filter.GetValue()); err != nil { badFields[globals.WorkerFilterField] = "Unable to successfully parse filter expression." } } + subtype := target.SubtypeFromId(req.GetId()) + _, err := subtypeRegistry.get(subtype) + if err != nil { + badFields[globals.TypeField] = "Unknown type provided." + } else { + if req.GetItem().GetType() != "" && target.SubtypeFromType(req.GetItem().GetType()) != subtype { + badFields[globals.TypeField] = "Cannot modify the resource type." + } + + a, err := subtypeRegistry.newAttribute(subtype, withStruct(req.GetItem().GetAttributes())) + if err != nil { + badFields[globals.AttributesField] = "Attribute fields do not match the expected format." + } else { + for k, v := range a.Vet() { + badFields[k] = v + } + } + } return badFields - }, tcp.TargetPrefix) + }, target.Prefixes()...) } func validateDeleteRequest(req *pbs.DeleteTargetRequest) error { - return handlers.ValidateDeleteRequest(handlers.NoopValidatorFn, req, tcp.TargetPrefix) + return handlers.ValidateDeleteRequest(handlers.NoopValidatorFn, req, target.Prefixes()...) } func validateListRequest(req *pbs.ListTargetsRequest) error { @@ -1723,7 +1728,7 @@ func validateListRequest(req *pbs.ListTargetsRequest) error { func validateAddSetsRequest(req *pbs.AddTargetHostSetsRequest) error { badFields := map[string]string{} - if !handlers.ValidId(handlers.Id(req.GetId()), tcp.TargetPrefix) { + if !handlers.ValidId(handlers.Id(req.GetId()), target.Prefixes()...) { badFields[globals.IdField] = "Incorrectly formatted identifier." } if req.GetVersion() == 0 { @@ -1747,7 +1752,7 @@ func validateAddSetsRequest(req *pbs.AddTargetHostSetsRequest) error { func validateSetSetsRequest(req *pbs.SetTargetHostSetsRequest) error { badFields := map[string]string{} - if !handlers.ValidId(handlers.Id(req.GetId()), tcp.TargetPrefix) { + if !handlers.ValidId(handlers.Id(req.GetId()), target.Prefixes()...) { badFields[globals.IdField] = "Incorrectly formatted identifier." } if req.GetVersion() == 0 { @@ -1768,7 +1773,7 @@ func validateSetSetsRequest(req *pbs.SetTargetHostSetsRequest) error { func validateRemoveSetsRequest(req *pbs.RemoveTargetHostSetsRequest) error { badFields := map[string]string{} - if !handlers.ValidId(handlers.Id(req.GetId()), tcp.TargetPrefix) { + if !handlers.ValidId(handlers.Id(req.GetId()), target.Prefixes()...) { badFields[globals.IdField] = "Incorrectly formatted identifier." } if req.GetVersion() == 0 { @@ -1792,7 +1797,7 @@ func validateRemoveSetsRequest(req *pbs.RemoveTargetHostSetsRequest) error { func validateAddHostSourcesRequest(req *pbs.AddTargetHostSourcesRequest) error { badFields := map[string]string{} - if !handlers.ValidId(handlers.Id(req.GetId()), tcp.TargetPrefix) { + if !handlers.ValidId(handlers.Id(req.GetId()), target.Prefixes()...) { badFields[globals.IdField] = "Incorrectly formatted identifier." } if req.GetVersion() == 0 { @@ -1816,7 +1821,7 @@ func validateAddHostSourcesRequest(req *pbs.AddTargetHostSourcesRequest) error { func validateSetHostSourcesRequest(req *pbs.SetTargetHostSourcesRequest) error { badFields := map[string]string{} - if !handlers.ValidId(handlers.Id(req.GetId()), tcp.TargetPrefix) { + if !handlers.ValidId(handlers.Id(req.GetId()), target.Prefixes()...) { badFields[globals.IdField] = "Incorrectly formatted identifier." } if req.GetVersion() == 0 { @@ -1837,7 +1842,7 @@ func validateSetHostSourcesRequest(req *pbs.SetTargetHostSourcesRequest) error { func validateRemoveHostSourcesRequest(req *pbs.RemoveTargetHostSourcesRequest) error { badFields := map[string]string{} - if !handlers.ValidId(handlers.Id(req.GetId()), tcp.TargetPrefix) { + if !handlers.ValidId(handlers.Id(req.GetId()), target.Prefixes()...) { badFields[globals.IdField] = "Incorrectly formatted identifier." } if req.GetVersion() == 0 { @@ -1861,7 +1866,7 @@ func validateRemoveHostSourcesRequest(req *pbs.RemoveTargetHostSourcesRequest) e func validateAddLibrariesRequest(req *pbs.AddTargetCredentialLibrariesRequest) error { badFields := map[string]string{} - if !handlers.ValidId(handlers.Id(req.GetId()), tcp.TargetPrefix) { + if !handlers.ValidId(handlers.Id(req.GetId()), target.Prefixes()...) { badFields[globals.IdField] = "Incorrectly formatted identifier." } if req.GetVersion() == 0 { @@ -1884,7 +1889,7 @@ func validateAddLibrariesRequest(req *pbs.AddTargetCredentialLibrariesRequest) e func validateSetLibrariesRequest(req *pbs.SetTargetCredentialLibrariesRequest) error { badFields := map[string]string{} - if !handlers.ValidId(handlers.Id(req.GetId()), tcp.TargetPrefix) { + if !handlers.ValidId(handlers.Id(req.GetId()), target.Prefixes()...) { badFields[globals.IdField] = "Incorrectly formatted identifier." } if req.GetVersion() == 0 { @@ -1904,7 +1909,7 @@ func validateSetLibrariesRequest(req *pbs.SetTargetCredentialLibrariesRequest) e func validateRemoveLibrariesRequest(req *pbs.RemoveTargetCredentialLibrariesRequest) error { badFields := map[string]string{} - if !handlers.ValidId(handlers.Id(req.GetId()), tcp.TargetPrefix) { + if !handlers.ValidId(handlers.Id(req.GetId()), target.Prefixes()...) { badFields[globals.IdField] = "Incorrectly formatted identifier." } if req.GetVersion() == 0 { @@ -1927,7 +1932,7 @@ func validateRemoveLibrariesRequest(req *pbs.RemoveTargetCredentialLibrariesRequ func validateAddCredentialSourcesRequest(req *pbs.AddTargetCredentialSourcesRequest) error { badFields := map[string]string{} - if !handlers.ValidId(handlers.Id(req.GetId()), tcp.TargetPrefix) { + if !handlers.ValidId(handlers.Id(req.GetId()), target.Prefixes()...) { badFields[globals.IdField] = "Incorrectly formatted identifier." } if req.GetVersion() == 0 { @@ -1957,7 +1962,7 @@ func validateAddCredentialSourcesRequest(req *pbs.AddTargetCredentialSourcesRequ func validateSetCredentialSourcesRequest(req *pbs.SetTargetCredentialSourcesRequest) error { badFields := map[string]string{} - if !handlers.ValidId(handlers.Id(req.GetId()), tcp.TargetPrefix) { + if !handlers.ValidId(handlers.Id(req.GetId()), target.Prefixes()...) { badFields[globals.IdField] = "Incorrectly formatted identifier." } if req.GetVersion() == 0 { @@ -1983,7 +1988,7 @@ func validateSetCredentialSourcesRequest(req *pbs.SetTargetCredentialSourcesRequ func validateRemoveCredentialSourcesRequest(req *pbs.RemoveTargetCredentialSourcesRequest) error { badFields := map[string]string{} - if !handlers.ValidId(handlers.Id(req.GetId()), tcp.TargetPrefix) { + if !handlers.ValidId(handlers.Id(req.GetId()), target.Prefixes()...) { badFields[globals.IdField] = "Incorrectly formatted identifier." } if req.GetVersion() == 0 { @@ -2017,7 +2022,7 @@ func validateAuthorizeSessionRequest(req *pbs.AuthorizeSessionRequest) error { scopeIdEmpty := req.GetScopeId() == "" scopeNameEmpty := req.GetScopeName() == "" if nameEmpty { - if !handlers.ValidId(handlers.Id(req.GetId()), tcp.TargetPrefix) { + if !handlers.ValidId(handlers.Id(req.GetId()), target.Prefixes()...) { badFields[globals.IdField] = "Incorrectly formatted identifier." } if !scopeIdEmpty { diff --git a/internal/servers/controller/handlers/targets/target_service_test.go b/internal/servers/controller/handlers/targets/tcp/target_service_test.go similarity index 88% rename from internal/servers/controller/handlers/targets/target_service_test.go rename to internal/servers/controller/handlers/targets/tcp/target_service_test.go index 33c278252..15963bfa4 100644 --- a/internal/servers/controller/handlers/targets/target_service_test.go +++ b/internal/servers/controller/handlers/targets/tcp/target_service_test.go @@ -1,4 +1,4 @@ -package targets_test +package tcp_test import ( "context" @@ -53,6 +53,8 @@ import ( "google.golang.org/protobuf/types/known/structpb" "google.golang.org/protobuf/types/known/timestamppb" "google.golang.org/protobuf/types/known/wrapperspb" + + _ "github.com/hashicorp/boundary/internal/servers/controller/handlers/targets/tcp" ) var testAuthorizedActions = []string{ @@ -115,14 +117,15 @@ func TestGet(t *testing.T) { hc := static.TestCatalogs(t, conn, proj.GetPublicId(), 1)[0] hs := static.TestSets(t, conn, hc.GetPublicId(), 2) - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), "test", target.WithHostSources([]string{hs[0].GetPublicId(), hs[1].GetPublicId()})) + ctx := context.Background() + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "test", target.WithHostSources([]string{hs[0].GetPublicId(), hs[1].GetPublicId()})) pTar := &pb.Target{ Id: tar.GetPublicId(), ScopeId: proj.GetPublicId(), Name: wrapperspb.String("test"), - CreatedTime: tar.CreateTime.GetTimestamp(), - UpdatedTime: tar.UpdateTime.GetTimestamp(), + CreatedTime: tar.GetCreateTime().GetTimestamp(), + UpdatedTime: tar.GetUpdateTime().GetTimestamp(), Scope: &scopes.ScopeInfo{Id: proj.GetPublicId(), Type: scope.Project.String(), ParentScopeId: o.GetPublicId()}, Type: tcp.Subtype.String(), HostSetIds: []string{hs[0].GetPublicId(), hs[1].GetPublicId()}, @@ -205,12 +208,13 @@ func TestList(t *testing.T) { otherHc := static.TestCatalogs(t, conn, otherProj.GetPublicId(), 1)[0] hss := static.TestSets(t, conn, hc.GetPublicId(), 2) otherHss := static.TestSets(t, conn, otherHc.GetPublicId(), 2) + ctx := context.Background() var wantTars []*pb.Target var totalTars []*pb.Target for i := 0; i < 5; i++ { name := fmt.Sprintf("tar%d", i) - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), name, target.WithHostSources([]string{hss[0].GetPublicId(), hss[1].GetPublicId()})) + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), name, target.WithHostSources([]string{hss[0].GetPublicId(), hss[1].GetPublicId()})) wantTars = append(wantTars, &pb.Target{ Id: tar.GetPublicId(), ScopeId: proj.GetPublicId(), @@ -226,7 +230,7 @@ func TestList(t *testing.T) { AuthorizedActions: testAuthorizedActions, }) totalTars = append(totalTars, wantTars[i]) - tar = tcp.TestTarget(t, conn, otherProj.GetPublicId(), name, target.WithHostSources([]string{otherHss[0].GetPublicId(), otherHss[1].GetPublicId()})) + tar = tcp.TestTarget(ctx, t, conn, otherProj.GetPublicId(), name, target.WithHostSources([]string{otherHss[0].GetPublicId(), otherHss[1].GetPublicId()})) totalTars = append(totalTars, &pb.Target{ Id: tar.GetPublicId(), ScopeId: otherProj.GetPublicId(), @@ -333,8 +337,9 @@ func TestDelete(t *testing.T) { return iamRepo, nil } + ctx := context.Background() _, proj := iam.TestScopes(t, iamRepo) - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), "test") + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "test") s, err := testService(t, conn, kms, wrapper) require.NoError(t, err, "Couldn't create a new target service.") @@ -390,6 +395,7 @@ func TestDelete_twice(t *testing.T) { wrapper := db.TestWrapper(t) kms := kms.TestKms(t, conn, wrapper) + ctx := context.Background() iamRepo := iam.TestRepo(t, conn, wrapper) iamRepoFn := func() (*iam.Repository, error) { return iamRepo, nil @@ -397,14 +403,14 @@ func TestDelete_twice(t *testing.T) { _, proj := iam.TestScopes(t, iamRepo) - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), "test") + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "test") s, err := testService(t, conn, kms, wrapper) require.NoError(err, "Couldn't create a new target service.") req := &pbs.DeleteTargetRequest{ Id: tar.GetPublicId(), } - ctx := auth.DisabledAuthTestContext(iamRepoFn, proj.GetPublicId()) + ctx = auth.DisabledAuthTestContext(iamRepoFn, proj.GetPublicId()) _, gErr := s.DeleteTarget(ctx, req) assert.NoError(gErr, "First attempt") _, gErr = s.DeleteTarget(ctx, req) @@ -588,7 +594,10 @@ func TestUpdate(t *testing.T) { {Id: hs[1].GetPublicId(), HostCatalogId: hs[1].GetCatalogId()}, } - tar, err := tcp.New(proj.GetPublicId(), target.WithName("default"), target.WithDescription("default")) + ctx := context.Background() + ttar, err := target.New(ctx, tcp.Subtype, proj.GetPublicId(), target.WithName("default"), target.WithDescription("default")) + require.NoError(t, err) + tar := ttar.(*tcp.Target) tar.DefaultPort = 2 require.NoError(t, err) gtar, _, _, err := repo.CreateTarget(context.Background(), tar) @@ -958,7 +967,9 @@ func TestUpdate_BadVersion(t *testing.T) { repo, err := repoFn() require.NoError(t, err, "Couldn't create new target repo.") - tar, err := tcp.New(proj.GetPublicId(), target.WithName("default"), target.WithDescription("default")) + ctx := context.Background() + ttar, err := target.New(ctx, tcp.Subtype, proj.GetPublicId(), target.WithName("default"), target.WithDescription("default")) + tar := ttar.(*tcp.Target) tar.DefaultPort = 2 require.NoError(t, err) gtar, _, _, err := repo.CreateTarget(context.Background(), tar) @@ -1004,40 +1015,41 @@ func TestAddTargetHostSets(t *testing.T) { pluginHs := plugin.TestSet(t, conn, kms, sche, pluginHc, map[string]plgpb.HostPluginServiceClient{ plg.GetPublicId(): plugin.NewWrappingPluginClient(&plugin.TestPluginServer{}), }) + ctx := context.Background() addCases := []struct { name string - tar *tcp.Target + tar target.Target addHostSets []string resultHostSets []string }{ { name: "Add set on empty target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "empty"), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "empty"), addHostSets: []string{hs[1].GetPublicId()}, resultHostSets: []string{hs[1].GetPublicId()}, }, { name: "Add set on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "populated", target.WithHostSources([]string{hs[0].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "populated", target.WithHostSources([]string{hs[0].GetPublicId()})), addHostSets: []string{hs[1].GetPublicId()}, resultHostSets: []string{hs[0].GetPublicId(), hs[1].GetPublicId()}, }, { name: "Add duplicated sets on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "duplicated", target.WithHostSources([]string{hs[0].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "duplicated", target.WithHostSources([]string{hs[0].GetPublicId()})), addHostSets: []string{hs[1].GetPublicId(), hs[1].GetPublicId()}, resultHostSets: []string{hs[0].GetPublicId(), hs[1].GetPublicId()}, }, { name: "Add plugin set on empty target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "plugin empty"), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "plugin empty"), addHostSets: []string{pluginHs.GetPublicId()}, resultHostSets: []string{pluginHs.GetPublicId()}, }, { name: "Add plugin set on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "plugin populated", target.WithHostSources([]string{hs[0].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "plugin populated", target.WithHostSources([]string{hs[0].GetPublicId()})), addHostSets: []string{pluginHs.GetPublicId()}, resultHostSets: []string{hs[0].GetPublicId(), pluginHs.GetPublicId()}, }, @@ -1060,7 +1072,7 @@ func TestAddTargetHostSets(t *testing.T) { }) } - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), "test") + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "test") failCases := []struct { name string @@ -1140,45 +1152,46 @@ func TestSetTargetHostSets(t *testing.T) { plg.GetPublicId(): plugin.NewWrappingPluginClient(&plugin.TestPluginServer{}), }) + ctx := context.Background() setCases := []struct { name string - tar *tcp.Target + tar target.Target setHostSets []string resultHostSets []string }{ { name: "Set on empty target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "empty"), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "empty"), setHostSets: []string{hs[1].GetPublicId()}, resultHostSets: []string{hs[1].GetPublicId()}, }, { name: "Set on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "populated", target.WithHostSources([]string{hs[0].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "populated", target.WithHostSources([]string{hs[0].GetPublicId()})), setHostSets: []string{hs[1].GetPublicId()}, resultHostSets: []string{hs[1].GetPublicId()}, }, { name: "Set duplicate host set on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "duplicate", target.WithHostSources([]string{hs[0].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "duplicate", target.WithHostSources([]string{hs[0].GetPublicId()})), setHostSets: []string{hs[1].GetPublicId(), hs[1].GetPublicId()}, resultHostSets: []string{hs[1].GetPublicId()}, }, { name: "Set empty on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "another populated", target.WithHostSources([]string{hs[0].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "another populated", target.WithHostSources([]string{hs[0].GetPublicId()})), setHostSets: []string{}, resultHostSets: nil, }, { name: "Set plugin set on empty target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "plugin empty"), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "plugin empty"), setHostSets: []string{pluginHs.GetPublicId()}, resultHostSets: []string{pluginHs.GetPublicId()}, }, { name: "Set plugin set on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "plugin populated", target.WithHostSources([]string{hs[0].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "plugin populated", target.WithHostSources([]string{hs[0].GetPublicId()})), setHostSets: []string{pluginHs.GetPublicId()}, resultHostSets: []string{pluginHs.GetPublicId()}, }, @@ -1197,7 +1210,7 @@ func TestSetTargetHostSets(t *testing.T) { }) } - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), "test name") + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "test name") failCases := []struct { name string @@ -1269,40 +1282,41 @@ func TestRemoveTargetHostSets(t *testing.T) { plg.GetPublicId(): plugin.NewWrappingPluginClient(&plugin.TestPluginServer{}), }) + ctx := context.Background() removeCases := []struct { name string - tar *tcp.Target + tar target.Target removeHosts []string resultHosts []string wantErr bool }{ { name: "Remove from empty", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "empty"), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "empty"), removeHosts: []string{hs[1].GetPublicId()}, wantErr: true, }, { name: "Remove 1 of 2 sets", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "remove partial", target.WithHostSources([]string{hs[0].GetPublicId(), hs[1].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "remove partial", target.WithHostSources([]string{hs[0].GetPublicId(), hs[1].GetPublicId()})), removeHosts: []string{hs[1].GetPublicId()}, resultHosts: []string{hs[0].GetPublicId()}, }, { name: "Remove 1 duplicate set of 2 sets", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "remove duplicate", target.WithHostSources([]string{hs[0].GetPublicId(), hs[1].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "remove duplicate", target.WithHostSources([]string{hs[0].GetPublicId(), hs[1].GetPublicId()})), removeHosts: []string{hs[1].GetPublicId(), hs[1].GetPublicId()}, resultHosts: []string{hs[0].GetPublicId()}, }, { name: "Remove all hosts from set", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "remove all", target.WithHostSources([]string{hs[0].GetPublicId(), hs[1].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "remove all", target.WithHostSources([]string{hs[0].GetPublicId(), hs[1].GetPublicId()})), removeHosts: []string{hs[0].GetPublicId(), hs[1].GetPublicId()}, resultHosts: []string{}, }, { name: "Remove 1 plugin of 2 sets", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "remove plugin partial", target.WithHostSources([]string{hs[0].GetPublicId(), pluginHs.GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "remove plugin partial", target.WithHostSources([]string{hs[0].GetPublicId(), pluginHs.GetPublicId()})), removeHosts: []string{pluginHs.GetPublicId()}, resultHosts: []string{hs[0].GetPublicId()}, }, @@ -1329,7 +1343,7 @@ func TestRemoveTargetHostSets(t *testing.T) { }) } - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), "testing") + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "testing") failCases := []struct { name string @@ -1410,39 +1424,40 @@ func TestAddTargetHostSources(t *testing.T) { plg.GetPublicId(): plugin.NewWrappingPluginClient(&plugin.TestPluginServer{}), }) + ctx := context.Background() addCases := []struct { name string - tar *tcp.Target + tar target.Target addHostSources []string resultHostSources []string }{ { name: "Add set on empty target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "empty"), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "empty"), addHostSources: []string{hs[1].GetPublicId()}, resultHostSources: []string{hs[1].GetPublicId()}, }, { name: "Add set on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "populated", target.WithHostSources([]string{hs[0].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "populated", target.WithHostSources([]string{hs[0].GetPublicId()})), addHostSources: []string{hs[1].GetPublicId()}, resultHostSources: []string{hs[0].GetPublicId(), hs[1].GetPublicId()}, }, { name: "Add duplicated sets on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "duplicated", target.WithHostSources([]string{hs[0].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "duplicated", target.WithHostSources([]string{hs[0].GetPublicId()})), addHostSources: []string{hs[1].GetPublicId(), hs[1].GetPublicId()}, resultHostSources: []string{hs[0].GetPublicId(), hs[1].GetPublicId()}, }, { name: "Add plugin set on empty target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "plugin empty"), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "plugin empty"), addHostSources: []string{pluginHs.GetPublicId()}, resultHostSources: []string{pluginHs.GetPublicId()}, }, { name: "Add plugin set on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "plugin populated", target.WithHostSources([]string{hs[0].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "plugin populated", target.WithHostSources([]string{hs[0].GetPublicId()})), addHostSources: []string{pluginHs.GetPublicId()}, resultHostSources: []string{hs[0].GetPublicId(), pluginHs.GetPublicId()}, }, @@ -1465,7 +1480,7 @@ func TestAddTargetHostSources(t *testing.T) { }) } - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), "test") + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "test") failCases := []struct { name string @@ -1545,39 +1560,40 @@ func TestSetTargetHostSources(t *testing.T) { plg.GetPublicId(): plugin.NewWrappingPluginClient(&plugin.TestPluginServer{}), }) + ctx := context.Background() setCases := []struct { name string - tar *tcp.Target + tar target.Target setHostSources []string resultHostSources []string }{ { name: "Set on empty target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "empty"), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "empty"), setHostSources: []string{hs[1].GetPublicId()}, resultHostSources: []string{hs[1].GetPublicId()}, }, { name: "Set on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "populated", target.WithHostSources([]string{hs[0].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "populated", target.WithHostSources([]string{hs[0].GetPublicId()})), setHostSources: []string{hs[1].GetPublicId()}, resultHostSources: []string{hs[1].GetPublicId()}, }, { name: "Set plugin set on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "plugin populated", target.WithHostSources([]string{hs[0].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "plugin populated", target.WithHostSources([]string{hs[0].GetPublicId()})), setHostSources: []string{pluginHs.GetPublicId()}, resultHostSources: []string{pluginHs.GetPublicId()}, }, { name: "Set duplicate host set on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "duplicate", target.WithHostSources([]string{hs[0].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "duplicate", target.WithHostSources([]string{hs[0].GetPublicId()})), setHostSources: []string{hs[1].GetPublicId(), hs[1].GetPublicId()}, resultHostSources: []string{hs[1].GetPublicId()}, }, { name: "Set empty on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "another populated", target.WithHostSources([]string{hs[0].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "another populated", target.WithHostSources([]string{hs[0].GetPublicId()})), setHostSources: []string{}, resultHostSources: nil, }, @@ -1596,7 +1612,7 @@ func TestSetTargetHostSources(t *testing.T) { }) } - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), "test name") + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "test name") failCases := []struct { name string @@ -1668,40 +1684,41 @@ func TestRemoveTargetHostSources(t *testing.T) { plg.GetPublicId(): plugin.NewWrappingPluginClient(&plugin.TestPluginServer{}), }) + ctx := context.Background() removeCases := []struct { name string - tar *tcp.Target + tar target.Target removeHostSources []string resultHostSources []string wantErr bool }{ { name: "Remove from empty", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "empty"), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "empty"), removeHostSources: []string{hs[1].GetPublicId()}, wantErr: true, }, { name: "Remove 1 of 2 sets", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "remove partial", target.WithHostSources([]string{hs[0].GetPublicId(), hs[1].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "remove partial", target.WithHostSources([]string{hs[0].GetPublicId(), hs[1].GetPublicId()})), removeHostSources: []string{hs[1].GetPublicId()}, resultHostSources: []string{hs[0].GetPublicId()}, }, { name: "Remove 1 plugin set of 2 sets", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "remove plugin partial", target.WithHostSources([]string{hs[0].GetPublicId(), pluginHs.GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "remove plugin partial", target.WithHostSources([]string{hs[0].GetPublicId(), pluginHs.GetPublicId()})), removeHostSources: []string{pluginHs.GetPublicId()}, resultHostSources: []string{hs[0].GetPublicId()}, }, { name: "Remove 1 duplicate set of 2 sets", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "remove duplicate", target.WithHostSources([]string{hs[0].GetPublicId(), hs[1].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "remove duplicate", target.WithHostSources([]string{hs[0].GetPublicId(), hs[1].GetPublicId()})), removeHostSources: []string{hs[1].GetPublicId(), hs[1].GetPublicId()}, resultHostSources: []string{hs[0].GetPublicId()}, }, { name: "Remove all hosts from set", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "remove all", target.WithHostSources([]string{hs[0].GetPublicId(), hs[1].GetPublicId()})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "remove all", target.WithHostSources([]string{hs[0].GetPublicId(), hs[1].GetPublicId()})), removeHostSources: []string{hs[0].GetPublicId(), hs[1].GetPublicId()}, resultHostSources: []string{}, }, @@ -1728,7 +1745,7 @@ func TestRemoveTargetHostSources(t *testing.T) { }) } - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), "testing") + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "testing") failCases := []struct { name string @@ -1802,27 +1819,28 @@ func TestAddTargetLibraries(t *testing.T) { store := vault.TestCredentialStores(t, conn, wrapper, proj.GetPublicId(), 1)[0] cls := vault.TestCredentialLibraries(t, conn, wrapper, store.GetPublicId(), 2) + ctx := context.Background() addCases := []struct { name string - tar *tcp.Target + tar target.Target addLibraries []string resultLibraryIds []string }{ { name: "Add library on empty target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "empty for libraries"), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "empty for libraries"), addLibraries: []string{cls[1].GetPublicId()}, resultLibraryIds: []string{cls[1].GetPublicId()}, }, { name: "Add library on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "populated for libraries", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "populated for libraries", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), addLibraries: []string{cls[1].GetPublicId()}, resultLibraryIds: []string{cls[0].GetPublicId(), cls[1].GetPublicId()}, }, { name: "Add duplicated libraries on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "duplicated for libraries", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "duplicated for libraries", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), addLibraries: []string{cls[1].GetPublicId(), cls[1].GetPublicId()}, resultLibraryIds: []string{cls[0].GetPublicId(), cls[1].GetPublicId()}, }, @@ -1853,7 +1871,7 @@ func TestAddTargetLibraries(t *testing.T) { }) } - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), "test") + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "test") failCases := []struct { name string @@ -1937,34 +1955,35 @@ func TestSetTargetLibraries(t *testing.T) { } } + ctx := context.Background() setCases := []struct { name string - tar *tcp.Target + tar target.Target setLibraries []string resultLibraryIds []string resultLibraries []*pb.CredentialLibrary }{ { name: "Set on empty target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "empty"), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "empty"), setLibraries: []string{cls[1].GetPublicId()}, resultLibraryIds: []string{cls[1].GetPublicId()}, }, { name: "Set on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "populated", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "populated", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), setLibraries: []string{cls[1].GetPublicId()}, resultLibraryIds: []string{cls[1].GetPublicId()}, }, { name: "Set duplicate libraries on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "duplicate", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "duplicate", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), setLibraries: []string{cls[1].GetPublicId(), cls[1].GetPublicId()}, resultLibraryIds: []string{cls[1].GetPublicId()}, }, { name: "Set empty on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "another populated", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "another populated", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), setLibraries: []string{}, resultLibraryIds: nil, }, @@ -1998,7 +2017,7 @@ func TestSetTargetLibraries(t *testing.T) { }) } - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), "test name") + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "test name") failCases := []struct { name string @@ -2063,28 +2082,29 @@ func TestRemoveTargetLibraries(t *testing.T) { store := vault.TestCredentialStores(t, conn, wrapper, proj.GetPublicId(), 1)[0] cls := vault.TestCredentialLibraries(t, conn, wrapper, store.GetPublicId(), 2) + ctx := context.Background() removeCases := []struct { name string - tar *tcp.Target + tar target.Target removeLibs []string resultLibIds []string wantErr bool }{ { name: "Remove from empty", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "empty"), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "empty"), removeLibs: []string{cls[1].GetPublicId()}, wantErr: true, }, { name: "Remove 1 of 2 libraries", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "remove partial", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose), target.TestNewCredentialLibrary("", cls[1].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "remove partial", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose), target.TestNewCredentialLibrary("", cls[1].GetPublicId(), credential.ApplicationPurpose)})), removeLibs: []string{cls[1].GetPublicId()}, resultLibIds: []string{cls[0].GetPublicId()}, }, { name: "Remove 1 duplicate set of 2 libraries", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "remove duplicate", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose), target.TestNewCredentialLibrary("", cls[1].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "remove duplicate", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose), target.TestNewCredentialLibrary("", cls[1].GetPublicId(), credential.ApplicationPurpose)})), removeLibs: []string{ cls[1].GetPublicId(), cls[1].GetPublicId(), }, @@ -2092,7 +2112,7 @@ func TestRemoveTargetLibraries(t *testing.T) { }, { name: "Remove all libraries from target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "remove all", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose), target.TestNewCredentialLibrary("", cls[1].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "remove all", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose), target.TestNewCredentialLibrary("", cls[1].GetPublicId(), credential.ApplicationPurpose)})), removeLibs: []string{ cls[0].GetPublicId(), cls[1].GetPublicId(), }, @@ -2119,7 +2139,7 @@ func TestRemoveTargetLibraries(t *testing.T) { }) } - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), "testing") + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "testing") failCases := []struct { name string @@ -2199,27 +2219,28 @@ func TestAddTargetSources(t *testing.T) { store := vault.TestCredentialStores(t, conn, wrapper, proj.GetPublicId(), 1)[0] cls := vault.TestCredentialLibraries(t, conn, wrapper, store.GetPublicId(), 2) + ctx := context.Background() addCases := []struct { name string - tar *tcp.Target + tar target.Target addSources []string resultSourceIds []string }{ { name: "Add source on empty target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "empty for sources"), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "empty for sources"), addSources: []string{cls[1].GetPublicId()}, resultSourceIds: []string{cls[1].GetPublicId()}, }, { name: "Add source on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "populated for sources", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "populated for sources", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), addSources: []string{cls[1].GetPublicId()}, resultSourceIds: []string{cls[0].GetPublicId(), cls[1].GetPublicId()}, }, { name: "Add duplicated sources on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "duplicated for sources", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "duplicated for sources", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), addSources: []string{cls[1].GetPublicId(), cls[1].GetPublicId()}, resultSourceIds: []string{cls[0].GetPublicId(), cls[1].GetPublicId()}, }, @@ -2250,7 +2271,7 @@ func TestAddTargetSources(t *testing.T) { }) } - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), "test") + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "test") failCases := []struct { name string @@ -2334,34 +2355,35 @@ func TestSetTargetCredentialSources(t *testing.T) { } } + ctx := context.Background() setCases := []struct { name string - tar *tcp.Target + tar target.Target setCredentialSources []string resultCredentialSourceIds []string resultCredentialSources []*pb.CredentialLibrary }{ { name: "Set on empty target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "empty"), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "empty"), setCredentialSources: []string{cls[1].GetPublicId()}, resultCredentialSourceIds: []string{cls[1].GetPublicId()}, }, { name: "Set on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "populated", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "populated", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), setCredentialSources: []string{cls[1].GetPublicId()}, resultCredentialSourceIds: []string{cls[1].GetPublicId()}, }, { name: "Set duplicate sources on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "duplicate", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "duplicate", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), setCredentialSources: []string{cls[1].GetPublicId(), cls[1].GetPublicId()}, resultCredentialSourceIds: []string{cls[1].GetPublicId()}, }, { name: "Set empty on populated target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "another populated", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "another populated", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose)})), setCredentialSources: []string{}, resultCredentialSourceIds: nil, }, @@ -2395,7 +2417,7 @@ func TestSetTargetCredentialSources(t *testing.T) { }) } - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), "test name") + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "test name") failCases := []struct { name string @@ -2460,28 +2482,29 @@ func TestRemoveTargetCredentialSources(t *testing.T) { store := vault.TestCredentialStores(t, conn, wrapper, proj.GetPublicId(), 1)[0] cls := vault.TestCredentialLibraries(t, conn, wrapper, store.GetPublicId(), 2) + ctx := context.Background() removeCases := []struct { name string - tar *tcp.Target + tar target.Target removeCredentialSources []string resultCredentialSourceIds []string wantErr bool }{ { name: "Remove from empty", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "empty"), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "empty"), removeCredentialSources: []string{cls[1].GetPublicId()}, wantErr: true, }, { name: "Remove 1 of 2 sources", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "remove partial", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose), target.TestNewCredentialLibrary("", cls[1].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "remove partial", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose), target.TestNewCredentialLibrary("", cls[1].GetPublicId(), credential.ApplicationPurpose)})), removeCredentialSources: []string{cls[1].GetPublicId()}, resultCredentialSourceIds: []string{cls[0].GetPublicId()}, }, { name: "Remove 1 duplicate set of 2 sources", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "remove duplicate", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose), target.TestNewCredentialLibrary("", cls[1].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "remove duplicate", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose), target.TestNewCredentialLibrary("", cls[1].GetPublicId(), credential.ApplicationPurpose)})), removeCredentialSources: []string{ cls[1].GetPublicId(), cls[1].GetPublicId(), }, @@ -2489,7 +2512,7 @@ func TestRemoveTargetCredentialSources(t *testing.T) { }, { name: "Remove all sources from target", - tar: tcp.TestTarget(t, conn, proj.GetPublicId(), "remove all", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose), target.TestNewCredentialLibrary("", cls[1].GetPublicId(), credential.ApplicationPurpose)})), + tar: tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "remove all", target.WithCredentialLibraries([]*target.CredentialLibrary{target.TestNewCredentialLibrary("", cls[0].GetPublicId(), credential.ApplicationPurpose), target.TestNewCredentialLibrary("", cls[1].GetPublicId(), credential.ApplicationPurpose)})), removeCredentialSources: []string{ cls[0].GetPublicId(), cls[1].GetPublicId(), }, @@ -2516,7 +2539,7 @@ func TestRemoveTargetCredentialSources(t *testing.T) { }) } - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), "testing") + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), "testing") failCases := []struct { name string @@ -2714,7 +2737,7 @@ func TestAuthorizeSession(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), tc.name) + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), tc.name) apiTar, err := s.AddTargetHostSets(ctx, &pbs.AddTargetHostSetsRequest{ Id: tar.GetPublicId(), Version: tar.GetVersion(), @@ -2878,7 +2901,7 @@ func TestAuthorizeSession_Errors(t *testing.T) { sec, tok := v.CreateToken(t, vault.WithPolicies([]string{"default", "database"})) store := vault.TestCredentialStore(t, conn, wrapper, proj.GetPublicId(), v.Addr, tok, sec.Auth.Accessor) - workerExists := func(tar *tcp.Target) (version uint32) { + workerExists := func(tar target.Target) (version uint32) { workerService := workers.NewWorkerServiceServer(serversRepoFn, sessionRepoFn, &sync.Map{}, kms) _, err := workerService.Status(context.Background(), &spbs.StatusRequest{ Worker: &spb.Server{ @@ -2890,7 +2913,7 @@ func TestAuthorizeSession_Errors(t *testing.T) { return tar.GetVersion() } - hostSetNoHostExists := func(tar *tcp.Target) (version uint32) { + hostSetNoHostExists := func(tar target.Target) (version uint32) { hc := static.TestCatalogs(t, conn, proj.GetPublicId(), 1)[0] hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] @@ -2903,7 +2926,7 @@ func TestAuthorizeSession_Errors(t *testing.T) { return tr.GetItem().GetVersion() } - hostExists := func(tar *tcp.Target) (version uint32) { + hostExists := func(tar target.Target) (version uint32) { hc := static.TestCatalogs(t, conn, proj.GetPublicId(), 1)[0] h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0] hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0] @@ -2917,7 +2940,7 @@ func TestAuthorizeSession_Errors(t *testing.T) { return apiTar.GetItem().GetVersion() } - libraryExists := func(tar *tcp.Target) (version uint32) { + libraryExists := func(tar target.Target) (version uint32) { credService, err := credentiallibraries.NewService(credentialRepoFn, iamRepoFn) require.NoError(t, err) clsResp, err := credService.CreateCredentialLibrary(ctx, &pbs.CreateCredentialLibraryRequest{Item: &credpb.CredentialLibrary{ @@ -2939,7 +2962,7 @@ func TestAuthorizeSession_Errors(t *testing.T) { return tr.GetItem().GetVersion() } - misConfiguredlibraryExists := func(tar *tcp.Target) (version uint32) { + misConfiguredlibraryExists := func(tar target.Target) (version uint32) { credService, err := credentiallibraries.NewService(credentialRepoFn, iamRepoFn) require.NoError(t, err) clsResp, err := credService.CreateCredentialLibrary(ctx, &pbs.CreateCredentialLibraryRequest{Item: &credpb.CredentialLibrary{ @@ -2963,37 +2986,37 @@ func TestAuthorizeSession_Errors(t *testing.T) { cases := []struct { name string - setup []func(*tcp.Target) uint32 + setup []func(target.Target) uint32 err bool }{ { // This one must be run first since it relies on the DB not having any worker details name: "no worker", - setup: []func(tcpTarget *tcp.Target) uint32{hostExists, libraryExists}, + setup: []func(tcpTarget target.Target) uint32{hostExists, libraryExists}, err: true, }, { name: "success", - setup: []func(tcpTarget *tcp.Target) uint32{workerExists, hostExists, libraryExists}, + setup: []func(tcpTarget target.Target) uint32{workerExists, hostExists, libraryExists}, }, { name: "no hosts", - setup: []func(tcpTarget *tcp.Target) uint32{workerExists, hostSetNoHostExists, libraryExists}, + setup: []func(tcpTarget target.Target) uint32{workerExists, hostSetNoHostExists, libraryExists}, err: true, }, { name: "bad library configuration", - setup: []func(tcpTarget *tcp.Target) uint32{workerExists, hostExists, misConfiguredlibraryExists}, + setup: []func(tcpTarget target.Target) uint32{workerExists, hostExists, misConfiguredlibraryExists}, err: true, }, } for i, tc := range cases { t.Run(tc.name, func(t *testing.T) { - tar := tcp.TestTarget(t, conn, proj.GetPublicId(), fmt.Sprintf("test-%d", i)) + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), fmt.Sprintf("test-%d", i)) for _, fn := range tc.setup { ver := fn(tar) - tar.Version = ver + tar.SetVersion(ver) } res, err := s.AuthorizeSession(ctx, &pbs.AuthorizeSessionRequest{ diff --git a/internal/servers/controller/handlers/targets/tcp/tcp.go b/internal/servers/controller/handlers/targets/tcp/tcp.go new file mode 100644 index 000000000..1fca85bbc --- /dev/null +++ b/internal/servers/controller/handlers/targets/tcp/tcp.go @@ -0,0 +1,57 @@ +package tcp + +import ( + "github.com/golang/protobuf/ptypes/wrappers" + "github.com/hashicorp/boundary/internal/servers/controller/handlers" + "github.com/hashicorp/boundary/internal/servers/controller/handlers/targets" + "github.com/hashicorp/boundary/internal/target" + "github.com/hashicorp/boundary/internal/target/tcp" + "github.com/hashicorp/boundary/internal/target/tcp/store" + pb "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/targets" +) + +type attribute struct { + *pb.TcpTargetAttributes +} + +func (a *attribute) Options() []target.Option { + var opts []target.Option + if a.GetDefaultPort().GetValue() != 0 { + opts = append(opts, target.WithDefaultPort(a.GetDefaultPort().GetValue())) + } + return opts +} + +func (a *attribute) Vet() map[string]string { + badFields := map[string]string{} + if a.GetDefaultPort() != nil && a.GetDefaultPort().GetValue() == 0 { + badFields["attributes.default_port"] = "This optional field cannot be set to 0." + } + return badFields +} + +func newAttribute(t target.Target) targets.Attributes { + a := &attribute{ + &pb.TcpTargetAttributes{}, + } + if t != nil { + if t.GetDefaultPort() > 0 { + a.DefaultPort = &wrappers.UInt32Value{Value: t.GetDefaultPort()} + } + } + return a +} + +func init() { + var maskManager handlers.MaskManager + var err error + + if maskManager, err = handlers.NewMaskManager( + handlers.MaskDestination{&store.Target{}}, + handlers.MaskSource{&pb.Target{}, &pb.TcpTargetAttributes{}}, + ); err != nil { + panic(err) + } + + targets.Register(tcp.Subtype, maskManager, newAttribute) +} diff --git a/internal/session/testing.go b/internal/session/testing.go index 7a96e2778..ac23b25f2 100644 --- a/internal/session/testing.go +++ b/internal/session/testing.go @@ -133,7 +133,7 @@ func TestSessionParams(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, iamR sets := static.TestSets(t, conn, cats[0].PublicId, 1) _ = static.TestSetMembers(t, conn, sets[0].PublicId, hosts) - tcpTarget := tcp.TestTarget(t, conn, proj.PublicId, "test target") + tcpTarget := tcp.TestTarget(ctx, t, conn, proj.PublicId, "test target") kms := kms.TestKms(t, conn, wrapper) targetRepo, err := target.NewRepository(rw, rw, kms) @@ -155,10 +155,10 @@ func TestSessionParams(t *testing.T, conn *db.DB, wrapper wrapping.Wrapper, iamR return ComposedOf{ UserId: user.PublicId, HostId: hosts[0].PublicId, - TargetId: tcpTarget.PublicId, + TargetId: tcpTarget.GetPublicId(), HostSetId: sets[0].PublicId, AuthTokenId: at.PublicId, - ScopeId: tcpTarget.ScopeId, + ScopeId: tcpTarget.GetScopeId(), Endpoint: "tcp://127.0.0.1:22", ExpirationTime: ×tamp.Timestamp{Timestamp: expTime}, ConnectionLimit: tcpTarget.GetSessionConnectionLimit(), diff --git a/internal/tests/api/targets/target_test.go b/internal/tests/api/targets/target_test.go index a5d901228..bcad5e1fa 100644 --- a/internal/tests/api/targets/target_test.go +++ b/internal/tests/api/targets/target_test.go @@ -17,6 +17,8 @@ import ( "github.com/hashicorp/boundary/internal/target/tcp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + _ "github.com/hashicorp/boundary/internal/servers/controller/handlers/targets/tcp" ) func TestHostSetASD(t *testing.T) { diff --git a/internal/tests/cluster/package_registry_test.go b/internal/tests/cluster/package_registry_test.go new file mode 100644 index 000000000..4ca900c60 --- /dev/null +++ b/internal/tests/cluster/package_registry_test.go @@ -0,0 +1,7 @@ +package cluster + +import ( + // Enable tcp target support. + _ "github.com/hashicorp/boundary/internal/servers/controller/handlers/targets/tcp" + _ "github.com/hashicorp/boundary/internal/target/tcp" +) From 0cfd8279b0699ac07496f2d736b6bce1d19fbae5 Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Thu, 2 Dec 2021 16:32:19 -0500 Subject: [PATCH 4/4] fix(controller/targets): Fix creation of credential libraries The arg order was incorrect for ingress/egress credentials. Ref: 2cc58ff9014deb3cf48d1d2fe158467978ff2a0e Blame: 36f1ae75f714efc3091e28714cb18e9a9148f298 --- .../servers/controller/handlers/targets/target_service.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/servers/controller/handlers/targets/target_service.go b/internal/servers/controller/handlers/targets/target_service.go index 2d9da18b5..95f7a196e 100644 --- a/internal/servers/controller/handlers/targets/target_service.go +++ b/internal/servers/controller/handlers/targets/target_service.go @@ -1351,7 +1351,7 @@ func (s Service) addCredentialSourcesInRepo(ctx context.Context, targetId string if err != nil { return nil, nil, nil, err } - credLibs, err := createCredLibs(targetId, applicationIds, egressIds, nil) + credLibs, err := createCredLibs(targetId, applicationIds, nil, egressIds) if err != nil { return nil, nil, nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to set credential sources in target: %v.", err) } @@ -1374,7 +1374,7 @@ func (s Service) setCredentialSourcesInRepo(ctx context.Context, targetId string return nil, nil, nil, err } - credLibs, err := createCredLibs(targetId, applicationIds, egressIds, nil) + credLibs, err := createCredLibs(targetId, applicationIds, nil, egressIds) if err != nil { return nil, nil, nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to set credential sources in target: %v.", err) } @@ -1402,7 +1402,7 @@ func (s Service) removeCredentialSourcesInRepo(ctx context.Context, targetId str return nil, nil, nil, err } - credLibs, err := createCredLibs(targetId, applicationIds, egressIds, nil) + credLibs, err := createCredLibs(targetId, applicationIds, nil, egressIds) if err != nil { return nil, nil, nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to set credential sources in target: %v.", err) }