Skip to content

Commit 3bc1bb2

Browse files
dlukmanjulienduchesne
dlukman
and
julienduchesne
authored
Fix removal of old team members when new are added (#795)
* Fix removal of old team members when new are added * Add test case about adding a team member Also, reformat the entire team test suite: - Allow parallel testing (unique team and user names) - Merge all test definitions into a function --------- Co-authored-by: Julien Duchesne <[email protected]>
1 parent d609df0 commit 3bc1bb2

File tree

2 files changed

+68
-119
lines changed

2 files changed

+68
-119
lines changed

‎grafana/resource_team.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ A set of email addresses corresponding to users who should be given membership
7171
to the team. Note: users specified here must already exist in Grafana.
7272
`,
7373
DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
74-
if old == new || (new == "[]" && old == "") || (new == "" && old == "[]") {
74+
if (new == "[]" && old == "") || (new == "" && old == "[]") {
7575
return true
7676
}
7777
return false

‎grafana/resource_team_test.go‎

Lines changed: 67 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@ package grafana
33
import (
44
"fmt"
55
"strconv"
6+
"strings"
67
"testing"
78

89
gapi "github.com/grafana/grafana-api-golang-client"
10+
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
911
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
1012
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
1113
)
@@ -14,37 +16,29 @@ func TestAccTeam_basic(t *testing.T) {
1416
CheckOSSTestsEnabled(t)
1517

1618
var team gapi.Team
19+
teamName := acctest.RandString(5)
20+
teamNameUpdated := acctest.RandString(5)
1721

18-
// TODO: Make parallelizable
19-
resource.Test(t, resource.TestCase{
22+
resource.ParallelTest(t, resource.TestCase{
2023
ProviderFactories: testAccProviderFactories,
2124
CheckDestroy: testAccTeamCheckDestroy(&team),
2225
Steps: []resource.TestStep{
2326
{
24-
Config: testAccTeamConfig_basic,
27+
Config: testAccTeamDefinition(teamName, nil),
2528
Check: resource.ComposeTestCheckFunc(
2629
testAccTeamCheckExists("grafana_team.test", &team),
27-
resource.TestCheckResourceAttr(
28-
"grafana_team.test", "name", "terraform-acc-test",
29-
),
30-
resource.TestCheckResourceAttr(
31-
"grafana_team.test", "email", "[email protected]",
32-
),
33-
resource.TestMatchResourceAttr(
34-
"grafana_team.test", "id", idRegexp,
35-
),
30+
resource.TestCheckResourceAttr("grafana_team.test", "name", teamName),
31+
resource.TestCheckResourceAttr("grafana_team.test", "email", teamName+"@example.com"),
32+
resource.TestMatchResourceAttr("grafana_team.test", "id", idRegexp),
3633
),
3734
},
3835
{
39-
Config: testAccTeamConfig_updateName,
36+
Config: testAccTeamDefinition(teamNameUpdated, nil),
4037
Check: resource.ComposeTestCheckFunc(
4138
testAccTeamCheckExists("grafana_team.test", &team),
42-
resource.TestCheckResourceAttr(
43-
"grafana_team.test", "name", "terraform-acc-test-update",
44-
),
45-
resource.TestCheckResourceAttr(
46-
"grafana_team.test", "email", "[email protected]",
47-
),
39+
resource.TestCheckResourceAttr("grafana_team.test", "name", teamNameUpdated),
40+
resource.TestCheckResourceAttr("grafana_team.test", "email", teamNameUpdated+"@example.com"),
41+
resource.TestMatchResourceAttr("grafana_team.test", "id", idRegexp),
4842
),
4943
},
5044
{
@@ -61,46 +55,47 @@ func TestAccTeam_Members(t *testing.T) {
6155
CheckOSSTestsEnabled(t)
6256

6357
var team gapi.Team
58+
teamName := acctest.RandString(5)
6459

65-
// TODO: Make parallelizable
66-
resource.Test(t, resource.TestCase{
60+
resource.ParallelTest(t, resource.TestCase{
6761
ProviderFactories: testAccProviderFactories,
6862
CheckDestroy: testAccTeamCheckDestroy(&team),
6963
Steps: []resource.TestStep{
7064
{
71-
Config: testAccTeamConfig_memberAdd,
65+
Config: testAccTeamDefinition(teamName, []string{
66+
"grafana_user.users.0.email",
67+
"grafana_user.users.1.email",
68+
}),
7269
Check: resource.ComposeTestCheckFunc(
7370
testAccTeamCheckExists("grafana_team.test", &team),
74-
resource.TestCheckResourceAttr(
75-
"grafana_team.test", "name", "terraform-acc-test",
76-
),
77-
resource.TestCheckResourceAttr(
78-
"grafana_team.test", "members.#", "2",
79-
),
80-
resource.TestCheckResourceAttr(
81-
"grafana_team.test", "members.0", "[email protected]",
82-
),
83-
resource.TestCheckResourceAttr(
84-
"grafana_team.test", "members.1", "[email protected]",
85-
),
71+
resource.TestCheckResourceAttr("grafana_team.test", "name", teamName),
72+
resource.TestCheckResourceAttr("grafana_team.test", "members.#", "2"),
73+
resource.TestCheckResourceAttr("grafana_team.test", "members.0", teamName+"[email protected]"),
74+
resource.TestCheckResourceAttr("grafana_team.test", "members.1", teamName+"[email protected]"),
8675
),
8776
},
77+
// Reorder members but only plan changes. There should be no changes.
8878
{
89-
Config: testAccTeamConfig_memberReorder,
79+
Config: testAccTeamDefinition(teamName, []string{
80+
"grafana_user.users.1.email",
81+
"grafana_user.users.0.email",
82+
}),
83+
PlanOnly: true,
84+
},
85+
// When adding a new member, the state should be updated and re-sorted.
86+
{
87+
Config: testAccTeamDefinition(teamName, []string{
88+
"grafana_user.users.1.email",
89+
"grafana_user.users.0.email",
90+
"grafana_user.users.2.email",
91+
}),
9092
Check: resource.ComposeTestCheckFunc(
9193
testAccTeamCheckExists("grafana_team.test", &team),
92-
resource.TestCheckResourceAttr(
93-
"grafana_team.test", "name", "terraform-acc-test",
94-
),
95-
resource.TestCheckResourceAttr(
96-
"grafana_team.test", "members.#", "2",
97-
),
98-
resource.TestCheckResourceAttr(
99-
"grafana_team.test", "members.0", "[email protected]",
100-
),
101-
resource.TestCheckResourceAttr(
102-
"grafana_team.test", "members.1", "[email protected]",
103-
),
94+
resource.TestCheckResourceAttr("grafana_team.test", "name", teamName),
95+
resource.TestCheckResourceAttr("grafana_team.test", "members.#", "3"),
96+
resource.TestCheckResourceAttr("grafana_team.test", "members.0", teamName+"[email protected]"),
97+
resource.TestCheckResourceAttr("grafana_team.test", "members.1", teamName+"[email protected]"),
98+
resource.TestCheckResourceAttr("grafana_team.test", "members.2", teamName+"[email protected]"),
10499
),
105100
},
106101
// Test the import with members
@@ -111,15 +106,11 @@ func TestAccTeam_Members(t *testing.T) {
111106
ImportStateVerifyIgnore: []string{"ignore_externally_synced_members"},
112107
},
113108
{
114-
Config: testAccTeamConfig_memberRemove,
109+
Config: testAccTeamDefinition(teamName, nil),
115110
Check: resource.ComposeTestCheckFunc(
116111
testAccTeamCheckExists("grafana_team.test", &team),
117-
resource.TestCheckResourceAttr(
118-
"grafana_team.test", "name", "terraform-acc-test",
119-
),
120-
resource.TestCheckResourceAttr(
121-
"grafana_team.test", "members.#", "0",
122-
),
112+
resource.TestCheckResourceAttr("grafana_team.test", "name", teamName),
113+
resource.TestCheckResourceAttr("grafana_team.test", "members.#", "0"),
123114
),
124115
},
125116
},
@@ -133,9 +124,9 @@ func TestAccTeam_RemoveUnexistingMember(t *testing.T) {
133124

134125
var team gapi.Team
135126
var userID int64 = -1
127+
teamName := acctest.RandString(5)
136128

137-
// TODO: Make parallelizable
138-
resource.Test(t, resource.TestCase{
129+
resource.ParallelTest(t, resource.TestCase{
139130
ProviderFactories: testAccProviderFactories,
140131
CheckDestroy: testAccTeamCheckDestroy(&team),
141132
Steps: []resource.TestStep{
@@ -154,7 +145,7 @@ func TestAccTeam_RemoveUnexistingMember(t *testing.T) {
154145
t.Fatal(err)
155146
}
156147
},
157-
Config: testAccTeam_withMember("[email protected]"),
148+
Config: testAccTeamDefinition(teamName, []string{`"[email protected]"`}),
158149
Check: resource.ComposeTestCheckFunc(
159150
testAccTeamCheckExists("grafana_team.test", &team),
160151
resource.TestCheckResourceAttr("grafana_team.test", "members.#", "1"),
@@ -168,7 +159,7 @@ func TestAccTeam_RemoveUnexistingMember(t *testing.T) {
168159
t.Fatal(err)
169160
}
170161
},
171-
Config: testAccTeamConfig_basic,
162+
Config: testAccTeamDefinition(teamName, nil),
172163
Check: resource.ComposeTestCheckFunc(
173164
testAccTeamCheckExists("grafana_team.test", &team),
174165
resource.TestCheckResourceAttr("grafana_team.test", "members.#", "0"),
@@ -216,71 +207,29 @@ func testAccTeamCheckDestroy(a *gapi.Team) resource.TestCheckFunc {
216207
}
217208
}
218209

219-
const testAccTeamConfig_basic = `
220-
resource "grafana_team" "test" {
221-
name = "terraform-acc-test"
222-
email = "[email protected]"
223-
}
224-
`
225-
const testAccTeamConfig_updateName = `
210+
func testAccTeamDefinition(name string, teamMembers []string) string {
211+
definition := fmt.Sprintf(`
226212
resource "grafana_team" "test" {
227-
name = "terraform-acc-test-update"
228-
email = "[email protected]"
229-
}
230-
`
231-
const testAccTeam_users = `
232-
resource "grafana_user" "user_one" {
233-
email = "[email protected]"
234-
name = "Team Test User 1"
235-
login = "test-team-1"
236-
password = "my-password"
237-
is_admin = false
213+
name = "%[1]s"
214+
email = "%[1][email protected]"
215+
members = [ %[2]s ]
238216
}
217+
`, name, strings.Join(teamMembers, `, `))
239218

240-
resource "grafana_user" "user_two" {
241-
email = "[email protected]"
242-
name = "Team Test User 2"
243-
login = "test-team-2"
219+
// If we're referencing a grafana_user resource, we need to create those users
220+
if len(teamMembers) > 0 && strings.Contains(teamMembers[0], "grafana_user") {
221+
definition += fmt.Sprintf(`
222+
resource "grafana_user" "users" {
223+
count = 3
224+
225+
email = "%[1]s-user-${count.index}@example.com"
226+
name = "%[1]s-user-${count.index}"
227+
login = "%[1]s-user-${count.index}"
244228
password = "my-password"
245229
is_admin = false
246230
}
247-
`
248-
249-
const testAccTeamConfig_memberAdd = testAccTeam_users + `
250-
resource "grafana_team" "test" {
251-
name = "terraform-acc-test"
252-
email = "[email protected]"
253-
members = [
254-
grafana_user.user_one.email,
255-
grafana_user.user_two.email,
256-
]
257-
}
258-
`
259-
260-
const testAccTeamConfig_memberReorder = testAccTeam_users + `
261-
resource "grafana_team" "test" {
262-
name = "terraform-acc-test"
263-
email = "[email protected]"
264-
members = [
265-
grafana_user.user_two.email,
266-
grafana_user.user_one.email,
267-
]
268-
}
269-
`
270-
271-
const testAccTeamConfig_memberRemove = testAccTeam_users + `
272-
resource "grafana_team" "test" {
273-
name = "terraform-acc-test"
274-
email = "[email protected]"
275-
members = [ ]
276-
}
277-
`
231+
`, name)
232+
}
278233

279-
func testAccTeam_withMember(user string) string {
280-
return fmt.Sprintf(`
281-
resource "grafana_team" "test" {
282-
name = "terraform-acc-test"
283-
email = "[email protected]"
284-
members = ["%s"]
285-
}`, user)
234+
return definition
286235
}

0 commit comments

Comments
 (0)