-
Notifications
You must be signed in to change notification settings - Fork 233
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
Ability to Get/Set Only the State Resource Identifier (Without id Attribute Handling) #541
Comments
To be extra confusing, this logic called by terraform-plugin-sdk/helper/schema/resource.go Lines 491 to 497 in b72757e
But in Terraform CLI 0.13.0 with the following applied configuration:
And removing the
So no magic bullet for data sources, at least there. |
… receiver methods and only passthrough id attribute state if present Reference: hashicorp#541 This allows resources to decouple from the implicit `id` attribute by only working with the underlying resource identifier (`InstanceState.ID`). Any `id` attribute handling is omitted from these new receiver methods. For reference, the existing `Id()` receiver method always read the `id` attribute as a fallback if the resource identifier was missing and `SetId()` always set the `id` attribute to match the resource identifier. Also part of this support, the `id` attribute is no longer always "promoted" using the resource identifier if the attribute is missing when retrieving the state, however it is always passed through if present in the previous or new state. Existing logic using `SetId()` and the existing import functionality are unaffected as they continue to set the `id` attribute.
… receiver methods and only passthrough id attribute state if present Reference: hashicorp#541 This allows resources to decouple from the implicit `id` attribute by only working with the underlying resource identifier (`InstanceState.ID`). Any `id` attribute handling is omitted from these new receiver methods. For reference, the existing `Id()` receiver method always read the `id` attribute as a fallback if the resource identifier was missing and `SetId()` always set the `id` attribute to match the resource identifier. Also part of this support, the `id` attribute is no longer always "promoted" using the resource identifier if the attribute is missing when retrieving the state, however it is always passed through if present in the previous or new state. Existing logic using `SetId()` and the existing import functionality are unaffected as they continue to set the `id` attribute.
Took a shot with proposal 1 here: #542 |
I'm a little confused here, and I think the shims may be muddying the issue. After 0.12, there's no such thing as a Resource ID, as far as I know. It's not part of the protocol at all, that I can find? Everything I'm being told, which I'm inclined to believe, indicates that IDs are a legacy bit of the SDK that doesn't actually matter to Terraform anymore. If it weren't for the backwards compatibility concerns, I think we could just.... remove them. Personally, I'm inclined to leave them alone at the moment, or if the need is great enough (to resolve #586 for example) then to add a flag to I think I can pull together a terraform-plugin-go example of a data source without any ID fairly easily to prove the concept before we invest in trying to untangle the SDK... |
Hi! 👋 I happened to coincidentally end up here as a result of following some other links today and saw the comment above, so wanted to offer some confirmation: Terraform Core mostly doesn't treat The SDK still has these special cases because we could see no other way in the pre-existing I think to remove it would therefore require introducing some other way to signal the presence or absence of an object, independently of setting its The "mostly" in my initial statement above deserves some elaboration: although the wire protocol and Terraform Core don't treat We have in the past occasionally discussed extending the schema model in the protocol to allow providers to optionally override that heuristic for resource types that have schemas that it doesn't work with, but with the SDK so far continuing to require |
Going to close this out -- I don't think we'll want to do this in this version of the framework and its not worth keeping it as an open issue. |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
SDK version
Use-cases
For some data sources and resources, there is no concept of an identifier. For example, the Terraform AWS Provider has quite a few "plural" data sources that return multiple objects of resource type. An
id
attribute is extraneous.Due to certain changes in Terraform CLI 0.13.0 (potentially some behaviors rolling back in 0.13.1) and confirmed for future versions of Terraform CLI, unstable data source attributes are expected to show in the Terraform plan output. Providers now must be careful to ensure all attributes are stable, including the
id
attribute, even if it is meaningless.Attempted Solutions
Trying to use the following inconsistently implemented or "this seems much harder than it should be" resource identifiers:
resource.UniqueID()
time.Now()
Proposal
Personally I lean towards 1 here, as it is the least risky overall, but present other random thoughts on the subject as well for discussion points. Proposal 3 seems kind of interesting but it does further couple providers to "special" Terraform Plugin SDK logic in this project, meaning changing the SDK or muxing it with other SDKs could be more cumbersome.
One caveat to any of these changes is that historically every Terraform Resource (using this Terraform Plugin SDK anyways in all its historical forms) has always had an
id
attribute. This "absolute" will no longer be the case.Proposal 1
Currently the
(*helper/schema.ResourceData).SetId()
receiver method combines two pieces of functionality:id
attributeWhile the related
(*helper/schema.ResourceData).Id()
receiver method performs the reverse of that functionality:id
attributeIt might make sense to add separate receiver methods that only work with the underlying resource identifier and do not work with the resource
id
attribute at all. For example:func (d *ResourceData) GetResourceID() string
func (d *ResourceData) SetResourceID(v string)
d.SetId("")
required today)func (d *ResourceData) RemoveFromState()
To further this concept and align with Terraform core's concepts of the resource identifier being wholly separate from the
id
attribute, it could be considered to deprecate the existingSetId()
andId()
receiver methods in preference of the new ones above. Existingd.SetId()
behavior could be preserved with:Proposal 2
Another consideration would be approaching this issue differently, where the resource identifier is automatically calculated from the schema and state values. In this proposal, instead of calling
SetId()
manually, the resource schema could declare which attributes participate in the value generation, e.g.And the SDK automatically does things to manage the identifier outside the resource logic, except for triggering resource removal. This has a ton more work associated with it and plenty of caveats to think through, e.g.
Proposal 3
Another variation of proposal 2 is changing the state resource identifier to a be completely opaque value to the resource, such as a one-time UUID after successful creation/import, or a hash of the
ResourceData
/InstanceState
. This removes the provider and resource logic from managing this identifier at all, except for triggering resource removal.References
The text was updated successfully, but these errors were encountered: