Skip to content

Commit 81933d9

Browse files
nickscybercyst
nicks
and
cybercyst
authored
org_member: handle role changes properly (#116)
* org_member: handle role changes properly before this PR, if you changed an org member's role, the provider would remove the member of the org and re-invite them. this is obviously pretty bad and breaks a bunch of stuff after this PR, the provider changes the role in place. our invite API is still pretty janky but we do the best we can with the api that exists. Signed-off-by: Nick Santos <[email protected]> * Update internal/provider/resource_org_member.go Co-authored-by: Forrest Loomis <[email protected]> Signed-off-by: Nick Santos <[email protected]> --------- Signed-off-by: Nick Santos <[email protected]> Co-authored-by: Forrest Loomis <[email protected]>
1 parent 93bad0d commit 81933d9

3 files changed

Lines changed: 158 additions & 11 deletions

File tree

‎internal/hubclient/client_organization.go‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,16 @@ type OrgMemberListResponse struct {
135135
Results []OrgMember `json:"results"` // List of accounts.
136136
}
137137

138+
// Weirdly, org role params are lowercase even though org role return values
139+
// are uppercase.
140+
type OrgRoleParam string
141+
142+
const (
143+
OrgRoleParamOwner OrgRoleParam = "owner"
144+
OrgRoleParamEditor OrgRoleParam = "editor"
145+
OrgRoleParamMember OrgRoleParam = "member"
146+
)
147+
138148
// OrgMember represents each organization member
139149
type OrgMember struct {
140150
Email string `json:"email"` // User's email address
@@ -254,7 +264,7 @@ func (c *Client) ListOrgTeamMembers(ctx context.Context, orgName string, teamNam
254264
if err != nil {
255265
return membersResponse, err
256266
}
257-
267+
258268
members := membersResponse.Results
259269
for membersResponse.Next != "" {
260270
nextResponse := OrgTeamMembersResponse{}
@@ -266,7 +276,7 @@ func (c *Client) ListOrgTeamMembers(ctx context.Context, orgName string, teamNam
266276
members = append(members, nextResponse.Results...)
267277
membersResponse = nextResponse
268278
}
269-
279+
270280
membersResponse.Results = members
271281
return membersResponse, nil
272282
}
@@ -340,6 +350,17 @@ func (c *Client) DeleteOrgMember(ctx context.Context, orgName string, userName s
340350
return c.sendRequest(ctx, "DELETE", url, nil, nil)
341351
}
342352

353+
func (c *Client) UpdateOrgMember(ctx context.Context, orgName string, userName string, role OrgRoleParam) error {
354+
url := fmt.Sprintf("/orgs/%s/members/%s/", orgName, userName)
355+
body := map[string]string{"role": string(role)}
356+
reqBody, err := json.Marshal(body)
357+
if err != nil {
358+
return err
359+
}
360+
361+
return c.sendRequest(ctx, "PUT", url, reqBody, nil)
362+
}
363+
343364
// func (c *Client) GetOrgInvitedMember(ctx context.Context, inviteID string) (OrgMembersResponse, error) {
344365
// url := fmt.Sprintf("/invites", inviteID)
345366
// var membersResponse OrgMembersResponse

‎internal/provider/resource_org_member.go‎

Lines changed: 75 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ resource "docker_org_member" "example" {
132132
"user_name": schema.StringAttribute{
133133
MarkdownDescription: "User name of the member. Either user_name or email must be specified.",
134134
Optional: true,
135+
Computed: true,
135136
PlanModifiers: []planmodifier.String{
136137
stringplanmodifier.RequiresReplace(),
137138
},
@@ -144,11 +145,11 @@ resource "docker_org_member" "example" {
144145
"role": schema.StringAttribute{
145146
MarkdownDescription: "Role assigned to the user within the organization (e.g., 'member', 'editor', 'owner').",
146147
Required: true,
147-
PlanModifiers: []planmodifier.String{
148-
stringplanmodifier.RequiresReplace(),
149-
},
150148
Validators: []validator.String{
151-
stringvalidator.OneOf("member", "editor", "owner"),
149+
stringvalidator.OneOf(
150+
string(hubclient.OrgRoleParamMember),
151+
string(hubclient.OrgRoleParamEditor),
152+
string(hubclient.OrgRoleParamOwner)),
152153
},
153154
},
154155
"invite_id": schema.StringAttribute{
@@ -187,6 +188,7 @@ func (r *OrgMemberResource) Create(ctx context.Context, req resource.CreateReque
187188
// Set computed fields.
188189
data.InviteID = current.InviteID
189190
data.Email = current.Email
191+
data.UserName = current.UserName
190192
} else {
191193
// If the member is not found, invite them now.
192194
inviteResp, err := r.client.InviteOrgMember(ctx,
@@ -207,6 +209,29 @@ func (r *OrgMemberResource) Create(ctx context.Context, req resource.CreateReque
207209
if invite.Invite.ID != "" {
208210
data.InviteID = types.StringValue(invite.Invite.ID)
209211
}
212+
213+
// If username or email or not set, we need to compute them
214+
if data.UserName.ValueString() == "" {
215+
data.UserName = types.StringValue("")
216+
}
217+
if data.Email.ValueString() == "" {
218+
data.Email = types.StringValue("")
219+
}
220+
}
221+
222+
isAlreadyMember := data.InviteID.ValueString() == "" && data.UserName.ValueString() != ""
223+
isRoleUpdated := current.Role.ValueString() != data.Role.ValueString()
224+
225+
if isAlreadyMember && isRoleUpdated {
226+
err = r.client.UpdateOrgMember(ctx,
227+
data.OrgName.ValueString(),
228+
data.UserName.ValueString(),
229+
hubclient.OrgRoleParam(data.Role.ValueString()))
230+
if err != nil {
231+
errMsg := fmt.Sprintf("Unable to update org_member role: %v", err)
232+
resp.Diagnostics.AddError("Error Updating Resource", errMsg)
233+
return
234+
}
210235
}
211236

212237
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
@@ -237,6 +262,7 @@ func (r *OrgMemberResource) Read(ctx context.Context, req resource.ReadRequest,
237262
if !found {
238263
resp.Diagnostics.AddError("Resource Not Found",
239264
fmt.Sprintf("Member not found in %s: %s", state.OrgName.ValueString(), invitee))
265+
return
240266
}
241267

242268
if data.Role.ValueString() == "" {
@@ -250,12 +276,52 @@ func (r *OrgMemberResource) Read(ctx context.Context, req resource.ReadRequest,
250276
}
251277
}
252278

253-
// NOTE(nicks): currently, we treat all fields as immutable,
254-
// so in theory update should never happen.
255-
//
256-
// Future work: update the provider to allow role changes.
257279
func (r *OrgMemberResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
258-
return
280+
var plan OrgMemberResourceModel
281+
resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...)
282+
if resp.Diagnostics.HasError() {
283+
return
284+
}
285+
286+
userNameOrEmail := plan.UserName.ValueString()
287+
if userNameOrEmail == "" {
288+
userNameOrEmail = plan.Email.ValueString()
289+
}
290+
291+
data, found, err := r.orgMember(ctx, plan.OrgName.ValueString(), userNameOrEmail)
292+
if err != nil {
293+
resp.Diagnostics.AddError("Error Reading Resource", err.Error())
294+
return
295+
}
296+
if !found {
297+
resp.Diagnostics.AddError("Resource Not Found",
298+
fmt.Sprintf("Member not found in %s: %s", plan.OrgName.ValueString(), userNameOrEmail))
299+
return
300+
}
301+
302+
// Role is the only field that can change.
303+
if plan.Role.ValueString() == data.Role.ValueString() {
304+
return
305+
}
306+
307+
if data.InviteID.ValueString() != "" {
308+
resp.Diagnostics.AddError("Resource cannot be changed",
309+
fmt.Sprintf("Member role cannot be changed until invitation is accepted: %s", userNameOrEmail))
310+
return
311+
}
312+
313+
err = r.client.UpdateOrgMember(ctx,
314+
data.OrgName.ValueString(),
315+
data.UserName.ValueString(),
316+
hubclient.OrgRoleParam(plan.Role.ValueString()))
317+
if err != nil {
318+
errMsg := fmt.Sprintf("Unable to update org_member role: %v", err)
319+
resp.Diagnostics.AddError("Error Updating Resource", errMsg)
320+
return
321+
}
322+
323+
data.Role = plan.Role
324+
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
259325
}
260326

261327
func (r *OrgMemberResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {

‎internal/provider/resource_org_member_test.go‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,66 @@ resource "docker_org_member" "test" {
5656
})
5757
}
5858

59+
// TestAccOrgMemberResource_ExistingMember tests managing the role of an existing
60+
// organization member.
61+
//
62+
// By design, this test requires the user "nick20241127" to already be a member
63+
// so that we don't have to deal with the invite flow.
64+
func TestAccOrgMemberResource_ExistingMember(t *testing.T) {
65+
org := envvar.GetWithDefault(envvar.AccTestOrganization)
66+
resource.Test(t, resource.TestCase{
67+
PreCheck: func() { testAccPreCheck(t) },
68+
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
69+
Steps: []resource.TestStep{
70+
{
71+
Config: fmt.Sprintf(`
72+
resource "docker_org_member" "test" {
73+
org_name = "%[1]s"
74+
user_name = "nick20241127"
75+
role = "member"
76+
77+
lifecycle {
78+
prevent_destroy = true
79+
}
80+
}`, org),
81+
Check: resource.ComposeAggregateTestCheckFunc(
82+
resource.TestCheckResourceAttr("docker_org_member.test", "org_name", org),
83+
resource.TestCheckResourceAttr("docker_org_member.test", "user_name", "nick20241127"),
84+
resource.TestCheckResourceAttr("docker_org_member.test", "email", "[email protected]"),
85+
resource.TestCheckResourceAttr("docker_org_member.test", "role", "member"),
86+
),
87+
},
88+
{
89+
Config: fmt.Sprintf(`
90+
resource "docker_org_member" "test" {
91+
org_name = "%[1]s"
92+
user_name = "nick20241127"
93+
role = "editor"
94+
95+
lifecycle{
96+
prevent_destroy = true
97+
}
98+
}`, org),
99+
Check: resource.ComposeAggregateTestCheckFunc(
100+
resource.TestCheckResourceAttr("docker_org_member.test", "org_name", org),
101+
resource.TestCheckResourceAttr("docker_org_member.test", "user_name", "nick20241127"),
102+
resource.TestCheckResourceAttr("docker_org_member.test", "email", "[email protected]"),
103+
resource.TestCheckResourceAttr("docker_org_member.test", "role", "editor"),
104+
),
105+
},
106+
{
107+
Config: `
108+
removed {
109+
from = docker_org_member.test
110+
lifecycle {
111+
destroy = false
112+
}
113+
}`,
114+
},
115+
},
116+
})
117+
}
118+
59119
func TestAccOrgMemberResourceImport(t *testing.T) {
60120
username := os.Getenv("DOCKER_USERNAME")
61121
org := envvar.GetWithDefault(envvar.AccTestOrganization)

0 commit comments

Comments
 (0)