[engine/import] Guess references to properties between dependant resources during import

[Zaid-Ajaj]

Description

Following up and extending #16208 such that now we guess references to properties between dependant resources when running an import based on their literal data retrieved from providers. See unit test added that showcases inferred dependencies when referencing deeply nested data from maps and arrays

Checklist

• I have run make tidy to update any new dependencies
• I have run make lint to verify my code passes the lint check
  • I have formatted my code using gofumpt

• I have added tests that prove my fix is effective or that my feature works

• I have run make changelog and committed the changelog/pending/<file> documenting my change

• Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version

[pulumi-bot]

Changelog

[uncommitted] (2024-06-25)

Features

• [engine] Guess references to (deeply nested) properties between dependant resources during import
  #16234

[Frassle]

Why is this just being checked for Identity values? Isn't this a problem for any value? If two resources both have an output "hello" and another resource has an input "hello" you can't decided which of the two resources to draw from.

[Zaid-Ajaj]

So the reason for this because if you have for example these pathed values computed

resourceGroup.id="group123"
resourceA.resourceGroup = "group123"
resourceB.resourceGroup = "group123"

Then I wanted to maintain the path resourceGroup.id even if that value is duplicated in paths resourceA.resourceGroup and resourceB.resourceGroup.

However, I think when building the pathed values, I should only be using state.Outputs instead of state.Inputs and only replace literal inputs with paths of resource outputs. Even then, resourceA.resourceGroup and resourceB.resourceGroup will occur both on state.Inputs and state.Outputs AFAIK

Would be great if I can actually test this with the CLI but getting weird behaviour with pulumi import

[Zaid-Ajaj]

Maybe I can actually do a second pass where we remove duplicate paths without taking identities into account and then merge non-duped identities and non-duped values 🤔

[Zaid-Ajaj]

Maybe I can actually do a second pass where we remove duplicate paths without taking identities into account and then merge non-duped identities and non-duped values 🤔

Implemented ☝️

Also found an edge case of "overguessing" that could result in a circular reference:

const bucket = new aws.s3.Bucket("my-bucket", {
    website: {
        indexDocument: "index.html",
    },
});

const bucketObject = new aws.s3.BucketObject("index.html", {
    bucket: bucket.id
});

Implemented a fallback such that if we encounter this error, we retry generating the program without guessing references.

[Zaid-Ajaj]

This has been resolved with the ancestorTypes feature where you can specify that BucketObject can guess values from outputs of Bucket but not the other way around ✅

[justinvp]

Some initial comments

[justinvp]

Is there a test case that covers this?

[Zaid-Ajaj]

I will add one!

[Zaid-Ajaj]

Done ✅

[justinvp]

What about non-string values, such as IsNumber()?

[justinvp]

Also, what about more complex values, like arrays and objects that are deeply equal? e.g. a resource has an output that's an array of strings and another resource has the same array of strings as an input?

[Zaid-Ajaj]

What about non-string values, such as IsNumber()?

I didn't account for numeric values, nor booleans because I am afraid of overguessing dependencies that have nothing to do with each other. For example instanceSize: 2 on a resource A and retryCount: 2 on a resource B would infer a dependency that is not necessarily what users wanted. Felt like starting with strings would make the most sense since AFAIK all IDs are strings and references between resources use a lot of them (i.e. resourceGroup.name in azure-native, vpc.id in aws, etc.)

Also, what about more complex values, like arrays and objects that are deeply equal? e.g. a resource has an output that's an array of strings and another resource has the same array of strings as an input?

First objects: for these we do not need to check for deep equality because their elements will be references. For example, consider the data here:

resourceA {
  // outputs
  config {
      first = "A"
      second = "B"
  }
}

resourceB {
  // inputs
  config {
      first = "A"
      second = "B"
  }
}

We could infer that resourceB.config = resourceA.config but chances are these are not the same type in the SDK so this might not work. That said, this is not needed in the first place because the current logic would infer dependency by values:

resourceB {
  config {
      first = resourceA.config.first
      second = resourceB.config.second
  }
}

This version is better handled by program-gen implementations.

As for arrays, I am not sure. The current logic would index each element where it is referenced:

resourceA {
  // outputs
  data = ["first", "second"]
}

resourceB {
  // inputs
  data = ["first", "second"]
}

Inferred:

resourceB {
  // inputs
  data = [resourceA.data[0], resourceA.data[1]]
}

so it will work even though it looks a bit ugly

[justinvp]

Can we have some test cases where there are multiple resources with the same output value, and therefore shouldn't be used as an input reference anywhere?

[Zaid-Ajaj]

TestGenerateHCL2DefinitionsWithAmbiguousReferencesMaintainsLiteralValue asserts just that but with IDs which are treated like outputs.