IAM: Implement update method in team API (#111660)

* implement team update in legacy store

* add unit tests

* add integration test

* set permissions for user in integration tests

* add more integration tests for update

* update validations

* add unit tests for ValidateOnUpdate() func

* fix integration test

pkg/registry/apis/iam/legacy/sql.go

@@ -31,6 +31,7 @@ type LegacyIdentityStore interface {
 
 	GetTeamInternalID(ctx context.Context, ns claims.NamespaceInfo, query GetTeamInternalIDQuery) (*GetTeamInternalIDResult, error)
 	CreateTeam(ctx context.Context, ns claims.NamespaceInfo, cmd CreateTeamCommand) (*CreateTeamResult, error)
+	UpdateTeam(ctx context.Context, ns claims.NamespaceInfo, cmd UpdateTeamCommand) (*UpdateTeamResult, error)
 	ListTeams(ctx context.Context, ns claims.NamespaceInfo, query ListTeamQuery) (*ListTeamResult, error)
 	DeleteTeam(ctx context.Context, ns claims.NamespaceInfo, cmd DeleteTeamCommand) error
 	ListTeamBindings(ctx context.Context, ns claims.NamespaceInfo, query ListTeamBindingsQuery) (*ListTeamBindingsResult, error)

pkg/registry/apis/iam/legacy/sql_test.go

@@ -60,6 +60,12 @@ func TestIdentityQueries(t *testing.T) {
 		return &v
 	}
 
+	updateTeam := func(cmd *UpdateTeamCommand) sqltemplate.SQLTemplate {
+		v := newUpdateTeam(nodb, cmd)
+		v.SQLTemplate = mocks.NewTestingSQLTemplate()
+		return &v
+	}
+
 	listTeams := func(q *ListTeamQuery) sqltemplate.SQLTemplate {
 		v := newListTeams(nodb, q)
 		v.SQLTemplate = mocks.NewTestingSQLTemplate()
@@ -393,6 +399,19 @@ func TestIdentityQueries(t *testing.T) {
 					}),
 				},
 			},
+			sqlUpdateTeamTemplate: {
+				{
+					Name: "update_team_basic",
+					Data: updateTeam(&UpdateTeamCommand{
+						UID:           "team-1",
+						Name:          "Team 1",
+						Email:         "team1@example.com",
+						IsProvisioned: true,
+						ExternalUID:   "team-1-uid",
+						Updated:       NewDBTime(time.Date(2023, 1, 1, 12, 0, 0, 0, time.UTC)),
+					}),
+				},
+			},
 			sqlDeleteTeamTemplate: {
 				{
 					Name: "delete_team_basic",

pkg/registry/apis/iam/legacy/team.go

@@ -265,6 +265,91 @@ func (s *legacySQLStore) CreateTeam(ctx context.Context, ns claims.NamespaceInfo
 	return &CreateTeamResult{Team: createdTeam}, nil
 }
 
+type UpdateTeamCommand struct {
+	UID           string
+	Name          string
+	Updated       DBTime
+	Email         string
+	ExternalID    string
+	IsProvisioned bool
+	ExternalUID   string
+}
+
+type UpdateTeamResult struct {
+	Team team.Team
+}
+
+var sqlUpdateTeamTemplate = mustTemplate("update_team.sql")
+
+func newUpdateTeam(sql *legacysql.LegacyDatabaseHelper, cmd *UpdateTeamCommand) updateTeamQuery {
+	return updateTeamQuery{
+		SQLTemplate: sqltemplate.New(sql.DialectForDriver()),
+		TeamTable:   sql.Table("team"),
+		Command:     cmd,
+	}
+}
+
+type updateTeamQuery struct {
+	sqltemplate.SQLTemplate
+	TeamTable string
+	Command   *UpdateTeamCommand
+}
+
+func (r updateTeamQuery) Validate() error {
+	return nil
+}
+
+func (s *legacySQLStore) UpdateTeam(ctx context.Context, ns claims.NamespaceInfo, cmd UpdateTeamCommand) (*UpdateTeamResult, error) {
+	now := time.Now().UTC().Truncate(time.Second)
+
+	cmd.Updated = NewDBTime(now)
+
+	sql, err := s.sql(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	req := 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)
+		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 {
+			return fmt.Errorf("failed to update team: %w", err)
+		}
+
+		updatedTeam = team.Team{
+			UID:           cmd.UID,
+			Name:          cmd.Name,
+			Email:         cmd.Email,
+			ExternalUID:   cmd.ExternalUID,
+			IsProvisioned: cmd.IsProvisioned,
+			Updated:       cmd.Updated.Time,
+		}
+
+		return nil
+	})
+
+	if err != nil {
+		return nil, err
+	}
+
+	return &UpdateTeamResult{Team: updatedTeam}, nil
+}
+
 type DeleteTeamCommand struct {
 	UID string
 }

