From 980a55a4e3a8d92f4c7eced3180832184c274745 Mon Sep 17 00:00:00 2001 From: Jim Date: Mon, 18 May 2020 17:51:09 -0400 Subject: [PATCH] Added DeleteScope() with tests and TestVerifyOplog() for easier testing (#49) --- internal/cmd/base/servers.go | 2 +- internal/db/domains_test.go | 12 +- internal/db/migrations/postgres.gen.go | 8 +- internal/db/migrations/postgres/04_iam.up.sql | 8 +- internal/db/testing.go | 99 +++++++ internal/db/testing_test.go | 113 ++++++++ internal/iam/options.go | 2 + internal/iam/options_test.go | 11 +- internal/iam/repository.go | 74 ++++- internal/iam/repository_scope.go | 61 ++-- internal/iam/repository_scope_test.go | 264 ++++++++++++------ internal/iam/repository_test.go | 125 ++++++--- internal/iam/scope.go | 37 ++- internal/iam/scope_test.go | 153 ++++++---- internal/iam/store/scanners_test.go | 14 +- .../handlers/projects/project_service.go | 2 +- 16 files changed, 736 insertions(+), 249 deletions(-) diff --git a/internal/cmd/base/servers.go b/internal/cmd/base/servers.go index 22e43882e..c9e1f445a 100644 --- a/internal/cmd/base/servers.go +++ b/internal/cmd/base/servers.go @@ -406,7 +406,7 @@ func (b *Server) CreateDevDatabase(dialect string) error { var scope *iam.Scope if b.DefaultOrgId != "" { - scope, err = repo.LookupScope(ctx, iam.WithPublicId(b.DefaultOrgId)) + scope, err = repo.LookupScope(ctx, b.DefaultOrgId) if err != nil { c() return fmt.Errorf("error looking up existing scope with org ID %q: %w", b.DefaultOrgId, err) diff --git a/internal/db/domains_test.go b/internal/db/domains_test.go index 5378870a2..21b075922 100644 --- a/internal/db/domains_test.go +++ b/internal/db/domains_test.go @@ -9,13 +9,13 @@ import ( func TestDomain_PublicId(t *testing.T) { const ( createTable = ` -create table if not exists test_table ( +create table if not exists test_table_public_id ( id bigint generated always as identity primary key, public_id wt_public_id ); ` insert = ` -insert into test_table (public_id) +insert into test_table_public_id (public_id) values ($1) returning id; ` @@ -71,7 +71,7 @@ returning id; func TestDomain_Timestamp(t *testing.T) { const ( createTable = ` -create table if not exists test_table ( +create table if not exists test_table_timestamp ( id bigint generated always as identity primary key, name text, good_time wt_timestamp, @@ -79,7 +79,7 @@ create table if not exists test_table ( ); ` insert = ` -insert into test_table (name) +insert into test_table_timestamp (name) values ($1) returning id; ` @@ -102,11 +102,11 @@ returning id; t.Fatalf("want no error, got error %v", err) } - if _, err := db.Query("select extract(timezone from good_time) from test_table;"); err != nil { + if _, err := db.Query("select extract(timezone from good_time) from test_table_timestamp;"); err != nil { t.Errorf("want no error, got error %v", err) } - if _, err := db.Query("select extract(timezone from bad_time) from test_table;"); err == nil { + if _, err := db.Query("select extract(timezone from bad_time) from test_table_timestamp;"); err == nil { t.Errorf("want error, got no error") } } diff --git a/internal/db/migrations/postgres.gen.go b/internal/db/migrations/postgres.gen.go index 7f5e799b6..6684238b4 100644 --- a/internal/db/migrations/postgres.gen.go +++ b/internal/db/migrations/postgres.gen.go @@ -158,7 +158,7 @@ COMMIT; bytes: []byte(` BEGIN; -CREATE TABLE if not exists iam_scope_type_enm ( +CREATE TABLE iam_scope_type_enm ( string text NOT NULL primary key CHECK(string IN ('unknown', 'organization', 'project')) ); INSERT INTO iam_scope_type_enm (string) @@ -168,7 +168,7 @@ values ('project'); -CREATE TABLE if not exists iam_scope ( +CREATE TABLE iam_scope ( public_id wt_public_id primary key, create_time wt_timestamp, update_time wt_timestamp, @@ -186,12 +186,12 @@ CREATE TABLE if not exists iam_scope ( description text, parent_id text REFERENCES iam_scope(public_id) ON DELETE CASCADE ON UPDATE CASCADE ); -create table if not exists iam_scope_organization ( +create table iam_scope_organization ( scope_id wt_public_id NOT NULL UNIQUE REFERENCES iam_scope(public_id) ON DELETE CASCADE ON UPDATE CASCADE, name text UNIQUE, primary key(scope_id) ); -create table if not exists iam_scope_project ( +create table iam_scope_project ( scope_id wt_public_id NOT NULL REFERENCES iam_scope(public_id) ON DELETE CASCADE ON UPDATE CASCADE, parent_id wt_public_id NOT NULL REFERENCES iam_scope_organization(scope_id) ON DELETE CASCADE ON UPDATE CASCADE, name text, diff --git a/internal/db/migrations/postgres/04_iam.up.sql b/internal/db/migrations/postgres/04_iam.up.sql index 9564f0850..1309f9790 100644 --- a/internal/db/migrations/postgres/04_iam.up.sql +++ b/internal/db/migrations/postgres/04_iam.up.sql @@ -1,6 +1,6 @@ BEGIN; -CREATE TABLE if not exists iam_scope_type_enm ( +CREATE TABLE iam_scope_type_enm ( string text NOT NULL primary key CHECK(string IN ('unknown', 'organization', 'project')) ); INSERT INTO iam_scope_type_enm (string) @@ -10,7 +10,7 @@ values ('project'); -CREATE TABLE if not exists iam_scope ( +CREATE TABLE iam_scope ( public_id wt_public_id primary key, create_time wt_timestamp, update_time wt_timestamp, @@ -28,12 +28,12 @@ CREATE TABLE if not exists iam_scope ( description text, parent_id text REFERENCES iam_scope(public_id) ON DELETE CASCADE ON UPDATE CASCADE ); -create table if not exists iam_scope_organization ( +create table iam_scope_organization ( scope_id wt_public_id NOT NULL UNIQUE REFERENCES iam_scope(public_id) ON DELETE CASCADE ON UPDATE CASCADE, name text UNIQUE, primary key(scope_id) ); -create table if not exists iam_scope_project ( +create table iam_scope_project ( scope_id wt_public_id NOT NULL REFERENCES iam_scope(public_id) ON DELETE CASCADE ON UPDATE CASCADE, parent_id wt_public_id NOT NULL REFERENCES iam_scope_organization(scope_id) ON DELETE CASCADE ON UPDATE CASCADE, name text, diff --git a/internal/db/testing.go b/internal/db/testing.go index 2c9edc5e0..a29e582f4 100644 --- a/internal/db/testing.go +++ b/internal/db/testing.go @@ -1,13 +1,17 @@ package db import ( + "context" "crypto/rand" "testing" + "time" _ "github.com/golang-migrate/migrate/v4/database/postgres" _ "github.com/golang-migrate/migrate/v4/source/file" wrapping "github.com/hashicorp/go-kms-wrapping" "github.com/hashicorp/go-kms-wrapping/wrappers/aead" + "github.com/hashicorp/watchtower/internal/oplog" + "github.com/hashicorp/watchtower/internal/oplog/store" "github.com/jinzhu/gorm" ) @@ -40,3 +44,98 @@ func TestWrapper(t *testing.T) wrapping.Wrapper { } return root } + +// TestVerifyOplog will verify that there is an oplog entry. An error is +// returned if the entry or it's metadata is not found. Returning an error +// allows clients to test if an entry was not written, which is a valid use case. +func TestVerifyOplog(t *testing.T, r Reader, resourcePublicId string, opt ...TestOption) error { + // sql where clauses + const ( + whereBase = ` + key = 'resource-public-id' +and value = ? +` + whereOptype = ` +and entry_id in ( + select entry_id + from oplog_metadata + where key = 'op-type' + and value = ? +) +` + whereCreateNotBefore = ` +and create_time > NOW()::timestamp - (interval '1 second' * ?) +` + ) + + opts := getTestOpts(opt...) + withOperation := opts.withOperation + withCreateNotBefore := opts.withCreateNotBefore + + where := whereBase + whereArgs := []interface{}{ + resourcePublicId, + } + + if withOperation != oplog.OpType_OP_TYPE_UNSPECIFIED { + where = where + whereOptype + whereArgs = append(whereArgs, withOperation.String()) + } + + if withCreateNotBefore != nil { + where = where + whereCreateNotBefore + whereArgs = append(whereArgs, int(*withCreateNotBefore)) + } + + var metadata store.Metadata + if err := r.LookupWhere(context.Background(), &metadata, where, whereArgs...); err != nil { + return err + } + + var foundEntry oplog.Entry + if err := r.LookupWhere(context.Background(), &foundEntry, "id = ?", metadata.EntryId); err != nil { + return err + } + return nil +} + +// getTestOpts - iterate the inbound TestOptions and return a struct +func getTestOpts(opt ...TestOption) testOptions { + opts := getDefaultTestOptions() + for _, o := range opt { + o(&opts) + } + return opts +} + +// TestOption - how Options are passed as arguments +type TestOption func(*testOptions) + +// options = how options are represented +type testOptions struct { + withCreateNotBefore *int + withOperation oplog.OpType +} + +func getDefaultTestOptions() testOptions { + return testOptions{ + withCreateNotBefore: nil, + withOperation: oplog.OpType_OP_TYPE_UNSPECIFIED, + } +} + +// WithCreateNotBefore provides an option to specify that the create time is not +// before (nbf) N seconds +func WithCreateNotBefore(nbfDuration time.Duration) TestOption { + return func(o *testOptions) { + secs := int(nbfDuration.Truncate(time.Second).Seconds()) + o.withCreateNotBefore = &secs + } +} + +// WithOperation provides an option to specify the operation type +func WithOperation(op oplog.OpType) TestOption { + return func(o *testOptions) { + o.withOperation = op + } +} diff --git a/internal/db/testing_test.go b/internal/db/testing_test.go index 57e897f46..de9582984 100644 --- a/internal/db/testing_test.go +++ b/internal/db/testing_test.go @@ -1,7 +1,17 @@ package db import ( + "context" + "strconv" "testing" + "time" + + _ "github.com/golang-migrate/migrate/v4/database/postgres" + _ "github.com/golang-migrate/migrate/v4/source/file" + "github.com/hashicorp/go-uuid" + "github.com/hashicorp/watchtower/internal/db/db_test" + "github.com/hashicorp/watchtower/internal/oplog" + "github.com/stretchr/testify/assert" ) func Test_Utils(t *testing.T) { @@ -13,3 +23,106 @@ func Test_Utils(t *testing.T) { }) } + +func TestVerifyOplogEntry(t *testing.T) { + cleanup, db, _ := TestSetup(t, "postgres") + defer cleanup() + assert := assert.New(t) + defer db.Close() + + t.Run("valid", func(t *testing.T) { + rw := Db{underlying: db} + id, err := uuid.GenerateUUID() + assert.NoError(err) + user, err := db_test.NewTestUser() + assert.NoError(err) + user.Name = "foo-" + id + err = rw.Create( + context.Background(), + user, + WithOplog( + TestWrapper(t), + oplog.Metadata{ + "op-type": []string{strconv.Itoa(int(oplog.OpType_OP_TYPE_CREATE))}, + "resource-public-id": []string{user.GetPublicId()}, + }), + ) + assert.NoError(err) + assert.NotZero(user.Id) + + foundUser, err := db_test.NewTestUser() + assert.NoError(err) + foundUser.PublicId = user.PublicId + err = rw.LookupByPublicId(context.Background(), foundUser) + assert.NoError(err) + assert.Equal(foundUser.Id, user.Id) + TestVerifyOplog(t, &rw, user.PublicId, WithOperation(oplog.OpType_OP_TYPE_CREATE), WithCreateNotBefore(5*time.Second)) + assert.NoError(err) + }) + t.Run("should-fail", func(t *testing.T) { + rw := Db{underlying: db} + id, err := uuid.GenerateUUID() + assert.NoError(err) + err = TestVerifyOplog(t, &rw, id) + assert.Error(err) + }) +} + +func Test_getTestOpts(t *testing.T) { + t.Parallel() + assert := assert.New(t) + t.Run("WithOperation", func(t *testing.T) { + opts := getTestOpts(WithOperation(oplog.OpType_OP_TYPE_CREATE)) + testOpts := getDefaultTestOptions() + testOpts.withOperation = oplog.OpType_OP_TYPE_CREATE + assert.Equal(opts, testOpts) + }) + t.Run("WithCreateNotBefore", func(t *testing.T) { + nbfSecs := 10 + opts := getTestOpts(WithCreateNotBefore(time.Second * 10)) + testOpts := getDefaultTestOptions() + testOpts.withCreateNotBefore = &nbfSecs + assert.Equal(opts, testOpts) + }) +} + +func TestWithCreateNotBefore(t *testing.T) { + type args struct { + nbfDuration time.Duration + } + tests := []struct { + name string + args args + want int + }{ + { + name: "0 seconds", + args: args{nbfDuration: 0}, + want: 0, + }, + { + name: "1 seconds", + args: args{nbfDuration: time.Millisecond * 1900}, + want: 1, + }, + { + name: "2 seconds", + args: args{nbfDuration: time.Millisecond * 2100}, + want: 2, + }, + { + name: "60 seconds", + args: args{nbfDuration: time.Minute * 1}, + want: 60, + }, + } + assert := assert.New(t) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + opts := getTestOpts(WithCreateNotBefore(tt.args.nbfDuration)) + testOpts := getDefaultTestOptions() + testOpts.withCreateNotBefore = &tt.want + assert.Equal(opts, testOpts) + }) + } +} diff --git a/internal/iam/options.go b/internal/iam/options.go index 0cd2600f0..80801c528 100644 --- a/internal/iam/options.go +++ b/internal/iam/options.go @@ -18,6 +18,7 @@ type options struct { withName string withScope *Scope withDescription string + withParentId *string } func getDefaultOptions() options { @@ -26,6 +27,7 @@ func getDefaultOptions() options { withScope: nil, withDescription: "", withName: "", + withParentId: nil, } } diff --git a/internal/iam/options_test.go b/internal/iam/options_test.go index 3304fd18a..f80e219f1 100644 --- a/internal/iam/options_test.go +++ b/internal/iam/options_test.go @@ -1,7 +1,6 @@ package iam import ( - "reflect" "testing" "github.com/stretchr/testify/assert" @@ -15,22 +14,22 @@ func Test_GetOpts(t *testing.T) { opts := getOpts(WithName("test")) testOpts := getDefaultOptions() testOpts.withName = "test" - assert.True(reflect.DeepEqual(opts, testOpts)) + assert.Equal(opts, testOpts) }) t.Run("withScope", func(t *testing.T) { s, err := NewOrganization() - assert.Nil(err) - assert.True(s.Scope != nil) + assert.NoError(err) + assert.NotNil(s.Scope) opts := getOpts(withScope(s)) testOpts := getDefaultOptions() testOpts.withScope = s - assert.True(reflect.DeepEqual(opts, testOpts)) + assert.Equal(opts, testOpts) }) t.Run("WithDescription", func(t *testing.T) { opts := getOpts(WithDescription("test desc")) testOpts := getDefaultOptions() testOpts.withDescription = "test desc" - assert.True(reflect.DeepEqual(opts, testOpts)) + assert.Equal(opts, testOpts) }) } diff --git a/internal/iam/repository.go b/internal/iam/repository.go index 5ddbaf3ea..7f69d1e4b 100644 --- a/internal/iam/repository.go +++ b/internal/iam/repository.go @@ -4,13 +4,16 @@ import ( "context" "errors" "fmt" - "strconv" wrapping "github.com/hashicorp/go-kms-wrapping" "github.com/hashicorp/watchtower/internal/db" "github.com/hashicorp/watchtower/internal/oplog" ) +var ( + ErrMetadataScopeNotFound = errors.New("scope not found for metadata") +) + // Repository is the iam database repository type Repository struct { reader db.Reader @@ -36,7 +39,7 @@ func NewRepository(r db.Reader, w db.Writer, wrapper wrapping.Wrapper) (*Reposit }, nil } -// Create will create a new iam resource in the db repository with an oplog entry +// create will create a new iam resource in the db repository with an oplog entry func (r *Repository) create(ctx context.Context, resource Resource, opt ...Option) (Resource, error) { if resource == nil { return nil, errors.New("error creating resource that is nil") @@ -49,7 +52,7 @@ func (r *Repository) create(ctx context.Context, resource Resource, opt ...Optio if err != nil { return nil, fmt.Errorf("error getting metadata for create: %w", err) } - metadata["op-type"] = []string{strconv.Itoa(int(oplog.OpType_OP_TYPE_CREATE))} + metadata["op-type"] = []string{oplog.OpType_OP_TYPE_CREATE.String()} var returnedResource interface{} _, err = r.writer.DoTx( @@ -68,7 +71,7 @@ func (r *Repository) create(ctx context.Context, resource Resource, opt ...Optio return returnedResource.(Resource), err } -// Update will update an iam resource in the db repository with an oplog entry +// update will update an iam resource in the db repository with an oplog entry func (r *Repository) update(ctx context.Context, resource Resource, fieldMaskPaths []string, opt ...Option) (Resource, int, error) { if resource == nil { return nil, db.NoRowsAffected, errors.New("error updating resource that is nil") @@ -81,7 +84,7 @@ func (r *Repository) update(ctx context.Context, resource Resource, fieldMaskPat if err != nil { return nil, db.NoRowsAffected, fmt.Errorf("error getting metadata for update: %w", err) } - metadata["op-type"] = []string{strconv.Itoa(int(oplog.OpType_OP_TYPE_UPDATE))} + metadata["op-type"] = []string{oplog.OpType_OP_TYPE_UPDATE.String()} var rowsUpdated int var returnedResource interface{} @@ -98,23 +101,82 @@ func (r *Repository) update(ctx context.Context, resource Resource, fieldMaskPat fieldMaskPaths, db.WithOplog(r.wrapper, metadata), ) + if err == nil && rowsUpdated > 1 { + // return err, which will result in a rollback of the update + return errors.New("error more than 1 resource would have been updated ") + } return err }, ) return returnedResource.(Resource), rowsUpdated, err } +// delete will delete an iam resource in the db repository with an oplog entry +func (r *Repository) delete(ctx context.Context, resource Resource, opt ...Option) (int, error) { + if resource == nil { + return db.NoRowsAffected, errors.New("error deleting resource that is nil") + } + resourceCloner, ok := resource.(Clonable) + if !ok { + return db.NoRowsAffected, errors.New("error resource is not clonable for delete") + } + metadata, err := r.stdMetadata(ctx, resource) + if err != nil { + return db.NoRowsAffected, fmt.Errorf("error getting metadata for delete: %w", err) + } + metadata["op-type"] = []string{oplog.OpType_OP_TYPE_DELETE.String()} + + var rowsDeleted int + var deleteResource interface{} + _, err = r.writer.DoTx( + ctx, + db.StdRetryCnt, + db.ExpBackoff{}, + func(w db.Writer) error { + deleteResource = resourceCloner.Clone() + var err error + rowsDeleted, err = w.Delete( + ctx, + deleteResource, + db.WithOplog(r.wrapper, metadata), + ) + if err == nil && rowsDeleted > 1 { + // return err, which will result in a rollback of the delete + return errors.New("error more than 1 resource would have been deleted ") + } + return err + }, + ) + return rowsDeleted, err +} + func (r *Repository) stdMetadata(ctx context.Context, resource Resource) (oplog.Metadata, error) { if s, ok := resource.(*Scope); ok { - if s.Type == OrganizationScope.String() { + if s.Type == "" { + if err := r.reader.LookupByPublicId(ctx, s); err != nil { + return nil, ErrMetadataScopeNotFound + } + } + switch s.Type { + case OrganizationScope.String(): return oplog.Metadata{ "resource-public-id": []string{resource.GetPublicId()}, "scope-id": []string{s.PublicId}, "scope-type": []string{s.Type}, "resource-type": []string{resource.ResourceType().String()}, }, nil + case ProjectScope.String(): + return oplog.Metadata{ + "resource-public-id": []string{resource.GetPublicId()}, + "scope-id": []string{s.ParentId}, + "scope-type": []string{s.Type}, + "resource-type": []string{resource.ResourceType().String()}, + }, nil + default: + return nil, fmt.Errorf("not a supported scope for metadata: %s", s.Type) } } + scope, err := resource.GetScope(ctx, r.reader) if err != nil { return nil, fmt.Errorf("unable to get scope for standard metadata: %w", err) diff --git a/internal/iam/repository_scope.go b/internal/iam/repository_scope.go index 1a2450db2..946752152 100644 --- a/internal/iam/repository_scope.go +++ b/internal/iam/repository_scope.go @@ -11,19 +11,22 @@ import ( // CreateScope will create a scope in the repository and return the written scope func (r *Repository) CreateScope(ctx context.Context, scope *Scope, opt ...Option) (*Scope, error) { if scope == nil { - return nil, errors.New("error scope is nil for create") + return nil, errors.New("scope is nil for create") } resource, err := r.create(ctx, scope) if err != nil { return nil, fmt.Errorf("failed to create scope: %w", err) } - return resource.(*Scope), err + return resource.(*Scope), nil } // UpdateScope will update a scope in the repository and return the written scope func (r *Repository) UpdateScope(ctx context.Context, scope *Scope, fieldMaskPaths []string, opt ...Option) (*Scope, int, error) { if scope == nil { - return nil, db.NoRowsAffected, errors.New("error scope is nil for update") + return nil, db.NoRowsAffected, errors.New("scope is nil for update") + } + if scope.PublicId == "" { + return nil, db.NoRowsAffected, errors.New("scope public id is unset for update") } resource, rowsUpdated, err := r.update(ctx, scope, fieldMaskPaths) if err != nil { @@ -34,32 +37,34 @@ func (r *Repository) UpdateScope(ctx context.Context, scope *Scope, fieldMaskPat // LookupScope will look up a scope in the repository. If the scope is not // found, it will return nil, nil. -func (r *Repository) LookupScope(ctx context.Context, opt ...Option) (*Scope, error) { - opts := getOpts(opt...) - withPublicId := opts.withPublicId - withName := opts.withName - - if withPublicId != "" { - scope := allocScope() - scope.PublicId = withPublicId - if err := r.reader.LookupByPublicId(ctx, &scope); err != nil { - if err == db.ErrRecordNotFound { - return nil, nil - } - return nil, err - } - return &scope, nil +func (r *Repository) LookupScope(ctx context.Context, withPublicId string, opt ...Option) (*Scope, error) { + if withPublicId == "" { + return nil, errors.New("cannot lookup a scope with an empty public id") } - if withName != "" { - scope := allocScope() - scope.Name = withName - if err := r.reader.LookupByName(ctx, &scope); err != nil { - if err == db.ErrRecordNotFound { - return nil, nil - } - return nil, err + scope := allocScope() + scope.PublicId = withPublicId + if err := r.reader.LookupByPublicId(ctx, &scope); err != nil { + if err == db.ErrRecordNotFound { + return nil, nil } - return &scope, nil + return nil, fmt.Errorf("unable to lookup scope by public id %s: %w", withPublicId, err) } - return nil, errors.New("you must look up scopes by id or name") + return &scope, nil +} + +// DeleteScope will delete a scope from the repository +func (r *Repository) DeleteScope(ctx context.Context, withPublicId string, opt ...Option) (int, error) { + if withPublicId == "" { + return db.NoRowsAffected, errors.New("cannot delete a scope with an empty public id") + } + scope := allocScope() + scope.PublicId = withPublicId + rowsDeleted, err := r.delete(ctx, &scope) + if err != nil { + if errors.Is(err, ErrMetadataScopeNotFound) { + return 0, nil + } + return db.NoRowsAffected, fmt.Errorf("unable to delete scope with public id %s: %w", withPublicId, err) + } + return rowsDeleted, nil } diff --git a/internal/iam/repository_scope_test.go b/internal/iam/repository_scope_test.go index ca4a2afb9..145c238c6 100644 --- a/internal/iam/repository_scope_test.go +++ b/internal/iam/repository_scope_test.go @@ -3,18 +3,23 @@ package iam import ( "context" "testing" + "time" "github.com/hashicorp/go-uuid" "github.com/hashicorp/watchtower/internal/db" "github.com/hashicorp/watchtower/internal/oplog" - "github.com/hashicorp/watchtower/internal/oplog/store" "github.com/stretchr/testify/assert" + "google.golang.org/protobuf/proto" ) func Test_Repository_CreateScope(t *testing.T) { t.Parallel() cleanup, conn, _ := db.TestSetup(t, "postgres") - defer cleanup() + defer func() { + if err := cleanup(); err != nil { + t.Error(err) + } + }() assert := assert.New(t) defer conn.Close() @@ -22,38 +27,85 @@ func Test_Repository_CreateScope(t *testing.T) { rw := db.New(conn) wrapper := db.TestWrapper(t) repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) id, err := uuid.GenerateUUID() - assert.Nil(err) + assert.NoError(err) - s, err := NewOrganization(WithName("fname-" + id)) + s, err := NewOrganization(WithName(id)) + assert.NoError(err) s, err = repo.CreateScope(context.Background(), s) - assert.Nil(err) - assert.True(s != nil) - assert.True(s.GetPublicId() != "") - assert.Equal(s.GetName(), "fname-"+id) + assert.NoError(err) + assert.NotNil(s) + assert.NotEmpty(s.GetPublicId()) + assert.Equal(s.GetName(), id) - foundScope, err := repo.LookupScope(context.Background(), WithPublicId(s.PublicId)) - assert.Nil(err) - assert.Equal(foundScope.GetPublicId(), s.GetPublicId()) + foundScope, err := repo.LookupScope(context.Background(), s.PublicId) + assert.NoError(err) + assert.True(proto.Equal(foundScope, s)) - foundScope, err = repo.LookupScope(context.Background(), WithName("fname-"+id)) - assert.Nil(err) - assert.Equal(foundScope.GetPublicId(), s.GetPublicId()) + err = db.TestVerifyOplog(t, rw, s.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_CREATE), db.WithCreateNotBefore(10*time.Second)) + assert.NoError(err) + }) + t.Run("dup-org-names", func(t *testing.T) { + rw := db.New(conn) + wrapper := db.TestWrapper(t) + repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) + id, err := uuid.GenerateUUID() + assert.NoError(err) - var metadata store.Metadata - err = conn.Where("key = ? and value = ?", "resource-public-id", s.PublicId).First(&metadata).Error - assert.Nil(err) + s, err := NewOrganization(WithName(id)) + assert.NoError(err) + s, err = repo.CreateScope(context.Background(), s) + assert.NoError(err) + assert.NotNil(s) + assert.NotEmpty(s.GetPublicId()) + assert.Equal(s.GetName(), id) - var foundEntry oplog.Entry - err = conn.Where("id = ?", metadata.EntryId).First(&foundEntry).Error - assert.Nil(err) + s2, err := NewOrganization(WithName(id)) + assert.NoError(err) + s2, err = repo.CreateScope(context.Background(), s2) + assert.Error(err) + assert.Nil(s2) + }) + t.Run("dup-proj-names", func(t *testing.T) { + rw := db.New(conn) + wrapper := db.TestWrapper(t) + repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) + id, err := uuid.GenerateUUID() + assert.NoError(err) + + s, err := NewOrganization(WithName(id)) + assert.NoError(err) + s, err = repo.CreateScope(context.Background(), s) + assert.NoError(err) + assert.NotNil(s) + assert.NotEmpty(s.GetPublicId()) + assert.Equal(s.GetName(), id) + + p, err := NewProject(s.PublicId, WithName(id)) + assert.NoError(err) + p, err = repo.CreateScope(context.Background(), p) + assert.NoError(err) + assert.NotEmpty(p.PublicId) + + p2, err := NewProject(s.PublicId, WithName(id)) + assert.NoError(err) + p2, err = repo.CreateScope(context.Background(), p2) + assert.Error(err) + assert.Nil(p2) }) } func Test_Repository_UpdateScope(t *testing.T) { t.Parallel() cleanup, conn, _ := db.TestSetup(t, "postgres") - defer cleanup() + defer func() { + if err := cleanup(); err != nil { + t.Error(err) + } + }() assert := assert.New(t) defer conn.Close() @@ -61,76 +113,68 @@ func Test_Repository_UpdateScope(t *testing.T) { rw := db.New(conn) wrapper := db.TestWrapper(t) repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) id, err := uuid.GenerateUUID() - assert.Nil(err) + assert.NoError(err) - s, err := NewOrganization(WithName("fname-" + id)) + s, err := NewOrganization(WithName(id)) + assert.NoError(err) s, err = repo.CreateScope(context.Background(), s) - assert.Nil(err) - assert.True(s != nil) - assert.True(s.GetPublicId() != "") - assert.Equal(s.GetName(), "fname-"+id) + assert.NoError(err) + assert.NotNil(s) + assert.NotEmpty(s.GetPublicId()) + assert.Equal(s.GetName(), id) - foundScope, err := repo.LookupScope(context.Background(), WithPublicId(s.PublicId)) - assert.Nil(err) - assert.Equal(foundScope.GetPublicId(), s.GetPublicId()) + foundScope, err := repo.LookupScope(context.Background(), s.PublicId) + assert.NoError(err) + assert.True(proto.Equal(foundScope, s)) - foundScope, err = repo.LookupScope(context.Background(), WithName("fname-"+id)) - assert.Nil(err) - assert.Equal(foundScope.GetPublicId(), s.GetPublicId()) + err = db.TestVerifyOplog(t, rw, s.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_CREATE), db.WithCreateNotBefore(10*time.Second)) + assert.NoError(err) - var metadata store.Metadata - err = conn.Where("key = ? and value = ?", "resource-public-id", s.PublicId).First(&metadata).Error - assert.Nil(err) - - var foundEntry oplog.Entry - err = conn.Where("id = ?", metadata.EntryId).First(&foundEntry).Error - assert.Nil(err) - - s.Name = "fname-" + id + s.Name = id s.Description = "desc-id" // not in the field mask paths s, updatedRows, err := repo.UpdateScope(context.Background(), s, []string{"Name"}) - assert.Nil(err) + assert.NoError(err) assert.Equal(1, updatedRows) - assert.True(s != nil) - assert.Equal(s.GetName(), "fname-"+id) - assert.Equal(foundScope.GetDescription(), "") // should be "" after update in db + assert.NotNil(s) + assert.Equal(s.GetName(), id) + assert.Empty(foundScope.GetDescription()) // should be "" after update in db - foundScope, err = repo.LookupScope(context.Background(), WithName("fname-"+id)) - assert.Nil(err) + foundScope, err = repo.LookupScope(context.Background(), s.PublicId) + assert.NoError(err) assert.Equal(foundScope.GetPublicId(), s.GetPublicId()) - assert.Equal(foundScope.GetDescription(), "") + assert.Empty(foundScope.GetDescription()) - err = conn.Where("key = ? and value = ?", "resource-public-id", s.PublicId).First(&metadata).Error - assert.Nil(err) - - err = conn.Where("id = ?", metadata.EntryId).First(&foundEntry).Error - assert.Nil(err) + err = db.TestVerifyOplog(t, rw, s.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) + assert.NoError(err) }) t.Run("bad-parent-scope", func(t *testing.T) { rw := db.New(conn) wrapper := db.TestWrapper(t) repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) id, err := uuid.GenerateUUID() - assert.Nil(err) + assert.NoError(err) - s, err := NewOrganization(WithName("fname-" + id)) - assert.Nil(err) + s, err := NewOrganization(WithName(id)) + assert.NoError(err) s, err = repo.CreateScope(context.Background(), s) - assert.Nil(err) - assert.True(s != nil) - assert.True(s.GetPublicId() != "") - assert.Equal(s.GetName(), "fname-"+id) + assert.NoError(err) + assert.NotNil(s) + assert.NotEmpty(s.GetPublicId()) + assert.Equal(s.GetName(), id) project, err := NewProject(s.PublicId) - assert.Nil(err) + assert.NoError(err) project, err = repo.CreateScope(context.Background(), project) - assert.Nil(err) - assert.True(project != nil) + assert.NoError(err) + assert.NotNil(project) project.ParentId = project.PublicId project, updatedRows, err := repo.UpdateScope(context.Background(), project, []string{"ParentId"}) - assert.True(err != nil) + assert.Error(err) + assert.Nil(project) assert.Equal(0, updatedRows) assert.Equal("failed to update scope: error on update you cannot change a scope's parent", err.Error()) }) @@ -139,7 +183,11 @@ func Test_Repository_UpdateScope(t *testing.T) { func Test_Repository_LookupScope(t *testing.T) { t.Parallel() cleanup, conn, _ := db.TestSetup(t, "postgres") - defer cleanup() + defer func() { + if err := cleanup(); err != nil { + t.Error(err) + } + }() assert := assert.New(t) defer conn.Close() @@ -147,32 +195,76 @@ func Test_Repository_LookupScope(t *testing.T) { rw := db.New(conn) wrapper := db.TestWrapper(t) repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) id, err := uuid.GenerateUUID() - assert.Nil(err) + assert.NoError(err) - s, err := NewOrganization(WithName("fname-" + id)) + s, err := NewOrganization(WithName(id)) + assert.NoError(err) s, err = repo.CreateScope(context.Background(), s) - assert.Nil(err) - assert.True(s != nil) - assert.True(s.GetPublicId() != "") - assert.Equal(s.GetName(), "fname-"+id) + assert.NoError(err) + assert.NotNil(s) + assert.NotEmpty(s.GetPublicId()) + assert.Equal(s.GetName(), id) - foundScope, err := repo.LookupScope(context.Background(), WithPublicId(s.PublicId)) - assert.Nil(err) - assert.Equal(foundScope.GetPublicId(), s.GetPublicId()) - - foundScope, err = repo.LookupScope(context.Background(), WithName("fname-"+id)) - assert.Nil(err) - assert.Equal(foundScope.GetPublicId(), s.GetPublicId()) + foundScope, err := repo.LookupScope(context.Background(), s.PublicId) + assert.NoError(err) + assert.True(proto.Equal(foundScope, s)) invalidId, err := uuid.GenerateUUID() - assert.Nil(err) - notFoundById, err := repo.LookupScope(context.Background(), WithPublicId(invalidId)) - assert.Nil(err) + assert.NoError(err) + notFoundById, err := repo.LookupScope(context.Background(), invalidId) + assert.NoError(err) assert.Nil(notFoundById) - - notFoundByName, err := repo.LookupScope(context.Background(), WithName(invalidId)) - assert.Nil(err) - assert.Nil(notFoundByName) + }) +} + +func Test_Repository_DeleteScope(t *testing.T) { + t.Parallel() + cleanup, conn, _ := db.TestSetup(t, "postgres") + defer func() { + if err := cleanup(); err != nil { + t.Error(err) + } + }() + assert := assert.New(t) + defer conn.Close() + rw := db.New(conn) + wrapper := db.TestWrapper(t) + repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) + t.Run("valid-with-public-id", func(t *testing.T) { + s, err := NewOrganization() + assert.NoError(err) + s, err = repo.CreateScope(context.Background(), s) + assert.NoError(err) + assert.NotNil(s) + assert.NotEmpty(s.GetPublicId()) + + foundScope, err := repo.LookupScope(context.Background(), s.PublicId) + assert.NoError(err) + assert.Equal(foundScope.GetPublicId(), s.GetPublicId()) + + rowsDeleted, err := repo.DeleteScope(context.Background(), s.PublicId) + assert.NoError(err) + assert.Equal(1, rowsDeleted) + + err = db.TestVerifyOplog(t, rw, s.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_DELETE), db.WithCreateNotBefore(10*time.Second)) + assert.NoError(err) + + foundScope, err = repo.LookupScope(context.Background(), s.PublicId) + assert.NoError(err) + assert.Nil(foundScope) + + }) + t.Run("valid-with-bad-id", func(t *testing.T) { + invalidId, err := uuid.GenerateUUID() + assert.NoError(err) + foundScope, err := repo.LookupScope(context.Background(), invalidId) + assert.NoError(err) + assert.Nil(foundScope) + rowsDeleted, err := repo.DeleteScope(context.Background(), invalidId) + assert.NoError(err) // no error is expected if the resource isn't in the db + assert.Equal(0, rowsDeleted) }) } diff --git a/internal/iam/repository_test.go b/internal/iam/repository_test.go index 17bdf1339..a9859d0c1 100644 --- a/internal/iam/repository_test.go +++ b/internal/iam/repository_test.go @@ -4,6 +4,7 @@ import ( "context" "reflect" "testing" + "time" wrapping "github.com/hashicorp/go-kms-wrapping" "github.com/hashicorp/go-uuid" @@ -11,12 +12,17 @@ import ( "github.com/hashicorp/watchtower/internal/oplog" "github.com/hashicorp/watchtower/internal/oplog/store" "github.com/stretchr/testify/assert" + "google.golang.org/protobuf/proto" ) func TestNewRepository(t *testing.T) { t.Parallel() cleanup, conn, _ := db.TestSetup(t, "postgres") - defer cleanup() + defer func() { + if err := cleanup(); err != nil { + t.Error(err) + } + }() assert := assert.New(t) defer conn.Close() @@ -101,7 +107,11 @@ func TestNewRepository(t *testing.T) { func Test_Repository_create(t *testing.T) { t.Parallel() cleanup, conn, _ := db.TestSetup(t, "postgres") - defer cleanup() + defer func() { + if err := cleanup(); err != nil { + t.Error(err) + } + }() assert := assert.New(t) defer conn.Close() @@ -109,48 +119,50 @@ func Test_Repository_create(t *testing.T) { rw := db.New(conn) wrapper := db.TestWrapper(t) repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) id, err := uuid.GenerateUUID() - assert.Nil(err) + assert.NoError(err) s, err := NewOrganization(WithName("fname-" + id)) + assert.NoError(err) retScope, err := repo.create(context.Background(), s) - assert.Nil(err) - assert.True(retScope != nil) - assert.True(retScope.GetPublicId() != "") + assert.NoError(err) + assert.NotNil(retScope) + assert.NotEmpty(retScope.GetPublicId()) assert.Equal(retScope.GetName(), "fname-"+id) - foundScope, err := repo.LookupScope(context.Background(), WithPublicId(s.PublicId)) - assert.Nil(err) - assert.Equal(foundScope.GetPublicId(), retScope.GetPublicId()) - - foundScope.Name = "fname-" + id - foundScope, err = repo.LookupScope(context.Background(), WithName("fname-"+id)) - assert.Nil(err) - assert.Equal(foundScope.GetPublicId(), retScope.GetPublicId()) + foundScope, err := repo.LookupScope(context.Background(), s.PublicId) + assert.NoError(err) + assert.True(proto.Equal(foundScope, retScope.(*Scope))) var metadata store.Metadata err = conn.Where("key = ? and value = ?", "resource-public-id", s.PublicId).First(&metadata).Error - assert.Nil(err) + assert.NoError(err) var foundEntry oplog.Entry err = conn.Where("id = ?", metadata.EntryId).First(&foundEntry).Error - assert.Nil(err) + assert.NoError(err) }) t.Run("nil-resource", func(t *testing.T) { rw := db.New(conn) wrapper := db.TestWrapper(t) repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) resource, err := repo.create(context.Background(), nil) - assert.True(err != nil) - assert.True(resource == nil) + assert.NotNil(err) + assert.Nil(resource) assert.Equal(err.Error(), "error creating resource that is nil") }) } -func Test_dbRepository_update(t *testing.T) { +func Test_Repository_update(t *testing.T) { t.Parallel() cleanup, conn, _ := db.TestSetup(t, "postgres") - defer cleanup() + defer func() { + if err := cleanup(); err != nil { + t.Error(err) + } + }() assert := assert.New(t) defer conn.Close() @@ -158,43 +170,90 @@ func Test_dbRepository_update(t *testing.T) { rw := db.New(conn) wrapper := db.TestWrapper(t) repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) id, err := uuid.GenerateUUID() - assert.Nil(err) + assert.NoError(err) s, err := NewOrganization() + assert.NoError(err) retScope, err := repo.create(context.Background(), s) - assert.Nil(err) - assert.True(retScope != nil) - assert.True(retScope.GetPublicId() != "") - assert.Equal(retScope.GetName(), "") + assert.NoError(err) + assert.NotNil(retScope) + assert.NotEmpty(retScope.GetPublicId()) + assert.Empty(retScope.GetName()) retScope.(*Scope).Name = "fname-" + id retScope, updatedRows, err := repo.update(context.Background(), retScope, []string{"Name"}) - assert.Nil(err) - assert.True(retScope != nil) + assert.NoError(err) + assert.NotNil(retScope) assert.Equal(1, updatedRows) assert.Equal(retScope.GetName(), "fname-"+id) - foundScope, err := repo.LookupScope(context.Background(), WithName("fname-"+id)) - assert.Nil(err) + foundScope, err := repo.LookupScope(context.Background(), s.PublicId) + assert.NoError(err) assert.Equal(foundScope.GetPublicId(), retScope.GetPublicId()) var metadata store.Metadata err = conn.Where("key = ? and value = ?", "resource-public-id", s.PublicId).First(&metadata).Error - assert.Nil(err) + assert.NoError(err) var foundEntry oplog.Entry err = conn.Where("id = ?", metadata.EntryId).First(&foundEntry).Error - assert.Nil(err) + assert.NoError(err) }) t.Run("nil-resource", func(t *testing.T) { rw := db.New(conn) wrapper := db.TestWrapper(t) repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) resource, updatedRows, err := repo.update(context.Background(), nil, nil) - assert.True(err != nil) - assert.True(resource == nil) + assert.NotNil(err) + assert.Nil(resource) assert.Equal(0, updatedRows) assert.Equal(err.Error(), "error updating resource that is nil") }) } + +func Test_Repository_delete(t *testing.T) { + t.Parallel() + cleanup, conn, _ := db.TestSetup(t, "postgres") + defer func() { + if err := cleanup(); err != nil { + t.Error(err) + } + }() + assert := assert.New(t) + defer conn.Close() + + t.Run("valid-org", func(t *testing.T) { + rw := db.New(conn) + wrapper := db.TestWrapper(t) + repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) + + s, err := NewOrganization() + assert.NoError(err) + retScope, err := repo.create(context.Background(), s) + assert.NoError(err) + assert.NotNil(retScope) + assert.NotEmpty(retScope.GetPublicId()) + assert.Equal(retScope.GetName(), "") + + rowsDeleted, err := repo.delete(context.Background(), s) + assert.NoError(err) + assert.Equal(1, rowsDeleted) + + err = db.TestVerifyOplog(t, rw, s.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_DELETE), db.WithCreateNotBefore(5*time.Second)) + assert.NoError(err) + }) + t.Run("nil-resource", func(t *testing.T) { + rw := db.New(conn) + wrapper := db.TestWrapper(t) + repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) + deletedRows, err := repo.delete(context.Background(), nil, nil) + assert.NotNil(err) + assert.Equal(0, deletedRows) + assert.Equal(err.Error(), "error deleting resource that is nil") + }) +} diff --git a/internal/iam/scope.go b/internal/iam/scope.go index 5d94ad0d1..7c530bf4b 100644 --- a/internal/iam/scope.go +++ b/internal/iam/scope.go @@ -193,14 +193,39 @@ func (s *Scope) GetScope(ctx context.Context, r db.Reader) (*Scope, error) { if r == nil { return nil, errors.New("error db is nil for get scope") } - if s.ParentId == "" { + if s.PublicId == "" { + return nil, errors.New("unable to get scope with unset public id") + } + if s.Type == "" && s.ParentId == "" { + if err := r.LookupByPublicId(ctx, s); err != nil { + return nil, fmt.Errorf("unable to get scope by public id: %w", err) + } + } + // HANDLE_ORG + switch s.Type { + case OrganizationScope.String(): return nil, nil + case ProjectScope.String(): + var p Scope + switch s.ParentId { + case "": + // no parent id, so use the public_id to find the parent scope. This + // won't work for if the scope hasn't been written to the db yet, + // like during create but in that case the parent id should be set + // for all scopes which are not organizations, and the organization + // case was handled at HANDLE_ORG + where := "public_id in (select parent_id from iam_scope where public_id = ?)" + if err := r.LookupWhere(ctx, &p, where, s.PublicId); err != nil { + return nil, fmt.Errorf("unable to lookup parent public id from public id: %w", err) + } + default: + if err := r.LookupWhere(ctx, &p, "public_id = ?", s.ParentId); err != nil { + return nil, fmt.Errorf("unable to lookup parent from public id: %w", err) + } + } + return &p, nil } - var p Scope - if err := r.LookupWhere(ctx, &p, "public_id = ?", s.ParentId); err != nil { - return nil, fmt.Errorf("error getting scope %w", err) - } - return &p, nil + return nil, fmt.Errorf("unable to get scope with type %s", s.Type) } // TableName returns the tablename to override the default gorm table name diff --git a/internal/iam/scope_test.go b/internal/iam/scope_test.go index 04255b214..297a811a1 100644 --- a/internal/iam/scope_test.go +++ b/internal/iam/scope_test.go @@ -13,76 +13,84 @@ import ( func Test_NewScope(t *testing.T) { t.Parallel() cleanup, conn, _ := db.TestSetup(t, "postgres") - defer cleanup() + defer func() { + if err := cleanup(); err != nil { + t.Error(err) + } + }() assert := assert.New(t) defer conn.Close() t.Run("valid-org-with-project", func(t *testing.T) { w := db.New(conn) s, err := NewOrganization() - assert.Nil(err) - assert.True(s.Scope != nil) + assert.NoError(err) + assert.NotNil(s.Scope) err = w.Create(context.Background(), s) - assert.Nil(err) - assert.True(s.PublicId != "") + assert.NoError(err) + assert.NotEmpty(s.PublicId) id, err := uuid.GenerateUUID() - assert.Nil(err) + assert.NoError(err) projScope, err := NewProject(s.PublicId, WithDescription(id)) - assert.Nil(err) - assert.True(projScope.Scope != nil) + assert.NoError(err) + assert.NotNil(projScope.Scope) assert.Equal(projScope.GetParentId(), s.PublicId) assert.Equal(projScope.GetDescription(), id) }) t.Run("unknown-scope", func(t *testing.T) { s, err := newScope(UnknownScope) - assert.True(err != nil) - assert.True(s == nil) + assert.NotNil(err) + assert.Nil(s) assert.Equal(err.Error(), "error unknown scope type for new scope") }) t.Run("proj-scope-with-no-org", func(t *testing.T) { s, err := NewProject("") - assert.True(err != nil) - assert.True(s == nil) + assert.NotNil(err) + assert.Nil(s) assert.Equal(err.Error(), "error creating new project: error project scope parent id is unset") }) } func Test_ScopeCreate(t *testing.T) { t.Parallel() cleanup, conn, _ := db.TestSetup(t, "postgres") - defer cleanup() + defer func() { + if err := cleanup(); err != nil { + t.Error(err) + } + }() assert := assert.New(t) defer conn.Close() t.Run("valid", func(t *testing.T) { w := db.New(conn) s, err := NewOrganization() - assert.Nil(err) - assert.True(s.Scope != nil) + assert.NoError(err) + assert.NotNil(s.Scope) err = w.Create(context.Background(), s) - assert.Nil(err) - assert.True(s.PublicId != "") + assert.NoError(err) + assert.NotEmpty(s.PublicId) }) t.Run("valid-with-parent", func(t *testing.T) { w := db.New(conn) s, err := NewOrganization() - assert.Nil(err) - assert.True(s.Scope != nil) + assert.NoError(err) + assert.NotNil(s.Scope) err = w.Create(context.Background(), s) - assert.Nil(err) - assert.True(s.PublicId != "") + assert.NoError(err) + assert.NotEmpty(s.PublicId) id, err := uuid.GenerateUUID() - assert.Nil(err) + assert.NoError(err) project, err := NewProject(s.PublicId, WithDescription(id)) - assert.Nil(err) - assert.True(project.Scope != nil) + assert.NoError(err) + assert.NotNil(project.Scope) assert.Equal(project.Scope.ParentId, s.PublicId) assert.Equal(project.GetDescription(), id) err = w.Create(context.Background(), project) - assert.Nil(err) + assert.NoError(err) assert.Equal(project.ParentId, s.PublicId) }) } @@ -90,67 +98,86 @@ func Test_ScopeCreate(t *testing.T) { func Test_ScopeUpdate(t *testing.T) { t.Parallel() cleanup, conn, _ := db.TestSetup(t, "postgres") - defer cleanup() + defer func() { + if err := cleanup(); err != nil { + t.Error(err) + } + }() assert := assert.New(t) defer conn.Close() t.Run("valid", func(t *testing.T) { w := db.New(conn) s, err := NewOrganization() - assert.Nil(err) - assert.True(s.Scope != nil) + assert.NoError(err) + assert.NotNil(s.Scope) err = w.Create(context.Background(), s) - assert.Nil(err) - assert.True(s.PublicId != "") + assert.NoError(err) + assert.NotEmpty(s.PublicId) id, err := uuid.GenerateUUID() - assert.Nil(err) + assert.NoError(err) s.Name = id updatedRows, err := w.Update(context.Background(), s, []string{"Name"}) - assert.Nil(err) + assert.NoError(err) assert.Equal(1, updatedRows) }) t.Run("type-update-not-allowed", func(t *testing.T) { w := db.New(conn) s, err := NewOrganization() - assert.Nil(err) - assert.True(s.Scope != nil) + assert.NoError(err) + assert.NotNil(s.Scope) err = w.Create(context.Background(), s) - assert.Nil(err) - assert.True(s.PublicId != "") + assert.NoError(err) + assert.NotEmpty(s.PublicId) s.Type = ProjectScope.String() updatedRows, err := w.Update(context.Background(), s, []string{"Type"}) - assert.True(err != nil) + assert.NotNil(err) assert.Equal(0, updatedRows) }) } func Test_ScopeGetScope(t *testing.T) { t.Parallel() cleanup, conn, _ := db.TestSetup(t, "postgres") - defer cleanup() + defer func() { + if err := cleanup(); err != nil { + t.Error(err) + } + }() assert := assert.New(t) defer conn.Close() t.Run("valid-scope", func(t *testing.T) { w := db.New(conn) s, err := NewOrganization() - assert.Nil(err) - assert.True(s.Scope != nil) + assert.NoError(err) + assert.NotNil(s.Scope) err = w.Create(context.Background(), s) - assert.Nil(err) - assert.True(s.PublicId != "") + assert.NoError(err) + assert.NotEmpty(s.PublicId) + + foundScope, err := s.GetScope(context.Background(), w) + assert.NoError(err) + assert.Nil(foundScope) project, err := NewProject(s.PublicId) - assert.Nil(err) - assert.True(project.Scope != nil) + assert.NoError(err) + assert.NotNil(project.Scope) assert.Equal(project.ParentId, s.PublicId) err = w.Create(context.Background(), project) - assert.Nil(err) + assert.NoError(err) projectOrg, err := project.GetScope(context.Background(), w) - assert.Nil(err) - assert.True(projectOrg != nil) + assert.NoError(err) + assert.NotNil(projectOrg) assert.Equal(projectOrg.PublicId, project.ParentId) + + p := allocScope() + p.PublicId = project.PublicId + p.Type = ProjectScope.String() + projectOrg, err = p.GetScope(context.Background(), w) + assert.NoError(err) + assert.Equal(s.PublicId, projectOrg.PublicId) }) } @@ -171,7 +198,7 @@ func TestScope_Actions(t *testing.T) { func TestScope_ResourceType(t *testing.T) { assert := assert.New(t) o, err := NewOrganization() - assert.Nil(err) + assert.NoError(err) ty := o.ResourceType() assert.Equal(ty, ResourceTypeOrganization) } @@ -179,18 +206,22 @@ func TestScope_ResourceType(t *testing.T) { func TestScope_Clone(t *testing.T) { t.Parallel() cleanup, conn, _ := db.TestSetup(t, "postgres") - defer cleanup() + defer func() { + if err := cleanup(); err != nil { + t.Error(err) + } + }() assert := assert.New(t) defer conn.Close() t.Run("valid", func(t *testing.T) { w := db.New(conn) s, err := NewOrganization() - assert.Nil(err) - assert.True(s.Scope != nil) + assert.NoError(err) + assert.NotNil(s.Scope) err = w.Create(context.Background(), s) - assert.Nil(err) - assert.True(s.PublicId != "") + assert.NoError(err) + assert.NotEmpty(s.PublicId) cp := s.Clone() assert.True(proto.Equal(cp.(*Scope).Scope, s.Scope)) @@ -198,18 +229,18 @@ func TestScope_Clone(t *testing.T) { t.Run("not-equal", func(t *testing.T) { w := db.New(conn) s, err := NewOrganization() - assert.Nil(err) - assert.True(s.Scope != nil) + assert.NoError(err) + assert.NotNil(s.Scope) err = w.Create(context.Background(), s) - assert.Nil(err) - assert.True(s.PublicId != "") + assert.NoError(err) + assert.NotEmpty(s.PublicId) s2, err := NewOrganization() - assert.Nil(err) - assert.True(s2.Scope != nil) + assert.NoError(err) + assert.NotNil(s2.Scope) err = w.Create(context.Background(), s2) - assert.Nil(err) - assert.True(s2.PublicId != "") + assert.NoError(err) + assert.NotEmpty(s2.PublicId) cp := s.Clone() assert.True(!proto.Equal(cp.(*Scope).Scope, s2.Scope)) diff --git a/internal/iam/store/scanners_test.go b/internal/iam/store/scanners_test.go index 3eb4c05cb..8e955b4ee 100644 --- a/internal/iam/store/scanners_test.go +++ b/internal/iam/store/scanners_test.go @@ -15,19 +15,19 @@ func Test_TimestampValue(t *testing.T) { t.Run("valid", func(t *testing.T) { ts := Timestamp{Timestamp: ×tamp.Timestamp{Seconds: 0, Nanos: 0}} v, err := ts.Value() - assert.Nil(err) + assert.NoError(err) assert.Equal(v, utcDate(1970, 1, 1)) }) t.Run("valid nil ts", func(t *testing.T) { var ts *Timestamp v, err := ts.Value() - assert.Nil(err) + assert.NoError(err) assert.Equal(v, nil) }) t.Run("invalid ts", func(t *testing.T) { ts := Timestamp{Timestamp: ×tamp.Timestamp{Seconds: maxValidSeconds, Nanos: 0}} v, err := ts.Value() - assert.True(err != nil) + assert.NotNil(err) assert.Equal(err.Error(), "timestamp: seconds:253402300800 after 10000-01-01") assert.Equal(v, utcDate(10000, 1, 1)) }) @@ -40,28 +40,28 @@ func Test_TimestampScan(t *testing.T) { v := time.Unix(0, 0) ts := Timestamp{} err := ts.Scan(v) - assert.Nil(err) + assert.NoError(err) assert.True(reflect.DeepEqual(ts.Timestamp, ×tamp.Timestamp{Seconds: 0, Nanos: 0})) }) t.Run("valid default time", func(t *testing.T) { var v time.Time ts := Timestamp{} err := ts.Scan(v) - assert.Nil(err) + assert.NoError(err) assert.True(reflect.DeepEqual(ts.Timestamp, ×tamp.Timestamp{Seconds: -62135596800, Nanos: 0})) }) t.Run("invalid type", func(t *testing.T) { v := 1 ts := Timestamp{} err := ts.Scan(v) - assert.True(err != nil) + assert.NotNil(err) assert.Equal(err.Error(), "Not a protobuf Timestamp") }) t.Run("invalid time", func(t *testing.T) { v := time.Unix(maxValidSeconds, 0) ts := Timestamp{} err := ts.Scan(v) - assert.True(err != nil) + assert.NotNil(err) assert.Equal(err.Error(), "error converting the timestamp: timestamp: seconds:253402300800 after 10000-01-01") }) } diff --git a/internal/servers/controller/handlers/projects/project_service.go b/internal/servers/controller/handlers/projects/project_service.go index 748b4cc3b..92d9083a9 100644 --- a/internal/servers/controller/handlers/projects/project_service.go +++ b/internal/servers/controller/handlers/projects/project_service.go @@ -72,7 +72,7 @@ func (s Service) UpdateProject(ctx context.Context, req *pbs.UpdateProjectReques } func (s Service) getFromRepo(ctx context.Context, req *pbs.GetProjectRequest) (*pb.Project, error) { - p, err := s.repo.LookupScope(ctx, iam.WithPublicId(req.GetId())) + p, err := s.repo.LookupScope(ctx, req.Id) if err != nil { return nil, err }