Skip to content
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

Better convert dynamically bridged providers #308

Merged
merged 1 commit into from
Feb 27, 2025
Merged

Conversation

lunaris
Copy link
Contributor

@lunaris lunaris commented Feb 21, 2025

When converting Terraform code to PCL, a big part of the work is mapping Terraform resources and their providers to the equivalent Pulumi bits and pieces. There are a few parts to this process:

  1. Given a Terraform provider P, what is the equivalent Pulumi provider?
  2. When a Pulumi provider has been determined, what are the mappings we should use to turn the various resources, data sources, etc. under that provider into their Pulumi equivalents?

For part 1, we mostly use a hard-coded list -- the set of managed, first-class Pulumi providers is finite, and anything not in that list can be dynamically bridged. Armed with this, part 2 is reduced to the problem of asking the engine (which is calling us as the converter) for the right set of mappings. Until now this part has been broken in the presence of providers which we need to dynamically bridge: while we can generate the correct PCL, with appropriate package blocks that describe the plugins and parameterization required, we have not been able to pass the same information (e.g. parameterization) to the mapper. With pulumi/pulumi#18671, we now can! This change thus does that while tidying up a few other things around dynamically bridged/parameterized providers, and gets conversion in the presence of these providers a bit further along.

Note

Things are likely still not 100% perfect, since we use
required_providers blocks to work out the versions we'll pass when
parameterizing. In the case of converting state, we don't have these,
and it's not a given that we'll always have them when converting
programs either, but we can iterate from this base and hopefully close
these gaps quickly.

Blocked by pulumi/pulumi-terraform-bridge#2905 release 3.153.0

Fixes pulumi/pulumi#18187

Copy link
Contributor

@brandonpollack23 brandonpollack23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went through all of this and can't find much to say. This change was all very subtle, great work. I'm not certain I would have come up with this on my own at all.

go.mod Outdated

toolchain go1.23.3

replace github.com/pulumi/pulumi-terraform-bridge/v3 v3.96.0 => ../pulumi-terraform-bridge
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a reminder dont forget to remove

@brandonpollack23 brandonpollack23 marked this pull request as ready for review February 25, 2025 12:04
@brandonpollack23 brandonpollack23 requested a review from a team as a code owner February 25, 2025 12:04
@brandonpollack23 brandonpollack23 marked this pull request as draft February 25, 2025 12:05
@brandonpollack23 brandonpollack23 marked this pull request as ready for review February 25, 2025 12:32
@lunaris lunaris force-pushed the wjones/param-convert branch from 2547320 to e2aba68 Compare February 27, 2025 11:49
Copy link
Contributor

@tgummerer tgummerer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not super familiar with this codebase, but the changes look reasonable to me. Could we add a test for this, or is this covered somehow already?

@Frassle
Copy link
Member

Frassle commented Feb 27, 2025

Two comments.
Firstly echo @tgummerer this should have a test.
Secondly it would have been nice to do the go.mod updates thats caused the code churn and diagnostics to all change in its own PR for simpler review.

@lunaris lunaris force-pushed the wjones/param-convert branch from e2aba68 to 3803785 Compare February 27, 2025 12:42
@lunaris lunaris changed the base branch from main to wjones/go-mod-bump February 27, 2025 12:43
@lunaris lunaris force-pushed the wjones/param-convert branch from 3803785 to 9789198 Compare February 27, 2025 12:54
Base automatically changed from wjones/go-mod-bump to main February 27, 2025 13:06
When converting Terraform code to PCL, a big part of the work is mapping
Terraform resources and their providers to the equivalent Pulumi bits
and pieces. There are a few parts to this process:

1. Given a Terraform provider P, what is the equivalent Pulumi provider?
2. When a Pulumi provider has been determined, what are the mappings we
   should use to turn the various resources, data sources, etc. under
   that provider into their Pulumi equivalents?

For part 1, we mostly use a hard-coded list -- the set of managed,
first-class Pulumi providers is finite, and anything not in that list
can be dynamically bridged. Armed with this, part 2 is reduced to the
problem of asking the engine (which is calling us as the converter) for
the right set of mappings. Until now this part has been broken in the
presence of providers which we need to dynamically bridge: while we can
generate the correct PCL, with appropriate `package` blocks that
describe the plugins and parameterization required, we have not been
able to pass the same information (e.g. parameterization) to the mapper.
With pulumi/pulumi#18671, we now can! This change thus does that while
tidying up a few other things around dynamically bridged/parameterized
providers, and gets conversion in the presence of these providers a bit
further along.

> [!NOTE]
> Things are likely still not 100% perfect, since we use
> `required_providers` blocks to work out the versions we'll pass when
> parameterizing. In the case of converting state, we don't have these,
> and it's not a given that we'll always have them when converting
> programs either, but we can iterate from this base and hopefully close
> these gaps quickly.

Fixes pulumi/pulumi#18187

Co-authored-by: Brandon Pollack <brandonpollack23@gmail.com>
@lunaris lunaris force-pushed the wjones/param-convert branch from 9789198 to 7c5db26 Compare February 27, 2025 13:07
@lunaris lunaris merged commit 9a4b84f into main Feb 27, 2025
4 checks passed
@lunaris lunaris deleted the wjones/param-convert branch February 27, 2025 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

after converting from terraform the sdk."index" is added to generated sdk packages when referenced
4 participants