pkg/registry/apis/iam/legacy/testdata/mysql--update_team-update_team_basic.sql

@@ -0,0 +1,7 @@
+UPDATE `grafana`.`team`
+SET name = 'Team 1',
+    updated = '2023-01-01 12:00:00',
+    email = 'team1@example.com',
+    is_provisioned = TRUE,
+    external_uid = 'team-1-uid'
+WHERE uid = 'team-1'

pkg/registry/apis/iam/legacy/testdata/postgres--update_team-update_team_basic.sql

@@ -0,0 +1,7 @@
+UPDATE "grafana"."team"
+SET name = 'Team 1',
+    updated = '2023-01-01 12:00:00',
+    email = 'team1@example.com',
+    is_provisioned = TRUE,
+    external_uid = 'team-1-uid'
+WHERE uid = 'team-1'

pkg/registry/apis/iam/legacy/testdata/sqlite--update_team-update_team_basic.sql

@@ -0,0 +1,7 @@
+UPDATE "grafana"."team"
+SET name = 'Team 1',
+    updated = '2023-01-01 12:00:00',
+    email = 'team1@example.com',
+    is_provisioned = TRUE,
+    external_uid = 'team-1-uid'
+WHERE uid = 'team-1'

pkg/registry/apis/iam/legacy/update_team.sql

@@ -0,0 +1,7 @@
+UPDATE {{ .Ident .TeamTable }}
+SET name = {{ .Arg .Command.Name }},
+    updated = {{ .Arg .Command.Updated }},
+    email = {{ .Arg .Command.Email }},
+    is_provisioned = {{ .Arg .Command.IsProvisioned }},
+    external_uid = {{ .Arg .Command.ExternalUID }}
+WHERE uid = {{ .Arg .Command.UID }}

pkg/registry/apis/iam/register.go

