-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
Conversation
e6c5f49
to
0252e5a
Compare
b6f8033
to
cc694a7
Compare
cc694a7
to
92ffcdd
Compare
92ffcdd
to
1dbb40e
Compare
1dbb40e
to
7e069d4
Compare
There was a problem hiding this 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
:
importedState := states.NewResourceInstanceObjectFromIR(imported[0]) terraform/internal/states/instance_object.go
Lines 70 to 76 in dcb06b9
func NewResourceInstanceObjectFromIR(ir providers.ImportedResource) *ResourceInstanceObject { return &ResourceInstanceObject{ Status: ObjectReady, Value: ir.State, Private: ir.Private, } }
With both of those resolved, looks like the data is properly passed around after that with some basic smoke tests 🎉
7e069d4
to
c39a786
Compare
There was a problem hiding this 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.
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