Skip to content

feat: Metrics read role for service accounts #444

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
captainbeardo wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: Metrics read role for service accounts #444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0379139
feat: Metrics read role for service accounts
captainbeardo Apr 29, 2025
c354bb8
fix: test needed actual role
captainbeardo Apr 29, 2025
547e3b5
fix: error message
captainbeardo Apr 29, 2025
fc9b584
fix: typo
captainbeardo Apr 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions app/role.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,24 @@ import (
)

var (
accountActionGroups = getAccountActionGroups()
namespaceActionGroups = getNamespaceActionGroups()
userAccountActionGroups = getUserAccountActionGroups()
allAccountActionGroups = getAllAccountActionGroups()
namespaceActionGroups = getNamespaceActionGroups()
)

func getAccountActionGroups() []string {
func getUserAccountActionGroups() []string {
var rv []string
for n, v := range auth.AccountActionGroup_value {
if v != int32(auth.ACCOUNT_ACTION_GROUP_UNSPECIFIED) &&
v != int32(auth.ACCOUNT_ACTION_GROUP_METRICS_READ) {
Copy link
Member

@mattkim mattkim Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can a user not have the metrics read role?

Copy link
Contributor Author

@captainbeardo captainbeardo Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we prevent that and this prevents tcld from trying.

rv = append(rv, n)
}
}
slices.Sort(rv)
return rv
}

func getAllAccountActionGroups() []string {
var rv []string
for n, v := range auth.AccountActionGroup_value {
if v != int32(auth.ACCOUNT_ACTION_GROUP_UNSPECIFIED) {
Expand Down Expand Up @@ -98,7 +111,7 @@ func toAccountActionGroup(actionGroup string) (auth.AccountActionGroup, error) {
}
if ag == auth.ACCOUNT_ACTION_GROUP_UNSPECIFIED {
return auth.ACCOUNT_ACTION_GROUP_UNSPECIFIED,
fmt.Errorf("invalid action group: should be one of: %s", accountActionGroups)
fmt.Errorf("invalid action group: should be one of: %s", allAccountActionGroups)
}
return ag, nil
}
Expand Down
8 changes: 4 additions & 4 deletions app/serviceaccount.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func NewServiceAccountCommand(getServiceAccountClientFn GetServiceAccountClientF
RequestIDFlag,
&cli.StringFlag{
Name: accountRoleFlagName,
Usage: fmt.Sprintf("The account role to set on the service account; valid types are: %v", accountActionGroups),
Usage: fmt.Sprintf("The account role to set on the service account; valid types are: %v", allAccountActionGroups),
Required: true,
Aliases: []string{"ar"},
},
Expand All @@ -227,7 +227,7 @@ func NewServiceAccountCommand(getServiceAccountClientFn GetServiceAccountClientF
}

if len(ctx.String(accountRoleFlagName)) == 0 {
return fmt.Errorf("account role must be specified; valid types are %v", accountActionGroups)
return fmt.Errorf("account role must be specified; valid types are %v", allAccountActionGroups)
}

ag, err := toAccountActionGroup(ctx.String(accountRoleFlagName))
Expand Down Expand Up @@ -425,15 +425,15 @@ func NewServiceAccountCommand(getServiceAccountClientFn GetServiceAccountClientF
ResourceVersionFlag,
&cli.StringFlag{
Name: accountRoleFlagName,
Usage: fmt.Sprintf("The account role to set on the service account; valid types are: %v", accountActionGroups),
Usage: fmt.Sprintf("The account role to set on the service account; valid types are: %v", allAccountActionGroups),
Required: true,
Aliases: []string{"ar"},
},
},
Action: func(ctx *cli.Context) error {
// validate input role
if _, ok := auth.AccountActionGroup_value[ctx.String(accountRoleFlagName)]; !ok {
return fmt.Errorf("invalid account role %v; valid types are: %v", ctx.String(accountRoleFlagName), accountActionGroups)
return fmt.Errorf("invalid account role %v; valid types are: %v", ctx.String(accountRoleFlagName), allAccountActionGroups)
}
// if account role is admin unset the namespace permissions
var namespacePermissions map[string]string
Expand Down
12 changes: 10 additions & 2 deletions app/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ func (c *UserClient) inviteUsers(
if err != nil {
return err
}
if role.Spec.AccountRole.ActionGroup == auth.ACCOUNT_ACTION_GROUP_METRICS_READ {
return errors.New("metrics read role is not allowed for users")
}

roleIDs = append(roleIDs, role.GetId())

// get any optional namespace permissions
Expand Down Expand Up @@ -261,6 +265,10 @@ func (c *UserClient) setAccountRole(
if err != nil {
return err
}
if accountRoleToSet.Spec.AccountRole.ActionGroup == auth.ACCOUNT_ACTION_GROUP_METRICS_READ {
return errors.New("metrics read role is not allowed for users")
}

if accountRoleToSet.Spec.AccountRole.ActionGroup == auth.ACCOUNT_ACTION_GROUP_ADMIN {
// set the user account admin role
y, err := ConfirmPrompt(ctx, "Setting admin role on user. All existing namespace permissions will be replaced, please confirm")
Expand Down Expand Up @@ -465,7 +473,7 @@ func NewUserCommand(getUserClientFn GetUserClientFn) (CommandOut, error) {
},
&cli.StringFlag{
Name: accountRoleFlagName,
Usage: fmt.Sprintf("The account role to set on the user; valid types are: %v", accountActionGroups),
Usage: fmt.Sprintf("The account role to set on the user; valid types are: %v", userAccountActionGroups),
Aliases: []string{"ar"},
Required: true,
},
Expand Down Expand Up @@ -531,7 +539,7 @@ func NewUserCommand(getUserClientFn GetUserClientFn) (CommandOut, error) {
ResourceVersionFlag,
&cli.StringFlag{
Name: accountRoleFlagName,
Usage: fmt.Sprintf("The account role to set on the user; valid types are: %v", accountActionGroups),
Usage: fmt.Sprintf("The account role to set on the user; valid types are: %v", userAccountActionGroups),
Required: true,
Aliases: []string{"ar"},
},
Expand Down
22 changes: 12 additions & 10 deletions app/user_group.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,12 @@ func NewUserGroupCommand(GetGroupClientFn GetGroupClientFn) (CommandOut, error)
Flags: []cli.Flag{
&cli.StringFlag{
Name: pageTokenFlagName,
Usage: "list groups starting from this page token",
Usage: "List groups starting from this page token",
Aliases: []string{"p"},
},
&cli.IntFlag{
Name: pageSizeFlagName,
Usage: "number of groups to list",
Usage: "Number of groups to list",
Aliases: []string{"s"},
},
},
Expand All @@ -208,9 +208,10 @@ func NewUserGroupCommand(GetGroupClientFn GetGroupClientFn) (CommandOut, error)
Aliases: []string{"g"},
Flags: []cli.Flag{
&cli.StringFlag{
Name: groupIDFlagName,
Usage: "group ID",
Aliases: []string{"id"},
Name: groupIDFlagName,
Usage: "The ID of the group",
Required: true,
Aliases: []string{"id"},
},
},
Action: func(ctx *cli.Context) error {
Expand All @@ -223,18 +224,19 @@ func NewUserGroupCommand(GetGroupClientFn GetGroupClientFn) (CommandOut, error)
Aliases: []string{"sa"},
Flags: []cli.Flag{
&cli.StringFlag{
Name: groupIDFlagName,
Usage: "group ID",
Aliases: []string{"id"},
Name: groupIDFlagName,
Usage: "The ID of the group",
Required: true,
Aliases: []string{"id"},
},
&cli.StringFlag{
Name: accountRoleFlagName,
Usage: "account role",
Usage: fmt.Sprintf("The account role to set on the group; valid types are: %v", userAccountActionGroups),
Aliases: []string{"ar"},
},
&cli.StringSliceFlag{
Name: namespaceRoleFlagName,
Usage: "namespace roles",
Usage: "Flag can be used multiple times",
Aliases: []string{"nr"},
},
},
Expand Down
20 changes: 18 additions & 2 deletions app/user_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@ package app
import (
"context"
"errors"
"reflect"
"testing"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/suite"
"github.com/temporalio/tcld/protogen/api/auth/v1"
"github.com/temporalio/tcld/protogen/api/authservice/v1"
"github.com/temporalio/tcld/protogen/api/request/v1"
authservicemock "github.com/temporalio/tcld/protogen/apimock/authservice/v1"
"github.com/urfave/cli/v2"
"reflect"
"testing"
)

func TestUser(t *testing.T) {
Expand Down Expand Up @@ -154,6 +155,11 @@ func (s *UserTestSuite) TestInviteErrors() {
Roles: []*auth.Role{{
Id: "account-admin-role",
Type: auth.ROLE_TYPE_PREDEFINED,
Spec: &auth.RoleSpec{
AccountRole: &auth.AccountRoleSpec{
ActionGroup: auth.ACCOUNT_ACTION_GROUP_ADMIN,
},
},
}},
}, nil)
s.mockAuthService.EXPECT().InviteUsers(gomock.Any(), gomock.Any()).Return(nil, errors.New("invite users error")).Times(1)
Expand All @@ -162,6 +168,11 @@ func (s *UserTestSuite) TestInviteErrors() {
Roles: []*auth.Role{{
Id: "account-dev-role",
Type: auth.ROLE_TYPE_PREDEFINED,
Spec: &auth.RoleSpec{
AccountRole: &auth.AccountRoleSpec{
ActionGroup: auth.ACCOUNT_ACTION_GROUP_DEVELOPER,
},
},
}},
}, nil).Times(1)
s.mockAuthService.EXPECT().GetRolesByPermissions(gomock.Any(), gomock.Any()).Return(&authservice.GetRolesByPermissionsResponse{
Expand Down Expand Up @@ -205,6 +216,11 @@ func (s *UserTestSuite) TestInviteSuccess() {
Roles: []*auth.Role{{
Id: "account-dev-role",
Type: auth.ROLE_TYPE_PREDEFINED,
Spec: &auth.RoleSpec{
AccountRole: &auth.AccountRoleSpec{
ActionGroup: auth.ACCOUNT_ACTION_GROUP_DEVELOPER,
},
},
}},
}, nil).Times(1)
s.mockAuthService.EXPECT().GetRolesByPermissions(gomock.Any(), gomock.Any()).Return(&authservice.GetRolesByPermissionsResponse{
Expand Down
Loading