Applying Host Catalog comments to Projects (#105)

* NewService for the project service now returns a non pointer and an error.

* Replacing t.Fatal calls with require.NoError calls.

* Initialize empty slices following the recommended way.

* Don't inline the prefix strings when validating requests.

* Check error when cleaning up the test.

* Add check that update updates the time.

* Use google.golang.org/protobuf where possible instead of github.com/golang/protobuf.

* Shorten the validateX function names.

* Inline API responses.

* Change test cleanup error logging to t.Errorf.

* Remove unneeded creation of an extra project.

* Inline regex check for invalid id.

* Build the pb.Project inline when converting from db to wire proto.

* Change stored repos into repo creating functions.

* Using iam.TestScopes instead of creating them myself.

* Remove out of date comments.

* Update name to match go test naming standards.

* Adding project service comments.

* Using scopeType's new prefix method inside scope.go

* Setting test project's name and desc to default to pass tests.

* Change host catalog to project in test.

* Fixing merge errors.

* Fixing some more merge errors.

* Fix typo in comments.

* Fix typos in project_service.go.

* Update error indicating provided id and the expected prefix.

* Make the RepoFactory types in a common directory.

internal/gen/controller/api/services/host_catalog_service.pb.go

@@ -1025,26 +1025,34 @@ const _ = grpc.SupportPackageIsVersion6
 //
 // For semantics around ctx use and closing/ending streaming RPCs, please refer to https://godoc.org/google.golang.org/grpc#ClientConn.NewStream.
 type HostCatalogServiceClient interface {
-	// GetHostCatalog returns a stored Host Catalog if present.  The provided request must include the org, project, and
-	// catalog id for the host catalog being retrieved. If any of those ids are missing, malformed, or reference a non
-	// existing resource an error is returned.
+	// GetHostCatalog returns a stored Host Catalog if present.  The provided
+	// request must include the org, project, and catalog id for the host catalog
+	//being retrieved. If any of those ids are missing, malformed, or reference a
+	// non existing resource an error is returned.
 	GetHostCatalog(ctx context.Context, in *GetHostCatalogRequest, opts ...grpc.CallOption) (*GetHostCatalogResponse, error)
-	// ListHostCatalogs returns a list of stored HostCatalogs which exist inside the project referenced in the request.
-	// The request must include the org and project ids for the HostCatalogs being retrieved.
-	// If any of those ids are missing, malformed, or reference a non existing parent resource an error is returned.
+	// ListHostCatalogs returns a list of stored HostCatalogs which exist inside
+	// the project referenced in the request. The request must include the org
+	// and project ids for the HostCatalogs being retrieved. If any of those ids
+	// are missing, malformed, or reference a non existing parent resource an
+	// error is returned.
 	ListHostCatalogs(ctx context.Context, in *ListHostCatalogsRequest, opts ...grpc.CallOption) (*ListHostCatalogsResponse, error)
-	// CreateHostCatalog creates and stores a HostCatalog in watchtower.  The provided request must include the org, and
-	// project id in which the catalog will be created and the catalog type to be created. If any of these required fields
-	// are missing, malformed, or reference a non existing parent resource an error is returned.
+	// CreateHostCatalog creates and stores a HostCatalog in watchtower.  The
+	// provided request must include the org, and project id in which the catalog
+	// will be created and the catalog type to be created. If any of these
+	// required fields are missing, malformed, or reference a non existing parent
+	// resource an error is returned.
 	CreateHostCatalog(ctx context.Context, in *CreateHostCatalogRequest, opts ...grpc.CallOption) (*CreateHostCatalogResponse, error)
-	// UpdateHostCatalog updates an exist HostCatalog in Watchtower.  The provided HostCatalog must not have any read only
-	// fields set except for potentially the id.  The update mask must be provided and include at least 1 mutable
-	// field.  To unset a field's value include that field name in the update mask and don't set the field in the HostCatalog.
+	// UpdateHostCatalog updates an exist HostCatalog in Watchtower.  The
+	// provided HostCatalog must not have any read only fields set.
+	// The update mask must be provided and include at least 1 mutable field.
+	// To unset a field's value include that field name in the update mask
+	// and don't set the field in the HostCatalog.
 	UpdateHostCatalog(ctx context.Context, in *UpdateHostCatalogRequest, opts ...grpc.CallOption) (*UpdateHostCatalogResponse, error)
-	// DeleteHostCatalog removes a HostCatalog and all associated hosts and host sets from Watchtower.  If the provided
-	// Org, Project, or HostCatalog ids are malformed or not provided DeleteHostCatalog returns an error.  It is not
-	// an error to call DeleteHostCatalog for an Org, Project, or HostCatalog that does not exist as the response indicates
-	// if it existed or not.
+	// DeleteHostCatalog removes a HostCatalog and all associated hosts and host
+	// sets from Watchtower.  If the provided Org, Project, or HostCatalog ids
+	// are malformed or not provided DeleteHostCatalog returns an error.  It is
+	// not an error to call DeleteHostCatalog for an Org, Project, or HostCatalog
+	// that does not exist as the response indicates if it existed or not.
 	DeleteHostCatalog(ctx context.Context, in *DeleteHostCatalogRequest, opts ...grpc.CallOption) (*DeleteHostCatalogResponse, error)
 }
 
@@ -1103,26 +1111,34 @@ func (c *hostCatalogServiceClient) DeleteHostCatalog(ctx context.Context, in *De
 
 // HostCatalogServiceServer is the server API for HostCatalogService service.
 type HostCatalogServiceServer interface {
-	// GetHostCatalog returns a stored Host Catalog if present.  The provided request must include the org, project, and
-	// catalog id for the host catalog being retrieved. If any of those ids are missing, malformed, or reference a non
-	// existing resource an error is returned.
+	// GetHostCatalog returns a stored Host Catalog if present.  The provided
+	// request must include the org, project, and catalog id for the host catalog
+	//being retrieved. If any of those ids are missing, malformed, or reference a
+	// non existing resource an error is returned.
 	GetHostCatalog(context.Context, *GetHostCatalogRequest) (*GetHostCatalogResponse, error)
-	// ListHostCatalogs returns a list of stored HostCatalogs which exist inside the project referenced in the request.
-	// The request must include the org and project ids for the HostCatalogs being retrieved.
-	// If any of those ids are missing, malformed, or reference a non existing parent resource an error is returned.
+	// ListHostCatalogs returns a list of stored HostCatalogs which exist inside
+	// the project referenced in the request. The request must include the org
+	// and project ids for the HostCatalogs being retrieved. If any of those ids
+	// are missing, malformed, or reference a non existing parent resource an
+	// error is returned.
 	ListHostCatalogs(context.Context, *ListHostCatalogsRequest) (*ListHostCatalogsResponse, error)
-	// CreateHostCatalog creates and stores a HostCatalog in watchtower.  The provided request must include the org, and
-	// project id in which the catalog will be created and the catalog type to be created. If any of these required fields
-	// are missing, malformed, or reference a non existing parent resource an error is returned.
+	// CreateHostCatalog creates and stores a HostCatalog in watchtower.  The
+	// provided request must include the org, and project id in which the catalog
+	// will be created and the catalog type to be created. If any of these
+	// required fields are missing, malformed, or reference a non existing parent
+	// resource an error is returned.
 	CreateHostCatalog(context.Context, *CreateHostCatalogRequest) (*CreateHostCatalogResponse, error)
-	// UpdateHostCatalog updates an exist HostCatalog in Watchtower.  The provided HostCatalog must not have any read only
-	// fields set except for potentially the id.  The update mask must be provided and include at least 1 mutable
-	// field.  To unset a field's value include that field name in the update mask and don't set the field in the HostCatalog.
+	// UpdateHostCatalog updates an exist HostCatalog in Watchtower.  The
+	// provided HostCatalog must not have any read only fields set.
+	// The update mask must be provided and include at least 1 mutable field.
+	// To unset a field's value include that field name in the update mask
+	// and don't set the field in the HostCatalog.
 	UpdateHostCatalog(context.Context, *UpdateHostCatalogRequest) (*UpdateHostCatalogResponse, error)
-	// DeleteHostCatalog removes a HostCatalog and all associated hosts and host sets from Watchtower.  If the provided
-	// Org, Project, or HostCatalog ids are malformed or not provided DeleteHostCatalog returns an error.  It is not
-	// an error to call DeleteHostCatalog for an Org, Project, or HostCatalog that does not exist as the response indicates
-	// if it existed or not.
+	// DeleteHostCatalog removes a HostCatalog and all associated hosts and host
+	// sets from Watchtower.  If the provided Org, Project, or HostCatalog ids
+	// are malformed or not provided DeleteHostCatalog returns an error.  It is
+	// not an error to call DeleteHostCatalog for an Org, Project, or HostCatalog
+	// that does not exist as the response indicates if it existed or not.
 	DeleteHostCatalog(context.Context, *DeleteHostCatalogRequest) (*DeleteHostCatalogResponse, error)
 }
 

internal/gen/controller/api/services/project_service.pb.go

@@ -933,10 +933,36 @@ const _ = grpc.SupportPackageIsVersion6
 //
 // For semantics around ctx use and closing/ending streaming RPCs, please refer to https://godoc.org/google.golang.org/grpc#ClientConn.NewStream.
 type ProjectServiceClient interface {
+	// GetProject returns a stored Project if present.  The provided request
+	// must include the org and project id for the project being retrieved. If
+	// any of those ids are missing, malformed or reference a non existing
+	// resource an error is returned.
 	GetProject(ctx context.Context, in *GetProjectRequest, opts ...grpc.CallOption) (*GetProjectResponse, error)
+	// ListProjects returns a list of stored projects which exist inside the org
+	// referenced inside the request.  The request must include the org id for
+	// the projects being retrieved.  If the org id is missing, malformed, or
+	// reference a non existing organization, an error is returned.
 	ListProjects(ctx context.Context, in *ListProjectsRequest, opts ...grpc.CallOption) (*ListProjectsResponse, error)
+	// CreateProject creates and stores a project in watchtower.  The provided
+	// request must include the org id in which the project will be created.
+	// If the org id is missing, malformed or references a non existing
+	// organization, an error is returned.  If a name is provided that is in
+	// use in another project in the same organization, an error is returned.
 	CreateProject(ctx context.Context, in *CreateProjectRequest, opts ...grpc.CallOption) (*CreateProjectResponse, error)
+	// UpdateProject updates an existing project in watchtower.  The provided
+	// project must not have any read only fields set.  The update mask must be
+	// included in the request and contain at least 1 mutable field.  To unset
+	// a field's value, include the field in the update mask and don't set it
+	// in the provided project. An error is returned if either the organization
+	// or project ids are missing or reference a non existing resource.  An error
+	// is also returned if the request attempts to update the name to one that is
+	// already in use in this organization.
 	UpdateProject(ctx context.Context, in *UpdateProjectRequest, opts ...grpc.CallOption) (*UpdateProjectResponse, error)
+	// DeleteProject remotes a project and all child resources from Watchtower.
+	// If the provided org or project ids are malformed or not provided an error
+	// is returned.  No error is returned if either ids reference resources that
+	// do not exist as the response itself specifies if the resource existed
+	// before the DeleteProject request was received.
 	DeleteProject(ctx context.Context, in *DeleteProjectRequest, opts ...grpc.CallOption) (*DeleteProjectResponse, error)
 }
 
@@ -995,10 +1021,36 @@ func (c *projectServiceClient) DeleteProject(ctx context.Context, in *DeleteProj
 
 // ProjectServiceServer is the server API for ProjectService service.
 type ProjectServiceServer interface {
+	// GetProject returns a stored Project if present.  The provided request
+	// must include the org and project id for the project being retrieved. If
+	// any of those ids are missing, malformed or reference a non existing
+	// resource an error is returned.
 	GetProject(context.Context, *GetProjectRequest) (*GetProjectResponse, error)
+	// ListProjects returns a list of stored projects which exist inside the org
+	// referenced inside the request.  The request must include the org id for
+	// the projects being retrieved.  If the org id is missing, malformed, or
+	// reference a non existing organization, an error is returned.
 	ListProjects(context.Context, *ListProjectsRequest) (*ListProjectsResponse, error)
+	// CreateProject creates and stores a project in watchtower.  The provided
+	// request must include the org id in which the project will be created.
+	// If the org id is missing, malformed or references a non existing
+	// organization, an error is returned.  If a name is provided that is in
+	// use in another project in the same organization, an error is returned.
 	CreateProject(context.Context, *CreateProjectRequest) (*CreateProjectResponse, error)
+	// UpdateProject updates an existing project in watchtower.  The provided
+	// project must not have any read only fields set.  The update mask must be
+	// included in the request and contain at least 1 mutable field.  To unset
+	// a field's value, include the field in the update mask and don't set it
+	// in the provided project. An error is returned if either the organization
+	// or project ids are missing or reference a non existing resource.  An error
+	// is also returned if the request attempts to update the name to one that is
+	// already in use in this organization.
 	UpdateProject(context.Context, *UpdateProjectRequest) (*UpdateProjectResponse, error)
+	// DeleteProject remotes a project and all child resources from Watchtower.
+	// If the provided org or project ids are malformed or not provided an error
+	// is returned.  No error is returned if either ids reference resources that
+	// do not exist as the response itself specifies if the resource existed
+	// before the DeleteProject request was received.
 	DeleteProject(context.Context, *DeleteProjectRequest) (*DeleteProjectResponse, error)
 }
 

internal/iam/scope.go

@@ -28,6 +28,14 @@ func (s ScopeType) String() string {
 	}[s]
 }
 
+func (s ScopeType) Prefix() string {
+	return [...]string{
+		"unknown",
+		"o",
+		"p",
+	}[s]
+}
+
 // Scope is used to create a hierarchy of "containers" that encompass the scope of
 // an IAM resource.  Scopes are Organizations and Projects.
 type Scope struct {
@@ -79,19 +87,15 @@ func newScope(scopeType ScopeType, opt ...Option) (*Scope, error) {
 			return nil, errors.New("error project scope parent id is unset")
 		}
 	}
-	prefix := "p"
-	if scopeType == OrganizationScope {
-		prefix = "o"
-	}
 	var publicId string
 	if withPublicId != "" {
-		if !strings.HasPrefix(withPublicId, prefix+"_") {
-			return nil, errors.New("passed-in public ID has wrong prefix for type")
+		if !strings.HasPrefix(withPublicId, scopeType.Prefix()+"_") {
+			return nil, fmt.Errorf("passed-in public ID %q has wrong prefix for type %q which uses prefix %q", withPublicId, scopeType.String(), scopeType.Prefix())
 		}
 		publicId = withPublicId
 	} else {
 		var err error
-		publicId, err = db.NewPublicId(prefix)
+		publicId, err = db.NewPublicId(scopeType.Prefix())
 		if err != nil {
 			return nil, fmt.Errorf("error generating public id %w for new scope", err)
 		}

internal/proto/local/controller/api/services/v1/project_service.proto

@@ -11,6 +11,11 @@ import "google/protobuf/wrappers.proto";
 import "controller/api/resources/scopes/v1/project.proto";
 
 service ProjectService {
+
+  // GetProject returns a stored Project if present.  The provided request
+  // must include the org and project id for the project being retrieved. If
+  // any of those ids are missing, malformed or reference a non existing
+  // resource an error is returned.
   rpc GetProject(GetProjectRequest) returns (GetProjectResponse) {
     option (google.api.http) = {
       get: "/v1/orgs/{org_id}/projects/{id}"
@@ -20,6 +25,11 @@ service ProjectService {
       summary: "Gets a single Project"
     };
   }
+
+  // ListProjects returns a list of stored projects which exist inside the org
+  // referenced inside the request.  The request must include the org id for
+  // the projects being retrieved.  If the org id is missing, malformed, or
+  // reference a non existing organization, an error is returned.
   rpc ListProjects(ListProjectsRequest) returns (ListProjectsResponse) {
     option (google.api.http) = {
       get: "/v1/orgs/{org_id}/projects"
@@ -28,6 +38,12 @@ service ProjectService {
       summary: "Lists all Projects"
     };
   }
+
+  // CreateProject creates and stores a project in watchtower.  The provided
+  // request must include the org id in which the project will be created.
+  // If the org id is missing, malformed or references a non existing
+  // organization, an error is returned.  If a name is provided that is in
+  // use in another project in the same organization, an error is returned.
   rpc CreateProject(CreateProjectRequest) returns (CreateProjectResponse) {
     option (google.api.http) = {
       post: "/v1/orgs/{org_id}/projects"
@@ -38,6 +54,15 @@ service ProjectService {
       summary: "Creates a single Project"
     };
   }
+
+  // UpdateProject updates an existing project in watchtower.  The provided
+  // project must not have any read only fields set.  The update mask must be
+  // included in the request and contain at least 1 mutable field.  To unset
+  // a field's value, include the field in the update mask and don't set it
+  // in the provided project. An error is returned if either the organization
+  // or project ids are missing or reference a non existing resource.  An error
+  // is also returned if the request attempts to update the name to one that is
+  // already in use in this organization.
   rpc UpdateProject(UpdateProjectRequest) returns (UpdateProjectResponse) {
     option (google.api.http) = {
       patch: "/v1/orgs/{org_id}/projects/{id}"
@@ -48,6 +73,12 @@ service ProjectService {
       summary: "Updates a Project"
     };
   }
+
+  // DeleteProject remotes a project and all child resources from Watchtower.
+  // If the provided org or project ids are malformed or not provided an error
+  // is returned.  No error is returned if either ids reference resources that
+  // do not exist as the response itself specifies if the resource existed
+  // before the DeleteProject request was received.
   rpc DeleteProject(DeleteProjectRequest) returns (DeleteProjectResponse) {
     option (google.api.http) = {
       delete: "/v1/orgs/{org_id}/projects/{id}"

internal/servers/controller/common/common.go

@@ -0,0 +1,11 @@
+package common
+
+import (
+	"github.com/hashicorp/watchtower/internal/host/static"
+	"github.com/hashicorp/watchtower/internal/iam"
+)
+
+type (
+	IamRepoFactory    func() (*iam.Repository, error)
+	StaticRepoFactory func() (*static.Repository, error)
+)

internal/servers/controller/common/doc.go

@@ -0,0 +1,3 @@
+// Package common contains types and helper functions that are used across the different packages under
+// internal/server/controller.
+package common

internal/servers/controller/controller.go

@@ -10,6 +10,7 @@ import (
 	"github.com/hashicorp/watchtower/internal/db"
 	"github.com/hashicorp/watchtower/internal/host/static"
 	"github.com/hashicorp/watchtower/internal/iam"
+	"github.com/hashicorp/watchtower/internal/servers/controller/common"
 )
 
 type Controller struct {
@@ -19,9 +20,9 @@ type Controller struct {
 	baseContext context.Context
 	baseCancel  context.CancelFunc
 
-	// Repos
-	IamRepoFn        func() (*iam.Repository, error)
-	StaticHostRepoFn func() (*static.Repository, error)
+	// Repo factory methods
+	IamRepoFn        common.IamRepoFactory
+	StaticHostRepoFn common.StaticRepoFactory
 }
 
 func New(conf *Config) (*Controller, error) {

internal/servers/controller/handler.go

@@ -77,7 +77,11 @@ func handleGrpcGateway(c *Controller) (http.Handler, error) {
 	if err := services.RegisterHostServiceHandlerServer(ctx, mux, &hosts.Service{}); err != nil {
 		return nil, fmt.Errorf("failed to register host service handler: %w", err)
 	}
-	if err := services.RegisterProjectServiceHandlerServer(ctx, mux, projects.NewService(c.IamRepoFn)); err != nil {
+	ps, err := projects.NewService(c.IamRepoFn)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create project handler service: %w", err)
+	}
+	if err := services.RegisterProjectServiceHandlerServer(ctx, mux, ps); err != nil {
 		return nil, fmt.Errorf("failed to register project service handler: %w", err)
 	}
 

internal/servers/controller/handlers/host_catalogs/host_catalog_service.go

@@ -9,6 +9,7 @@ import (
 	pb "github.com/hashicorp/watchtower/internal/gen/controller/api/resources/hosts"
 	pbs "github.com/hashicorp/watchtower/internal/gen/controller/api/services"
 	"github.com/hashicorp/watchtower/internal/host/static"
+	"github.com/hashicorp/watchtower/internal/servers/controller/common"
 	"github.com/hashicorp/watchtower/internal/servers/controller/handlers"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -69,14 +70,14 @@ var (
 )
 
 type Service struct {
-	staticRepoFn func() (*static.Repository, error)
+	staticRepoFn common.StaticRepoFactory
 }
 
 var _ pbs.HostCatalogServiceServer = Service{}
 
 // NewService returns a host catalog Service which handles host catalog related requests to watchtower and uses the provided
 // repositories for storage and retrieval.
-func NewService(repoFn func() (*static.Repository, error)) (Service, error) {
+func NewService(repoFn common.StaticRepoFactory) (Service, error) {
 	if repoFn == nil {
 		return Service{}, fmt.Errorf("nil static repository provided")
 	}

internal/servers/controller/handlers/projects/project_service.go

@@ -6,13 +6,14 @@ import (
 	"regexp"
 	"strings"
 
-	"github.com/golang/protobuf/ptypes/wrappers"
 	pb "github.com/hashicorp/watchtower/internal/gen/controller/api/resources/scopes"
 	pbs "github.com/hashicorp/watchtower/internal/gen/controller/api/services"
 	"github.com/hashicorp/watchtower/internal/iam"
+	"github.com/hashicorp/watchtower/internal/servers/controller/common"
 	"github.com/hashicorp/watchtower/internal/servers/controller/handlers"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/types/known/wrapperspb"
 )
 
 var (
@@ -24,75 +25,76 @@ var (
 	}
 )
 
+// Service handles request as described by the pbs.ProjectServiceServer interface.
 type Service struct {
-	pbs.UnimplementedProjectServiceServer
-	repoFn func() (*iam.Repository, error)
+	repo common.IamRepoFactory
 }
 
-func NewService(repoFn func() (*iam.Repository, error)) *Service {
-	if repoFn == nil {
-		return nil
+// NewService returns a project service which handles project related requests to watchtower.
+func NewService(repo common.IamRepoFactory) (Service, error) {
+	if repo == nil {
+		return Service{}, fmt.Errorf("nil iam repository provided")
 	}
-	return &Service{repoFn: repoFn}
+	return Service{repo: repo}, nil
 }
 
-var _ pbs.ProjectServiceServer = &Service{}
+var _ pbs.ProjectServiceServer = Service{}
 
+// ListProjects is not yet implemented but will implement the interface pbs.ProjectServiceServer.
+func (s Service) ListProjects(ctx context.Context, req *pbs.ListProjectsRequest) (*pbs.ListProjectsResponse, error) {
+	return nil, status.Error(codes.Unimplemented, "List not enabled for this resource.")
+}
+
+// GetProjects implements the interface pbs.ProjectServiceServer.
 func (s Service) GetProject(ctx context.Context, req *pbs.GetProjectRequest) (*pbs.GetProjectResponse, error) {
-	if err := validateGetProjectRequest(req); err != nil {
+	if err := validateGetRequest(req); err != nil {
 		return nil, err
 	}
 	p, err := s.getFromRepo(ctx, req.GetId())
 	if err != nil {
 		return nil, err
 	}
-	resp := &pbs.GetProjectResponse{}
-	resp.Item = p
-	return resp, nil
+	return &pbs.GetProjectResponse{Item: p}, nil
 }
 
+// CreateProject implements the interface pbs.ProjectServiceServer.
 func (s Service) CreateProject(ctx context.Context, req *pbs.CreateProjectRequest) (*pbs.CreateProjectResponse, error) {
-	if err := validateCreateProjectRequest(req); err != nil {
+	if err := validateCreateRequest(req); err != nil {
 		return nil, err
 	}
 	p, err := s.createInRepo(ctx, req.GetOrgId(), req.GetItem())
 	if err != nil {
 		return nil, err
 	}
-	resp := &pbs.CreateProjectResponse{}
-	resp.Uri = fmt.Sprintf("orgs/%s/projects/%s", req.GetOrgId(), p.GetId())
-	resp.Item = p
-	return resp, nil
+	return &pbs.CreateProjectResponse{Item: p, Uri: fmt.Sprintf("orgs/%s/projects/%s", req.GetOrgId(), p.GetId())}, nil
 }
 
+// UpdateProject implements the interface pbs.ProjectServiceServer.
 func (s Service) UpdateProject(ctx context.Context, req *pbs.UpdateProjectRequest) (*pbs.UpdateProjectResponse, error) {
-	if err := validateUpdateProjectRequest(req); err != nil {
+	if err := validateUpdateRequest(req); err != nil {
 		return nil, err
 	}
 	p, err := s.updateInRepo(ctx, req.GetOrgId(), req.GetId(), req.GetUpdateMask().GetPaths(), req.GetItem())
 	if err != nil {
 		return nil, err
 	}
-	resp := &pbs.UpdateProjectResponse{}
-	resp.Item = p
-	return resp, nil
+	return &pbs.UpdateProjectResponse{Item: p}, nil
 }
 
+// DeleteProject implements the interface pbs.ProjectServiceServer.
 func (s Service) DeleteProject(ctx context.Context, req *pbs.DeleteProjectRequest) (*pbs.DeleteProjectResponse, error) {
-	if err := validateDeleteProjectRequest(req); err != nil {
+	if err := validateDeleteRequest(req); err != nil {
 		return nil, err
 	}
 	existed, err := s.deleteFromRepo(ctx, req.GetId())
 	if err != nil {
 		return nil, err
 	}
-	resp := &pbs.DeleteProjectResponse{}
-	resp.Existed = existed
-	return resp, nil
+	return &pbs.DeleteProjectResponse{Existed: existed}, nil
 }
 
 func (s Service) getFromRepo(ctx context.Context, id string) (*pb.Project, error) {
-	repo, err := s.repoFn()
+	repo, err := s.repo()
 	if err != nil {
 		return nil, err
 	}
@@ -107,7 +109,7 @@ func (s Service) getFromRepo(ctx context.Context, id string) (*pb.Project, error
 }
 
 func (s Service) createInRepo(ctx context.Context, orgID string, item *pb.Project) (*pb.Project, error) {
-	opts := []iam.Option{}
+	var opts []iam.Option
 	if item.GetName() != nil {
 		opts = append(opts, iam.WithName(item.GetName().GetValue()))
 	}
@@ -118,7 +120,7 @@ func (s Service) createInRepo(ctx context.Context, orgID string, item *pb.Projec
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "Unable to build project for creation: %v.", err)
 	}
-	repo, err := s.repoFn()
+	repo, err := s.repo()
 	if err != nil {
 		return nil, err
 	}
@@ -133,7 +135,7 @@ func (s Service) createInRepo(ctx context.Context, orgID string, item *pb.Projec
 }
 
 func (s Service) updateInRepo(ctx context.Context, orgID, projId string, mask []string, item *pb.Project) (*pb.Project, error) {
-	opts := []iam.Option{iam.WithPublicId(projId)}
+	var opts []iam.Option
 	if desc := item.GetDescription(); desc != nil {
 		opts = append(opts, iam.WithDescription(desc.GetValue()))
 	}
@@ -144,6 +146,7 @@ func (s Service) updateInRepo(ctx context.Context, orgID, projId string, mask []
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "Unable to build project for update: %v.", err)
 	}
+	p.PublicId = projId
 	dbMask, err := toDbUpdateMask(mask)
 	if err != nil {
 		return nil, err
@@ -151,7 +154,7 @@ func (s Service) updateInRepo(ctx context.Context, orgID, projId string, mask []
 	if len(dbMask) == 0 {
 		return nil, handlers.InvalidArgumentErrorf("No valid fields included in the update mask.", []string{"update_mask"})
 	}
-	repo, err := s.repoFn()
+	repo, err := s.repo()
 	if err != nil {
 		return nil, err
 	}
@@ -166,7 +169,7 @@ func (s Service) updateInRepo(ctx context.Context, orgID, projId string, mask []
 }
 
 func (s Service) deleteFromRepo(ctx context.Context, projId string) (bool, error) {
-	repo, err := s.repoFn()
+	repo, err := s.repo()
 	if err != nil {
 		return false, err
 	}
@@ -179,8 +182,8 @@ func (s Service) deleteFromRepo(ctx context.Context, projId string) (bool, error
 
 // toDbUpdateMask converts the wire format's FieldMask into a list of strings containing FieldMask paths used
 func toDbUpdateMask(paths []string) ([]string, error) {
-	dbPaths := []string{}
-	invalid := []string{}
+	var dbPaths []string
+	var invalid []string
 	for _, p := range paths {
 		for _, f := range strings.Split(p, ",") {
 			if dbField, ok := wireToStorageMask[strings.TrimSpace(f)]; ok {
@@ -197,15 +200,17 @@ func toDbUpdateMask(paths []string) ([]string, error) {
 }
 
 func toProto(in *iam.Scope) *pb.Project {
-	out := pb.Project{Id: in.GetPublicId()}
+	out := pb.Project{
+		Id:          in.GetPublicId(),
+		CreatedTime: in.GetCreateTime().GetTimestamp(),
+		UpdatedTime: in.GetUpdateTime().GetTimestamp(),
+	}
 	if in.GetDescription() != "" {
-		out.Description = &wrappers.StringValue{Value: in.GetDescription()}
+		out.Description = &wrapperspb.StringValue{Value: in.GetDescription()}
 	}
 	if in.GetName() != "" {
-		out.Name = &wrappers.StringValue{Value: in.GetName()}
+		out.Name = &wrapperspb.StringValue{Value: in.GetName()}
 	}
-	out.CreatedTime = in.GetCreateTime().GetTimestamp()
-	out.UpdatedTime = in.GetUpdateTime().GetTimestamp()
 	return &out
 }
 
@@ -215,35 +220,25 @@ func toProto(in *iam.Scope) *pb.Project {
 //  * All required parameters are set
 //  * There are no conflicting parameters provided
 // TODO: Populate the error in a way to allow it to be converted to the previously described error format and include all invalid fields instead of just the most recent.
-func validateGetProjectRequest(req *pbs.GetProjectRequest) error {
+func validateGetRequest(req *pbs.GetProjectRequest) error {
 	if err := validateAncestors(req); err != nil {
 		return err
 	}
-	badFormat := []string{}
-	if !validID(req.GetId(), "p_") {
-		badFormat = append(badFormat, "id")
-	}
-	if !validID(req.GetOrgId(), "o_") {
-		badFormat = append(badFormat, "id")
-	}
-	if len(badFormat) > 0 {
-		return handlers.InvalidArgumentErrorf("Improperly formatted identifier.", badFormat)
+	if !validId(req.GetId(), iam.ProjectScope.Prefix()+"_") {
+		return handlers.InvalidArgumentErrorf("Improperly formatted identifier.", []string{"id"})
 	}
 	return nil
 }
 
-func validateCreateProjectRequest(req *pbs.CreateProjectRequest) error {
+func validateCreateRequest(req *pbs.CreateProjectRequest) error {
 	if err := validateAncestors(req); err != nil {
 		return err
 	}
-	if !validID(req.GetOrgId(), "o_") {
-		handlers.InvalidArgumentErrorf("Improperly formatted identifier.", []string{"org_id"})
-	}
 	item := req.GetItem()
 	if item == nil {
 		return handlers.InvalidArgumentErrorf("A project's fields must be set to something.", []string{"item"})
 	}
-	immutableFieldsSet := []string{}
+	var immutableFieldsSet []string
 	if item.GetId() != "" {
 		immutableFieldsSet = append(immutableFieldsSet, "id")
 	}
@@ -259,19 +254,12 @@ func validateCreateProjectRequest(req *pbs.CreateProjectRequest) error {
 	return nil
 }
 
-func validateUpdateProjectRequest(req *pbs.UpdateProjectRequest) error {
+func validateUpdateRequest(req *pbs.UpdateProjectRequest) error {
 	if err := validateAncestors(req); err != nil {
 		return err
 	}
-	badFormat := []string{}
-	if !validID(req.GetId(), "p_") {
-		badFormat = append(badFormat, "id")
-	}
-	if !validID(req.GetOrgId(), "o_") {
-		badFormat = append(badFormat, "id")
-	}
-	if len(badFormat) > 0 {
-		handlers.InvalidArgumentErrorf("Improperly formatted identifier.", badFormat)
+	if !validId(req.GetId(), iam.ProjectScope.Prefix()+"_") {
+		return handlers.InvalidArgumentErrorf("Improperly formatted identifier.", []string{"id"})
 	}
 
 	if req.GetUpdateMask() == nil {
@@ -287,7 +275,7 @@ func validateUpdateProjectRequest(req *pbs.UpdateProjectRequest) error {
 	if item.GetId() != "" && item.GetId() != req.GetId() {
 		return handlers.InvalidArgumentErrorf("Id in provided item and url do not match.", []string{"id"})
 	}
-	immutableFieldsSet := []string{}
+	var immutableFieldsSet []string
 	if item.GetId() != "" {
 		immutableFieldsSet = append(immutableFieldsSet, "id")
 	}
@@ -304,32 +292,22 @@ func validateUpdateProjectRequest(req *pbs.UpdateProjectRequest) error {
 	return nil
 }
 
-func validateDeleteProjectRequest(req *pbs.DeleteProjectRequest) error {
+func validateDeleteRequest(req *pbs.DeleteProjectRequest) error {
 	if err := validateAncestors(req); err != nil {
 		return err
 	}
-	badFields := []string{}
-	if !validID(req.GetId(), "p_") {
-		badFields = append(badFields, "id")
-	}
-	if !validID(req.GetOrgId(), "o_") {
-		badFields = append(badFields, "org_id")
-	}
-	if len(badFields) > 0 {
-		return handlers.InvalidArgumentErrorf("Improperly formatted id.", badFields)
+	if !validId(req.GetId(), iam.ProjectScope.Prefix()+"_") {
+		return handlers.InvalidArgumentErrorf("Improperly formatted id.", []string{"id"})
 	}
 	return nil
 }
 
-func validID(id, prefix string) bool {
+func validId(id, prefix string) bool {
 	if !strings.HasPrefix(id, prefix) {
 		return false
 	}
 	id = strings.TrimPrefix(id, prefix)
-	if reInvalidID.Match([]byte(id)) {
-		return false
-	}
-	return true
+	return !reInvalidID.Match([]byte(id))
 }
 
 type ancestorProvider interface {
@@ -341,5 +319,8 @@ func validateAncestors(r ancestorProvider) error {
 	if r.GetOrgId() == "" {
 		return handlers.InvalidArgumentErrorf("Missing organization id.", []string{"org_id"})
 	}
+	if !validId(r.GetOrgId(), iam.OrganizationScope.Prefix()+"_") {
+		return handlers.InvalidArgumentErrorf("Poorly formatted org id.", []string{"org_id"})
+	}
 	return nil
 }

internal/servers/controller/handlers/projects/project_service_test.go

@@ -6,7 +6,6 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/golang/protobuf/proto"
 	"github.com/golang/protobuf/ptypes"
 	"github.com/golang/protobuf/ptypes/wrappers"
 	"github.com/hashicorp/watchtower/internal/db"
@@ -14,52 +13,46 @@ import (
 	pbs "github.com/hashicorp/watchtower/internal/gen/controller/api/services"
 	"github.com/hashicorp/watchtower/internal/iam"
 	"github.com/hashicorp/watchtower/internal/servers/controller/handlers/projects"
-	"github.com/stretchr/testify/require"
 	"google.golang.org/genproto/protobuf/field_mask"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/known/wrapperspb"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func createDefaultProjectAndRepo(t *testing.T) (*iam.Scope, func() (*iam.Repository, error)) {
 	t.Helper()
 	cleanup, conn, _ := db.TestSetup(t, "postgres")
 	t.Cleanup(func() {
-		conn.Close()
-		cleanup()
+		if err := conn.Close(); err != nil {
+			t.Errorf("Error when closing gorm DB: %v", err)
+		}
+		if err := cleanup(); err != nil {
+			t.Errorf("Error when cleaning up TestSetup: %v", err)
+		}
 	})
 	rw := db.New(conn)
 	wrap := db.TestWrapper(t)
 	repoFn := func() (*iam.Repository, error) {
 		return iam.NewRepository(rw, rw, wrap)
 	}
+
+	_, pRes := iam.TestScopes(t, conn)
+	pRes.Name = "default"
+	pRes.Description = "default"
 	repo, err := repoFn()
-	assert.Nil(t, err, "Unable to create new repoFn")
-
-	// Create a default org and project for our tests.
-	o, err := iam.NewOrganization(iam.WithName("default"))
-	if err != nil {
-		t.Fatalf("Could not get new org: %v", err)
-	}
-	oRes, err := repo.CreateScope(context.Background(), o)
-	if err != nil {
-		t.Fatalf("Could not create org scope: %v", err)
-	}
-
-	p, err := iam.NewProject(oRes.GetPublicId(), iam.WithName("default"), iam.WithDescription("default"))
-	if err != nil {
-		t.Fatalf("Could not get new project: %v", err)
-	}
-	pRes, err := repo.CreateScope(context.Background(), p)
-	if err != nil {
-		t.Fatalf("Could not create project scope: %v", err)
-	}
-
+	require.NoError(t, err)
+	pRes, _, err = repo.UpdateScope(context.Background(), pRes, []string{"Name", "Description"})
+	require.NoError(t, err)
 	return pRes, repoFn
 }
 
 func TestGet(t *testing.T) {
+	assert := assert.New(t)
+	require := require.New(t)
 	proj, repo := createDefaultProjectAndRepo(t)
 	toMerge := &pbs.GetProjectRequest{
 		OrgId: proj.GetParentId(),
@@ -68,8 +61,8 @@ func TestGet(t *testing.T) {
 
 	pProject := &pb.Project{
 		Id:          proj.GetPublicId(),
-		Name:        &wrappers.StringValue{Value: proj.GetName()},
-		Description: &wrappers.StringValue{Value: proj.GetDescription()},
+		Name:        &wrapperspb.StringValue{Value: proj.GetName()},
+		Description: &wrapperspb.StringValue{Value: proj.GetDescription()},
 		CreatedTime: proj.CreateTime.GetTimestamp(),
 		UpdatedTime: proj.UpdateTime.GetTimestamp(),
 	}
@@ -81,40 +74,37 @@ func TestGet(t *testing.T) {
 		errCode codes.Code
 	}{
 		{
-			name:    "Get an Existing Project",
+			name:    "Get an Existing project",
 			req:     &pbs.GetProjectRequest{Id: proj.GetPublicId()},
 			res:     &pbs.GetProjectResponse{Item: pProject},
 			errCode: codes.OK,
 		},
 		{
-			name: "Get a non existant Host Catalog",
-			req:  &pbs.GetProjectRequest{Id: "p_DoesntExis"},
-			res:  nil,
-			// This will be fixed with PR 42
+			name:    "Get a non existing project",
+			req:     &pbs.GetProjectRequest{Id: "p_DoesntExis"},
+			res:     nil,
 			errCode: codes.NotFound,
 		},
 		{
-			name: "Wrong id prefix",
-			req:  &pbs.GetProjectRequest{Id: "j_1234567890"},
-			res:  nil,
-			// This will be fixed with PR 42
+			name:    "Wrong id prefix",
+			req:     &pbs.GetProjectRequest{Id: "j_1234567890"},
+			res:     nil,
 			errCode: codes.InvalidArgument,
 		},
 		{
-			name: "space in id",
-			req:  &pbs.GetProjectRequest{Id: "p_1 23456789"},
-			res:  nil,
-			// This will be fixed with PR 42
+			name:    "space in id",
+			req:     &pbs.GetProjectRequest{Id: "p_1 23456789"},
+			res:     nil,
 			errCode: codes.InvalidArgument,
 		},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			assert := assert.New(t)
 			req := proto.Clone(toMerge).(*pbs.GetProjectRequest)
 			proto.Merge(req, tc.req)
 
-			s := projects.NewService(repo)
+			s, err := projects.NewService(repo)
+			require.NoError(err, "Couldn't create new project service.")
 
 			got, gErr := s.GetProject(context.Background(), req)
 			assert.Equal(tc.errCode, status.Code(gErr), "GetProject(%+v) got error %v, wanted %v", req, gErr, tc.errCode)
@@ -124,20 +114,11 @@ func TestGet(t *testing.T) {
 }
 
 func TestDelete(t *testing.T) {
-	proj, repoFn := createDefaultProjectAndRepo(t)
+	require := require.New(t)
+	proj, repo := createDefaultProjectAndRepo(t)
 
-	proj2, err := iam.NewProject(proj.GetParentId())
-	if err != nil {
-		t.Fatalf("Couldn't allocate a second project: %v", err)
-	}
-	repo, err := repoFn()
-	require.NoError(t, err, "Error creating new iam repo.")
-	proj2, err = repo.CreateScope(context.Background(), proj2)
-	if err != nil {
-		t.Fatalf("Couldn't persist a second project %v", err)
-	}
-
-	s := projects.NewService(repoFn)
+	s, err := projects.NewService(repo)
+	require.NoError(err, "Error when getting new project service.")
 
 	cases := []struct {
 		name    string
@@ -148,8 +129,8 @@ func TestDelete(t *testing.T) {
 		{
 			name: "Delete an Existing Project",
 			req: &pbs.DeleteProjectRequest{
-				OrgId: proj2.GetParentId(),
-				Id:    proj2.GetPublicId(),
+				OrgId: proj.GetParentId(),
+				Id:    proj.GetPublicId(),
 			},
 			res: &pbs.DeleteProjectResponse{
 				Existed: true,
@@ -159,7 +140,7 @@ func TestDelete(t *testing.T) {
 		{
 			name: "Delete bad project id Project",
 			req: &pbs.DeleteProjectRequest{
-				OrgId: proj2.GetParentId(),
+				OrgId: proj.GetParentId(),
 				Id:    "p_doesntexis",
 			},
 			res: &pbs.DeleteProjectResponse{
@@ -171,7 +152,7 @@ func TestDelete(t *testing.T) {
 			name: "Delete bad org id Project",
 			req: &pbs.DeleteProjectRequest{
 				OrgId: "o_doesntexis",
-				Id:    proj2.GetPublicId(),
+				Id:    proj.GetPublicId(),
 			},
 			res: &pbs.DeleteProjectResponse{
 				Existed: false,
@@ -182,7 +163,7 @@ func TestDelete(t *testing.T) {
 			name: "Bad org formatting",
 			req: &pbs.DeleteProjectRequest{
 				OrgId: "bad_format",
-				Id:    proj2.GetPublicId(),
+				Id:    proj.GetPublicId(),
 			},
 			res:     nil,
 			errCode: codes.InvalidArgument,
@@ -190,7 +171,7 @@ func TestDelete(t *testing.T) {
 		{
 			name: "Bad Project Id formatting",
 			req: &pbs.DeleteProjectRequest{
-				OrgId: proj2.GetParentId(),
+				OrgId: proj.GetParentId(),
 				Id:    "bad_format",
 			},
 			res:     nil,
@@ -209,9 +190,11 @@ func TestDelete(t *testing.T) {
 
 func TestDelete_twice(t *testing.T) {
 	assert := assert.New(t)
+	require := require.New(t)
 	proj, repo := createDefaultProjectAndRepo(t)
 
-	s := projects.NewService(repo)
+	s, err := projects.NewService(repo)
+	require.NoError(err, "Error when getting new project service")
 	req := &pbs.DeleteProjectRequest{
 		OrgId: proj.GetParentId(),
 		Id:    proj.GetPublicId(),
@@ -225,11 +208,10 @@ func TestDelete_twice(t *testing.T) {
 }
 
 func TestCreate(t *testing.T) {
+	require := require.New(t)
 	defaultProj, repo := createDefaultProjectAndRepo(t)
 	defaultProjCreated, err := ptypes.Timestamp(defaultProj.GetCreateTime().GetTimestamp())
-	if err != nil {
-		t.Fatalf("Error converting proto to timestamp: %v", err)
-	}
+	require.NoError(err, "Error converting proto to timestamp.")
 	toMerge := &pbs.CreateProjectRequest{
 		OrgId: defaultProj.GetParentId(),
 	}
@@ -243,14 +225,14 @@ func TestCreate(t *testing.T) {
 		{
 			name: "Create a valid Project",
 			req: &pbs.CreateProjectRequest{Item: &pb.Project{
-				Name:        &wrappers.StringValue{Value: "name"},
-				Description: &wrappers.StringValue{Value: "desc"},
+				Name:        &wrapperspb.StringValue{Value: "name"},
+				Description: &wrapperspb.StringValue{Value: "desc"},
 			}},
 			res: &pbs.CreateProjectResponse{
 				Uri: fmt.Sprintf("orgs/%s/projects/p_", defaultProj.GetParentId()),
 				Item: &pb.Project{
-					Name:        &wrappers.StringValue{Value: "name"},
-					Description: &wrappers.StringValue{Value: "desc"},
+					Name:        &wrapperspb.StringValue{Value: "name"},
+					Description: &wrapperspb.StringValue{Value: "desc"},
 				},
 			},
 			errCode: codes.OK,
@@ -286,7 +268,8 @@ func TestCreate(t *testing.T) {
 			req := proto.Clone(toMerge).(*pbs.CreateProjectRequest)
 			proto.Merge(req, tc.req)
 
-			s := projects.NewService(repo)
+			s, err := projects.NewService(repo)
+			require.NoError(err, "Error when getting new project service.")
 
 			got, gErr := s.CreateProject(context.Background(), req)
 			assert.Equal(tc.errCode, status.Code(gErr), "CreateProject(%+v) got error %v, wanted %v", req, gErr, tc.errCode)
@@ -294,13 +277,9 @@ func TestCreate(t *testing.T) {
 				assert.True(strings.HasPrefix(got.GetUri(), tc.res.Uri))
 				assert.True(strings.HasPrefix(got.GetItem().GetId(), "p_"))
 				gotCreateTime, err := ptypes.Timestamp(got.GetItem().GetCreatedTime())
-				if err != nil {
-					t.Fatalf("Error converting proto to timestamp: %v", err)
-				}
+				require.NoError(err, "Error converting proto to timestamp.")
 				gotUpdateTime, err := ptypes.Timestamp(got.GetItem().GetUpdatedTime())
-				if err != nil {
-					t.Fatalf("Error converting proto to timestamp: %v", err)
-				}
+				require.NoError(err, "Error converting proto to timestamp.")
 				// Verify it is a project created after the test setup's default project
 				assert.True(gotCreateTime.After(defaultProjCreated), "New project should have been created after default project. Was created %v, which is after %v", gotCreateTime, defaultProjCreated)
 				assert.True(gotUpdateTime.After(defaultProjCreated), "New project should have been updated after default project. Was updated %v, which is after %v", gotUpdateTime, defaultProjCreated)
@@ -316,22 +295,20 @@ func TestCreate(t *testing.T) {
 }
 
 func TestUpdate(t *testing.T) {
+	require := require.New(t)
 	proj, repoFn := createDefaultProjectAndRepo(t)
-	tested := projects.NewService(repoFn)
+	tested, err := projects.NewService(repoFn)
+	require.NoError(err, "Error when getting new project service.")
 
-	var err error
 	resetProject := func() {
 		repo, err := repoFn()
-		require.NoError(t, err, "Error creating new iam repo.")
-		if proj, _, err = repo.UpdateScope(context.Background(), proj, []string{"Name", "Description"}); err != nil {
-			t.Fatalf("Failed to reset the project")
-		}
+		require.NoError(err, "Couldn't get a new repo")
+		proj, _, err = repo.UpdateScope(context.Background(), proj, []string{"Name", "Description"})
+		require.NoError(err, "Failed to reset the project")
 	}
 
 	projCreated, err := ptypes.Timestamp(proj.GetCreateTime().GetTimestamp())
-	if err != nil {
-		t.Fatalf("Error converting proto to timestamp: %v", err)
-	}
+	require.NoError(err, "Error converting proto to timestamp")
 	toMerge := &pbs.UpdateProjectRequest{
 		OrgId: proj.GetParentId(),
 		Id:    proj.GetPublicId(),
@@ -350,15 +327,15 @@ func TestUpdate(t *testing.T) {
 					Paths: []string{"name", "description"},
 				},
 				Item: &pb.Project{
-					Name:        &wrappers.StringValue{Value: "new"},
-					Description: &wrappers.StringValue{Value: "desc"},
+					Name:        &wrapperspb.StringValue{Value: "new"},
+					Description: &wrapperspb.StringValue{Value: "desc"},
 				},
 			},
 			res: &pbs.UpdateProjectResponse{
 				Item: &pb.Project{
 					Id:          proj.GetPublicId(),
-					Name:        &wrappers.StringValue{Value: "new"},
-					Description: &wrappers.StringValue{Value: "desc"},
+					Name:        &wrapperspb.StringValue{Value: "new"},
+					Description: &wrapperspb.StringValue{Value: "desc"},
 					CreatedTime: proj.GetCreateTime().GetTimestamp(),
 				},
 			},
@@ -371,15 +348,15 @@ func TestUpdate(t *testing.T) {
 					Paths: []string{"name,description"},
 				},
 				Item: &pb.Project{
-					Name:        &wrappers.StringValue{Value: "new"},
-					Description: &wrappers.StringValue{Value: "desc"},
+					Name:        &wrapperspb.StringValue{Value: "new"},
+					Description: &wrapperspb.StringValue{Value: "desc"},
 				},
 			},
 			res: &pbs.UpdateProjectResponse{
 				Item: &pb.Project{
 					Id:          proj.GetPublicId(),
-					Name:        &wrappers.StringValue{Value: "new"},
-					Description: &wrappers.StringValue{Value: "desc"},
+					Name:        &wrapperspb.StringValue{Value: "new"},
+					Description: &wrapperspb.StringValue{Value: "desc"},
 					CreatedTime: proj.GetCreateTime().GetTimestamp(),
 				},
 			},
@@ -389,8 +366,8 @@ func TestUpdate(t *testing.T) {
 			name: "No Update Mask",
 			req: &pbs.UpdateProjectRequest{
 				Item: &pb.Project{
-					Name:        &wrappers.StringValue{Value: "updated name"},
-					Description: &wrappers.StringValue{Value: "updated desc"},
+					Name:        &wrapperspb.StringValue{Value: "updated name"},
+					Description: &wrapperspb.StringValue{Value: "updated desc"},
 				},
 			},
 			errCode: codes.InvalidArgument,
@@ -400,8 +377,8 @@ func TestUpdate(t *testing.T) {
 			req: &pbs.UpdateProjectRequest{
 				UpdateMask: &field_mask.FieldMask{Paths: []string{}},
 				Item: &pb.Project{
-					Name:        &wrappers.StringValue{Value: "updated name"},
-					Description: &wrappers.StringValue{Value: "updated desc"},
+					Name:        &wrapperspb.StringValue{Value: "updated name"},
+					Description: &wrapperspb.StringValue{Value: "updated desc"},
 				},
 			},
 			errCode: codes.InvalidArgument,
@@ -411,8 +388,8 @@ func TestUpdate(t *testing.T) {
 			req: &pbs.UpdateProjectRequest{
 				UpdateMask: &field_mask.FieldMask{Paths: []string{"nonexistant_field"}},
 				Item: &pb.Project{
-					Name:        &wrappers.StringValue{Value: "updated name"},
-					Description: &wrappers.StringValue{Value: "updated desc"},
+					Name:        &wrapperspb.StringValue{Value: "updated name"},
+					Description: &wrapperspb.StringValue{Value: "updated desc"},
 				},
 			},
 			errCode: codes.InvalidArgument,
@@ -424,13 +401,13 @@ func TestUpdate(t *testing.T) {
 					Paths: []string{"name"},
 				},
 				Item: &pb.Project{
-					Description: &wrappers.StringValue{Value: "ignored"},
+					Description: &wrapperspb.StringValue{Value: "ignored"},
 				},
 			},
 			res: &pbs.UpdateProjectResponse{
 				Item: &pb.Project{
 					Id:          proj.GetPublicId(),
-					Description: &wrappers.StringValue{Value: "default"},
+					Description: &wrapperspb.StringValue{Value: "default"},
 					CreatedTime: proj.GetCreateTime().GetTimestamp(),
 				},
 			},
@@ -462,15 +439,15 @@ func TestUpdate(t *testing.T) {
 					Paths: []string{"name"},
 				},
 				Item: &pb.Project{
-					Name:        &wrappers.StringValue{Value: "updated"},
-					Description: &wrappers.StringValue{Value: "ignored"},
+					Name:        &wrapperspb.StringValue{Value: "updated"},
+					Description: &wrapperspb.StringValue{Value: "ignored"},
 				},
 			},
 			res: &pbs.UpdateProjectResponse{
 				Item: &pb.Project{
 					Id:          proj.GetPublicId(),
-					Name:        &wrappers.StringValue{Value: "updated"},
-					Description: &wrappers.StringValue{Value: "default"},
+					Name:        &wrapperspb.StringValue{Value: "updated"},
+					Description: &wrapperspb.StringValue{Value: "default"},
 					CreatedTime: proj.GetCreateTime().GetTimestamp(),
 				},
 			},
@@ -483,21 +460,21 @@ func TestUpdate(t *testing.T) {
 					Paths: []string{"description"},
 				},
 				Item: &pb.Project{
-					Name:        &wrappers.StringValue{Value: "ignored"},
-					Description: &wrappers.StringValue{Value: "notignored"},
+					Name:        &wrapperspb.StringValue{Value: "ignored"},
+					Description: &wrapperspb.StringValue{Value: "notignored"},
 				},
 			},
 			res: &pbs.UpdateProjectResponse{
 				Item: &pb.Project{
 					Id:          proj.GetPublicId(),
-					Name:        &wrappers.StringValue{Value: "default"},
-					Description: &wrappers.StringValue{Value: "notignored"},
+					Name:        &wrapperspb.StringValue{Value: "default"},
+					Description: &wrapperspb.StringValue{Value: "notignored"},
 					CreatedTime: proj.GetCreateTime().GetTimestamp(),
 				},
 			},
 			errCode: codes.OK,
 		},
-		// TODO: Updating a non existing project should result in a NotFound exception but currently results in
+		// TODO: Updating a non existant project should result in a NotFound exception but currently results in
 		// the repoFn returning an internal error.
 		{
 			name: "Update a Non Existing Project",
@@ -507,8 +484,8 @@ func TestUpdate(t *testing.T) {
 					Paths: []string{"description"},
 				},
 				Item: &pb.Project{
-					Name:        &wrappers.StringValue{Value: "new"},
-					Description: &wrappers.StringValue{Value: "desc"},
+					Name:        &wrapperspb.StringValue{Value: "new"},
+					Description: &wrapperspb.StringValue{Value: "desc"},
 				},
 			},
 			errCode: codes.Internal,
@@ -522,8 +499,8 @@ func TestUpdate(t *testing.T) {
 				},
 				Item: &pb.Project{
 					Id:          "p_somethinge",
-					Name:        &wrappers.StringValue{Value: "new"},
-					Description: &wrappers.StringValue{Value: "new desc"},
+					Name:        &wrapperspb.StringValue{Value: "new"},
+					Description: &wrapperspb.StringValue{Value: "new desc"},
 				}},
 			res:     nil,
 			errCode: codes.InvalidArgument,
@@ -568,14 +545,9 @@ func TestUpdate(t *testing.T) {
 			if got != nil {
 				assert.NotNilf(tc.res, "Expected UpdateProject response to be nil, but was %v", got)
 				gotUpdateTime, err := ptypes.Timestamp(got.GetItem().GetUpdatedTime())
-				if err != nil {
-					t.Fatalf("Error converting proto to timestamp: %v", err)
-				}
+				require.NoError(err, "Error converting proto to timestamp")
 				// Verify it is a project updated after it was created
-				// TODO: This is currently failing.
-				//assert.True(gotUpdateTime.After(projCreated), "Updated project should have been updated after it's creation. Was updated %v, which is after %v", gotUpdateTime, projCreated)
-				_ = gotUpdateTime
-				_ = projCreated
+				assert.True(gotUpdateTime.After(projCreated), "Updated project should have been updated after it's creation. Was updated %v, which is after %v", gotUpdateTime, projCreated)
 
 				// Clear all values which are hard to compare against.
 				got.Item.UpdatedTime, tc.res.Item.UpdatedTime = nil, nil