pulumi / pulumi-terraform-bridge

A library allowing Terraform providers to be bridged into Pulumi.
Apache License 2.0
194 stars 43 forks source link

Muxed ID validation triggers on SDK based resources #2030

Closed iwahbe closed 4 months ago

iwahbe commented 4 months ago

Our ID validation is over-eager for muxed providers. It triggers missing ID errors on SDK based resources.

error: Resource test_res has a problem: no "id" attribute. To map this resource consider specifying ResourceInfo.ComputeID
t0yv0 commented 4 months ago

I had a question about the problem statement here. SDK based resources do not require an ID attribute? Do you have an example?

// InstanceState is used to track the unique state information belonging
// to a given instance.
type InstanceState struct {
    // A unique ID for this resource. This is opaque to Terraform
    // and is only meant as a lookup mechanism for the providers.
    ID string `json:"id"`

Reading this again, it does appear that while there is a provider-scoped opaque notion of ID in TF SDKv2, it is not necessarily obvious that it must imply there is an attribute called "id".

Scanning the code though, I am afraid there are still plenty of places in the bridge that assume exactly this:

func (s *v2InstanceState2) ID() string {
    id := s.stateValue.GetAttr("id")
    if !id.IsKnown() {
        return ""
    }
    contract.Assertf(id.Type() == cty.String, "expected id to be of type String")
    return id.AsString()
}

And I am not sure the PR fully removes that assumption. Perhaps something that needs doing later? Thoughts?