config/module: use the raw source as part of the key #9144
Conversation
This changes the key for the storage to be the _raw_ source from the module, not the fully expanded source. Example: it'll be a relative path instead of an absolute path. This allows the ".terraform/modules" directory to be portable when moving to other machines. This was a behavior that existed in <= 0.7.2 and was broken with #8398. This amends that and adds a test to verify.
|
|
||
| entry, err = os.Stat(target) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
Here's a prime example of having to be very careful with naked returns and name result parameters, since err was shadowed in this for loop. This is handled correctly here, but the mix of return values and naked returns in this code looks prime for introducing bugs later on. Can we drop the named result value for this function?
There was a problem hiding this comment.
I just copied this code from the gist at the top of the file. It isn't tested since it itself is only used in tests and the original source didn't come with it. I can add it if we want but I agree there is a LOT stylistically I disagree with in this code but it was a copy pasta.
| } | ||
| defer func() { | ||
| if e := out.Close(); e != nil { | ||
| err = e |
There was a problem hiding this comment.
You should check if err == nil before assigning e here, since an existing error is probably more important that the Close error.
| if err != nil && !os.IsNotExist(err) { | ||
| return | ||
| } | ||
| if err == nil { |
There was a problem hiding this comment.
passing a nil error to os.IsNotExist returns false, so you could just do a single check. Any other errors will be caught later anyway
if !os.IsNotExist(err) {
return fmt.Errorf("destination already exists")
}
(only mention it because the err == nil check threw me for a second)
|
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
Fixes customer ticket 1806.
This changes the key for the storage to be the raw source from the
module, not the fully expanded source. Example: it'll be a relative path
instead of an absolute path.
This allows the ".terraform/modules" directory to be portable when
moving to other machines. This was a behavior that existed in <= 0.7.2
and was broken with #8398. This amends that and adds a test to verify.