Skip to content

Commit 99877ea

Browse files
fix: support unmanaged roles on user resource (#250)
A customer ran into an issue when creating an OIDC user while role sync was enabled on the deployment: ``` Error: 'User Role Field' is set in the OIDC configuration. All role changes must come from the oidc identity provider. ``` This is because we always call `UpdateUserRoles` on `Create` and `Update`. OIDC User roles cannot be managed via the API if role sync is used, as the API always returns an error on any role update request. With this PR, `roles` can now be set to `null` in the config, whereby the Terraform provider will not attempt to read or update the user's roles under any circumstances. This prevents config drift when roles are set via Role Sync.
1 parent 399bfa7 commit 99877ea

File tree

4 files changed

+86
-40
lines changed

4 files changed

+86
-40
lines changed

docs/resources/user.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ resource "coderd_user" "admin" {
5656
- `login_type` (String) Type of login for the user. Valid types are `none`, `password`, `github`, and `oidc`.
5757
- `name` (String) Display name of the user. Defaults to username.
5858
- `password` (String, Sensitive) Password for the user. Required when `login_type` is `password`. Passwords are saved into the state as plain text and should only be used for testing purposes.
59-
- `roles` (Set of String) Roles assigned to the user. Valid roles are `owner`, `template-admin`, `user-admin`, and `auditor`.
59+
- `roles` (Set of String) Roles assigned to the user. Valid roles are `owner`, `template-admin`, `user-admin`, and `auditor`. If `null`, roles will not be managed by Terraform. This attribute must be null if the user is an OIDC user and role sync is configured
6060
- `suspended` (Boolean) Whether the user is suspended.
6161

6262
### Read-Only

integration/integration.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func StartCoder(ctx context.Context, t *testing.T, name string, useLicense bool)
9696
t.Logf("not ready yet: %s", err.Error())
9797
}
9898
return err == nil
99-
}, 20*time.Second, time.Second, "coder failed to become ready in time")
99+
}, 30*time.Second, time.Second, "coder failed to become ready in time")
100100
_, err = client.CreateFirstUser(ctx, codersdk.CreateFirstUserRequest{
101101
Email: testEmail,
102102
Username: testUsername,

internal/provider/user_resource.go

Lines changed: 45 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
1515
"github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault"
1616
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
17-
"github.com/hashicorp/terraform-plugin-framework/resource/schema/setdefault"
1817
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault"
1918
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
2019
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
@@ -88,16 +87,14 @@ func (r *UserResource) Schema(ctx context.Context, req resource.SchemaRequest, r
8887
Required: true,
8988
},
9089
"roles": schema.SetAttribute{
91-
MarkdownDescription: "Roles assigned to the user. Valid roles are `owner`, `template-admin`, `user-admin`, and `auditor`.",
92-
Computed: true,
90+
MarkdownDescription: "Roles assigned to the user. Valid roles are `owner`, `template-admin`, `user-admin`, and `auditor`. If `null`, roles will not be managed by Terraform. This attribute must be null if the user is an OIDC user and role sync is configured",
9391
Optional: true,
9492
ElementType: types.StringType,
9593
Validators: []validator.Set{
9694
setvalidator.ValueStringsAre(
9795
stringvalidator.OneOf("owner", "template-admin", "user-admin", "auditor"),
9896
),
9997
},
100-
Default: setdefault.StaticValue(types.SetValueMust(types.StringType, []attr.Value{})),
10198
},
10299
"login_type": schema.StringAttribute{
103100
MarkdownDescription: "Type of login for the user. Valid types are `none`, `password`, `github`, and `oidc`.",
@@ -209,21 +206,26 @@ func (r *UserResource) Create(ctx context.Context, req resource.CreateRequest, r
209206
tflog.Info(ctx, "successfully updated user profile")
210207
data.Name = types.StringValue(user.Name)
211208

212-
var roles []string
213-
resp.Diagnostics.Append(
214-
data.Roles.ElementsAs(ctx, &roles, false)...,
215-
)
216-
tflog.Info(ctx, "updating user roles", map[string]any{
217-
"new_roles": roles,
218-
})
219-
user, err = client.UpdateUserRoles(ctx, user.ID.String(), codersdk.UpdateRoles{
220-
Roles: roles,
221-
})
222-
if err != nil {
223-
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update newly created user roles, got error: %s", err))
224-
return
209+
if !data.Roles.IsNull() {
210+
var roles []string
211+
resp.Diagnostics.Append(
212+
data.Roles.ElementsAs(ctx, &roles, false)...,
213+
)
214+
if resp.Diagnostics.HasError() {
215+
return
216+
}
217+
tflog.Info(ctx, "updating user roles", map[string]any{
218+
"new_roles": roles,
219+
})
220+
user, err = client.UpdateUserRoles(ctx, user.ID.String(), codersdk.UpdateRoles{
221+
Roles: roles,
222+
})
223+
if err != nil {
224+
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update newly created user roles, got error: %s", err))
225+
return
226+
}
227+
tflog.Info(ctx, "successfully updated user roles")
225228
}
226-
tflog.Info(ctx, "successfully updated user roles")
227229

228230
if data.Suspended.ValueBool() {
229231
_, err = client.UpdateUserStatus(ctx, data.ID.ValueString(), codersdk.UserStatus("suspended"))
@@ -267,11 +269,13 @@ func (r *UserResource) Read(ctx context.Context, req resource.ReadRequest, resp
267269
data.Email = types.StringValue(user.Email)
268270
data.Name = types.StringValue(user.Name)
269271
data.Username = types.StringValue(user.Username)
270-
roles := make([]attr.Value, 0, len(user.Roles))
271-
for _, role := range user.Roles {
272-
roles = append(roles, types.StringValue(role.Name))
272+
if !data.Roles.IsNull() {
273+
roles := make([]attr.Value, 0, len(user.Roles))
274+
for _, role := range user.Roles {
275+
roles = append(roles, types.StringValue(role.Name))
276+
}
277+
data.Roles = types.SetValueMust(types.StringType, roles)
273278
}
274-
data.Roles = types.SetValueMust(types.StringType, roles)
275279
data.LoginType = types.StringValue(string(user.LoginType))
276280
data.Suspended = types.BoolValue(user.Status == codersdk.UserStatusSuspended)
277281

@@ -344,21 +348,26 @@ func (r *UserResource) Update(ctx context.Context, req resource.UpdateRequest, r
344348
data.Name = name
345349
tflog.Info(ctx, "successfully updated user profile")
346350

347-
var roles []string
348-
resp.Diagnostics.Append(
349-
data.Roles.ElementsAs(ctx, &roles, false)...,
350-
)
351-
tflog.Info(ctx, "updating user roles", map[string]any{
352-
"new_roles": roles,
353-
})
354-
_, err = client.UpdateUserRoles(ctx, user.ID.String(), codersdk.UpdateRoles{
355-
Roles: roles,
356-
})
357-
if err != nil {
358-
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update user roles, got error: %s", err))
359-
return
351+
if !data.Roles.IsNull() {
352+
var roles []string
353+
resp.Diagnostics.Append(
354+
data.Roles.ElementsAs(ctx, &roles, false)...,
355+
)
356+
if resp.Diagnostics.HasError() {
357+
return
358+
}
359+
tflog.Info(ctx, "updating user roles", map[string]any{
360+
"new_roles": roles,
361+
})
362+
_, err = client.UpdateUserRoles(ctx, user.ID.String(), codersdk.UpdateRoles{
363+
Roles: roles,
364+
})
365+
if err != nil {
366+
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update user roles, got error: %s", err))
367+
return
368+
}
369+
tflog.Info(ctx, "successfully updated user roles")
360370
}
361-
tflog.Info(ctx, "successfully updated user roles")
362371

363372
if data.LoginType.ValueString() == string(codersdk.LoginTypePassword) && !data.Password.IsNull() {
364373
tflog.Info(ctx, "updating password")

internal/provider/user_resource_test.go

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ func TestAccUserResource(t *testing.T) {
4242
cfg4.LoginType = ptr.Ref("github")
4343
cfg4.Password = nil
4444

45+
cfg5 := cfg4
46+
cfg5.Roles = nil
47+
4548
resource.Test(t, resource.TestCase{
4649
IsUnitTest: true,
4750
PreCheck: func() { testAccPreCheck(t) },
@@ -68,7 +71,7 @@ func TestAccUserResource(t *testing.T) {
6871
ImportState: true,
6972
ImportStateVerify: true,
7073
// We can't pull the password from the API.
71-
ImportStateVerifyIgnore: []string{"password"},
74+
ImportStateVerifyIgnore: []string{"password", "roles"},
7275
},
7376
// ImportState by username
7477
{
@@ -77,7 +80,7 @@ func TestAccUserResource(t *testing.T) {
7780
ImportStateVerify: true,
7881
ImportStateId: "example",
7982
// We can't pull the password from the API.
80-
ImportStateVerifyIgnore: []string{"password"},
83+
ImportStateVerifyIgnore: []string{"password", "roles"},
8184
},
8285
// Update and Read testing
8386
{
@@ -114,8 +117,42 @@ func TestAccUserResource(t *testing.T) {
114117
// The Plan should be to create the entire resource
115118
ExpectNonEmptyPlan: true,
116119
},
120+
// Unmanaged roles
121+
{
122+
Config: cfg5.String(t),
123+
Check: resource.ComposeAggregateTestCheckFunc(
124+
resource.TestCheckNoResourceAttr("coderd_user.test", "roles"),
125+
),
126+
},
117127
},
118128
})
129+
130+
t.Run("CreateUnmanagedRolesOk", func(t *testing.T) {
131+
cfg := testAccUserResourceConfig{
132+
URL: client.URL.String(),
133+
Token: client.SessionToken(),
134+
Username: ptr.Ref("unmanaged"),
135+
Name: ptr.Ref("Unmanaged User"),
136+
Email: ptr.Ref("unmanaged@coder.com"),
137+
Roles: nil, // Start with unmanaged roles
138+
LoginType: ptr.Ref("password"),
139+
Password: ptr.Ref("SomeSecurePassword!"),
140+
}
141+
142+
resource.Test(t, resource.TestCase{
143+
IsUnitTest: true,
144+
PreCheck: func() { testAccPreCheck(t) },
145+
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
146+
Steps: []resource.TestStep{
147+
{
148+
Config: cfg.String(t),
149+
Check: resource.ComposeAggregateTestCheckFunc(
150+
resource.TestCheckNoResourceAttr("coderd_user.test", "roles"),
151+
),
152+
},
153+
},
154+
})
155+
})
119156
}
120157

121158
type testAccUserResourceConfig struct {

0 commit comments

Comments
 (0)