providers/aws: fix source_dest_check on instance creation #1021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `SourceDestCheck` attribute can only be changed via `ModifyInstance`, so the AWS instance resource's `Create` function calls out to `Update` before it returns to take care of applying `source_dest_check` properly. The `Update` function originally guarded against unnecessary API calls with `GetOk`, which worked fine until #993 when we changed the `GetOk` semantics to no longer distinguish between "configured and zero-value" and "not configured". I attempted in #1003 to fix this by switching to `HasChange` for the guard, but this does not work in the `Create` case. I played around with a few different ideas, none of which worked: (a) Setting `Default: true` on `source_dest_check' has no effect (b) Setting `Computed: true` on `source_dest_check' and adding a `d.Set` call in the `Read` function (which will initially set the value to `true` after instance creation). I really thought I could get this to work, but it results in the following: ```go d.Get('source_dest_check') // true d.HasChange('source_dest_check') // false d.GetChange('source_dest_check') // old: false, new: false ``` I couldn't figure out a way of coherently dealing with that result, so I ended up throwing up my hands and giving up on the guard altogether. We'll call `ModifyInstance` more than we have to, but this at least yields expected behavior for both Creates and Updates. Fixes #1020
|
This is a good example of the fallout of the cc @mitchellh to join me in said think |
|
Yeah... going to merge this LGTM. But yeah, I noticed this. Also, we should talk about doing a branch to backport a 0.3.8 out to fix this while we do 0.4. |
mitchellh
added a commit
that referenced
this pull request
Feb 23, 2015
…eck-on-create providers/aws: fix source_dest_check on instance creation
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The
SourceDestCheckattribute can only be changed viaModifyInstance, so the AWS instance resource'sCreatefunction callsout to
Updatebefore it returns to take care of applyingsource_dest_checkproperly.The
Updatefunction originally guarded against unnecessary API callswith
GetOk, which worked fine until #993 when we changed theGetOksemantics to no longer distinguish between "configured and zero-value"
and "not configured".
I attempted in #1003 to fix this by switching to
HasChangefor theguard, but this does not work in the
Createcase.I played around with a few different ideas, none of which worked:
(a) Setting
Default: trueon `source_dest_check' has no effect(b) Setting
Computed: trueonsource_dest_check' and adding ad.Setcall in theReadfunction (which will initially set the value totrue`after instance creation). I really thought I could get this to work,
but it results in the following:
I couldn't figure out a way of coherently dealing with that result, so I
ended up throwing up my hands and giving up on the guard altogether.
We'll call
ModifyInstancemore than we have to, but this at leastyields expected behavior for both Creates and Updates.
Fixes #1020