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.

Sign up for GitHub

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

terraform: copy RawConfigs to avoid races #1454

Merged
merged 2 commits into from
Apr 9, 2015
Merged

terraform: copy RawConfigs to avoid races #1454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b201983
terraform: copy RawConfigs
mitchellh Apr 9, 2015
06beab6
config: copy the key on Copy
mitchellh Apr 9, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions config/raw_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package config
import (
"bytes"
"encoding/gob"
"sync"

"github.com/hashicorp/terraform/config/lang"
"github.com/hashicorp/terraform/config/lang/ast"
Expand Down Expand Up @@ -31,6 +32,7 @@ type RawConfig struct {
Interpolations []ast.Node
Variables map[string]InterpolatedVariable

lock sync.Mutex
config map[string]interface{}
unknownKeys []string
}
Expand All @@ -46,6 +48,20 @@ func NewRawConfig(raw map[string]interface{}) (*RawConfig, error) {
return result, nil
}

// Copy returns a copy of this RawConfig, uninterpolated.
func (r *RawConfig) Copy() *RawConfig {
r.lock.Lock()
defer r.lock.Unlock()

result, err := NewRawConfig(r.Raw)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is fine, but double checking is always good. It is okay that we don't copy Key?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it isn't okay, good catch. fixing that now.

if err != nil {
panic("copy failed: " + err.Error())
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I panic here because if the *RawConfig was made once, it shouldn't fail ever again. It is a pure function in that way.


result.Key = r.Key
return result
}

// Value returns the value of the configuration if this configuration
// has a Key set. If this does not have a Key set, nil will be returned.
func (r *RawConfig) Value() interface{} {
Expand All @@ -55,6 +71,8 @@ func (r *RawConfig) Value() interface{} {
}
}

r.lock.Lock()
defer r.lock.Unlock()
return r.Raw[r.Key]
}

Expand All @@ -81,6 +99,9 @@ func (r *RawConfig) Config() map[string]interface{} {
//
// If a variable key is missing, this will panic.
func (r *RawConfig) Interpolate(vs map[string]ast.Variable) error {
r.lock.Lock()
defer r.lock.Unlock()

config := langEvalConfig(vs)
return r.interpolate(func(root ast.Node) (string, error) {
// We detect the variables again and check if the value of any
Expand Down Expand Up @@ -119,6 +140,9 @@ func (r *RawConfig) Interpolate(vs map[string]ast.Variable) error {
// values in this config) and returns a new config. The original config
// is not modified.
func (r *RawConfig) Merge(other *RawConfig) *RawConfig {
r.lock.Lock()
defer r.lock.Unlock()

// Merge the raw configurations
raw := make(map[string]interface{})
for k, v := range r.Raw {
Expand Down Expand Up @@ -252,6 +276,9 @@ func (r *RawConfig) GobDecode(b []byte) error {
// tree of interpolated variables is recomputed on decode, since it is
// referentially transparent.
func (r *RawConfig) GobEncode() ([]byte, error) {
r.lock.Lock()
defer r.lock.Unlock()

data := gobRawConfig{
Key: r.Key,
Raw: r.Raw,
Expand Down
8 changes: 4 additions & 4 deletions terraform/transform_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode {
Output: &provider,
})
vseq.Nodes = append(vseq.Nodes, &EvalInterpolate{
Config: n.Resource.RawConfig,
Config: n.Resource.RawConfig.Copy(),
Resource: resource,
Output: &resourceConfig,
})
Expand All @@ -189,7 +189,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode {
Name: p.Type,
Output: &provisioner,
}, &EvalInterpolate{
Config: p.RawConfig,
Config: p.RawConfig.Copy(),
Resource: resource,
Output: &resourceConfig,
}, &EvalValidateProvisioner{
Expand Down Expand Up @@ -243,7 +243,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode {
Node: &EvalSequence{
Nodes: []EvalNode{
&EvalInterpolate{
Config: n.Resource.RawConfig,
Config: n.Resource.RawConfig.Copy(),
Resource: resource,
Output: &resourceConfig,
},
Expand Down Expand Up @@ -354,7 +354,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode {
},

&EvalInterpolate{
Config: n.Resource.RawConfig,
Config: n.Resource.RawConfig.Copy(),
Resource: resource,
Output: &resourceConfig,
},
Expand Down