Skip to content

Commit 7bcb796

Browse files
deigadeiga
authored
[MAINT] Consolidate ID format of github_emu_group_mapping (#3215)
* Update resource ID to be <group-id>:<team-id> Signed-off-by: Timo Sand <[email protected]> * Fix `tflog.SetField` usage Signed-off-by: Timo Sand <[email protected]> * Update tests to use `terraform-plugin-testing` and fix broken tests Signed-off-by: Timo Sand <[email protected]> * Update docs Signed-off-by: Timo Sand <[email protected]> * Cleanup extra `tflog.SetField` calls Signed-off-by: Timo Sand <[email protected]> * Reducing logger complexity Signed-off-by: Timo Sand <[email protected]> * Improve error messages in `Import` Signed-off-by: Timo Sand <[email protected]> --------- Signed-off-by: Timo Sand <[email protected]>
1 parent 94f71cf commit 7bcb796

6 files changed

Lines changed: 210 additions & 91 deletions

‎github/resource_github_emu_group_mapping.go‎

Lines changed: 36 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package github
22

33
import (
44
"context"
5+
"fmt"
56
"net/http"
67
"strconv"
78

@@ -49,13 +50,18 @@ func resourceGithubEMUGroupMapping() *schema.Resource {
4950
Computed: true,
5051
},
5152
},
52-
SchemaVersion: 1,
53+
SchemaVersion: 2,
5354
StateUpgraders: []schema.StateUpgrader{
5455
{
5556
Type: resourceGithubEMUGroupMappingV0().CoreConfigSchema().ImpliedType(),
5657
Upgrade: resourceGithubEMUGroupMappingStateUpgradeV0,
5758
Version: 0,
5859
},
60+
{
61+
Type: resourceGithubEMUGroupMappingV1().CoreConfigSchema().ImpliedType(),
62+
Upgrade: resourceGithubEMUGroupMappingStateUpgradeV1,
63+
Version: 1,
64+
},
5965
},
6066
}
6167
}
@@ -69,18 +75,19 @@ func resourceGithubEMUGroupMappingCreate(ctx context.Context, d *schema.Resource
6975
}
7076
client := meta.(*Owner).v3client
7177
orgName := meta.(*Owner).name
72-
tflog.SetField(ctx, "org_name", orgName)
7378

7479
teamSlug := d.Get("team_slug").(string)
75-
tflog.SetField(ctx, "team_slug", teamSlug)
7680

7781
groupID := toInt64(d.Get("group_id"))
78-
tflog.SetField(ctx, "group_id", groupID)
7982
eg := &github.ExternalGroup{
8083
GroupID: new(groupID),
8184
}
8285

83-
tflog.Debug(ctx, "Connecting external group to team via GitHub API")
86+
tflog.Debug(ctx, "Connecting external group to team via GitHub API", map[string]any{
87+
"org_name": orgName,
88+
"team_slug": teamSlug,
89+
"group_id": groupID,
90+
})
8491

