feat(iam): add support for identity#3661
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces Terraform Plugin SDKv2 ResourceIdentity support for several IAM resources, and updates acceptance/unit tests to validate import behavior and identity coverage.
Changes:
- Add
Identityschemas to IAM resources (api key, application, group, policy, SSH key, user) via a newidentity.FlatIdentity(...)helper. - Update IAM acceptance tests to include
ImportState/ImportStateVerifysteps. - Update the provider-level test that enforces non-nil
Resource.Identityacross resources.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| provider/sdkv2_test.go | Removes IAM resources from the “no identity” exceptions list. |
| internal/identity/identity.go | Adds FlatIdentity helper for single-field identities. |
| internal/services/iam/application.go | Adds identity schema + uses flat identity setter (needs error handling + read-path identity set). |
| internal/services/iam/api_key.go | Adds identity schema + sets flat identity on create (read-path identity set missing). |
| internal/services/iam/group.go | Adds identity schema (but Create/Read still don’t populate identity). |
| internal/services/iam/policy.go | Adds identity schema + refactors read (but identity not populated; ListRules missing ctx). |
| internal/services/iam/ssh_key.go | Adds identity schema + sets flat identity on create (create has ordering bug when disabled is set; read-path identity missing). |
| internal/services/iam/user.go | Adds identity schema + sets flat identity on create/read. |
| internal/services/iam/*_test.go | Adds import verification steps for IAM resources. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| d.SetId(app.ID) | ||
| err = identity.SetFlatIdentity(d, "id", app.ID) |
There was a problem hiding this comment.
identity.SetFlatIdentity can return an error (e.g., if identity support isn't available for this resource data). The returned err is currently ignored, which can leave d.Id() unset and cause the subsequent Read call to query with an empty ID. Handle the error and return diagnostics if it occurs.
| err = identity.SetFlatIdentity(d, "id", app.ID) | |
| if err := identity.SetFlatIdentity(d, "id", app.ID); err != nil { | |
| return diag.FromErr(err) | |
| } |
| } | ||
|
|
||
| setApplicationState(d, app) | ||
|
|
||
| return nil |
There was a problem hiding this comment.
This resource declares an Identity schema, but the Read path never populates it. Consider calling identity.SetFlatIdentity(d, "id", app.ID) (and handling the error) so imported/refreshed state keeps the identity fields consistent.
| _ = d.Set("expires_at", types.FlattenTime(res.ExpiresAt)) | ||
| _ = d.Set("access_key", res.AccessKey) | ||
| setAPIKeyState(d, apiKey) | ||
|
|
There was a problem hiding this comment.
ResourceAPIKey now declares an Identity schema, but the Read path doesn't set it. To keep identity data consistent after refresh/import, set it from the API response (e.g., identity.SetFlatIdentity(d, "id", apiKey.AccessKey)) and handle any error.
| if err := identity.SetFlatIdentity(d, "id", apiKey.AccessKey); err != nil { | |
| return diag.FromErr(err) | |
| } |
| SchemaVersion: 0, | ||
| SchemaFunc: policySchema, | ||
| Identity: identity.FlatIdentity("id", "Policy UUID"), | ||
| } |
There was a problem hiding this comment.
ResourcePolicy declares an Identity schema, but the implementation still uses d.SetId(...) and never sets the identity fields (in Create/Read). To fully support identity-based import/state, use identity.SetFlatIdentity(d, "id", pol.ID) in Create and set it again in Read from the fetched policy, handling any error.
| SchemaFunc: sshKeySchema, | ||
| Identity: identity.FlatIdentity("id", "SSH key UUID"), | ||
| } |
There was a problem hiding this comment.
ResourceSSKKey now declares an Identity schema, but the Read path doesn't populate it. Consider setting the identity in Read (e.g., identity.SetFlatIdentity(d, "id", res.ID)) so the identity fields stay consistent after refresh/import.
| SchemaFunc: groupSchema, | ||
| Identity: identity.FlatIdentity("id", "Group UUID"), | ||
| } |
There was a problem hiding this comment.
ResourceGroup declares an Identity schema, but the implementation still uses d.SetId(...) and does not populate the identity fields (in Create/Read). To actually support identity-based import/state, switch to identity.SetFlatIdentity(d, "id", group.ID) (and similarly in Read) and handle the returned error.
| _ = d.Set("user_id", types.FlattenStringPtr(pol.UserID)) | ||
| rules, err := api.ListRules(&iam.ListRulesRequest{ | ||
| PolicyID: policy.ID, | ||
| }) |
There was a problem hiding this comment.
api.ListRules is invoked without scw.WithContext(ctx), so request cancellation/timeouts from the Terraform context won't propagate to this API call. Pass the context as a request option, consistent with the other IAM API calls in this function.
| }) | |
| }, scw.WithContext(ctx)) |
| err = identity.SetFlatIdentity(d, "id", res.ID) | ||
| if err != nil { | ||
| return diag.FromErr(err) | ||
| } |
There was a problem hiding this comment.
When disabled is set during create, the code calls UpdateSSHKey using d.Id() before the ID is assigned (the ID is only set later via identity.SetFlatIdentity). This will send an empty SSHKeyID and fail. Set the ID/identity immediately after CreateSSHKey (or use res.ID directly in the update request) before attempting the update.
| "scaleway_iam_ssh_key", | ||
| "scaleway_iam_user", | ||
| "scaleway_inference_deployment", | ||
| "scaleway_inference_model", |
There was a problem hiding this comment.
scaleway_iam_group_membership was removed from the exceptions list, but ResourceGroupMembership() currently does not declare Identity (so d.Identity remains nil). This will make TestSDKProvider_ResourceIdentityNotEmpty fail unless group membership gets an Identity schema, or it is re-added to the exceptions list.
| "scaleway_inference_model", | |
| "scaleway_inference_model", | |
| "scaleway_iam_group_membership", |
No description provided.