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

Config-driven importing through identity (TF-23179) #36703

Merged
merged 10 commits into from
Apr 2, 2025

Conversation

dbanck
Copy link
Member

@dbanck dbanck commented Mar 14, 2025

This PR adds support for importing resources via their identity

Target Release

1.12.x

CHANGELOG entry

I'm planning to add changelog entries for the whole feature in a separate PR

@dbanck dbanck force-pushed the dbanck/import-via-identity branch from 92ffcdd to 1dbb40e Compare March 20, 2025 19:43
@dbanck dbanck marked this pull request as ready for review March 20, 2025 19:44
@dbanck dbanck requested a review from a team as a code owner March 20, 2025 19:44
Copy link
Member

@austinvalle austinvalle left a comment

Choose a reason for hiding this comment

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

Apologies for creeping on this PR 👀 , but I was just perusing how close this PR is to ready. I found one (already in main) decode problem here -> #36806 .

The other problem I ran into was something this PR may eventually have resolved so I'm just dropping it here 😄.

Currently, when importing by identity, the actual identity data isn't passed to the following ReadResource RPC call since it's not being added to the ResourceInstanceObject:


With both of those resolved, looks like the data is properly passed around after that with some basic smoke tests 🎉

Copy link
Member

@jbardin jbardin left a comment

Choose a reason for hiding this comment

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

LGTM! We can probably get rid of those getProvider calls since you're not using the provider. That little wrapper is a bit weird, and a result of refactoring long ago, but I'm not sure what to do with it for now since it does cut down on some repetition when providers are used.

@dbanck dbanck merged commit 6917e69 into main Apr 2, 2025
8 checks passed
@dbanck dbanck deleted the dbanck/import-via-identity branch April 2, 2025 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement no-changelog-needed Add this to your PR if the change does not require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants