From c9b99306e17f89352ef301996ddcdbe559d43cd0 Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Thu, 4 Nov 2021 11:37:32 -0400 Subject: [PATCH] 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 {