-
Notifications
You must be signed in to change notification settings - Fork 9
silo: use previous state for id attribute #529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Added a plan modifier to the `id` attribute to prevent it from being
marked as `(known after apply)` during a call to `Update`. This
prevents downstream resources that rely on `id` in an interpolation from
recomputing their state as well.
With this change, notice how the `id` attribute no longer is marked as
`(known after apply`).
```
tls_private_key.self-signed: Refreshing state... [id=c622d58fe2be7f6933c9bea4acade2bed4e4d050]
tls_self_signed_cert.self-signed: Refreshing state... [id=215195527778788990587349674206992319739]
oxide_silo.example: Refreshing state... [id=aed4e113-5db7-4025-abd6-dbac748e38c1]
Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# oxide_silo.example will be updated in-place
~ resource "oxide_silo" "example" {
id = "aed4e113-5db7-4025-abd6-dbac748e38c1"
name = "example"
~ quotas = {
~ cpus = 2 -> 4
# (2 unchanged attributes hidden)
}
~ time_created = "2025-09-24 20:15:42.838197 +0000 UTC" -> (known after apply)
~ time_modified = "2025-09-24 20:15:42.838197 +0000 UTC" -> (known after apply)
# (5 unchanged attributes hidden)
}
Plan: 0 to add, 1 to change, 0 to destroy.
Do you want to perform these actions?
Terraform will perform the actions described above.
Only 'yes' will be accepted to approve.
Enter a value: yes
oxide_silo.example: Modifying... [id=aed4e113-5db7-4025-abd6-dbac748e38c1]
oxide_silo.example: Modifications complete after 1s [id=aed4e113-5db7-4025-abd6-dbac748e38c1]
Apply complete! Resources: 0 added, 1 changed, 0 destroyed.
```
Closes #528.
| Computed: true, | ||
| Description: "Unique, immutable, system-controlled identifier of the silo.", | ||
| PlanModifiers: []planmodifier.String{ | ||
| stringplanmodifier.UseStateForUnknown(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable based on the docs:
This is useful for reducing (known after apply) plan outputs for computed attributes which are known to not change over time.
I assume we can count on generated IDs not changing over time? Anyway, this LGTM, just wondering if we need to do something similar for other resources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct! There's no API that I'm aware of that'll change the ID of the top-level resource on a call to Update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving, with a question on the diff. I think this is correct, just wondering if we need to make the same change elsewhere too.
I believe we need to make this change on all resources whose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh interesting, TIL!
I was wondering why I never ran into this problem and I think it's because the SDKv2 uses SetID() to explicitly set the resource identifier, so I guess it handles the plan diff behind the scenes.
I should probably be safe to apply this plan modifier to time_created fields as well.
Added a plan modifier to the `id` attribute to prevent it from being
marked as `(known after apply)` during a call to `Update`. This prevents
downstream resources that rely on `id` in an interpolation from
recomputing their state as well.
With this change, notice how the `id` attribute no longer is marked as
`(known after apply`).
```
tls_private_key.self-signed: Refreshing state... [id=c622d58fe2be7f6933c9bea4acade2bed4e4d050]
tls_self_signed_cert.self-signed: Refreshing state... [id=215195527778788990587349674206992319739]
oxide_silo.example: Refreshing state... [id=aed4e113-5db7-4025-abd6-dbac748e38c1]
Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# oxide_silo.example will be updated in-place
~ resource "oxide_silo" "example" {
id = "aed4e113-5db7-4025-abd6-dbac748e38c1"
name = "example"
~ quotas = {
~ cpus = 2 -> 4
# (2 unchanged attributes hidden)
}
~ time_created = "2025-09-24 20:15:42.838197 +0000 UTC" -> (known after apply)
~ time_modified = "2025-09-24 20:15:42.838197 +0000 UTC" -> (known after apply)
# (5 unchanged attributes hidden)
}
Plan: 0 to add, 1 to change, 0 to destroy.
Do you want to perform these actions?
Terraform will perform the actions described above.
Only 'yes' will be accepted to approve.
Enter a value: yes
oxide_silo.example: Modifying... [id=aed4e113-5db7-4025-abd6-dbac748e38c1]
oxide_silo.example: Modifications complete after 1s [id=aed4e113-5db7-4025-abd6-dbac748e38c1]
Apply complete! Resources: 0 added, 1 changed, 0 destroyed.
```
Closes
#528.
I forgot to update the changelog in #529.
I was also wondering why I never ran into this. Thanks for the link. I submitted #530 to update the remaining resources. I'll update it also to account for the |
I forgot to update the changelog in #529.
I would think |
You're right! For some reason last night I lumped |
Added a plan modifier to the
idattribute to prevent it from being marked as(known after apply)during a call toUpdate. This prevents downstream resources that rely onidin an interpolation from recomputing their state as well.With this change, notice how the
idattribute no longer is marked as(known after apply).Closes #528.