Skip to content

Commit 3109c39

Browse files
deigadeiga
authored
[BUG] ensure failed membership invite can be deleted (#3058)
* `github_membership`: Refactor to use Context-aware functions Signed-off-by: Timo Sand <[email protected]> * Ignore 404 on membership delete Signed-off-by: Timo Sand <[email protected]> * Refactor to use `tflog` instead of `log` package Signed-off-by: Timo Sand <[email protected]> * Remove calling `Read` at the end of `Create` Signed-off-by: Timo Sand <[email protected]> --------- Signed-off-by: Timo Sand <[email protected]>
1 parent 1ecdc16 commit 3109c39

2 files changed

Lines changed: 65 additions & 31 deletions

File tree

‎github/resource_github_membership.go‎

Lines changed: 64 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,21 @@ package github
33
import (
44
"context"
55
"errors"
6-
"log"
6+
"fmt"
77
"net/http"
88

99
"github.com/google/go-github/v81/github"
10+
"github.com/hashicorp/terraform-plugin-log/tflog"
11+
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
1012
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1113
)
1214

1315
func resourceGithubMembership() *schema.Resource {
1416
return &schema.Resource{
15-
Create: resourceGithubMembershipCreateOrUpdate,
16-
Read: resourceGithubMembershipRead,
17-
Update: resourceGithubMembershipCreateOrUpdate,
18-
Delete: resourceGithubMembershipDelete,
17+
CreateContext: resourceGithubMembershipCreateOrUpdate,
18+
ReadContext: resourceGithubMembershipRead,
19+
UpdateContext: resourceGithubMembershipCreateOrUpdate,
20+
DeleteContext: resourceGithubMembershipDelete,
1921
Importer: &schema.ResourceImporter{
2022
StateContext: schema.ImportStatePassthroughContext,
2123
},
@@ -49,52 +51,55 @@ func resourceGithubMembership() *schema.Resource {
4951
}
5052
}
5153

52-
func resourceGithubMembershipCreateOrUpdate(d *schema.ResourceData, meta any) error {
54+
func resourceGithubMembershipCreateOrUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
5355
err := checkOrganization(meta)
5456
if err != nil {
55-
return err
57+
return diag.FromErr(err)
5658
}
5759

5860
client := meta.(*Owner).v3client
5961

6062
orgName := meta.(*Owner).name
6163
username := d.Get("username").(string)
6264
roleName := d.Get("role").(string)
63-
ctx := context.Background()
6465
if !d.IsNewResource() {
6566
ctx = context.WithValue(ctx, ctxId, d.Id())
6667
}
6768

68-
_, _, err = client.Organizations.EditOrgMembership(ctx,
69+
_, resp, err := client.Organizations.EditOrgMembership(ctx,
6970
username,
7071
orgName,
7172
&github.Membership{
7273
Role: github.Ptr(roleName),
7374
},
7475
)
7576
if err != nil {
76-
return err
77+
return diag.FromErr(err)
7778
}
7879

7980
d.SetId(buildTwoPartID(orgName, username))
8081

81-
return resourceGithubMembershipRead(d, meta)
82+
if err = d.Set("etag", resp.Header.Get("ETag")); err != nil {
83+
return diag.FromErr(err)
84+
}
85+
86+
return nil
8287
}
8388

84-
func resourceGithubMembershipRead(d *schema.ResourceData, meta any) error {
89+
func resourceGithubMembershipRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
8590
err := checkOrganization(meta)
8691
if err != nil {
87-
return err
92+
return diag.FromErr(err)
8893
}
8994

9095
client := meta.(*Owner).v3client
9196

9297
orgName := meta.(*Owner).name
9398
_, username, err := parseTwoPartID(d.Id(), "organization", "username")
9499
if err != nil {
95-
return err
100+
return diag.FromErr(err)
96101
}
97-
ctx := context.WithValue(context.Background(), ctxId, d.Id())
102+
ctx = context.WithValue(ctx, ctxId, d.Id())
98103
if !d.IsNewResource() {
99104
ctx = context.WithValue(ctx, ctxEtag, d.Get("etag").(string))
100105
}
@@ -108,44 +113,49 @@ func resourceGithubMembershipRead(d *schema.ResourceData, meta any) error {
108113
return nil
109114
}
110115
if ghErr.Response.StatusCode == http.StatusNotFound {
111-
log.Printf("[INFO] Removing membership %s from state because it no longer exists in GitHub",
112-
d.Id())
116+
tflog.Info(ctx, fmt.Sprintf("Removing membership %s from state because it no longer exists in GitHub", d.Id()), map[string]any{
117+
"membership_id": d.Id(),
118+
})
113119
d.SetId("")
114120
return nil
115121
}
116122
}
117-
return err
123+
return diag.FromErr(err)
118124
}
119125

120126
if err = d.Set("etag", resp.Header.Get("ETag")); err != nil {
121-
return err
127+
return diag.FromErr(err)
122128
}
123129
if err = d.Set("username", username); err != nil {
124-
return err
130+
return diag.FromErr(err)
125131
}
126132
if err = d.Set("role", membership.GetRole()); err != nil {
127-
return err
133+
return diag.FromErr(err)
128134
}
129135

130136
return nil
131137
}
132138

133-
func resourceGithubMembershipDelete(d *schema.ResourceData, meta any) error {
139+
func resourceGithubMembershipDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
134140
err := checkOrganization(meta)
135141
if err != nil {
136-
return err
142+
return diag.FromErr(err)
137143
}
138144

139145
client := meta.(*Owner).v3client
140146
orgName := meta.(*Owner).name
141-
ctx := context.WithValue(context.Background(), ctxId, d.Id())
147+
ctx = context.WithValue(ctx, ctxId, d.Id())
142148

143149
username := d.Get("username").(string)
144150
downgradeOnDestroy := d.Get("downgrade_on_destroy").(bool)
145151
downgradeTo := "member"
146152

147153
if downgradeOnDestroy {
148-
log.Printf("[INFO] Downgrading '%s' membership for '%s' to '%s'", orgName, username, downgradeTo)
154+
tflog.Info(ctx, fmt.Sprintf("Downgrading '%s' membership for '%s' to '%s'", orgName, username, downgradeTo), map[string]any{
155+
"org_name": orgName,
156+
"username": username,
157+
"role": downgradeTo,
158+
})
149159

150160
// Check to make sure this member still has access to the organization before downgrading.
151161
// If we don't do this, the member would just be re-added to the organization.
@@ -155,26 +165,50 @@ func resourceGithubMembershipDelete(d *schema.ResourceData, meta any) error {
155165
var ghErr *github.ErrorResponse
156166
if errors.As(err, &ghErr) {
157167
if ghErr.Response.StatusCode == http.StatusNotFound {
158-
log.Printf("[INFO] Not downgrading '%s' membership for '%s' because they are not a member of the org anymore", orgName, username)
168+
tflog.Info(ctx, fmt.Sprintf("Not downgrading '%s' membership for '%s' because they are not a member of the org anymore", orgName, username), map[string]any{
169+
"org_name": orgName,
170+
"username": username,
171+
})
159172
return nil
160173
}
161174
}
162175

163-
return err
176+
return diag.FromErr(err)
164177
}
165178

166179
if *membership.Role == downgradeTo {
167-
log.Printf("[INFO] Not downgrading '%s' membership for '%s' because they are already '%s'", orgName, username, downgradeTo)
180+
tflog.Info(ctx, fmt.Sprintf("Not downgrading '%s' membership for '%s' because they are already '%s'", orgName, username, downgradeTo), map[string]any{
181+
"org_name": orgName,
182+
"username": username,
183+
"role": downgradeTo,
184+
})
168185
return nil
169186
}
170187

171188
_, _, err = client.Organizations.EditOrgMembership(ctx, username, orgName, &github.Membership{
172189
Role: github.Ptr(downgradeTo),
173190
})
174191
} else {
175-
log.Printf("[INFO] Revoking '%s' membership for '%s'", orgName, username)
192+
tflog.Info(ctx, fmt.Sprintf("Revoking '%s' membership for '%s'", orgName, username), map[string]any{
193+
"org_name": orgName,
194+
"username": username,
195+
})
176196
_, err = client.Organizations.RemoveOrgMembership(ctx, username, orgName)
197+
if err != nil {
198+
var ghErr *github.ErrorResponse
199+
if errors.As(err, &ghErr) {
200+
if ghErr.Response.StatusCode == http.StatusNotFound {
201+
tflog.Info(ctx, fmt.Sprintf("Not removing '%s' membership for '%s' because they are not a member of the org anymore", orgName, username), map[string]any{
202+
"org_name": orgName,
203+
"username": username,
204+
})
205+
return nil
206+
}
207+
}
208+
209+
return diag.FromErr(err)
210+
}
177211
}
178212

179-
return err
213+
return diag.FromErr(err)
180214
}

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ require (
77
github.com/google/go-github/v81 v81.0.0
88
github.com/google/uuid v1.6.0
99
github.com/hashicorp/go-cty v1.5.0
10+
github.com/hashicorp/terraform-plugin-log v0.9.0
1011
github.com/hashicorp/terraform-plugin-sdk/v2 v2.38.1
1112
github.com/shurcooL/githubv4 v0.0.0-20221126192849-0b5c4c7994eb
1213
github.com/stretchr/testify v1.11.1
@@ -39,7 +40,6 @@ require (
3940
github.com/hashicorp/terraform-exec v0.23.1 // indirect
4041
github.com/hashicorp/terraform-json v0.27.1 // indirect
4142
github.com/hashicorp/terraform-plugin-go v0.29.0 // indirect
42-
github.com/hashicorp/terraform-plugin-log v0.9.0 // indirect
4343
github.com/hashicorp/terraform-registry-address v0.4.0 // indirect
4444
github.com/hashicorp/terraform-svchost v0.1.1 // indirect
4545
github.com/hashicorp/yamux v0.1.2 // indirect

0 commit comments

Comments
 (0)