@@ -2,6 +2,7 @@ package iam
 
 import (
 	"context"
+	"fmt"
 	"maps"
 	"strings"
 
@@ -353,6 +354,12 @@ func (b *IdentityAccessManagementAPIBuilder) Validate(ctx context.Context, a adm
 		switch typedObj := a.GetObject().(type) {
 		case *iamv0.ResourcePermission:
 			return resourcepermission.ValidateCreateAndUpdateInput(ctx, typedObj)
+		case *iamv0.Team:
+			oldTeamObj, ok := a.GetOldObject().(*iamv0.Team)
+			if !ok {
+				return fmt.Errorf("expected old object to be a Team, got %T", oldTeamObj)
+			}
+			return team.ValidateOnUpdate(ctx, typedObj, oldTeamObj)
 		}
 		return nil
 	case admission.Delete:

pkg/registry/apis/iam/team/store.go

@@ -112,7 +112,52 @@ func (s *LegacyStore) Delete(ctx context.Context, name string, deleteValidation
 
 // Update implements rest.Updater.
 func (s *LegacyStore) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) {
-	return nil, false, apierrors.NewMethodNotSupported(resource.GroupResource(), "update")
+	if !s.enableAuthnMutation {
+		return nil, false, apierrors.NewMethodNotSupported(resource.GroupResource(), "update")
+	}
+
+	ns, err := request.NamespaceInfoFrom(ctx, true)
+	if err != nil {
+		return nil, false, err
+	}
+
+	oldObj, err := s.Get(ctx, name, nil)
+	if err != nil {
+		return oldObj, false, err
+	}
+
+	obj, err := objInfo.UpdatedObject(ctx, oldObj)
+	if err != nil {
+		return oldObj, false, err
+	}
+
+	teamObj, ok := obj.(*iamv0alpha1.Team)
+	if !ok {
+		return nil, false, fmt.Errorf("expected Team object, got %T", obj)
+	}
+
+	if updateValidation != nil {
+		if err := updateValidation(ctx, obj, oldObj); err != nil {
+			return oldObj, false, err
+		}
+	}
+
+	updateCmd := legacy.UpdateTeamCommand{
+		UID:           teamObj.Name,
+		Name:          teamObj.Spec.Title,
+		Email:         teamObj.Spec.Email,
+		IsProvisioned: teamObj.Spec.Provisioned,
+		ExternalUID:   teamObj.Spec.ExternalUID,
+	}
+
+	result, err := s.store.UpdateTeam(ctx, ns, updateCmd)
+	if err != nil {
+		return oldObj, false, err
+	}
+
+	iamTeam := toTeamObject(result.Team, ns)
+
+	return &iamTeam, false, nil
 }
 
 func (s *LegacyStore) List(ctx context.Context, options *internalversion.ListOptions) (runtime.Object, error) {

pkg/registry/apis/iam/team/validate.go

@@ -30,3 +30,28 @@ func ValidateOnCreate(ctx context.Context, obj *iamv0alpha1.Team) error {
 
 	return nil
 }
+
+func ValidateOnUpdate(ctx context.Context, obj, old *iamv0alpha1.Team) error {
+	requester, err := identity.GetRequester(ctx)
+	if err != nil {
+		return apierrors.NewUnauthorized("no identity found")
+	}
+
+	if obj.Spec.Title == "" {
+		return apierrors.NewBadRequest("the team must have a title")
+	}
+
+	if !requester.IsIdentityType(types.TypeServiceAccount) && obj.Spec.Provisioned && !old.Spec.Provisioned {
+		return apierrors.NewBadRequest("provisioned teams are only allowed for service accounts")
+	}
+
+	if old.Spec.Provisioned && !obj.Spec.Provisioned {
+		return apierrors.NewBadRequest("provisioned teams cannot be updated to non-provisioned teams")
+	}
+
+	if !obj.Spec.Provisioned && obj.Spec.ExternalUID != "" {
+		return apierrors.NewBadRequest("externalUID is only allowed for provisioned teams")
+	}
+
+	return nil
+}

pkg/registry/apis/iam/team/validate_test.go

@@ -115,3 +115,213 @@ func TestValidateOnCreate(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateOnUpdate(t *testing.T) {
+	tests := []struct {
+		name      string
+		requester *identity.StaticRequester
+		obj       *iamv0alpha1.Team
+		old       *iamv0alpha1.Team
+		want      error
+	}{
+		{
+			name: "valid update - no changes to provisioned status",
+			requester: &identity.StaticRequester{
+				Type:    types.TypeUser,
+				OrgRole: identity.RoleAdmin,
+			},
+			obj: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title: "updated title",
+					Email: "updated@test.com",
+				},
+			},
+			old: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title: "original title",
+					Email: "original@test.com",
+				},
+			},
+			want: nil,
+		},
+		{
+			name: "valid update - service account changing to provisioned",
+			requester: &identity.StaticRequester{
+				Type:    types.TypeServiceAccount,
+				OrgRole: identity.RoleAdmin,
+			},
+			obj: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title:       "updated title",
+					Email:       "updated@test.com",
+					Provisioned: true,
+					ExternalUID: "test-uid",
+				},
+			},
+			old: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title: "original title",
+					Email: "original@test.com",
+				},
+			},
+			want: nil,
+		},
+		{
+			name: "valid update - already provisioned team",
+			requester: &identity.StaticRequester{
+				Type:    types.TypeServiceAccount,
+				OrgRole: identity.RoleAdmin,
+			},
+			obj: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title:       "updated title",
+					Email:       "updated@test.com",
+					Provisioned: true,
+					ExternalUID: "updated-uid",
+				},
+			},
+			old: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title:       "original title",
+					Email:       "original@test.com",
+					Provisioned: true,
+					ExternalUID: "original-uid",
+				},
+			},
+			want: nil,
+		},
+		{
+			name: "invalid update - no title",
+			requester: &identity.StaticRequester{
+				Type:    types.TypeUser,
+				OrgRole: identity.RoleAdmin,
+			},
+			obj: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title: "",
+					Email: "updated@test.com",
+				},
+			},
+			old: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title: "original title",
+					Email: "original@test.com",
+				},
+			},
+			want: apierrors.NewBadRequest("the team must have a title"),
+		},
+		{
+			name: "invalid update - user trying to change to provisioned",
+			requester: &identity.StaticRequester{
+				Type:    types.TypeUser,
+				OrgRole: identity.RoleAdmin,
+			},
+			obj: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title:       "updated title",
+					Email:       "updated@test.com",
+					Provisioned: true,
+					ExternalUID: "test-uid",
+				},
+			},
+			old: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title: "original title",
+					Email: "original@test.com",
+				},
+			},
+			want: apierrors.NewBadRequest("provisioned teams are only allowed for service accounts"),
+		},
+		{
+			name: "invalid update - changing from provisioned to non-provisioned",
+			requester: &identity.StaticRequester{
+				Type:    types.TypeServiceAccount,
+				OrgRole: identity.RoleAdmin,
+			},
+			obj: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title: "updated title",
+					Email: "updated@test.com",
+				},
+			},
+			old: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title:       "original title",
+					Email:       "original@test.com",
+					Provisioned: true,
+					ExternalUID: "original-uid",
+				},
+			},
+			want: apierrors.NewBadRequest("provisioned teams cannot be updated to non-provisioned teams"),
+		},
+		{
+			name: "invalid update - has externalUID but not provisioned",
+			requester: &identity.StaticRequester{
+				Type:    types.TypeUser,
+				OrgRole: identity.RoleAdmin,
+			},
+			obj: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title:       "updated title",
+					Email:       "updated@test.com",
+					ExternalUID: "test-uid",
+				},
+			},
+			old: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title: "original title",
+					Email: "original@test.com",
+				},
+			},
+			want: apierrors.NewBadRequest("externalUID is only allowed for provisioned teams"),
+		},
+		{
+			name:      "invalid update - no requester in context",
+			requester: nil,
+			obj: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title: "updated title",
+					Email: "updated@test.com",
+				},
+			},
+			old: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title: "original title",
+					Email: "original@test.com",
+				},
+			},
+			want: apierrors.NewUnauthorized("no identity found"),
+		},
+		{
+			name: "valid update - adding externalUID to provisioned team",
+			requester: &identity.StaticRequester{
+				Type:    types.TypeServiceAccount,
+				OrgRole: identity.RoleAdmin,
+			},
+			obj: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title:       "updated title",
+					Email:       "updated@test.com",
+					Provisioned: true,
+					ExternalUID: "new-uid",
+				},
+			},
+			old: &iamv0alpha1.Team{
+				Spec: iamv0alpha1.TeamSpec{
+					Title:       "original title",
+					Email:       "original@test.com",
+					Provisioned: true,
+				},
+			},
+			want: nil,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			ctx := identity.WithRequester(context.Background(), test.requester)
+			err := ValidateOnUpdate(ctx, test.obj, test.old)
+			assert.Equal(t, test.want, err)
+		})
+	}
+}

