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

Update Registry to respect provider aliases #7166

Merged
merged 11 commits into from
Jul 28, 2021

Conversation

EvanBoyle
Copy link
Contributor

Description

This change makes the provider registry aware of aliases. This allows provider aliases to work properly, ensuring that resources that use aliased providers don't get replaced unnecessarily.

Fixes #3979

Checklist

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

@pgavlin pgavlin self-assigned this Jun 2, 2021
@pgavlin
Copy link
Member

pgavlin commented Jun 2, 2021

It feels like this will behave subtly differently than other aliases, and I'm not sure that's what we want.

For other resource types, an alias just means "consider this resource to have the same identity as the resource in the old state". For providers, that identity influences a couple things:

  1. Which provider instance is used for a particular resource, which is managed by the provider map
  2. Whether or not resources that refer to the provider will be considered as requiring replacement due to a change in their provider

I think that your changes address (1)--updating the provider map ensures that resources in the old state will treat the new provider as having the same identity as their provider if it has been aliased--but I'm not sure that it addresses (2). I think that the referenced issue requires that (2) is fixed. Can you add a test for that scenario?

@EvanBoyle
Copy link
Contributor Author

Just tested (2) out and you are correct, altering the provider results in it getting replaced but not the resources that depend on it.

Do have any suggestions on what layer to look at for addressing (2)?

@joeduffy
Copy link
Member

joeduffy commented Jul 9, 2021

Hey @EvanBoyle I was super excited to see you working on a fix for this. Any chance it will be landing soonish?

@EvanBoyle
Copy link
Contributor Author

EvanBoyle commented Jul 13, 2021

Update, it looks like the incorrect behavior for (2) in my test program is due to pulumi/pulumi-terraform-bridge#244

I'll write some lifecycle tests for (2) to verify.

@EvanBoyle
Copy link
Contributor Author

@pgavlin I've added a passing test that covers those two scenarios, would you mind taking another look?

pkg/resource/deploy/providers/registry.go Outdated Show resolved Hide resolved
pkg/resource/deploy/step_generator.go Outdated Show resolved Hide resolved
@EvanBoyle EvanBoyle requested a review from pgavlin July 17, 2021 00:27
@EvanBoyle EvanBoyle merged commit f4efb75 into master Jul 28, 2021
@pulumi-bot pulumi-bot deleted the evan/respectProviderAliases branch July 28, 2021 19:12
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.

Provider aliases aren't working
3 participants