terraform: deep copy shadow arguments to avoid state modifications

The arguments passed into Apply, Refresh, Diff could be modified which
caused the shadow comparison later to cause errors. Also, the result
should be deep copied so that it isn't modified.
This commit is contained in:
Mitchell Hashimoto 2016-10-06 14:56:02 -07:00
parent 4de803622d
commit fdeb4656c9
No known key found for this signature in database
GPG Key ID: 744E147AA52F5B0A
4 changed files with 138 additions and 13 deletions

View File

@ -381,6 +381,16 @@ func (d *InstanceDiff) Empty() bool {
return !d.Destroy && !d.DestroyTainted && len(d.Attributes) == 0 return !d.Destroy && !d.DestroyTainted && len(d.Attributes) == 0
} }
// DeepCopy performs a deep copy of all parts of the InstanceDiff
func (d *InstanceDiff) DeepCopy() *InstanceDiff {
copy, err := copystructure.Config{Lock: true}.Copy(d)
if err != nil {
panic(err)
}
return copy.(*InstanceDiff)
}
func (d *InstanceDiff) GoString() string { func (d *InstanceDiff) GoString() string {
return fmt.Sprintf("*%#v", InstanceDiff{ return fmt.Sprintf("*%#v", InstanceDiff{
Attributes: d.Attributes, Attributes: d.Attributes,

View File

@ -104,7 +104,14 @@ func (f *shadowComponentFactory) CloseShadow() error {
} }
} }
// TODO: provisioners once they're done for _, n := range shared.provisionerKeys {
_, shadow, err := shared.ResourceProvisioner(n, n)
if err == nil && shadow != nil {
if err := shadow.CloseShadow(); err != nil {
result = multierror.Append(result, err)
}
}
}
// Mark ourselves as closed // Mark ourselves as closed
shared.closed = true shared.closed = true
@ -141,6 +148,15 @@ func (f *shadowComponentFactory) ShadowError() error {
} }
} }
for _, n := range shared.provisionerKeys {
_, shadow, err := shared.ResourceProvisioner(n, n)
if err == nil && shadow != nil {
if err := shadow.ShadowError(); err != nil {
result = multierror.Append(result, err)
}
}
}
return result return result
} }
@ -168,7 +184,7 @@ type shadowComponentFactoryProviderEntry struct {
type shadowComponentFactoryProvisionerEntry struct { type shadowComponentFactoryProvisionerEntry struct {
Real ResourceProvisioner Real ResourceProvisioner
Shadow ResourceProvisioner Shadow shadowResourceProvisioner
Err error Err error
} }
@ -211,7 +227,7 @@ func (f *shadowComponentFactoryShared) ResourceProvider(
} }
func (f *shadowComponentFactoryShared) ResourceProvisioner( func (f *shadowComponentFactoryShared) ResourceProvisioner(
n, uid string) (ResourceProvisioner, ResourceProvisioner, error) { n, uid string) (ResourceProvisioner, shadowResourceProvisioner, error) {
// Determine if we already have a value // Determine if we already have a value
raw, ok := f.provisioners.ValueOk(uid) raw, ok := f.provisioners.ValueOk(uid)
if !ok { if !ok {
@ -227,8 +243,7 @@ func (f *shadowComponentFactoryShared) ResourceProvisioner(
if p != nil { if p != nil {
// For now, just create a mock since we don't support provisioners yet // For now, just create a mock since we don't support provisioners yet
real := p real, shadow := newShadowResourceProvisioner(p)
shadow := new(MockResourceProvisioner)
entry.Real = real entry.Real = real
entry.Shadow = shadow entry.Shadow = shadow
} }

View File

@ -146,11 +146,15 @@ func (p *shadowResourceProviderReal) Apply(
info *InstanceInfo, info *InstanceInfo,
state *InstanceState, state *InstanceState,
diff *InstanceDiff) (*InstanceState, error) { diff *InstanceDiff) (*InstanceState, error) {
// Thse have to be copied before the call since call can modify
stateCopy := state.DeepCopy()
diffCopy := diff.DeepCopy()
result, err := p.ResourceProvider.Apply(info, state, diff) result, err := p.ResourceProvider.Apply(info, state, diff)
p.Shared.Apply.SetValue(info.HumanId(), &shadowResourceProviderApply{ p.Shared.Apply.SetValue(info.HumanId(), &shadowResourceProviderApply{
State: state, State: stateCopy,
Diff: diff, Diff: diffCopy,
Result: result, Result: result.DeepCopy(),
ResultErr: err, ResultErr: err,
}) })
@ -161,11 +165,14 @@ func (p *shadowResourceProviderReal) Diff(
info *InstanceInfo, info *InstanceInfo,
state *InstanceState, state *InstanceState,
desired *ResourceConfig) (*InstanceDiff, error) { desired *ResourceConfig) (*InstanceDiff, error) {
// Thse have to be copied before the call since call can modify
stateCopy := state.DeepCopy()
result, err := p.ResourceProvider.Diff(info, state, desired) result, err := p.ResourceProvider.Diff(info, state, desired)
p.Shared.Diff.SetValue(info.HumanId(), &shadowResourceProviderDiff{ p.Shared.Diff.SetValue(info.HumanId(), &shadowResourceProviderDiff{
State: state, State: stateCopy,
Desired: desired, Desired: desired,
Result: result, Result: result.DeepCopy(),
ResultErr: err, ResultErr: err,
}) })
@ -175,10 +182,13 @@ func (p *shadowResourceProviderReal) Diff(
func (p *shadowResourceProviderReal) Refresh( func (p *shadowResourceProviderReal) Refresh(
info *InstanceInfo, info *InstanceInfo,
state *InstanceState) (*InstanceState, error) { state *InstanceState) (*InstanceState, error) {
// Thse have to be copied before the call since call can modify
stateCopy := state.DeepCopy()
result, err := p.ResourceProvider.Refresh(info, state) result, err := p.ResourceProvider.Refresh(info, state)
p.Shared.Refresh.SetValue(info.HumanId(), &shadowResourceProviderRefresh{ p.Shared.Refresh.SetValue(info.HumanId(), &shadowResourceProviderRefresh{
State: state, State: stateCopy,
Result: result, Result: result.DeepCopy(),
ResultErr: err, ResultErr: err,
}) })
@ -430,7 +440,7 @@ func (p *shadowResourceProviderShadow) Apply(
if !state.Equal(result.State) { if !state.Equal(result.State) {
p.ErrorLock.Lock() p.ErrorLock.Lock()
p.Error = multierror.Append(p.Error, fmt.Errorf( p.Error = multierror.Append(p.Error, fmt.Errorf(
"State had unequal states (real, then shadow):\n\n%#v\n\n%#v", "Apply: state had unequal states (real, then shadow):\n\n%#v\n\n%#v",
result.State, state)) result.State, state))
p.ErrorLock.Unlock() p.ErrorLock.Unlock()
} }

View File

@ -324,6 +324,96 @@ func TestShadowResourceProviderApply(t *testing.T) {
} }
} }
func TestShadowResourceProviderApply_modifyDiff(t *testing.T) {
mock := new(MockResourceProvider)
real, shadow := newShadowResourceProvider(mock)
// Test values
info := &InstanceInfo{Id: "foo"}
state := &InstanceState{ID: "foo"}
diff := &InstanceDiff{}
mockResult := &InstanceState{ID: "foo"}
// Configure the mock
mock.ApplyFn = func(
info *InstanceInfo,
s *InstanceState, d *InstanceDiff) (*InstanceState, error) {
d.Destroy = true
return s, nil
}
// Call the real func
realResult, realErr := real.Apply(info, state.DeepCopy(), diff.DeepCopy())
if !realResult.Equal(mockResult) {
t.Fatalf("bad: %#v", realResult)
}
if realErr != nil {
t.Fatalf("bad: %#v", realErr)
}
// Verify the shadow returned the same values
result, err := shadow.Apply(info, state.DeepCopy(), diff.DeepCopy())
if !result.Equal(mockResult) {
t.Fatalf("bad: %#v", result)
}
if err != nil {
t.Fatalf("bad: %#v", err)
}
// Verify we have no errors
if err := shadow.CloseShadow(); err != nil {
t.Fatalf("bad: %s", err)
}
if err := shadow.ShadowError(); err != nil {
t.Fatalf("bad: %s", err)
}
}
func TestShadowResourceProviderApply_modifyState(t *testing.T) {
mock := new(MockResourceProvider)
real, shadow := newShadowResourceProvider(mock)
// Test values
info := &InstanceInfo{Id: "foo"}
state := &InstanceState{ID: ""}
diff := &InstanceDiff{}
mockResult := &InstanceState{ID: "foo"}
// Configure the mock
mock.ApplyFn = func(
info *InstanceInfo,
s *InstanceState, d *InstanceDiff) (*InstanceState, error) {
s.ID = "foo"
return s, nil
}
// Call the real func
realResult, realErr := real.Apply(info, state.DeepCopy(), diff)
if !realResult.Equal(mockResult) {
t.Fatalf("bad: %#v", realResult)
}
if realErr != nil {
t.Fatalf("bad: %#v", realErr)
}
// Verify the shadow returned the same values
result, err := shadow.Apply(info, state.DeepCopy(), diff)
if !result.Equal(mockResult) {
t.Fatalf("bad: %#v", result)
}
if err != nil {
t.Fatalf("bad: %#v", err)
}
// Verify we have no errors
if err := shadow.CloseShadow(); err != nil {
t.Fatalf("bad: %s", err)
}
if err := shadow.ShadowError(); err != nil {
t.Fatalf("bad: %s", err)
}
}
func TestShadowResourceProviderDiff(t *testing.T) { func TestShadowResourceProviderDiff(t *testing.T) {
mock := new(MockResourceProvider) mock := new(MockResourceProvider)
real, shadow := newShadowResourceProvider(mock) real, shadow := newShadowResourceProvider(mock)