Skip to content

Commit 5503d71

Browse files
fix: handle user deletion config drift (#209)
Closes #208. Querying `api/v2/users/{ID}` returns a valid response for deleted users, as deleted users in `coderd` are merely tombstoned. To handle this, we perform an additional query by username. If the user has been deleted, the username will be available, or belong to a user with a different ID, in which case we can mark the user resource as deleted. Also has the `isNotFound` check include the specific response for when a user does not exist: ``` sdkErr.StatusCode() == http.StatusBadRequest && strings.Contains(sdkErr.Message, "must be an existing uuid or username") ```
1 parent 17f2a01 commit 5503d71

File tree

3 files changed

+53
-3
lines changed

3 files changed

+53
-3
lines changed

internal/provider/user_resource.go

+27-2
Original file line numberDiff line numberDiff line change
@@ -248,14 +248,15 @@ func (r *UserResource) Read(ctx context.Context, req resource.ReadRequest, resp
248248

249249
client := r.data.Client
250250

251+
// Lookup by ID to handle imports
251252
user, err := client.User(ctx, data.ID.ValueString())
252253
if err != nil {
253254
if isNotFound(err) {
254-
resp.Diagnostics.AddWarning("Client Warning", fmt.Sprintf("User with ID %q not found. Marking as deleted.", data.ID.ValueString()))
255+
resp.Diagnostics.AddWarning("Client Warning", fmt.Sprintf("User with ID %q not found. Marking resource as deleted.", data.ID.ValueString()))
255256
resp.State.RemoveResource(ctx)
256257
return
257258
}
258-
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to get current user, got error: %s", err))
259+
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to get current user by ID, got error: %s", err))
259260
return
260261
}
261262
if len(user.OrganizationIDs) < 1 {
@@ -274,6 +275,30 @@ func (r *UserResource) Read(ctx context.Context, req resource.ReadRequest, resp
274275
data.LoginType = types.StringValue(string(user.LoginType))
275276
data.Suspended = types.BoolValue(user.Status == codersdk.UserStatusSuspended)
276277

278+
// The user-by-ID API returns deleted users if the authorized user has
279+
// permission. It does not indicate whether the user is deleted or not.
280+
// The user-by-username API will never return deleted users.
281+
// So, we do another lookup by username.
282+
userByName, err := client.User(ctx, data.Username.ValueString())
283+
if err != nil {
284+
if isNotFound(err) {
285+
resp.Diagnostics.AddWarning("Client Warning", fmt.Sprintf(
286+
"User with username %q not found. Marking resource as deleted.",
287+
data.Username.ValueString()))
288+
resp.State.RemoveResource(ctx)
289+
return
290+
}
291+
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to get current user by username, got error: %s", err))
292+
return
293+
}
294+
if userByName.ID != data.ID.ValueUUID() {
295+
resp.Diagnostics.AddWarning("Client Error", fmt.Sprintf(
296+
"The username %q has been reassigned to a new user not managed by this Terraform resource. Marking resource as deleted.",
297+
user.Username))
298+
resp.State.RemoveResource(ctx)
299+
return
300+
}
301+
277302
// Save updated data into Terraform state
278303
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
279304
}

internal/provider/user_resource_test.go

+14
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/coder/coder/v2/coderd/util/ptr"
1010
"github.com/coder/terraform-provider-coderd/integration"
1111
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
12+
"github.com/hashicorp/terraform-plugin-testing/terraform"
1213
"github.com/stretchr/testify/require"
1314
)
1415

@@ -100,6 +101,19 @@ func TestAccUserResource(t *testing.T) {
100101
resource.TestCheckResourceAttr("coderd_user.test", "login_type", "github"),
101102
),
102103
},
104+
// Verify config drift via deletion is handled
105+
{
106+
Config: cfg4.String(t),
107+
Check: func(*terraform.State) error {
108+
user, err := client.User(ctx, "exampleNew")
109+
if err != nil {
110+
return err
111+
}
112+
return client.DeleteUser(ctx, user.ID)
113+
},
114+
// The Plan should be to create the entire resource
115+
ExpectNonEmptyPlan: true,
116+
},
103117
},
104118
})
105119
}

internal/provider/util.go

+12-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"net/http"
99
"os"
1010
"path/filepath"
11+
"strings"
1112

1213
"github.com/coder/coder/v2/codersdk"
1314
"github.com/google/uuid"
@@ -110,5 +111,15 @@ func memberDiff(currentMembers []uuid.UUID, plannedMembers []UUID) (add, remove
110111

111112
func isNotFound(err error) bool {
112113
var sdkErr *codersdk.Error
113-
return errors.As(err, &sdkErr) && sdkErr.StatusCode() == http.StatusNotFound
114+
if !errors.As(err, &sdkErr) {
115+
return false
116+
}
117+
if sdkErr.StatusCode() == http.StatusNotFound {
118+
return true
119+
}
120+
// `httpmw/ExtractUserContext` returns a 400 w/ this message if the user is not found
121+
if sdkErr.StatusCode() == http.StatusBadRequest && strings.Contains(sdkErr.Message, "must be an existing uuid or username") {
122+
return true
123+
}
124+
return false
114125
}

0 commit comments

Comments
 (0)