Skip to content

Commit 421c932

Browse files
twuspinillos
andauthored
fix(roles): Handle issue with duplicate IDs cause 400 despite using existing resources (#2254)
* fix(roles): Handle issue where duplicate ids cause invalid requests * make generate-templates * make generate-templates * make docs * Update schema * Regenerate schema --------- Co-authored-by: spinillos <[email protected]>
1 parent b95f74a commit 421c932

File tree

4 files changed

+103
-4
lines changed

4 files changed

+103
-4
lines changed

internal/resources/grafana/resource_role_assignment_item.go

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,17 @@ func (r *resourceRoleAssignmentItem) Create(ctx context.Context, req resource.Cr
163163
resp.Diagnostics.AddError("Failed to parse team ID", err.Error())
164164
return
165165
}
166-
roleAssignments.Teams = append(roleAssignments.Teams, teamID)
166+
// Check if team is already assigned to avoid duplicates
167+
teamExists := false
168+
for _, existingTeamID := range roleAssignments.Teams {
169+
if existingTeamID == teamID {
170+
teamExists = true
171+
break
172+
}
173+
}
174+
if !teamExists {
175+
roleAssignments.Teams = append(roleAssignments.Teams, teamID)
176+
}
167177
assignmentType = "team"
168178
resourceID = teamIDStr
169179
case !data.UserID.IsNull():
@@ -172,7 +182,17 @@ func (r *resourceRoleAssignmentItem) Create(ctx context.Context, req resource.Cr
172182
resp.Diagnostics.AddError("Failed to parse user ID", err.Error())
173183
return
174184
}
175-
roleAssignments.Users = append(roleAssignments.Users, userID)
185+
// Check if user is already assigned to avoid duplicates
186+
userExists := false
187+
for _, existingUserID := range roleAssignments.Users {
188+
if existingUserID == userID {
189+
userExists = true
190+
break
191+
}
192+
}
193+
if !userExists {
194+
roleAssignments.Users = append(roleAssignments.Users, userID)
195+
}
176196
assignmentType = "user"
177197
resourceID = data.UserID.ValueString()
178198
case !data.ServiceAccountID.IsNull():
@@ -182,7 +202,17 @@ func (r *resourceRoleAssignmentItem) Create(ctx context.Context, req resource.Cr
182202
resp.Diagnostics.AddError("Failed to parse service account ID", err.Error())
183203
return
184204
}
185-
roleAssignments.ServiceAccounts = append(roleAssignments.ServiceAccounts, serviceAccountID)
205+
// Check if service account is already assigned to avoid duplicates
206+
saExists := false
207+
for _, existingSAID := range roleAssignments.ServiceAccounts {
208+
if existingSAID == serviceAccountID {
209+
saExists = true
210+
break
211+
}
212+
}
213+
if !saExists {
214+
roleAssignments.ServiceAccounts = append(roleAssignments.ServiceAccounts, serviceAccountID)
215+
}
186216
assignmentType = "service_account"
187217
resourceID = serviceAccountIDStr
188218
}

internal/resources/grafana/resource_role_assignment_item_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,29 @@ func TestAccRoleAssignmentItem_withCloudServiceAccount(t *testing.T) {
121121
})
122122
}
123123

124+
func TestAccRoleAssignmentItem_NoDuplicates(t *testing.T) {
125+
testutils.CheckEnterpriseTestsEnabled(t, ">=9.0.0")
126+
127+
testName := acctest.RandString(10)
128+
var role models.RoleDTO
129+
130+
resource.ParallelTest(t, resource.TestCase{
131+
ProtoV5ProviderFactories: testutils.ProtoV5ProviderFactories,
132+
CheckDestroy: roleAssignmentCheckExists.destroyed(&role, nil),
133+
Steps: []resource.TestStep{
134+
{
135+
Config: roleAssignmentItemNoDuplicatesConfig(testName),
136+
Check: resource.ComposeTestCheckFunc(
137+
roleAssignmentCheckExists.exists("grafana_role.test", &role),
138+
// Verify the role assignment doesn't contain duplicates
139+
resource.TestCheckResourceAttr("grafana_role_assignment_item.user1", "role_uid", testName),
140+
resource.TestCheckResourceAttr("grafana_role_assignment_item.team", "role_uid", testName),
141+
),
142+
},
143+
},
144+
})
145+
}
146+
124147
func roleAssignmentItemConfig(name string) string {
125148
return fmt.Sprintf(`
126149
resource "grafana_role" "test" {
@@ -289,3 +312,46 @@ resource "grafana_role_assignment_item" "service_account" {
289312
}
290313
`, name)
291314
}
315+
316+
func roleAssignmentItemNoDuplicatesConfig(name string) string {
317+
return fmt.Sprintf(`
318+
// Create a test role that will have multiple assignments
319+
// This role will be the target for both user and team assignments
320+
resource "grafana_role" "test" {
321+
name = "%[1]s"
322+
description = "test desc"
323+
version = 1
324+
uid = "%[1]s"
325+
global = true
326+
group = "testgroup"
327+
display_name = "testdisplay"
328+
hidden = true
329+
}
330+
331+
// Create a test team that will be assigned to the role
332+
resource "grafana_team" "test_team" {
333+
name = "%[1]s"
334+
}
335+
336+
// Create a test user that will be assigned to the same role
337+
resource "grafana_user" "test_user" {
338+
email = "%[1][email protected]"
339+
login = "%[1][email protected]"
340+
password = "12345"
341+
}
342+
343+
// Multiple grafana_role_assignment_item resources targeting the SAME role.
344+
// This setup reproduces the bug where duplicate assignments could be created
345+
// when multiple assignment items reference the same role UID.
346+
347+
resource grafana_role_assignment_item "user1" {
348+
role_uid = grafana_role.test.uid // Same role as team assignment
349+
user_id = grafana_user.test_user.id
350+
}
351+
352+
resource grafana_role_assignment_item "team" {
353+
role_uid = grafana_role.test.uid // Same role as user1 assignment
354+
team_id = grafana_team.test_team.id
355+
}
356+
`, name)
357+
}

pkg/generate/postprocessing/replace_references.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,9 @@ var knownReferences = []string{
142142
"grafana_team_external_group.team_id=grafana_team.id",
143143
"grafana_team_preferences.home_dashboard_uid=grafana_dashboard.uid",
144144
"grafana_team_preferences.team_id=grafana_team.id",
145+
"grafana_user.role_uid=grafana_role.uid",
146+
"grafana_user.team_id=grafana_team.id",
147+
"grafana_user.user_id=grafana_user.id",
145148
}
146149

147150
func ReplaceReferences(fpath string, plannedState *tfjson.Plan, extraKnownReferences []string) error {

provider_schema.json

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)