Skip to content

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

Merged
dbanck merged 10 commits intomainfrom
dbanck/import-via-identity
Apr 2, 2025
Merged

Config-driven importing through identity (TF-23179)#36703
dbanck merged 10 commits intomainfrom
dbanck/import-via-identity

Conversation

@dbanck
Copy link
Copy Markdown
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 e6c5f49 to 0252e5a Compare March 19, 2025 13:19
@dbanck dbanck force-pushed the dbanck/import-via-identity branch from b6f8033 to cc694a7 Compare March 20, 2025 17:54
@dbanck dbanck added the no-changelog-needed Add this to your PR if the change does not require a changelog entry label Mar 20, 2025
@dbanck dbanck force-pushed the dbanck/import-via-identity branch from cc694a7 to 92ffcdd Compare March 20, 2025 19:34
@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
Copy Markdown
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 🎉

@dbanck dbanck force-pushed the dbanck/import-via-identity branch from 7e069d4 to c39a786 Compare April 1, 2025 19:23
Copy link
Copy Markdown
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 requested a review from dsa0x April 2, 2025 08:39
@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
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 3, 2025

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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