IAM: Persist team members on the Team resource (step 1) (#123468)

* IAM: Persist team members on the Team resource (step 1)

* Add shared CUE TeamMember + TeamPermission types and expose
  TeamSpec.Members as a list of those.
* team LegacyStore: hydrate spec.members on Get/List and reconcile
  add/update/delete deltas on Create/Update against the legacy team
  tables. Reconciliation runs in the same SQL transaction as the team
  row update; team-internal-ID resolution happens before BEGIN to avoid
  a SQLite self-deadlock when the write tx holds the file lock.
* Admission rejects duplicate members and flips of the immutable
  `external` flag so the error is consistent regardless of dual-write
  mode.
* Legacy CreateTeam now returns apierrors.NewAlreadyExists on a unique
  constraint violation so clients see a proper 409 instead of a 500
  wrapping the raw SQL error.
* Index member UIDs on the Team search document.
* Integration tests cover CRUD with members, hydration, and the new
  admission rules.

Scope of this commit is the data model + legacy store + index path.
The user-teams / team-members subresources, the enterprise team-group
synchronizer, and the legacy member-search adapter land in follow-up
steps on this branch.

* Regenerate openapi spec

* IAM: fix testdata path in team integration subtests

The team integration test file lives in `pkg/tests/apis/iam/team/`
while the shared testdata fixtures live one level up in
`pkg/tests/apis/iam/testdata/`. The new members/AlreadyExists subtests
referenced `testdata/…` instead of `../testdata/…`, so CI failed with
"no such file or directory" when loading the fixture. Use the correct
relative path and switch the helper deletions from `defer` to
`t.Cleanup` for consistency with the rest of the file.

* Fix frontend

* IAM: address review on team members path

* Create is now transactional. CreateTeamCommand gained MemberCreates
  and legacy.CreateTeam inserts the team row and all seed members in
  one WithTransaction; a failing member rolls back the team too,
  matching Update's atomicity.
* Update now maps team.ErrTeamMemberAlreadyAdded → apierrors.NewConflict
  so the concurrent-add race returns 409 instead of 500, matching the
  Create path.
* buildCreateCommand / buildUpdateCommand translate GetUserInternalID
  failures into apierrors.NewBadRequest so an unknown user UID in
  spec.members returns 400 rather than a 500.
* Replace the per-team hydration on List with a single bulk query via
  a new ListTeamBindingsQuery.TeamUIDs + ArgList in team_bindings_query.sql,
  backed by listTeamMembersForTeams. Drops the N+1.
* mapTeamPermission / toLegacyPermission use explicit switches with a
  default panic so adding a new permission variant upstream without
  updating the mapping fails loud instead of silently collapsing to
  "member".
* Clarify comments on CreateTeamCommand.MemberCreates and
  UpdateTeamCommand.Member{Deletes,Updates,Creates} to spell out the
  atomicity guarantee and which fields the caller must pre-resolve.
* Document the remaining TOCTOU gap between listAllTeamMembers and the
  write tx in Update, and why the race that matters (add-add) is
  covered by the unique-constraint → 409 path.

* IAM: integration test for Update 409 on concurrent member adds

Exercises the ErrTeamMemberAlreadyAdded → apierrors.NewConflict mapping
on the team Update path by racing 10 goroutines that each add the same
user to an empty team. The test asserts no goroutine ever receives a
500 (the pre-fix failure mode), at least one succeeds, and the final
team state contains the user exactly once — i.e. the race converges
through the 409/retry path rather than duplicating or corrupting rows.

* IAM: propagate permission-mapping errors and bulk team-member writes

* mapTeamPermission / toLegacyPermission / mapToTeamMember / toTeamObject
  now return (value, error) instead of panicking on unknown variants.
  diffMembers, buildCreateCommand, buildUpdateCommand, and the Get /
  List / Create / Update flows in team/store.go propagate the error so
  a corrupt SQL row or a new upstream variant surfaces as a 500 at the
  HTTP boundary instead of crashing the apiserver.
* legacy.CreateTeam and legacy.UpdateTeam now issue one bulk INSERT
  (multi-row VALUES) for MemberCreates and one bulk DELETE (… WHERE
  uid IN (...)) for MemberDeletes, replacing the per-row loops.
  DeleteTeamMembersBulkCommand is scoped by OrgID so a UID collision
  across orgs cannot remove the wrong row. Permission UPDATEs stay as
  one statement per row — each has a distinct target value and a
  CASE-WHEN bulk isn't worth the complexity today.
* New golden-SQL tests (single + many variants) cover the two bulk
  templates across mysql/postgres/sqlite.
This commit is contained in:
Misi
2026-04-24 17:10:45 +02:00
committed by GitHub
parent 9ab6c3c467
commit ffb4fea402
36 changed files with 1165 additions and 52 deletions
@@ -21,5 +21,3 @@ TeamRef: {
// Name is the unique identifier for a team.
name: string
}
TeamPermission: "admin" | "member"
+14
View File
@@ -0,0 +1,14 @@
package v0alpha1
TeamMember: {
// kind of the identity
kind: "User"
// uid of the identity
name: string
// permission of the identity in the team
permission: TeamPermission
// whether the member was added externally (e.g. team sync)
external: bool
}
TeamPermission: "admin" | "member"
+1
View File
@@ -5,4 +5,5 @@ TeamSpec: {
email: string
provisioned: bool
externalUID: string
members: [...TeamMember]
}
+45 -5
View File
@@ -2,17 +2,57 @@
package v0alpha1
// +k8s:openapi-gen=true
type TeamTeamMember struct {
// kind of the identity
Kind string `json:"kind"`
// uid of the identity
Name string `json:"name"`
// permission of the identity in the team
Permission TeamTeamPermission `json:"permission"`
// whether the member was added externally (e.g. team sync)
External bool `json:"external"`
}
// NewTeamTeamMember creates a new TeamTeamMember object.
func NewTeamTeamMember() *TeamTeamMember {
return &TeamTeamMember{
Kind: "User",
}
}
// OpenAPIModelName returns the OpenAPI model name for TeamTeamMember.
func (TeamTeamMember) OpenAPIModelName() string {
return "com.github.grafana.grafana.apps.iam.pkg.apis.iam.v0alpha1.TeamTeamMember"
}
// +k8s:openapi-gen=true
type TeamTeamPermission string
const (
TeamTeamPermissionAdmin TeamTeamPermission = "admin"
TeamTeamPermissionMember TeamTeamPermission = "member"
)
// OpenAPIModelName returns the OpenAPI model name for TeamTeamPermission.
func (TeamTeamPermission) OpenAPIModelName() string {
return "com.github.grafana.grafana.apps.iam.pkg.apis.iam.v0alpha1.TeamTeamPermission"
}
// +k8s:openapi-gen=true
type TeamSpec struct {
Title string `json:"title"`
Email string `json:"email"`
Provisioned bool `json:"provisioned"`
ExternalUID string `json:"externalUID"`
Title string `json:"title"`
Email string `json:"email"`
Provisioned bool `json:"provisioned"`
ExternalUID string `json:"externalUID"`
Members []TeamTeamMember `json:"members"`
}
// NewTeamSpec creates a new TeamSpec object.
func NewTeamSpec() *TeamSpec {
return &TeamSpec{}
return &TeamSpec{
Members: []TeamTeamMember{},
}
}
// OpenAPIModelName returns the OpenAPI model name for TeamSpec.
+62 -1
View File
@@ -76,6 +76,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA
TeamLBACRuleSpec{}.OpenAPIModelName(): schema_pkg_apis_iam_v0alpha1_TeamLBACRuleSpec(ref),
TeamList{}.OpenAPIModelName(): schema_pkg_apis_iam_v0alpha1_TeamList(ref),
TeamSpec{}.OpenAPIModelName(): schema_pkg_apis_iam_v0alpha1_TeamSpec(ref),
TeamTeamMember{}.OpenAPIModelName(): schema_pkg_apis_iam_v0alpha1_TeamTeamMember(ref),
User{}.OpenAPIModelName(): schema_pkg_apis_iam_v0alpha1_User(ref),
UserList{}.OpenAPIModelName(): schema_pkg_apis_iam_v0alpha1_UserList(ref),
UserSpec{}.OpenAPIModelName(): schema_pkg_apis_iam_v0alpha1_UserSpec(ref),
@@ -2879,8 +2880,68 @@ func schema_pkg_apis_iam_v0alpha1_TeamSpec(ref common.ReferenceCallback) common.
Format: "",
},
},
"members": {
SchemaProps: spec.SchemaProps{
Type: []string{"array"},
Items: &spec.SchemaOrArray{
Schema: &spec.Schema{
SchemaProps: spec.SchemaProps{
Default: map[string]interface{}{},
Ref: ref(TeamTeamMember{}.OpenAPIModelName()),
},
},
},
},
},
},
Required: []string{"title", "email", "provisioned", "externalUID"},
Required: []string{"title", "email", "provisioned", "externalUID", "members"},
},
},
Dependencies: []string{
TeamTeamMember{}.OpenAPIModelName()},
}
}
func schema_pkg_apis_iam_v0alpha1_TeamTeamMember(ref common.ReferenceCallback) common.OpenAPIDefinition {
return common.OpenAPIDefinition{
Schema: spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{"object"},
Properties: map[string]spec.Schema{
"kind": {
SchemaProps: spec.SchemaProps{
Description: "kind of the identity",
Default: "",
Type: []string{"string"},
Format: "",
},
},
"name": {
SchemaProps: spec.SchemaProps{
Description: "uid of the identity",
Default: "",
Type: []string{"string"},
Format: "",
},
},
"permission": {
SchemaProps: spec.SchemaProps{
Description: "permission of the identity in the team",
Default: "",
Type: []string{"string"},
Format: "",
},
},
"external": {
SchemaProps: spec.SchemaProps{
Description: "whether the member was added externally (e.g. team sync)",
Default: false,
Type: []string{"boolean"},
Format: "",
},
},
},
Required: []string{"kind", "name", "permission", "external"},
},
},
}
@@ -2045,9 +2045,20 @@ export type TeamBindingList = {
kind?: string;
metadata: ListMeta;
};
export type TeamTeamMember = {
/** whether the member was added externally (e.g. team sync) */
external: boolean;
/** kind of the identity */
kind: string;
/** uid of the identity */
name: string;
/** permission of the identity in the team */
permission: string;
};
export type TeamSpec = {
email: string;
externalUID: string;
members: TeamTeamMember[];
provisioned: boolean;
title: string;
};
@@ -6193,7 +6193,7 @@
},
"TeamSpec": {
"type": "object",
"required": ["title", "email", "provisioned", "externalUID"],
"required": ["title", "email", "provisioned", "externalUID", "members"],
"properties": {
"email": {
"type": "string",
@@ -6203,6 +6203,17 @@
"type": "string",
"default": ""
},
"members": {
"type": "array",
"items": {
"default": {},
"allOf": [
{
"$ref": "#/components/schemas/TeamTeamMember"
}
]
}
},
"provisioned": {
"type": "boolean",
"default": false
@@ -6213,6 +6224,32 @@
}
}
},
"TeamTeamMember": {
"type": "object",
"required": ["kind", "name", "permission", "external"],
"properties": {
"external": {
"description": "whether the member was added externally (e.g. team sync)",
"type": "boolean",
"default": false
},
"kind": {
"description": "kind of the identity",
"type": "string",
"default": ""
},
"name": {
"description": "uid of the identity",
"type": "string",
"default": ""
},
"permission": {
"description": "permission of the identity in the team",
"type": "string",
"default": ""
}
}
},
"User": {
"type": "object",
"required": ["metadata", "spec", "status"],
@@ -0,0 +1,7 @@
INSERT INTO {{ .Ident .TeamMemberTable }}
(uid, team_id, user_id, org_id, created, updated, external, permission)
VALUES
{{- range $i, $m := .Command.Members }}
{{- if $i }},{{ end }}
({{ $.Arg $m.UID }}, {{ $.Arg $m.TeamID }}, {{ $.Arg $m.UserID }}, {{ $.Arg $m.OrgID }}, {{ $.Arg $m.Created }}, {{ $.Arg $m.Updated }}, {{ $.Arg $m.External }}, {{ $.Arg $m.Permission }})
{{- end }}
@@ -0,0 +1,3 @@
DELETE FROM {{ .Ident .TeamMemberTable }}
WHERE org_id = {{ .Arg .Command.OrgID }}
AND uid IN ({{ .ArgList .Command.UIDs }})
+88
View File
@@ -103,6 +103,18 @@ func TestIdentityQueries(t *testing.T) {
return &v
}
deleteTeamMembersBulk := func(q *DeleteTeamMembersBulkCommand) sqltemplate.SQLTemplate {
v := newDeleteTeamMembersBulk(nodb, q)
v.SQLTemplate = mocks.NewTestingSQLTemplate()
return &v
}
createTeamMembersBulk := func(q *CreateTeamMembersBulkCommand) sqltemplate.SQLTemplate {
v := newCreateTeamMembersBulk(nodb, q)
v.SQLTemplate = mocks.NewTestingSQLTemplate()
return &v
}
deleteTeam := func(q *DeleteTeamCommand) sqltemplate.SQLTemplate {
v := newDeleteTeam(nodb, q)
v.SQLTemplate = mocks.NewTestingSQLTemplate()
@@ -346,6 +358,14 @@ func TestIdentityQueries(t *testing.T) {
External: boolPtr(true),
}),
},
{
Name: "team_bindings_team_uids",
Data: listTeamBindings(&ListTeamBindingsQuery{
OrgID: 1,
TeamUIDs: []string{"team-1", "team-2"},
Pagination: common.Pagination{Limit: 1},
}),
},
},
sqlUpdateTeamMemberQuery: {
{
@@ -383,6 +403,74 @@ func TestIdentityQueries(t *testing.T) {
}),
},
},
sqlDeleteTeamMembersBulkQuery: {
{
Name: "delete_team_members_bulk_single",
Data: deleteTeamMembersBulk(&DeleteTeamMembersBulkCommand{
OrgID: 1,
UIDs: []string{"team-member-1"},
}),
},
{
Name: "delete_team_members_bulk_many",
Data: deleteTeamMembersBulk(&DeleteTeamMembersBulkCommand{
OrgID: 1,
UIDs: []string{"team-member-1", "team-member-2", "team-member-3"},
}),
},
},
sqlCreateTeamMembersBulkQuery: {
{
Name: "create_team_members_bulk_single",
Data: createTeamMembersBulk(&CreateTeamMembersBulkCommand{
Members: []CreateTeamMemberCommand{
{
UID: "team-member-1",
TeamID: 1,
TeamUID: "team-1",
UserID: 10,
UserUID: "user-10",
OrgID: 1,
Created: legacysql.NewDBTime(time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC)),
Updated: legacysql.NewDBTime(time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC)),
External: false,
Permission: 0,
},
},
}),
},
{
Name: "create_team_members_bulk_many",
Data: createTeamMembersBulk(&CreateTeamMembersBulkCommand{
Members: []CreateTeamMemberCommand{
{
UID: "team-member-1",
TeamID: 1,
TeamUID: "team-1",
UserID: 10,
UserUID: "user-10",
OrgID: 1,
Created: legacysql.NewDBTime(time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC)),
Updated: legacysql.NewDBTime(time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC)),
External: false,
Permission: 0,
},
{
UID: "team-member-2",
TeamID: 1,
TeamUID: "team-1",
UserID: 11,
UserUID: "user-11",
OrgID: 1,
Created: legacysql.NewDBTime(time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC)),
Updated: legacysql.NewDBTime(time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC)),
External: true,
Permission: 4,
},
},
}),
},
},
sqlQueryUserTeamsTemplate: {
{
Name: "team_1_members_page_1",
+108 -16
View File
@@ -7,6 +7,9 @@ import (
"time"
claims "github.com/grafana/authlib/types"
apierrors "k8s.io/apimachinery/pkg/api/errors"
iamv0 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1"
"github.com/grafana/grafana/pkg/registry/apis/iam/common"
"github.com/grafana/grafana/pkg/services/sqlstore/session"
"github.com/grafana/grafana/pkg/services/team"
@@ -275,6 +278,13 @@ type CreateTeamCommand struct {
ExternalID string
IsProvisioned bool
ExternalUID string
// MemberCreates optionally seeds members alongside the team in the same
// SQL transaction as the team row insert. Only UserID needs to be
// pre-resolved by the caller; TeamID, TeamUID, OrgID and timestamps are
// populated here from the team row that was just inserted, so if the
// team insert rolls back the member inserts roll back with it.
MemberCreates []CreateTeamMemberCommand
}
type CreateTeamResult struct {
@@ -328,9 +338,34 @@ func (s *legacySQLStore) CreateTeam(ctx context.Context, ns claims.NamespaceInfo
teamID, err := st.ExecWithReturningId(ctx, teamQuery, req.GetArgs()...)
if err != nil {
if sql.DB.GetDialect().IsUniqueConstraintViolation(err) {
return apierrors.NewAlreadyExists(iamv0.TeamResourceInfo.GroupResource(), cmd.UID)
}
return fmt.Errorf("failed to create team: %w", err)
}
if len(cmd.MemberCreates) > 0 {
for i := range cmd.MemberCreates {
cmd.MemberCreates[i].TeamID = teamID
cmd.MemberCreates[i].TeamUID = cmd.UID
cmd.MemberCreates[i].OrgID = ns.OrgID
cmd.MemberCreates[i].Created = legacysql.NewDBTime(now)
cmd.MemberCreates[i].Updated = legacysql.NewDBTime(now)
}
bulk := CreateTeamMembersBulkCommand{Members: cmd.MemberCreates}
breq := newCreateTeamMembersBulk(sql, &bulk)
bq, err := sqltemplate.Execute(sqlCreateTeamMembersBulkQuery, breq)
if err != nil {
return fmt.Errorf("failed to execute bulk create team members template: %w", err)
}
if _, err := st.Exec(ctx, bq, breq.GetArgs()...); err != nil {
if sql.DB.GetDialect().IsUniqueConstraintViolation(err) {
return team.ErrTeamMemberAlreadyAdded
}
return fmt.Errorf("failed to create team members: %w", err)
}
}
createdTeam = team.Team{
ID: teamID,
UID: cmd.UID,
@@ -361,6 +396,14 @@ type UpdateTeamCommand struct {
ExternalID string
IsProvisioned bool
ExternalUID string
// MemberDeletes / MemberUpdates / MemberCreates reconcile the team's
// members in the same SQL transaction as the team row update. Callers
// must pre-resolve both TeamID and UserID on MemberCreates entries;
// OrgID and timestamps are filled in here.
MemberDeletes []DeleteTeamMemberCommand
MemberUpdates []UpdateTeamMemberCommand
MemberCreates []CreateTeamMemberCommand
}
type UpdateTeamResult struct {
@@ -387,9 +430,11 @@ func (r updateTeamQuery) Validate() error {
return nil
}
// UpdateTeam updates a team and, when the command carries member changes,
// also reconciles those members (deletes, permission updates, additions) in
// the same SQL transaction.
func (s *legacySQLStore) UpdateTeam(ctx context.Context, ns claims.NamespaceInfo, cmd UpdateTeamCommand) (*UpdateTeamResult, error) {
now := time.Now().UTC()
cmd.Updated = legacysql.NewDBTime(now)
sql, err := s.getDB(ctx)
@@ -397,28 +442,77 @@ func (s *legacySQLStore) UpdateTeam(ctx context.Context, ns claims.NamespaceInfo
return nil, err
}
req := newUpdateTeam(sql, &cmd)
// Resolve the team's internal ID before opening the write transaction. Doing
// the read inside WithTransaction would acquire a second DB connection while
// the tx holds the write lock, which deadlocks on SQLite.
if _, err := s.GetTeamInternalID(ctx, ns, GetTeamInternalIDQuery{OrgID: ns.OrgID, UID: cmd.UID}); err != nil {
return nil, fmt.Errorf("team not found: %w", err)
}
teamReq := newUpdateTeam(sql, &cmd)
var updatedTeam team.Team
err = sql.DB.GetSqlxSession().WithTransaction(ctx, func(st *session.SessionTx) error {
_, err := s.GetTeamInternalID(ctx, ns, GetTeamInternalIDQuery{
OrgID: ns.OrgID,
UID: cmd.UID,
})
if err != nil {
return fmt.Errorf("team not found: %w", err)
}
teamQuery, err := sqltemplate.Execute(sqlUpdateTeamTemplate, req)
err = sql.DB.GetSqlxSession().WithTransaction(ctx, func(st *session.SessionTx) error {
teamQuery, err := sqltemplate.Execute(sqlUpdateTeamTemplate, teamReq)
if err != nil {
return fmt.Errorf("failed to execute team update template %q: %w", sqlUpdateTeamTemplate.Name(), err)
}
_, err = st.Exec(ctx, teamQuery, req.GetArgs()...)
if err != nil {
if _, err := st.Exec(ctx, teamQuery, teamReq.GetArgs()...); err != nil {
return fmt.Errorf("failed to update team: %w", err)
}
if len(cmd.MemberDeletes) > 0 {
uids := make([]string, len(cmd.MemberDeletes))
for i, d := range cmd.MemberDeletes {
uids[i] = d.UID
}
bulk := DeleteTeamMembersBulkCommand{OrgID: ns.OrgID, UIDs: uids}
dreq := newDeleteTeamMembersBulk(sql, &bulk)
dq, err := sqltemplate.Execute(sqlDeleteTeamMembersBulkQuery, dreq)
if err != nil {
return fmt.Errorf("failed to execute bulk delete team members template: %w", err)
}
if _, err := st.Exec(ctx, dq, dreq.GetArgs()...); err != nil {
return fmt.Errorf("failed to delete team members: %w", err)
}
}
// Permission updates stay as one statement per row: each row changes to
// a different target value, so a single bulk UPDATE would need CASE
// WHEN scaffolding that's not worth the complexity until a profile
// shows it.
for i := range cmd.MemberUpdates {
cmd.MemberUpdates[i].Updated = legacysql.NewDBTime(now)
ureq := newUpdateTeamMember(sql, &cmd.MemberUpdates[i])
uq, err := sqltemplate.Execute(sqlUpdateTeamMemberQuery, ureq)
if err != nil {
return fmt.Errorf("failed to execute update team member template: %w", err)
}
if _, err := st.Exec(ctx, uq, ureq.GetArgs()...); err != nil {
return fmt.Errorf("failed to update team member: %w", err)
}
}
if len(cmd.MemberCreates) > 0 {
for i := range cmd.MemberCreates {
cmd.MemberCreates[i].Created = legacysql.NewDBTime(now)
cmd.MemberCreates[i].Updated = legacysql.NewDBTime(now)
cmd.MemberCreates[i].OrgID = ns.OrgID
}
bulk := CreateTeamMembersBulkCommand{Members: cmd.MemberCreates}
creq := newCreateTeamMembersBulk(sql, &bulk)
cq, err := sqltemplate.Execute(sqlCreateTeamMembersBulkQuery, creq)
if err != nil {
return fmt.Errorf("failed to execute bulk create team members template: %w", err)
}
if _, err := st.Exec(ctx, cq, creq.GetArgs()...); err != nil {
if sql.DB.GetDialect().IsUniqueConstraintViolation(err) {
return team.ErrTeamMemberAlreadyAdded
}
return fmt.Errorf("failed to create team members: %w", err)
}
}
updatedTeam = team.Team{
UID: cmd.UID,
Name: cmd.Name,
@@ -427,14 +521,12 @@ func (s *legacySQLStore) UpdateTeam(ctx context.Context, ns claims.NamespaceInfo
IsProvisioned: cmd.IsProvisioned,
Updated: cmd.Updated.Time,
}
return nil
})
if err != nil {
return nil, err
}
return &UpdateTeamResult{Team: updatedTeam}, nil
}
+60 -3
View File
@@ -15,9 +15,13 @@ import (
)
type ListTeamBindingsQuery struct {
UID string
OrgID int64
TeamUID string
UID string
OrgID int64
TeamUID string
// TeamUIDs lets callers fetch bindings for many teams in one query.
// Mutually exclusive with TeamUID — if both are set the single-UID
// filter wins (see team_bindings_query.sql).
TeamUIDs []string
UserUID string
External *bool
Pagination common.Pagination
@@ -390,7 +394,60 @@ type DeleteTeamMemberCommand struct {
UID string
}
// DeleteTeamMembersBulkCommand removes multiple team_member rows by binding
// UID in a single SQL DELETE. OrgID scopes the DELETE so a UID from another
// org cannot be deleted even if it happens to collide.
type DeleteTeamMembersBulkCommand struct {
OrgID int64
UIDs []string
}
// CreateTeamMembersBulkCommand inserts multiple team_member rows in a single
// multi-row INSERT. Each member must be fully populated (TeamID/TeamUID/UserID
// already resolved, OrgID set, timestamps filled).
type CreateTeamMembersBulkCommand struct {
Members []CreateTeamMemberCommand
}
var sqlDeleteTeamMemberQuery = mustTemplate("delete_team_member_query.sql")
var sqlDeleteTeamMembersBulkQuery = mustTemplate("delete_team_members_bulk.sql")
var sqlCreateTeamMembersBulkQuery = mustTemplate("create_team_members_bulk.sql")
type deleteTeamMembersBulkQuery struct {
sqltemplate.SQLTemplate
TeamMemberTable string
Command *DeleteTeamMembersBulkCommand
}
func (r deleteTeamMembersBulkQuery) Validate() error {
return nil
}
func newDeleteTeamMembersBulk(sql *legacysql.LegacyDatabaseHelper, cmd *DeleteTeamMembersBulkCommand) deleteTeamMembersBulkQuery {
return deleteTeamMembersBulkQuery{
SQLTemplate: sqltemplate.New(sql.DialectForDriver()),
TeamMemberTable: sql.Table("team_member"),
Command: cmd,
}
}
type createTeamMembersBulkQuery struct {
sqltemplate.SQLTemplate
TeamMemberTable string
Command *CreateTeamMembersBulkCommand
}
func (r createTeamMembersBulkQuery) Validate() error {
return nil
}
func newCreateTeamMembersBulk(sql *legacysql.LegacyDatabaseHelper, cmd *CreateTeamMembersBulkCommand) createTeamMembersBulkQuery {
return createTeamMembersBulkQuery{
SQLTemplate: sqltemplate.New(sql.DialectForDriver()),
TeamMemberTable: sql.Table("team_member"),
Command: cmd,
}
}
func newDeleteTeamMember(sql *legacysql.LegacyDatabaseHelper, cmd *DeleteTeamMemberCommand) deleteTeamMemberQuery {
return deleteTeamMemberQuery{
@@ -9,6 +9,8 @@ WHERE
{{ end }}
{{ if .Query.TeamUID }}
AND t.uid = {{ .Arg .Query.TeamUID }}
{{ else if .Query.TeamUIDs }}
AND t.uid IN ({{ .ArgList .Query.TeamUIDs }})
{{ end }}
{{ if .Query.UserUID }}
AND u.uid = {{ .Arg .Query.UserUID }}
@@ -0,0 +1,5 @@
INSERT INTO `grafana`.`team_member`
(uid, team_id, user_id, org_id, created, updated, external, permission)
VALUES
('team-member-1', 1, 10, 1, '2023-01-01 12:00:00', '2023-01-01 12:00:00', FALSE, 'Member'),
('team-member-2', 1, 11, 1, '2023-01-01 12:00:00', '2023-01-01 12:00:00', TRUE, 'Admin')
@@ -0,0 +1,4 @@
INSERT INTO `grafana`.`team_member`
(uid, team_id, user_id, org_id, created, updated, external, permission)
VALUES
('team-member-1', 1, 10, 1, '2023-01-01 12:00:00', '2023-01-01 12:00:00', FALSE, 'Member')
@@ -0,0 +1,3 @@
DELETE FROM `grafana`.`team_member`
WHERE org_id = 1
AND uid IN ('team-member-1', 'team-member-2', 'team-member-3')
@@ -0,0 +1,3 @@
DELETE FROM `grafana`.`team_member`
WHERE org_id = 1
AND uid IN ('team-member-1')
@@ -0,0 +1,9 @@
SELECT tm.id as id, tm.uid as uid, t.uid as team_uid, t.id as team_id, u.uid as user_uid, u.id as user_id, tm.created, tm.updated, tm.permission, tm.external
FROM `grafana`.`team_member` tm
INNER JOIN `grafana`.`team` t ON tm.team_id = t.id
INNER JOIN `grafana`.`user` u ON tm.user_id = u.id
WHERE
tm.org_id = 1
AND t.uid IN ('team-1', 'team-2')
ORDER BY tm.id ASC
LIMIT 1;
@@ -0,0 +1,5 @@
INSERT INTO "grafana"."team_member"
(uid, team_id, user_id, org_id, created, updated, external, permission)
VALUES
('team-member-1', 1, 10, 1, '2023-01-01 12:00:00', '2023-01-01 12:00:00', FALSE, 'Member'),
('team-member-2', 1, 11, 1, '2023-01-01 12:00:00', '2023-01-01 12:00:00', TRUE, 'Admin')
@@ -0,0 +1,4 @@
INSERT INTO "grafana"."team_member"
(uid, team_id, user_id, org_id, created, updated, external, permission)
VALUES
('team-member-1', 1, 10, 1, '2023-01-01 12:00:00', '2023-01-01 12:00:00', FALSE, 'Member')
@@ -0,0 +1,3 @@
DELETE FROM "grafana"."team_member"
WHERE org_id = 1
AND uid IN ('team-member-1', 'team-member-2', 'team-member-3')
@@ -0,0 +1,3 @@
DELETE FROM "grafana"."team_member"
WHERE org_id = 1
AND uid IN ('team-member-1')
@@ -0,0 +1,9 @@
SELECT tm.id as id, tm.uid as uid, t.uid as team_uid, t.id as team_id, u.uid as user_uid, u.id as user_id, tm.created, tm.updated, tm.permission, tm.external
FROM "grafana"."team_member" tm
INNER JOIN "grafana"."team" t ON tm.team_id = t.id
INNER JOIN "grafana"."user" u ON tm.user_id = u.id
WHERE
tm.org_id = 1
AND t.uid IN ('team-1', 'team-2')
ORDER BY tm.id ASC
LIMIT 1;
@@ -0,0 +1,5 @@
INSERT INTO "grafana"."team_member"
(uid, team_id, user_id, org_id, created, updated, external, permission)
VALUES
('team-member-1', 1, 10, 1, '2023-01-01 12:00:00', '2023-01-01 12:00:00', FALSE, 'Member'),
('team-member-2', 1, 11, 1, '2023-01-01 12:00:00', '2023-01-01 12:00:00', TRUE, 'Admin')
@@ -0,0 +1,4 @@
INSERT INTO "grafana"."team_member"
(uid, team_id, user_id, org_id, created, updated, external, permission)
VALUES
('team-member-1', 1, 10, 1, '2023-01-01 12:00:00', '2023-01-01 12:00:00', FALSE, 'Member')
@@ -0,0 +1,3 @@
DELETE FROM "grafana"."team_member"
WHERE org_id = 1
AND uid IN ('team-member-1', 'team-member-2', 'team-member-3')
@@ -0,0 +1,3 @@
DELETE FROM "grafana"."team_member"
WHERE org_id = 1
AND uid IN ('team-member-1')
@@ -0,0 +1,9 @@
SELECT tm.id as id, tm.uid as uid, t.uid as team_uid, t.id as team_id, u.uid as user_uid, u.id as user_id, tm.created, tm.updated, tm.permission, tm.external
FROM "grafana"."team_member" tm
INNER JOIN "grafana"."team" t ON tm.team_id = t.id
INNER JOIN "grafana"."user" u ON tm.user_id = u.id
WHERE
tm.org_id = 1
AND t.uid IN ('team-1', 'team-2')
ORDER BY tm.id ASC
LIMIT 1;
+234 -23
View File
@@ -2,6 +2,7 @@ package team
import (
"context"
"errors"
"fmt"
"strconv"
@@ -142,20 +143,53 @@ func (s *LegacyStore) Update(ctx context.Context, name string, objInfo rest.Upda
}
}
updateCmd := legacy.UpdateTeamCommand{
UID: teamObj.Name,
Name: teamObj.Spec.Title,
Email: teamObj.Spec.Email,
IsProvisioned: teamObj.Spec.Provisioned,
ExternalUID: teamObj.Spec.ExternalUID,
// TOCTOU note: the current-members read happens outside the write tx
// that runs inside legacy.UpdateTeam, so a concurrent writer may alter
// the team_member rows between diffMembers and the write. The apiserver
// resourceVersion check on the Team row does not cover team_member, so
// full-replace Updates can interleave. The two races that matter:
// * Two writers adding the same user: the second INSERT hits the
// UNIQUE(org_id, team_id, user_id) constraint; legacy.UpdateTeam
// returns ErrTeamMemberAlreadyAdded and we surface 409 below so the
// client re-reads and retries.
// * A writer deleting a member that is already gone: the DELETE is a
// no-op (affects 0 rows) — harmless.
// Moving this read into WithTransaction would close the remaining gap
// but requires the legacy store to expose a tx-scoped ListTeamBindings
// so we don't re-trigger the SQLite self-deadlock described on
// GetTeamInternalID.
currentMembers, err := s.listAllTeamMembers(ctx, ns, teamObj.Name)
if err != nil {
return oldObj, false, err
}
diff, err := diffMembers(currentMembers, teamObj.Spec.Members)
if err != nil {
return oldObj, false, apierrors.NewBadRequest(err.Error())
}
result, err := s.store.UpdateTeam(ctx, ns, updateCmd)
updateCmd, err := s.buildUpdateCommand(ctx, ns, teamObj, diff)
if err != nil {
return oldObj, false, err
}
iamTeam := toTeamObject(result.Team, ns)
result, err := s.store.UpdateTeam(ctx, ns, updateCmd)
if err != nil {
// Race with another writer adding the same member first — surface as
// 409 so retry.RetryOnConflict (or the client) can re-read and recompute.
if errors.Is(err, team.ErrTeamMemberAlreadyAdded) {
return oldObj, false, apierrors.NewConflict(teamResource.GroupResource(), name, err)
}
return oldObj, false, err
}
members, err := s.listAllTeamMembers(ctx, ns, teamObj.Name)
if err != nil {
return oldObj, false, err
}
iamTeam, err := toTeamObject(result.Team, ns, members)
if err != nil {
return oldObj, false, err
}
return &iamTeam, false, nil
}
@@ -178,10 +212,22 @@ func (s *LegacyStore) List(ctx context.Context, options *internalversion.ListOpt
return nil, err
}
teamUIDs := make([]string, len(found.Teams))
for i, t := range found.Teams {
teamUIDs[i] = t.UID
}
membersByTeam, err := s.listTeamMembersForTeams(ctx, ns, teamUIDs)
if err != nil {
return nil, err
}
teams := make([]*iamv0alpha1.Team, 0, len(found.Teams))
for _, t := range found.Teams {
team := toTeamObject(t, ns)
teams = append(teams, &team)
teamObj, err := toTeamObject(t, ns, membersByTeam[t.UID])
if err != nil {
return nil, err
}
teams = append(teams, &teamObj)
}
return &common.ListResponse[*iamv0alpha1.Team]{
@@ -229,7 +275,14 @@ func (s *LegacyStore) Get(ctx context.Context, name string, options *metav1.GetO
return nil, teamResource.NewNotFound(name)
}
obj := toTeamObject(found.Teams[0], ns)
members, err := s.listAllTeamMembers(ctx, ns, name)
if err != nil {
return nil, fmt.Errorf("failed to list members for team %s: %w", name, err)
}
obj, err := toTeamObject(found.Teams[0], ns, members)
if err != nil {
return nil, err
}
return &obj, nil
}
@@ -258,20 +311,27 @@ func (s *LegacyStore) Create(ctx context.Context, obj runtime.Object, createVali
}
}
createCmd := legacy.CreateTeamCommand{
UID: teamObj.Name,
Name: teamObj.Spec.Title,
Email: teamObj.Spec.Email,
IsProvisioned: teamObj.Spec.Provisioned,
ExternalUID: teamObj.Spec.ExternalUID,
}
result, err := s.store.CreateTeam(ctx, ns, createCmd)
createCmd, err := s.buildCreateCommand(ctx, ns, teamObj)
if err != nil {
return nil, err
}
iamTeam := toTeamObject(result.Team, ns)
result, err := s.store.CreateTeam(ctx, ns, createCmd)
if err != nil {
if errors.Is(err, team.ErrTeamMemberAlreadyAdded) {
return nil, apierrors.NewConflict(teamResource.GroupResource(), teamObj.Name, err)
}
return nil, err
}
members, err := s.listAllTeamMembers(ctx, ns, result.Team.UID)
if err != nil {
return nil, err
}
iamTeam, err := toTeamObject(result.Team, ns, members)
if err != nil {
return nil, err
}
return &iamTeam, nil
}
@@ -304,7 +364,15 @@ func getDeprecatedInternalIDFromLabelSelectors(options *internalversion.ListOpti
return 0
}
func toTeamObject(t team.Team, ns claims.NamespaceInfo) iamv0alpha1.Team {
func toTeamObject(t team.Team, ns claims.NamespaceInfo, members []legacy.TeamMember) (iamv0alpha1.Team, error) {
specMembers := make([]iamv0alpha1.TeamTeamMember, 0, len(members))
for _, m := range members {
mapped, err := mapToTeamMember(m)
if err != nil {
return iamv0alpha1.Team{}, err
}
specMembers = append(specMembers, mapped)
}
obj := iamv0alpha1.Team{
ObjectMeta: metav1.ObjectMeta{
Name: t.UID,
@@ -317,11 +385,154 @@ func toTeamObject(t team.Team, ns claims.NamespaceInfo) iamv0alpha1.Team {
Email: t.Email,
Provisioned: t.IsProvisioned,
ExternalUID: t.ExternalUID,
Members: specMembers,
},
}
meta, _ := utils.MetaAccessor(&obj)
meta.SetUpdatedTimestamp(&t.Updated)
meta.SetDeprecatedInternalID(t.ID) // nolint:staticcheck
return obj
return obj, nil
}
// listAllTeamMembers paginates ListTeamBindings to completion for a given
// team. Used on the single-team Get / Create / Update read paths.
func (s *LegacyStore) listAllTeamMembers(ctx context.Context, ns claims.NamespaceInfo, teamUID string) ([]legacy.TeamMember, error) {
var all []legacy.TeamMember
var continueToken int64
for {
page, err := s.store.ListTeamBindings(ctx, ns, legacy.ListTeamBindingsQuery{
TeamUID: teamUID,
Pagination: common.Pagination{Limit: common.MaxListLimit, Continue: continueToken},
})
if err != nil {
return nil, err
}
all = append(all, page.Bindings...)
if page.Continue == 0 {
break
}
continueToken = page.Continue
}
return all, nil
}
// listTeamMembersForTeams fetches all members for the given team UIDs in a
// single (paginated) query and groups them by team UID. Removes the List-time
// N+1 of one round trip per team.
func (s *LegacyStore) listTeamMembersForTeams(ctx context.Context, ns claims.NamespaceInfo, teamUIDs []string) (map[string][]legacy.TeamMember, error) {
out := make(map[string][]legacy.TeamMember, len(teamUIDs))
if len(teamUIDs) == 0 {
return out, nil
}
var continueToken int64
for {
page, err := s.store.ListTeamBindings(ctx, ns, legacy.ListTeamBindingsQuery{
TeamUIDs: teamUIDs,
Pagination: common.Pagination{Limit: common.MaxListLimit, Continue: continueToken},
})
if err != nil {
return nil, err
}
for _, m := range page.Bindings {
out[m.TeamUID] = append(out[m.TeamUID], m)
}
if page.Continue == 0 {
break
}
continueToken = page.Continue
}
return out, nil
}
// buildCreateCommand assembles the legacy CreateTeamCommand with any initial
// members pre-resolved so the legacy store can insert team + members in one
// SQL transaction. Unknown user UIDs surface as 400 Bad Request.
func (s *LegacyStore) buildCreateCommand(ctx context.Context, ns claims.NamespaceInfo, teamObj *iamv0alpha1.Team) (legacy.CreateTeamCommand, error) {
cmd := legacy.CreateTeamCommand{
UID: teamObj.Name,
Name: teamObj.Spec.Title,
Email: teamObj.Spec.Email,
IsProvisioned: teamObj.Spec.Provisioned,
ExternalUID: teamObj.Spec.ExternalUID,
}
diff, err := diffMembers(nil, teamObj.Spec.Members)
if err != nil {
return cmd, apierrors.NewBadRequest(err.Error())
}
for _, add := range diff.toAdd {
userObj, err := s.store.GetUserInternalID(ctx, ns, legacy.GetUserInternalIDQuery{UID: add.Name})
if err != nil {
return cmd, apierrors.NewBadRequest(fmt.Sprintf("unknown user %q in spec.members", add.Name))
}
perm, err := toLegacyPermission(add.Permission)
if err != nil {
return cmd, err
}
cmd.MemberCreates = append(cmd.MemberCreates, legacy.CreateTeamMemberCommand{
UID: util.GenerateShortUID(),
UserID: userObj.ID,
UserUID: add.Name,
Permission: perm,
External: add.External,
})
}
return cmd, nil
}
// buildUpdateCommand assembles the legacy UpdateTeamCommand — the team row
// update and all member-level changes — so the legacy store can apply them
// atomically in one SQL transaction. Unknown user UIDs surface as 400 Bad
// Request.
func (s *LegacyStore) buildUpdateCommand(ctx context.Context, ns claims.NamespaceInfo, teamObj *iamv0alpha1.Team, diff memberDiff) (legacy.UpdateTeamCommand, error) {
cmd := legacy.UpdateTeamCommand{
UID: teamObj.Name,
Name: teamObj.Spec.Title,
Email: teamObj.Spec.Email,
IsProvisioned: teamObj.Spec.Provisioned,
ExternalUID: teamObj.Spec.ExternalUID,
}
for _, del := range diff.toDelete {
cmd.MemberDeletes = append(cmd.MemberDeletes, legacy.DeleteTeamMemberCommand{UID: del.UID})
}
for _, up := range diff.toUpdate {
perm, err := toLegacyPermission(up.permission)
if err != nil {
return cmd, err
}
cmd.MemberUpdates = append(cmd.MemberUpdates, legacy.UpdateTeamMemberCommand{
UID: up.binding.UID,
Permission: perm,
})
}
if len(diff.toAdd) == 0 {
return cmd, nil
}
teamInfo, err := s.store.GetTeamInternalID(ctx, ns, legacy.GetTeamInternalIDQuery{UID: teamObj.Name})
if err != nil {
return cmd, fmt.Errorf("failed to fetch team %s: %w", teamObj.Name, err)
}
for _, add := range diff.toAdd {
userObj, err := s.store.GetUserInternalID(ctx, ns, legacy.GetUserInternalIDQuery{UID: add.Name})
if err != nil {
return cmd, apierrors.NewBadRequest(fmt.Sprintf("unknown user %q in spec.members", add.Name))
}
perm, err := toLegacyPermission(add.Permission)
if err != nil {
return cmd, err
}
cmd.MemberCreates = append(cmd.MemberCreates, legacy.CreateTeamMemberCommand{
UID: util.GenerateShortUID(),
TeamID: teamInfo.ID,
TeamUID: teamObj.Name,
UserID: userObj.ID,
UserUID: add.Name,
Permission: perm,
External: add.External,
})
}
return cmd, nil
}
+105
View File
@@ -0,0 +1,105 @@
package team
import (
"fmt"
iamv0alpha1 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1"
"github.com/grafana/grafana/pkg/registry/apis/iam/legacy"
"github.com/grafana/grafana/pkg/services/team"
)
// mapTeamPermission translates a legacy team.PermissionType to the generated
// enum on the Team CRD. An unknown variant returns an error instead of
// silently collapsing to "member" — callers surface this as a 500 at the
// HTTP boundary so a buggy row / new upstream variant is visible rather
// than silently corrupt.
func mapTeamPermission(p team.PermissionType) (iamv0alpha1.TeamTeamPermission, error) {
switch p {
case team.PermissionTypeAdmin:
return iamv0alpha1.TeamTeamPermissionAdmin, nil
case team.PermissionTypeMember:
return iamv0alpha1.TeamTeamPermissionMember, nil
default:
return "", fmt.Errorf("team: unhandled legacy PermissionType %d", p)
}
}
// toLegacyPermission is the inverse of mapTeamPermission with the same
// unknown-variant behavior.
func toLegacyPermission(p iamv0alpha1.TeamTeamPermission) (team.PermissionType, error) {
switch p {
case iamv0alpha1.TeamTeamPermissionAdmin:
return team.PermissionTypeAdmin, nil
case iamv0alpha1.TeamTeamPermissionMember:
return team.PermissionTypeMember, nil
default:
return 0, fmt.Errorf("team: unhandled TeamTeamPermission %q", p)
}
}
func mapToTeamMember(tm legacy.TeamMember) (iamv0alpha1.TeamTeamMember, error) {
perm, err := mapTeamPermission(tm.Permission)
if err != nil {
return iamv0alpha1.TeamTeamMember{}, err
}
return iamv0alpha1.TeamTeamMember{
Kind: "User",
Name: tm.UserUID,
Permission: perm,
External: tm.External,
}, nil
}
type memberDiff struct {
toAdd []iamv0alpha1.TeamTeamMember
toUpdate []memberUpdate
toDelete []legacy.TeamMember
}
type memberUpdate struct {
binding legacy.TeamMember
permission iamv0alpha1.TeamTeamPermission
}
func diffMembers(current []legacy.TeamMember, desired []iamv0alpha1.TeamTeamMember) (memberDiff, error) {
var out memberDiff
currentByUser := make(map[string]legacy.TeamMember, len(current))
for _, tm := range current {
currentByUser[tm.UserUID] = tm
}
desiredByUser := make(map[string]iamv0alpha1.TeamTeamMember, len(desired))
for _, m := range desired {
if _, dup := desiredByUser[m.Name]; dup {
return memberDiff{}, fmt.Errorf("duplicate member %q in spec.members", m.Name)
}
desiredByUser[m.Name] = m
}
for _, want := range desired {
have, exists := currentByUser[want.Name]
if !exists {
out.toAdd = append(out.toAdd, want)
continue
}
if want.External != have.External {
return memberDiff{}, fmt.Errorf("cannot change external flag for member %q", want.Name)
}
havePerm, err := mapTeamPermission(have.Permission)
if err != nil {
return memberDiff{}, err
}
if havePerm != want.Permission {
out.toUpdate = append(out.toUpdate, memberUpdate{binding: have, permission: want.Permission})
}
}
for _, have := range current {
if _, exists := desiredByUser[have.UserUID]; !exists {
out.toDelete = append(out.toDelete, have)
}
}
return out, nil
}
+35
View File
@@ -28,6 +28,10 @@ func ValidateOnCreate(ctx context.Context, obj *iamv0alpha1.Team) error {
return apierrors.NewBadRequest("externalUID is only allowed for provisioned teams")
}
if err := validateNoDuplicateMembers(obj.Spec.Members); err != nil {
return err
}
return nil
}
@@ -53,5 +57,36 @@ func ValidateOnUpdate(ctx context.Context, obj, old *iamv0alpha1.Team) error {
return apierrors.NewBadRequest("externalUID is only allowed for provisioned teams")
}
if err := validateNoDuplicateMembers(obj.Spec.Members); err != nil {
return err
}
if err := validateMemberExternalImmutable(old.Spec.Members, obj.Spec.Members); err != nil {
return err
}
return nil
}
func validateNoDuplicateMembers(members []iamv0alpha1.TeamTeamMember) error {
seen := make(map[string]struct{}, len(members))
for _, m := range members {
if _, dup := seen[m.Name]; dup {
return apierrors.NewBadRequest("duplicate member " + m.Name + " in spec.members")
}
seen[m.Name] = struct{}{}
}
return nil
}
func validateMemberExternalImmutable(old, new []iamv0alpha1.TeamTeamMember) error {
oldByName := make(map[string]bool, len(old))
for _, m := range old {
oldByName[m.Name] = m.External
}
for _, m := range new {
if was, ok := oldByName[m.Name]; ok && was != m.External {
return apierrors.NewBadRequest("external flag is immutable on existing member " + m.Name)
}
}
return nil
}
@@ -14,6 +14,7 @@ const (
TEAM_SEARCH_EMAIL = "email"
TEAM_SEARCH_PROVISIONED = "provisioned"
TEAM_SEARCH_EXTERNAL_UID = "externalUID"
TEAM_SEARCH_MEMBERS = "members"
)
// TeamSortableExtraFields are the additional fields that can be used for sorting team search results.
@@ -38,6 +39,15 @@ var TeamSearchTableColumnDefinitions = map[string]*resourcepb.ResourceTableColum
Type: resourcepb.ResourceTableColumnDefinition_STRING,
Description: "External UID of the team",
},
TEAM_SEARCH_MEMBERS: {
Name: TEAM_SEARCH_MEMBERS,
Type: resourcepb.ResourceTableColumnDefinition_STRING,
IsArray: true,
Description: "UIDs of users that are members of the team",
Properties: &resourcepb.ResourceTableColumnDefinition_Properties{
Filterable: true,
},
},
}
func GetTeamSearchBuilder() (resource.DocumentBuilderInfo, error) {
@@ -77,6 +87,13 @@ func (t *teamSearchBuilder) BuildDocument(ctx context.Context, key *resourcepb.R
if team.Spec.ExternalUID != "" {
doc.Fields[TEAM_SEARCH_EXTERNAL_UID] = team.Spec.ExternalUID
}
if len(team.Spec.Members) > 0 {
uids := make([]string, 0, len(team.Spec.Members))
for _, m := range team.Spec.Members {
uids = append(uids, m.Name)
}
doc.Fields[TEAM_SEARCH_MEMBERS] = uids
}
return doc, nil
}
+5
View File
@@ -6,6 +6,7 @@ import (
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"github.com/grafana/grafana/pkg/services/featuremgmt"
@@ -57,6 +58,10 @@ func TestIntegrationIdentity(t *testing.T) {
})
rsp, err := teamClient.Resource.List(ctx, metav1.ListOptions{})
require.NoError(t, err)
// Members have randomly-generated UIDs; drop them from comparison.
for i := range rsp.Items {
unstructured.RemoveNestedField(rsp.Items[i].Object, "spec", "members")
}
found := teamClient.SanitizeJSONList(rsp, "name", "labels")
require.JSONEq(t, `{
"items": [
@@ -3,12 +3,14 @@ package team
import (
"context"
"fmt"
"sync"
"testing"
"time"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"github.com/grafana/grafana/pkg/apiserver/rest"
"github.com/grafana/grafana/pkg/services/featuremgmt"
@@ -41,6 +43,7 @@ func TestIntegrationTeams(t *testing.T) {
})
doTeamCRUDTestsUsingTheNewAPIs(t, helper)
doTeamSpecMembersTests(t, helper)
if mode < 3 {
doTeamCRUDTestsUsingTheLegacyAPIs(t, helper, mode)
@@ -192,6 +195,25 @@ func doTeamCRUDTestsUsingTheNewAPIs(t *testing.T, helper *apis.K8sTestHelper) {
require.Contains(t, statusErr.ErrStatus.Message, "externalUID is only allowed for provisioned teams")
})
t.Run("should return AlreadyExists when creating a team with a taken name", func(t *testing.T) {
ctx := context.Background()
teamClient := helper.GetResourceClient(apis.ResourceClientArgs{
User: helper.Org1.Admin,
Namespace: helper.Namespacer(helper.Org1.Admin.Identity.GetOrgID()),
GVR: gvrTeams,
})
created, err := teamClient.Resource.Create(ctx, helper.LoadYAMLOrJSONFile("../testdata/team-test-create-v0.yaml"), metav1.CreateOptions{})
require.NoError(t, err)
t.Cleanup(func() { _ = teamClient.Resource.Delete(ctx, created.GetName(), metav1.DeleteOptions{}) })
_, err = teamClient.Resource.Create(ctx, helper.LoadYAMLOrJSONFile("../testdata/team-test-create-v0.yaml"), metav1.CreateOptions{})
require.Error(t, err)
var statusErr *errors.StatusError
require.ErrorAs(t, err, &statusErr)
require.Equal(t, int32(409), statusErr.ErrStatus.Code)
})
t.Run("should create team with generateName and get it using the new APIs as a GrafanaAdmin", func(t *testing.T) {
ctx := context.Background()
@@ -340,3 +362,193 @@ func doTeamCRUDTestsUsingTheLegacyAPIs(t *testing.T, helper *apis.K8sTestHelper,
require.Contains(t, statusErr.ErrStatus.Message, "not found")
})
}
func doTeamSpecMembersTests(t *testing.T, helper *apis.K8sTestHelper) {
teamClient := helper.GetResourceClient(apis.ResourceClientArgs{
User: helper.Org1.Admin,
Namespace: helper.Namespacer(helper.Org1.Admin.Identity.GetOrgID()),
GVR: gvrTeams,
})
editorUID := helper.Org1.Editor.Identity.GetIdentifier()
viewerUID := helper.Org1.Viewer.Identity.GetIdentifier()
newTeamWithMembers := func(prefix string, members []map[string]interface{}) *unstructured.Unstructured {
body := map[string]interface{}{
"apiVersion": "iam.grafana.app/v0alpha1",
"kind": "Team",
"metadata": map[string]interface{}{"generateName": prefix},
"spec": map[string]interface{}{
"title": "Team " + prefix,
"email": prefix + "@example.com",
"provisioned": false,
"externalUID": "",
"members": members,
},
}
return &unstructured.Unstructured{Object: body}
}
memberSpec := func(uid, permission string, external bool) map[string]interface{} {
return map[string]interface{}{
"kind": "User",
"name": uid,
"permission": permission,
"external": external,
}
}
t.Run("should create team with members and hydrate on Get", func(t *testing.T) {
ctx := context.Background()
obj := newTeamWithMembers("team-members-", []map[string]interface{}{
memberSpec(editorUID, "member", false),
})
created, err := teamClient.Resource.Create(ctx, obj, metav1.CreateOptions{})
require.NoError(t, err)
t.Cleanup(func() { _ = teamClient.Resource.Delete(ctx, created.GetName(), metav1.DeleteOptions{}) })
fetched, err := teamClient.Resource.Get(ctx, created.GetName(), metav1.GetOptions{})
require.NoError(t, err)
members, found, err := unstructured.NestedSlice(fetched.Object, "spec", "members")
require.NoError(t, err)
require.True(t, found)
require.Len(t, members, 1)
m := members[0].(map[string]interface{})
require.Equal(t, "User", m["kind"])
require.Equal(t, editorUID, m["name"])
require.Equal(t, "member", m["permission"])
require.Equal(t, false, m["external"])
})
t.Run("should add, update permission, and remove members via Update", func(t *testing.T) {
ctx := context.Background()
obj := newTeamWithMembers("team-members-upd-", []map[string]interface{}{
memberSpec(editorUID, "member", false),
})
created, err := teamClient.Resource.Create(ctx, obj, metav1.CreateOptions{})
require.NoError(t, err)
t.Cleanup(func() { _ = teamClient.Resource.Delete(ctx, created.GetName(), metav1.DeleteOptions{}) })
// Add viewer as admin alongside editor
fetched, err := teamClient.Resource.Get(ctx, created.GetName(), metav1.GetOptions{})
require.NoError(t, err)
require.NoError(t, unstructured.SetNestedSlice(fetched.Object, []interface{}{
memberSpec(editorUID, "member", false),
memberSpec(viewerUID, "admin", false),
}, "spec", "members"))
updated, err := teamClient.Resource.Update(ctx, fetched, metav1.UpdateOptions{})
require.NoError(t, err)
members, _, _ := unstructured.NestedSlice(updated.Object, "spec", "members")
require.Len(t, members, 2)
// Promote editor to admin
fetched, err = teamClient.Resource.Get(ctx, created.GetName(), metav1.GetOptions{})
require.NoError(t, err)
require.NoError(t, unstructured.SetNestedSlice(fetched.Object, []interface{}{
memberSpec(editorUID, "admin", false),
memberSpec(viewerUID, "admin", false),
}, "spec", "members"))
updated, err = teamClient.Resource.Update(ctx, fetched, metav1.UpdateOptions{})
require.NoError(t, err)
members, _, _ = unstructured.NestedSlice(updated.Object, "spec", "members")
require.Len(t, members, 2)
for _, raw := range members {
m := raw.(map[string]interface{})
require.Equal(t, "admin", m["permission"])
}
// Remove viewer
fetched, err = teamClient.Resource.Get(ctx, created.GetName(), metav1.GetOptions{})
require.NoError(t, err)
require.NoError(t, unstructured.SetNestedSlice(fetched.Object, []interface{}{
memberSpec(editorUID, "admin", false),
}, "spec", "members"))
updated, err = teamClient.Resource.Update(ctx, fetched, metav1.UpdateOptions{})
require.NoError(t, err)
members, _, _ = unstructured.NestedSlice(updated.Object, "spec", "members")
require.Len(t, members, 1)
m := members[0].(map[string]interface{})
require.Equal(t, editorUID, m["name"])
})
t.Run("should reject toggling external on an existing member", func(t *testing.T) {
ctx := context.Background()
obj := newTeamWithMembers("team-members-ext-", []map[string]interface{}{
memberSpec(editorUID, "member", false),
})
created, err := teamClient.Resource.Create(ctx, obj, metav1.CreateOptions{})
require.NoError(t, err)
t.Cleanup(func() { _ = teamClient.Resource.Delete(ctx, created.GetName(), metav1.DeleteOptions{}) })
require.NoError(t, unstructured.SetNestedSlice(created.Object, []interface{}{
memberSpec(editorUID, "member", true),
}, "spec", "members"))
_, err = teamClient.Resource.Update(ctx, created, metav1.UpdateOptions{})
require.Error(t, err)
var se *errors.StatusError
require.ErrorAs(t, err, &se)
require.Equal(t, int32(400), se.ErrStatus.Code)
require.Contains(t, se.ErrStatus.Message, "external")
})
// Guards the ErrTeamMemberAlreadyAdded → apierrors.NewConflict mapping on
// the Update path. Several goroutines race to add the same user; if two
// reach legacy.UpdateTeam before either commits, the second INSERT hits the
// UNIQUE(org_id, team_id, user_id) constraint and the store must surface a
// 409 (not a 500) so the client can retry.
t.Run("concurrent adds of the same member never return 500", func(t *testing.T) {
ctx := context.Background()
obj := newTeamWithMembers("team-members-race-", []map[string]interface{}{})
created, err := teamClient.Resource.Create(ctx, obj, metav1.CreateOptions{})
require.NoError(t, err)
t.Cleanup(func() { _ = teamClient.Resource.Delete(ctx, created.GetName(), metav1.DeleteOptions{}) })
const parallel = 10
var wg sync.WaitGroup
errs := make([]error, parallel)
start := make(chan struct{})
for i := 0; i < parallel; i++ {
wg.Add(1)
go func(i int) {
defer wg.Done()
<-start
fetched, getErr := teamClient.Resource.Get(ctx, created.GetName(), metav1.GetOptions{})
if getErr != nil {
errs[i] = getErr
return
}
if err := unstructured.SetNestedSlice(fetched.Object, []interface{}{
memberSpec(editorUID, "member", false),
}, "spec", "members"); err != nil {
errs[i] = err
return
}
_, errs[i] = teamClient.Resource.Update(ctx, fetched, metav1.UpdateOptions{})
}(i)
}
close(start)
wg.Wait()
successes := 0
for i, e := range errs {
if e == nil {
successes++
continue
}
var se *errors.StatusError
require.ErrorAs(t, e, &se, "goroutine %d returned non-status error: %v", i, e)
require.NotEqual(t, int32(500), se.ErrStatus.Code,
"goroutine %d got 500 (%q); member-add race must surface as 409", i, se.ErrStatus.Message)
}
require.Greater(t, successes, 0, "expected at least one goroutine to succeed")
// Final state must contain the user exactly once regardless of race
// outcomes, proving the retries converge.
final, err := teamClient.Resource.Get(ctx, created.GetName(), metav1.GetOptions{})
require.NoError(t, err)
members, _, _ := unstructured.NestedSlice(final.Object, "spec", "members")
require.Len(t, members, 1)
m := members[0].(map[string]interface{})
require.Equal(t, editorUID, m["name"])
})
}
@@ -6724,7 +6724,8 @@
"title",
"email",
"provisioned",
"externalUID"
"externalUID",
"members"
],
"properties": {
"email": {
@@ -6735,6 +6736,17 @@
"type": "string",
"default": ""
},
"members": {
"type": "array",
"items": {
"default": {},
"allOf": [
{
"$ref": "#/components/schemas/com.github.grafana.grafana.apps.iam.pkg.apis.iam.v0alpha1.TeamTeamMember"
}
]
}
},
"provisioned": {
"type": "boolean",
"default": false
@@ -6745,6 +6757,37 @@
}
}
},
"com.github.grafana.grafana.apps.iam.pkg.apis.iam.v0alpha1.TeamTeamMember": {
"type": "object",
"required": [
"kind",
"name",
"permission",
"external"
],
"properties": {
"external": {
"description": "whether the member was added externally (e.g. team sync)",
"type": "boolean",
"default": false
},
"kind": {
"description": "kind of the identity",
"type": "string",
"default": ""
},
"name": {
"description": "uid of the identity",
"type": "string",
"default": ""
},
"permission": {
"description": "permission of the identity in the team",
"type": "string",
"default": ""
}
}
},
"com.github.grafana.grafana.apps.iam.pkg.apis.iam.v0alpha1.User": {
"type": "object",
"required": [
+2
View File
@@ -147,6 +147,8 @@ export function teamDtoToTeam(dto: TeamDto): Team {
email: dto.email ?? '',
externalUID: dto.externalUID ?? '',
provisioned: dto.isProvisioned,
// FIXME: Legacy API does not return team members, so this will always be an empty array. We should either update the legacy API to include members or make a separate call to fetch them.
members: [],
},
};
}