refactor(product_enablement): make Read() logic consistent with other resource types #773
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In a prior PR we fixed a bunch of bugs with the design of the
product_enablement
nested resource block.I noticed that we still had logic related to the old implementation.
For example, in the original implementation, we were always setting data into the state file when doing the TF 'read flow'. This caused problems with users seeing an unexpected diff because their config didn't contain something that now existed in the state file. Still part of the original implementation was a follow-up bug fix for that problem (i.e. we would only execute the 'read flow' if the user actually had
product_enablement
defined in their config).As part of the recent PR rewrite for
product_enablement
, I noticed that we still had the conditional check that meant we wouldn't execute the 'read flow' unless the user had actually definedproduct_enablement
in their config.So this PR fixes that by removing those unnecessary requirements, and in the course of doing that I've fixed two other bugs that then surfaced because of that change...
name
attribute if the user hasproduct_enablement
defined in their config. If we didn't do this, then we would A.) still be setting state data (albeit just thename
attribute) which is something we were trying to avoid, and B.) if the user had allowed their service version to drift outside of TF, then the TF logic for handling that and bringing the versions back inline with what TF was tracking would cause a runtime panic because we would force the execution of the 'read flow' which would have tried to index (e.g.localState[0]
) into an empty list.