Skip to content

addrs: Fix LegacyString for builtin providers#25861

Merged
alisdair merged 2 commits intomasterfrom
alisdair/builtin-provider-legacy-string
Aug 17, 2020
Merged

addrs: Fix LegacyString for builtin providers#25861
alisdair merged 2 commits intomasterfrom
alisdair/builtin-provider-legacy-string

Conversation

@alisdair
Copy link
Copy Markdown
Contributor

Builtin provider addrs (i.e. "terraform.io/builtin/terraform") should be able to convert to legacy string form (i.e. "terraform"). This ensures that we can safely round-trip through ParseLegacyAbsProviderConfig,
which can return either a legacy or a builtin provider addr.

Fixes #25803. The test in the second commit reproduces the panic without the first commit.

Builtin provider addrs (i.e. "terraform.io/builtin/terraform") should be
able to convert to legacy string form (i.e. "terraform"). This ensures
that we can safely round-trip through ParseLegacyAbsProviderConfig,
which can return either a legacy or a builtin provider addr.
@alisdair alisdair requested a review from a team August 14, 2020 19:50
@alisdair alisdair self-assigned this Aug 14, 2020
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 14, 2020

Codecov Report

Merging #25861 into master will increase coverage by 0.00%.
The diff coverage is 0.00%.

Impacted Files Coverage Δ
addrs/provider.go 66.85% <0.00%> (ø)
terraform/evaluate.go 53.93% <0.00%> (-0.44%) ⬇️
states/statefile/version3_upgrade.go 67.09% <0.00%> (+2.16%) ⬆️

Copy link
Copy Markdown
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

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

🎉

@alisdair alisdair merged commit eb9e32d into master Aug 17, 2020
@alisdair alisdair deleted the alisdair/builtin-provider-legacy-string branch August 17, 2020 15:22
@jamroks
Copy link
Copy Markdown

jamroks commented Aug 20, 2020

🎉

I maybe mistaken but this hasn't been released yet, right ?

@alisdair
Copy link
Copy Markdown
Contributor Author

That's correct. It will be released shortly as part of 0.13.1.

@jamroks
Copy link
Copy Markdown

jamroks commented Aug 20, 2020

Thank you ! @alisdair

@ghost
Copy link
Copy Markdown

ghost commented Oct 11, 2020

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.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

panic: terraform.io/builtin/terraform is not a legacy addrs.Provider

3 participants