Address some extra review feedback on multi scope grants (#4272)

internal/iam/group.go

@@ -34,7 +34,7 @@ var (
 )
 
 // NewGroup creates a new in memory group with a scope (project/org)
-// and allowed options include: withDescripion, WithName.
+// and allowed options include: withDescription, WithName.
 func NewGroup(ctx context.Context, scopeId string, opt ...Option) (*Group, error) {
 	const op = "iam.NewGroup"
 	if scopeId == "" {

internal/iam/repository_grant_scope.go

@@ -18,13 +18,13 @@ import (
 // for the WithVersion option and will return an error.
 func (r *Repository) AddRoleGrantScopes(ctx context.Context, roleId string, roleVersion uint32, grantScopes []string, _ ...Option) ([]*RoleGrantScope, error) {
 	const op = "iam.(Repository).AddRoleGrantScopes"
-	if roleId == "" {
+
+	switch {
+	case roleId == "":
 		return nil, errors.New(ctx, errors.InvalidParameter, op, "missing role id")
-	}
-	if len(grantScopes) == 0 {
+	case len(grantScopes) == 0:
 		return nil, errors.New(ctx, errors.InvalidParameter, op, "missing grant scopes")
-	}
-	if roleVersion == 0 {
+	case roleVersion == 0:
 		return nil, errors.New(ctx, errors.InvalidParameter, op, "missing version")
 	}
 
@@ -128,13 +128,13 @@ func (r *Repository) AddRoleGrantScopes(ctx context.Context, roleId string, role
 // for the WithVersion option and will return an error.
 func (r *Repository) DeleteRoleGrantScopes(ctx context.Context, roleId string, roleVersion uint32, grantScopes []string, _ ...Option) (int, error) {
 	const op = "iam.(Repository).DeleteRoleGrantScopes"
-	if roleId == "" {
+
+	switch {
+	case roleId == "":
 		return db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "missing role id")
-	}
-	if len(grantScopes) == 0 {
+	case len(grantScopes) == 0:
 		return db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "missing grant scopes")
-	}
-	if roleVersion == 0 {
+	case roleVersion == 0:
 		return db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "missing version")
 	}
 
@@ -224,15 +224,14 @@ func (r *Repository) DeleteRoleGrantScopes(ctx context.Context, roleId string, r
 // value for the WithVersion option and will return an error.
 func (r *Repository) SetRoleGrantScopes(ctx context.Context, roleId string, roleVersion uint32, grantScopes []string, opt ...Option) ([]*RoleGrantScope, int, error) {
 	const op = "iam.(Repository).SetRoleGrantScopes"
-	if roleId == "" {
-		return nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "missing role id")
-	}
-	if roleVersion == 0 {
-		return nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "missing version")
-	}
 
-	// Explicitly set to zero clears, but treat nil as a mistake
-	if grantScopes == nil {
+	switch {
+	case roleId == "":
+		return nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "missing role id")
+	case roleVersion == 0:
+		return nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "missing version")
+	case grantScopes == nil:
+		// Explicitly set to zero clears, but treat nil as a mistake
 		return nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "missing grants")
 	}
 

internal/iam/repository_role.go

@@ -21,18 +21,18 @@ import (
 // role.  No options are currently supported.
 func (r *Repository) CreateRole(ctx context.Context, role *Role, opt ...Option) (*Role, []*PrincipalRole, []*RoleGrant, []*RoleGrantScope, error) {
 	const op = "iam.(Repository).CreateRole"
-	if role == nil {
+
+	switch {
+	case role == nil:
 		return nil, nil, nil, nil, errors.New(ctx, errors.InvalidParameter, op, "missing role")
-	}
-	if role.Role == nil {
+	case role.Role == nil:
 		return nil, nil, nil, nil, errors.New(ctx, errors.InvalidParameter, op, "missing role store")
-	}
-	if role.PublicId != "" {
+	case role.PublicId != "":
 		return nil, nil, nil, nil, errors.New(ctx, errors.InvalidParameter, op, "public id not empty")
-	}
-	if role.ScopeId == "" {
+	case role.ScopeId == "":
 		return nil, nil, nil, nil, errors.New(ctx, errors.InvalidParameter, op, "missing scope id")
 	}
+
 	id, err := newRoleId(ctx)
 	if err != nil {
 		return nil, nil, nil, nil, errors.Wrap(ctx, err, op)

internal/iam/role.go

@@ -34,7 +34,7 @@ var (
 )
 
 // NewRole creates a new in memory role with a scope (project/org)
-// allowed options include: withDescripion, WithName.
+// allowed options include: withDescription, WithName.
 func NewRole(ctx context.Context, scopeId string, opt ...Option) (*Role, error) {
 	const op = "iam.NewRole"
 	if scopeId == "" {
@@ -81,7 +81,7 @@ func (role *Role) validScopeTypes() []scope.Type {
 	return []scope.Type{scope.Global, scope.Org, scope.Project}
 }
 
-// Getscope returns the scope for the Role.
+// GetScope returns the scope for the Role.
 func (role *Role) GetScope(ctx context.Context, r db.Reader) (*Scope, error) {
 	return LookupScope(ctx, r, role)
 }

internal/iam/role_grant_scope.go

@@ -30,17 +30,16 @@ var (
 	_ db.VetForWriter = (*RoleGrantScope)(nil)
 )
 
-// NewRoleGrantScope creates a new in memory role grant scope
+// NewRoleGrantScope creates a new in memory role grant scope. No options are
+// supported.
 func NewRoleGrantScope(ctx context.Context, roleId string, grantScope string, _ ...Option) (*RoleGrantScope, error) {
 	const op = "iam.NewRoleGrantScope"
-	if roleId == "" {
-		return nil, errors.New(ctx, errors.InvalidParameter, op, "missing role id")
-	}
-	if grantScope == "" {
-		return nil, errors.New(ctx, errors.InvalidParameter, op, "missing grant scope")
-	}
 
 	switch {
+	case roleId == "":
+		return nil, errors.New(ctx, errors.InvalidParameter, op, "missing role id")
+	case grantScope == "":
+		return nil, errors.New(ctx, errors.InvalidParameter, op, "missing grant scope")
 	case grantScope == scope.Global.String(),
 		grantScope == globals.GrantScopeThis,
 		grantScope == globals.GrantScopeChildren,
@@ -49,12 +48,14 @@ func NewRoleGrantScope(ctx context.Context, roleId string, grantScope string, _
 	default:
 		return nil, errors.New(ctx, errors.InvalidParameter, op, fmt.Sprintf("unknown grant scope id %q", grantScope))
 	}
+
 	rgs := &RoleGrantScope{
 		RoleGrantScope: &store.RoleGrantScope{
 			RoleId:           roleId,
 			ScopeIdOrSpecial: grantScope,
 		},
 	}
+
 	return rgs, nil
 }
 

internal/proto/controller/api/resources/roles/v1/role.proto

@@ -107,7 +107,7 @@ message Role {
   // match all descendant scopes (e.g. child scopes, children of child scopes;
   // only valid at "global" scope since it is the only one with children of
   // children).
-  repeated string grant_scope_ids = 91 [json_name = "grant_scope_ids"]; // @gotags: `class:"public"`
+  repeated string grant_scope_ids = 91 [json_name = "grant_scope_ids"]; // @gotags: `class:"public" eventstream:"observation"`
 
   // Output only. The IDs (only) of principals that are assigned to this role.
   repeated string principal_ids = 100 [json_name = "principal_ids"]; // @gotags: `class:"public" eventstream:"observation"`

sdk/pbs/controller/api/resources/roles/role.pb.go

@@ -277,7 +277,7 @@ type Role struct {
 	// match all descendant scopes (e.g. child scopes, children of child scopes;
 	// only valid at "global" scope since it is the only one with children of
 	// children).
-	GrantScopeIds []string `protobuf:"bytes,91,rep,name=grant_scope_ids,proto3" json:"grant_scope_ids,omitempty" class:"public"` // @gotags: `class:"public"`
+	GrantScopeIds []string `protobuf:"bytes,91,rep,name=grant_scope_ids,proto3" json:"grant_scope_ids,omitempty" class:"public" eventstream:"observation"` // @gotags: `class:"public" eventstream:"observation"`
 	// Output only. The IDs (only) of principals that are assigned to this role.
 	PrincipalIds []string `protobuf:"bytes,100,rep,name=principal_ids,proto3" json:"principal_ids,omitempty" class:"public" eventstream:"observation"` // @gotags: `class:"public" eventstream:"observation"`
 	// Output only. The principals that are assigned to this role.