pkg/tests/apis/iam/team_integration_test.go

@@ -38,17 +38,18 @@ func TestIntegrationTeams(t *testing.T) {
 					featuremgmt.FlagKubernetesAuthnMutation,
 				},
 			})
+
 			doTeamCRUDTestsUsingTheNewAPIs(t, helper)
 
 			if mode < 3 {
-				doTeamCRUDTestsUsingTheLegacyAPIs(t, helper)
+				doTeamCRUDTestsUsingTheLegacyAPIs(t, helper, mode)
 			}
 		})
 	}
 }
 
 func doTeamCRUDTestsUsingTheNewAPIs(t *testing.T, helper *apis.K8sTestHelper) {
-	t.Run("should create/get/delete team using the new APIs as a GrafanaAdmin", func(t *testing.T) {
+	t.Run("should create/get/update/delete team using the new APIs as a GrafanaAdmin", func(t *testing.T) {
 		ctx := context.Background()
 
 		teamClient := helper.GetResourceClient(apis.ResourceClientArgs{
@@ -57,6 +58,7 @@ func doTeamCRUDTestsUsingTheNewAPIs(t *testing.T, helper *apis.K8sTestHelper) {
 			GVR:       gvrTeams,
 		})
 
+		// Create the team
 		created, err := teamClient.Resource.Create(ctx, helper.LoadYAMLOrJSONFile("testdata/team-test-create-v0.yaml"), metav1.CreateOptions{})
 		require.NoError(t, err)
 		require.NotNil(t, created)
@@ -69,6 +71,7 @@ func doTeamCRUDTestsUsingTheNewAPIs(t *testing.T, helper *apis.K8sTestHelper) {
 		createdUID := created.GetName()
 		require.NotEmpty(t, createdUID)
 
+		// Get the team
 		fetched, err := teamClient.Resource.Get(ctx, createdUID, metav1.GetOptions{})
 		require.NoError(t, err)
 		require.NotNil(t, fetched)
@@ -81,6 +84,26 @@ func doTeamCRUDTestsUsingTheNewAPIs(t *testing.T, helper *apis.K8sTestHelper) {
 		require.Equal(t, createdUID, fetched.GetName())
 		require.Equal(t, "default", fetched.GetNamespace())
 
+		// Update the team
+		updatedTeam, err := teamClient.Resource.Update(ctx, helper.LoadYAMLOrJSONFile("testdata/team-test-update-v0.yaml"), metav1.UpdateOptions{})
+		require.NoError(t, err)
+		require.NotNil(t, updatedTeam)
+
+		updatedSpec := updatedTeam.Object["spec"].(map[string]interface{})
+		require.Equal(t, "Test Team 2", updatedSpec["title"])
+		require.Equal(t, "testteam2@example123.com", updatedSpec["email"])
+		require.Equal(t, false, updatedSpec["provisioned"])
+
+		verifiedTeam, err := teamClient.Resource.Get(ctx, createdUID, metav1.GetOptions{})
+		require.NoError(t, err)
+		require.NotNil(t, verifiedTeam)
+
+		verifiedSpec := verifiedTeam.Object["spec"].(map[string]interface{})
+		require.Equal(t, "Test Team 2", verifiedSpec["title"])
+		require.Equal(t, "testteam2@example123.com", verifiedSpec["email"])
+		require.Equal(t, false, verifiedSpec["provisioned"])
+
+		// Delete the team
 		err = teamClient.Resource.Delete(ctx, createdUID, metav1.DeleteOptions{})
 		require.NoError(t, err)
 
@@ -202,12 +225,14 @@ func doTeamCRUDTestsUsingTheNewAPIs(t *testing.T, helper *apis.K8sTestHelper) {
 	})
 }
 
-func doTeamCRUDTestsUsingTheLegacyAPIs(t *testing.T, helper *apis.K8sTestHelper) {
-	t.Run("should create team using legacy APIs and get/delete it using the new APIs", func(t *testing.T) {
+func doTeamCRUDTestsUsingTheLegacyAPIs(t *testing.T, helper *apis.K8sTestHelper, mode rest.DualWriterMode) {
+	t.Run("should create team using legacy APIs and get/update/delete it using the new APIs", func(t *testing.T) {
 		ctx := context.Background()
+
 		teamClient := helper.GetResourceClient(apis.ResourceClientArgs{
-			User: helper.Org1.Admin,
-			GVR:  gvrTeams,
+			User:      helper.Org1.Admin,
+			Namespace: helper.Namespacer(helper.Org1.Admin.Identity.GetOrgID()),
+			GVR:       gvrTeams,
 		})
 
 		legacyTeamPayload := `{
@@ -243,6 +268,31 @@ func doTeamCRUDTestsUsingTheLegacyAPIs(t *testing.T, helper *apis.K8sTestHelper)
 		require.Equal(t, rsp.Result.UID, team.GetName())
 		require.Equal(t, "default", team.GetNamespace())
 
+		// Updating the team is not supported in Mode2 if the team has been created using the legacy APIs
+		if mode < rest.Mode2 {
+			team.Object["spec"].(map[string]interface{})["title"] = "Updated Test Team 2"
+			team.Object["spec"].(map[string]interface{})["email"] = "updated@example.com"
+
+			updatedTeam, err := teamClient.Resource.Update(ctx, team, metav1.UpdateOptions{})
+			require.NoError(t, err)
+			require.NotNil(t, updatedTeam)
+
+			updatedSpec := updatedTeam.Object["spec"].(map[string]interface{})
+			require.Equal(t, "Updated Test Team 2", updatedSpec["title"])
+			require.Equal(t, "updated@example.com", updatedSpec["email"])
+			require.Equal(t, false, updatedSpec["provisioned"])
+
+			verifiedTeam, err := teamClient.Resource.Get(ctx, rsp.Result.UID, metav1.GetOptions{})
+			require.NoError(t, err)
+			require.NotNil(t, verifiedTeam)
+
+			verifiedSpec := verifiedTeam.Object["spec"].(map[string]interface{})
+			require.Equal(t, "Updated Test Team 2", verifiedSpec["title"])
+			require.Equal(t, "updated@example.com", verifiedSpec["email"])
+			require.Equal(t, false, verifiedSpec["provisioned"])
+		}
+
+		// Delete the team
 		err = teamClient.Resource.Delete(ctx, rsp.Result.UID, metav1.DeleteOptions{})
 		require.NoError(t, err)
 

pkg/tests/apis/iam/testdata/team-test-update-v0.yaml

@@ -0,0 +1,7 @@
+apiVersion: iam.grafana.app/v0alpha1
+kind: Team
+metadata:
+  name: test-team-1
+spec:
+  title: "Test Team 2"
+  email: testteam2@example123.com