From e18cdc52b3c0f38b8df1bdb7ec80f2eef43ebdfc Mon Sep 17 00:00:00 2001 From: Michael Gaffney Date: Mon, 17 Aug 2020 17:06:31 -0400 Subject: [PATCH] Create host (#276) * Change testHostCatalog methods to be like other resource test methods * Implement CreateHost * Make allocHost return a pointer to a Host --- internal/host/static/errors.go | 9 + internal/host/static/host.go | 27 +++ internal/host/static/host_catalog_test.go | 9 +- internal/host/static/host_set_member_test.go | 4 +- internal/host/static/host_set_test.go | 4 +- internal/host/static/host_test.go | 5 +- internal/host/static/immutable_fields_test.go | 12 +- internal/host/static/repository_host.go | 61 +++++ .../static/repository_host_catalog_test.go | 13 +- internal/host/static/repository_host_test.go | 223 ++++++++++++++++++ 10 files changed, 351 insertions(+), 16 deletions(-) create mode 100644 internal/host/static/errors.go create mode 100644 internal/host/static/repository_host.go create mode 100644 internal/host/static/repository_host_test.go diff --git a/internal/host/static/errors.go b/internal/host/static/errors.go new file mode 100644 index 0000000000..3c1c6ca0d2 --- /dev/null +++ b/internal/host/static/errors.go @@ -0,0 +1,9 @@ +package static + +import "errors" + +var ( + // ErrInvalidAddress results from attempting to perform an operation + // that sets an address on a host to an invalid value. + ErrInvalidAddress = errors.New("invalid address") +) diff --git a/internal/host/static/host.go b/internal/host/static/host.go index 10f93b483b..8f445a62bb 100644 --- a/internal/host/static/host.go +++ b/internal/host/static/host.go @@ -5,6 +5,8 @@ import ( "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/host/static/store" + "github.com/hashicorp/boundary/internal/oplog" + "google.golang.org/protobuf/proto" ) // A Host contains a static address. @@ -49,3 +51,28 @@ func (h *Host) TableName() string { func (h *Host) SetTableName(n string) { h.tableName = n } + +func allocHost() *Host { + return &Host{ + Host: &store.Host{}, + } +} + +func (h *Host) clone() *Host { + cp := proto.Clone(h.Host) + return &Host{ + Host: cp.(*store.Host), + } +} + +func (h *Host) oplog(op oplog.OpType) oplog.Metadata { + metadata := oplog.Metadata{ + "resource-public-id": []string{h.PublicId}, + "resource-type": []string{"static-host"}, + "op-type": []string{op.String()}, + } + if h.CatalogId != "" { + metadata["catalog-id"] = []string{h.CatalogId} + } + return metadata +} diff --git a/internal/host/static/host_catalog_test.go b/internal/host/static/host_catalog_test.go index 24a89c3c97..ec0f7211bd 100644 --- a/internal/host/static/host_catalog_test.go +++ b/internal/host/static/host_catalog_test.go @@ -107,13 +107,12 @@ func TestHostCatalog_New(t *testing.T) { } } -func testCatalogs(t *testing.T, conn *gorm.DB, count int) []*HostCatalog { +func testCatalogs(t *testing.T, conn *gorm.DB, scopeId string, count int) []*HostCatalog { t.Helper() assert := assert.New(t) - _, prj := iam.TestScopes(t, conn) var cats []*HostCatalog for i := 0; i < count; i++ { - cat, err := NewHostCatalog(prj.GetPublicId()) + cat, err := NewHostCatalog(scopeId) assert.NoError(err) assert.NotNil(cat) id, err := newHostCatalogId() @@ -129,9 +128,9 @@ func testCatalogs(t *testing.T, conn *gorm.DB, count int) []*HostCatalog { return cats } -func testCatalog(t *testing.T, conn *gorm.DB) *HostCatalog { +func testCatalog(t *testing.T, conn *gorm.DB, scopeId string) *HostCatalog { t.Helper() - cats := testCatalogs(t, conn, 1) + cats := testCatalogs(t, conn, scopeId, 1) return cats[0] } diff --git a/internal/host/static/host_set_member_test.go b/internal/host/static/host_set_member_test.go index 19a50cbdc4..59ad0ebb30 100644 --- a/internal/host/static/host_set_member_test.go +++ b/internal/host/static/host_set_member_test.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/host/static/store" + "github.com/hashicorp/boundary/internal/iam" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -14,7 +15,8 @@ func TestHostSetMember_New(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") conn.LogMode(false) - cats := testCatalogs(t, conn, 2) + _, prj := iam.TestScopes(t, conn) + cats := testCatalogs(t, conn, prj.PublicId, 2) blueCat := cats[0] blueSets := testSets(t, conn, blueCat.GetPublicId(), 2) diff --git a/internal/host/static/host_set_test.go b/internal/host/static/host_set_test.go index b42e6cb139..8e0ab17696 100644 --- a/internal/host/static/host_set_test.go +++ b/internal/host/static/host_set_test.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/host/static/store" + "github.com/hashicorp/boundary/internal/iam" "github.com/jinzhu/gorm" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -13,7 +14,8 @@ import ( func TestHostSet_New(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") - cat := testCatalog(t, conn) + _, prj := iam.TestScopes(t, conn) + cat := testCatalog(t, conn, prj.PublicId) conn.LogMode(false) type args struct { diff --git a/internal/host/static/host_test.go b/internal/host/static/host_test.go index 869e737ca1..af976cdffd 100644 --- a/internal/host/static/host_test.go +++ b/internal/host/static/host_test.go @@ -7,13 +7,16 @@ import ( "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/host/static/store" + "github.com/hashicorp/boundary/internal/iam" "github.com/jinzhu/gorm" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) + func TestHost_New(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") - cat := testCatalog(t, conn) + _, prj := iam.TestScopes(t, conn) + cat := testCatalog(t, conn, prj.PublicId) conn.LogMode(false) type args struct { diff --git a/internal/host/static/immutable_fields_test.go b/internal/host/static/immutable_fields_test.go index fd031329c5..7d182d5391 100644 --- a/internal/host/static/immutable_fields_test.go +++ b/internal/host/static/immutable_fields_test.go @@ -21,7 +21,8 @@ func TestHostCatalog_ImmutableFields(t *testing.T) { _, proj := iam.TestScopes(t, conn) ts := timestamp.Timestamp{Timestamp: ×tamppb.Timestamp{Seconds: 0, Nanos: 0}} - new := testCatalog(t, conn) + _, prj := iam.TestScopes(t, conn) + new := testCatalog(t, conn, prj.PublicId) var tests = []struct { name string @@ -84,7 +85,8 @@ func TestStaticHost_ImmutableFields(t *testing.T) { w := db.New(conn) ts := timestamp.Timestamp{Timestamp: ×tamppb.Timestamp{Seconds: 0, Nanos: 0}} - cat := testCatalog(t, conn) + _, prj := iam.TestScopes(t, conn) + cat := testCatalog(t, conn, prj.PublicId) hosts := testHosts(t, conn, cat.GetPublicId(), 1) new := hosts[0] @@ -157,7 +159,8 @@ func TestStaticHostSet_ImmutableFields(t *testing.T) { w := db.New(conn) ts := timestamp.Timestamp{Timestamp: ×tamppb.Timestamp{Seconds: 0, Nanos: 0}} - cat := testCatalog(t, conn) + _, prj := iam.TestScopes(t, conn) + cat := testCatalog(t, conn, prj.PublicId) sets := testSets(t, conn, cat.GetPublicId(), 1) new := sets[0] @@ -229,7 +232,8 @@ func TestStaticHostSetMember_ImmutableFields(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") w := db.New(conn) - cat := testCatalog(t, conn) + _, prj := iam.TestScopes(t, conn) + cat := testCatalog(t, conn, prj.PublicId) sets := testSets(t, conn, cat.GetPublicId(), 1) hosts := testHosts(t, conn, cat.GetPublicId(), 1) diff --git a/internal/host/static/repository_host.go b/internal/host/static/repository_host.go new file mode 100644 index 0000000000..70c8434a6e --- /dev/null +++ b/internal/host/static/repository_host.go @@ -0,0 +1,61 @@ +package static + +import ( + "context" + "fmt" + + "github.com/hashicorp/boundary/internal/db" + "github.com/hashicorp/boundary/internal/oplog" +) + +// CreateHost inserts h into the repository and returns a new Host +// containing the host's PublicId. h is not changed. h must contain a valid +// CatalogId. h must not contain a PublicId. The PublicId is generated and +// assigned by this method. opt is ignored. +// +// h must contain a valid Address. +// +// Both h.Name and h.Description are optional. If h.Name is set, it must be +// unique within h.CatalogId. +func (r *Repository) CreateHost(ctx context.Context, h *Host, opt ...Option) (*Host, error) { + if h == nil { + return nil, fmt.Errorf("create: static host: %w", db.ErrNilParameter) + } + if h.Host == nil { + return nil, fmt.Errorf("create: static host: embedded Host: %w", db.ErrNilParameter) + } + if h.CatalogId == "" { + return nil, fmt.Errorf("create: static host: no catalog id: %w", db.ErrInvalidParameter) + } + if h.PublicId != "" { + return nil, fmt.Errorf("create: static host: public id not empty: %w", db.ErrInvalidParameter) + } + h = h.clone() + + id, err := newHostId() + if err != nil { + return nil, fmt.Errorf("create: static host: %w", err) + } + h.PublicId = id + + var newHost *Host + _, err = r.writer.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, + func(_ db.Reader, w db.Writer) error { + newHost = h.clone() + return w.Create(ctx, newHost, db.WithOplog(r.wrapper, h.oplog(oplog.OpType_OP_TYPE_CREATE))) + }, + ) + + if err != nil { + if db.IsUniqueError(err) { + return nil, fmt.Errorf("create: static host: in catalog: %s: name %s already exists: %w", + h.CatalogId, h.Name, db.ErrNotUnique) + } + if db.IsCheckConstraintError(err) { + return nil, fmt.Errorf("create: static host: in catalog: %s: %q: %w", + h.CatalogId, h.Address, ErrInvalidAddress) + } + return nil, fmt.Errorf("create: static host: in catalog: %s: %w", h.CatalogId, err) + } + return newHost, nil +} diff --git a/internal/host/static/repository_host_catalog_test.go b/internal/host/static/repository_host_catalog_test.go index f3eebe6324..0ed4ec6329 100644 --- a/internal/host/static/repository_host_catalog_test.go +++ b/internal/host/static/repository_host_catalog_test.go @@ -479,7 +479,8 @@ func TestRepository_UpdateCatalog(t *testing.T) { assert.NotNil(repo) name := "test-dup-name" - cats := testCatalogs(t, conn, 2) + _, prj := iam.TestScopes(t, conn) + cats := testCatalogs(t, conn, prj.PublicId, 2) c1 := cats[0] c1.Name = name got1, gotCount1, err := repo.UpdateCatalog(context.Background(), c1, 1, []string{"name"}) @@ -540,7 +541,9 @@ func TestRepository_UpdateCatalog(t *testing.T) { assert.NoError(err) assert.NotNil(repo) - c1, c2 := testCatalog(t, conn), testCatalog(t, conn) + _, prj1 := iam.TestScopes(t, conn) + _, prj2 := iam.TestScopes(t, conn) + c1, c2 := testCatalog(t, conn, prj1.PublicId), testCatalog(t, conn, prj2.PublicId) assert.NotEqual(c1.ScopeId, c2.ScopeId) orig := c1.clone() @@ -558,7 +561,8 @@ func TestRepository_UpdateCatalog(t *testing.T) { func TestRepository_LookupCatalog(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") - cat := testCatalog(t, conn) + _, prj := iam.TestScopes(t, conn) + cat := testCatalog(t, conn, prj.PublicId) badId, err := newHostCatalogId() assert.NoError(t, err) assert.NotNil(t, badId) @@ -618,7 +622,8 @@ func TestRepository_LookupCatalog(t *testing.T) { func TestRepository_DeleteCatalog(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") - cat := testCatalog(t, conn) + _, prj := iam.TestScopes(t, conn) + cat := testCatalog(t, conn, prj.PublicId) badId, err := newHostCatalogId() assert.NoError(t, err) assert.NotNil(t, badId) diff --git a/internal/host/static/repository_host_test.go b/internal/host/static/repository_host_test.go new file mode 100644 index 0000000000..8cc116f2e1 --- /dev/null +++ b/internal/host/static/repository_host_test.go @@ -0,0 +1,223 @@ +package static + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/hashicorp/boundary/internal/db" + "github.com/hashicorp/boundary/internal/host/static/store" + "github.com/hashicorp/boundary/internal/iam" + "github.com/hashicorp/boundary/internal/oplog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRepository_CreateHost(t *testing.T) { + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + + _, prj := iam.TestScopes(t, conn) + catalog := testCatalogs(t, conn, prj.PublicId, 1)[0] + + var tests = []struct { + name string + in *Host + opts []Option + want *Host + wantIsErr error + }{ + { + name: "nil-Host", + wantIsErr: db.ErrNilParameter, + }, + { + name: "nil-embedded-Host", + in: &Host{}, + wantIsErr: db.ErrNilParameter, + }, + { + name: "invalid-no-catalog-id", + in: &Host{ + Host: &store.Host{}, + }, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "invalid-public-id-set", + in: &Host{ + Host: &store.Host{ + CatalogId: catalog.PublicId, + PublicId: "abcd_OOOOOOOOOO", + }, + }, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "valid-no-options", + in: &Host{ + Host: &store.Host{ + CatalogId: catalog.PublicId, + Address: "127.0.0.1", + }, + }, + want: &Host{ + Host: &store.Host{ + CatalogId: catalog.PublicId, + Address: "127.0.0.1", + }, + }, + }, + { + name: "valid-with-name", + in: &Host{ + Host: &store.Host{ + CatalogId: catalog.PublicId, + Name: "test-name-repo", + Address: "127.0.0.1", + }, + }, + want: &Host{ + Host: &store.Host{ + CatalogId: catalog.PublicId, + Name: "test-name-repo", + Address: "127.0.0.1", + }, + }, + }, + { + name: "valid-with-description", + in: &Host{ + Host: &store.Host{ + CatalogId: catalog.PublicId, + Description: ("test-description-repo"), + Address: "127.0.0.1", + }, + }, + want: &Host{ + Host: &store.Host{ + CatalogId: catalog.PublicId, + Description: ("test-description-repo"), + Address: "127.0.0.1", + }, + }, + }, + { + name: "invalid-no-address", + in: &Host{ + Host: &store.Host{ + CatalogId: catalog.PublicId, + }, + }, + wantIsErr: ErrInvalidAddress, + }, + { + name: "invalid-address-to-short", + in: &Host{ + Host: &store.Host{ + CatalogId: catalog.PublicId, + Address: "127", + }, + }, + wantIsErr: ErrInvalidAddress, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, wrapper) + require.NoError(err) + require.NotNil(repo) + got, err := repo.CreateHost(context.Background(), tt.in, tt.opts...) + if tt.wantIsErr != nil { + assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + assert.Nil(got) + return + } + require.NoError(err) + assert.Empty(tt.in.PublicId) + require.NotNil(got) + assertPublicId(t, HostPrefix, got.PublicId) + assert.NotSame(tt.in, got) + assert.Equal(tt.want.Name, got.Name) + assert.Equal(tt.want.Description, got.Description) + assert.Equal(got.CreateTime, got.UpdateTime) + assert.NoError(db.TestVerifyOplog(t, rw, got.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_CREATE), db.WithCreateNotBefore(10*time.Second))) + }) + } + + t.Run("invalid-duplicate-names", func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, wrapper) + require.NoError(err) + require.NotNil(repo) + + _, prj := iam.TestScopes(t, conn) + catalog := testCatalogs(t, conn, prj.PublicId, 1)[0] + + in := &Host{ + Host: &store.Host{ + CatalogId: catalog.PublicId, + Name: "test-name-repo", + Address: "127.0.0.1", + }, + } + + got, err := repo.CreateHost(context.Background(), in) + require.NoError(err) + require.NotNil(got) + assertPublicId(t, HostPrefix, got.PublicId) + assert.NotSame(in, got) + assert.Equal(in.Name, got.Name) + assert.Equal(in.Description, got.Description) + assert.Equal(got.CreateTime, got.UpdateTime) + + got2, err := repo.CreateHost(context.Background(), in) + assert.Truef(errors.Is(err, db.ErrNotUnique), "want err: %v got: %v", db.ErrNotUnique, err) + assert.Nil(got2) + }) + + t.Run("valid-duplicate-names-diff-catalogs", func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, wrapper) + require.NoError(err) + require.NotNil(repo) + + _, prj := iam.TestScopes(t, conn) + catalogs := testCatalogs(t, conn, prj.PublicId, 2) + + catalogA, catalogB := catalogs[0], catalogs[1] + + in := &Host{ + Host: &store.Host{ + Name: "test-name-repo", + Address: "127.0.0.1", + }, + } + in2 := in.clone() + + in.CatalogId = catalogA.PublicId + got, err := repo.CreateHost(context.Background(), in) + require.NoError(err) + require.NotNil(got) + assertPublicId(t, HostPrefix, got.PublicId) + assert.NotSame(in, got) + assert.Equal(in.Name, got.Name) + assert.Equal(in.Description, got.Description) + assert.Equal(got.CreateTime, got.UpdateTime) + + in2.CatalogId = catalogB.PublicId + got2, err := repo.CreateHost(context.Background(), in2) + require.NoError(err) + require.NotNil(got2) + assertPublicId(t, HostPrefix, got2.PublicId) + assert.NotSame(in2, got2) + assert.Equal(in2.Name, got2.Name) + assert.Equal(in2.Description, got2.Description) + assert.Equal(got2.CreateTime, got2.UpdateTime) + }) +}