8592
group, resp, err := client.Teams.UpdateConnectedExternalGroup(ctx, orgName, teamSlug, eg)
8693
if err != nil {
@@ -94,20 +101,23 @@ func resourceGithubEMUGroupMappingCreate(ctx context.Context, d *schema.Resource
94101
return diag.FromErr(err)
95102
}
96103

97-
newResourceID, err := buildID(strconv.FormatInt(teamID, 10), teamSlug, strconv.FormatInt(groupID, 10))
104+
newResourceID, err := buildID(strconv.FormatInt(groupID, 10), strconv.FormatInt(teamID, 10))
98105
if err != nil {
99106
return diag.FromErr(err)
100107
}
101108

102-
if err := d.Set("team_id", int(teamID)); err != nil {
103-
return diag.FromErr(err)
104-
}
105-
106109
tflog.Trace(ctx, "Setting resource ID", map[string]any{
107110
"resource_id": newResourceID,
108111
})
109112
d.SetId(newResourceID)
110113

114+
tflog.Trace(ctx, "Setting team_id", map[string]any{
115+
"team_id": teamID,
116+
})
117+
if err := d.Set("team_id", int(teamID)); err != nil {
118+
return diag.FromErr(err)
119+
}
120+
111121
etag := resp.Header.Get("ETag")
112122
tflog.Trace(ctx, "Setting state attribute: etag", map[string]any{
113123
"etag": etag,
@@ -116,21 +126,23 @@ func resourceGithubEMUGroupMappingCreate(ctx context.Context, d *schema.Resource
116126
return diag.FromErr(err)
117127
}
118128

129+
tflog.Trace(ctx, "Setting group_name", map[string]any{
130+
"group_name": group.GetGroupName(),
131+
})
119132
if err := d.Set("group_name", group.GetGroupName()); err != nil {
120133
return diag.FromErr(err)
121134
}
122135

123-
tflog.Trace(ctx, "Resource created or updated successfully", map[string]any{
136+
tflog.Trace(ctx, "Resource created successfully", map[string]any{
124137
"resource_id": d.Id(),
125138
})
126139

127140
return nil
128141
}
129142

130143
func resourceGithubEMUGroupMappingRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
131-
tflog.Trace(ctx, "Reading EMU group mapping", map[string]any{
132-
"resource_id": d.Id(),
133-
})
144+
ctx = tflog.SetField(ctx, "resource_id", d.Id())
145+
tflog.Trace(ctx, "Reading EMU group mapping")
134146

135147
err := checkOrganization(meta)
136148
if err != nil {
@@ -142,36 +154,26 @@ func resourceGithubEMUGroupMappingRead(ctx context.Context, d *schema.ResourceDa
142154
groupID := toInt64(d.Get("group_id"))
143155
teamSlug := d.Get("team_slug").(string)
144156

145-
tflog.SetField(ctx, "group_id", groupID)
146-
tflog.SetField(ctx, "team_slug", teamSlug)
147-
tflog.SetField(ctx, "org_name", orgName)
148-
149157
tflog.Debug(ctx, "Querying external groups linked to team from GitHub API")
150158

151159
groupsList, resp, err := client.Teams.ListExternalGroupsForTeamBySlug(ctx, orgName, teamSlug)
152160
if err != nil {
153161
if resp != nil && resp.StatusCode == http.StatusBadRequest {
154-
tflog.Info(ctx, "Removing EMU group mapping from state because the team has explicit members in GitHub", map[string]any{
155-
"resource_id": d.Id(),
156-
})
162+
tflog.Info(ctx, "Removing EMU group mapping from state because the team has explicit members in GitHub")
157163
d.SetId("")
158164
return nil
159165
}
160166
if resp != nil && (resp.StatusCode == http.StatusNotFound) {
161167
// If the Group is not found, remove it from state
162-
tflog.Info(ctx, "Removing EMU group mapping from state because team no longer exists in GitHub", map[string]any{
163-
"resource_id": d.Id(),
164-
})
168+
tflog.Info(ctx, "Removing EMU group mapping from state because team no longer exists in GitHub")
165169
d.SetId("")
166170
return nil
167171
}
168172
return diag.FromErr(err)
169173
}
170174

171175
if len(groupsList.Groups) < 1 {
172-
tflog.Info(ctx, "Removing EMU group mapping from state because no external groups are linked to the team", map[string]any{
173-
"resource_id": d.Id(),
174-
})
176+
tflog.Info(ctx, "Removing EMU group mapping from state because no external groups are linked to the team")
175177
d.SetId("")
176178
return nil
177179
}
@@ -180,7 +182,6 @@ func resourceGithubEMUGroupMappingRead(ctx context.Context, d *schema.ResourceDa
180182
group := groupsList.Groups[0]
181183

182184
tflog.Debug(ctx, "Successfully retrieved external group from GitHub API", map[string]any{
183-
"group_id": group.GetGroupID(),
184185
"group_name": group.GetGroupName(),
185186
})
186187

@@ -205,28 +206,24 @@ func resourceGithubEMUGroupMappingRead(ctx context.Context, d *schema.ResourceDa
205206
}
206207

207208
func resourceGithubEMUGroupMappingUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
208-
tflog.Trace(ctx, "Updating EMU group mapping", map[string]any{
209-
"resource_id": d.Id(),
210-
})
209+
ctx = tflog.SetField(ctx, "resource_id", d.Id())
210+
tflog.Trace(ctx, "Updating EMU group mapping")
211211

212212
err := checkOrganization(meta)
213213
if err != nil {
214214
return diag.FromErr(err)
215215
}
216216
client := meta.(*Owner).v3client
217217
orgName := meta.(*Owner).name
218-
tflog.SetField(ctx, "org_name", orgName)
219218

220219
teamSlug := d.Get("team_slug").(string)
221-
tflog.SetField(ctx, "team_slug", teamSlug)
222220

223221
groupID := toInt64(d.Get("group_id"))
224-
tflog.SetField(ctx, "group_id", groupID)
225222
eg := &github.ExternalGroup{
226223
GroupID: new(groupID),
227224
}
228225

229-
if d.HasChanges("group_id", "team_slug") {
226+
if d.HasChange("team_slug") {
230227

231228
tflog.Debug(ctx, "Updating connected external group via GitHub API")
232229

@@ -248,23 +245,9 @@ func resourceGithubEMUGroupMappingUpdate(ctx context.Context, d *schema.Resource
248245
if err := d.Set("group_name", group.GetGroupName()); err != nil {
249246
return diag.FromErr(err)
250247
}
251-
252-
teamID := toInt64(d.Get("team_id"))
253-
254-
newResourceID, err := buildID(strconv.FormatInt(teamID, 10), teamSlug, strconv.FormatInt(groupID, 10))
255-
if err != nil {
256-
return diag.FromErr(err)
257-
}
258-
259-
tflog.Trace(ctx, "Setting resource ID", map[string]any{
260-
"resource_id": newResourceID,
261-
})
262-
d.SetId(newResourceID)
263248
}
264249

265-
tflog.Trace(ctx, "Updated successfully", map[string]any{
266-
"resource_id": d.Id(),
267-
})
250+
tflog.Trace(ctx, "Updated successfully")
268251

269252
return nil
270253
}
@@ -320,11 +303,11 @@ func resourceGithubEMUGroupMappingImport(ctx context.Context, d *schema.Resource
320303
// <group-id>:<team-slug>
321304
groupIDString, teamSlug, err := parseID2(d.Id())
322305
if err != nil {
323-
return nil, err
306+
return nil, fmt.Errorf("could not parse import ID (%s), expected format: <group-id>:<team-slug>. Parse error: %w", importID, err)
324307
}
325308
groupID, err := strconv.Atoi(groupIDString)
326309
if err != nil {
327-
return nil, err
310+
return nil, unconvertibleIdErr(groupIDString, err)
328311
}
329312

330313
tflog.Debug(ctx, "Parsed two-part import ID", map[string]any{
@@ -350,7 +333,7 @@ func resourceGithubEMUGroupMappingImport(ctx context.Context, d *schema.Resource
350333
return nil, err
351334
}
352335

353-
resourceID, err := buildID(strconv.FormatInt(teamID, 10), teamSlug, groupIDString)
336+
resourceID, err := buildID(groupIDString, strconv.FormatInt(teamID, 10))
354337
if err != nil {
355338
return nil, err
356339
}

‎github/resource_github_emu_group_mapping_migration.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package github
22

33
import (
44
"context"
5+
"fmt"
56
"net/http"
67
"strconv"
78

@@ -65,3 +66,56 @@ func resourceGithubEMUGroupMappingStateUpgradeV0(ctx context.Context, rawState m
6566
tflog.Trace(ctx, "GitHub EMU Group Mapping State after migration", map[string]any{"state": rawState})
6667
return rawState, nil
6768
}
69+
70+
func resourceGithubEMUGroupMappingV1() *schema.Resource {
71+
return &schema.Resource{
72+
Schema: map[string]*schema.Schema{
73+
"team_id": {
74+
Type: schema.TypeInt,
75+
Computed: true,
76+
Description: "ID of the GitHub team.",
77+
},
78+
"team_slug": {
79+
Type: schema.TypeString,
80+
Required: true,
81+
Description: "Slug of the GitHub team.",
82+
},
83+
"group_id": {
84+
Type: schema.TypeInt,
85+
Required: true,
86+
ForceNew: true,
87+
Description: "Integer corresponding to the external group ID to be linked.",
88+
},
89+
"group_name": {
90+
Type: schema.TypeString,
91+
Computed: true,
92+
Description: "Name of the external group.",
93+
},
94+
"etag": {
95+
Type: schema.TypeString,
96+
Computed: true,
97+
},
98+
},
99+
}
100+
}
101+
102+
func resourceGithubEMUGroupMappingStateUpgradeV1(ctx context.Context, rawState map[string]any, meta any) (map[string]any, error) {
103+
tflog.Trace(ctx, "GitHub EMU Group Mapping State before migration v1 => v2", map[string]any{"state": rawState})
104+
105+
oldResourceID, ok := rawState["id"].(string)
106+
if !ok {
107+
return nil, fmt.Errorf("id is not a string")
108+
}
109+
teamID, _, groupID, err := parseID3(oldResourceID)
110+
if err != nil {
111+
return nil, fmt.Errorf("could not parse ID: %w", err)
112+
}
113+
resourceID, err := buildID(groupID, teamID)
114+
if err != nil {
115+
return nil, fmt.Errorf("could not build ID: %w", err)
116+
}
117+
rawState["id"] = resourceID
118+
119+
tflog.Trace(ctx, "GitHub EMU Group Mapping State after migration", map[string]any{"state": rawState})
120+
return rawState, nil
121+
}

‎github/resource_github_emu_group_mapping_migration_test.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package github
2+
3+
import (
4+
"testing"
5+
6+
"github.com/google/go-cmp/cmp"
7+
)
8+
9+
func Test_resourceGithubEMUGroupMappingStateUpgradeV1(t *testing.T) {
10+
t.Parallel()
11+
12+
for _, d := range []struct {
13+
testName string
14+
rawState map[string]any
15+
want map[string]any
16+
shouldError bool
17+
}{
18+
{
19+
testName: "migrates v1 to v2",
20+
rawState: map[string]any{
21+
"id": "123456:test-team:7765543",
22+
},
23+
want: map[string]any{
24+
"id": "7765543:123456",
25+
},
26+
shouldError: false,
27+
},
28+
} {
29+
t.Run(d.testName, func(t *testing.T) {
30+
t.Parallel()
31+
32+
got, err := resourceGithubEMUGroupMappingStateUpgradeV1(t.Context(), d.rawState, nil)
33+
if (err != nil) != d.shouldError {
34+
t.Fatalf("unexpected error state")
35+
}
36+
37+
if diff := cmp.Diff(got, d.want); !d.shouldError && diff != "" {
38+
t.Fatalf("got %+v, want %+v: %s", got, d.want, diff)
39+
}
40+
})
41+
}
42+
}

0 commit comments

Comments
 (0)