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

evaluation of missing instances during import #31878

Merged
merged 1 commit into from
Oct 4, 2022
Merged

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Sep 27, 2022

Because import does not yet plan new instances as part of the import process, we can end up evaluating references to resources which have no state at all, and the fallback for this situation could result in slightly better values. The count and for_each values were technically incorrect, since the length is not known to be zero, so we return a DynamicVal instead. The single instance case does have a concrete type which we can return as an unknown.

This whole process was meant as a step towards a complete import plan, but unfortunately is still contains a few similar edge cases which plagued the original import graph. While these new values will allow more expressions to evaluate correctly (mostly in count/for_each where known-ness is important), there are going to be situations which won't work until we have the complete planned values.

Fixes #31852

@jbardin jbardin added the 1.3-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Sep 27, 2022
@jbardin jbardin requested a review from a team September 27, 2022 18:23
@jbardin jbardin self-assigned this Sep 27, 2022
@jbardin jbardin changed the title unknown evaluation of missing instances in import evaluation of missing instances during import Sep 27, 2022
Because import does not yet plan new instances as part of the import
process, we can end up evaluating references to resources which have no
state at all. The fallback for this situation could result in slightly
better values during import. The count and for_each values were
technically incorrect, since the length is not known to be zero, and the
single instance does have a concrete type which we can return.
@jbardin jbardin merged commit 3047b8c into main Oct 4, 2022
@jbardin jbardin deleted the jbardin/import-plan-new branch October 4, 2022 16:45
@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2022

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2022

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 Nov 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1.3-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: Invalid count argument in Terraform v1.3.0
2 participants