Added DeleteScope() with tests and TestVerifyOplog() for easier testing (#49)

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)

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")
 	}
 }

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,

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,

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
+	}
+}

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)
+		})
+	}
+}

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,
 	}
 }
 

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)
 	})
 }

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)

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
 }

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)
 	})
 }

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")
+	})
+}

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

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))

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: &timestamp.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: &timestamp.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, &timestamp.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, &timestamp.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")
 	})
 }

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
 	}