diff --git a/backend/remote-state/consul/backend.go b/backend/remote-state/consul/backend.go index 979dc368c..79aeea402 100644 --- a/backend/remote-state/consul/backend.go +++ b/backend/remote-state/consul/backend.go @@ -60,6 +60,13 @@ func New() backend.Backend { Description: "Compress the state data using gzip", Default: false, }, + + "lock": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + Description: "Lock state access", + Default: true, + }, }, } @@ -71,13 +78,18 @@ func New() backend.Backend { type Backend struct { *schema.Backend + // The fields below are set from configure configData *schema.ResourceData + lock bool } func (b *Backend) configure(ctx context.Context) error { // Grab the resource data b.configData = schema.FromContextBackendConfig(ctx) + // Store the lock information + b.lock = b.configData.Get("lock").(bool) + // Initialize a client to test config _, err := b.clientRaw() return err diff --git a/backend/remote-state/consul/backend_state.go b/backend/remote-state/consul/backend_state.go index 0339dd79a..4c0851871 100644 --- a/backend/remote-state/consul/backend_state.go +++ b/backend/remote-state/consul/backend_state.go @@ -89,7 +89,7 @@ func (b *Backend) State(name string) (state.State, error) { gzip := b.configData.Get("gzip").(bool) // Build the state client - stateMgr := &remote.State{ + var stateMgr state.State = &remote.State{ Client: &RemoteClient{ Client: client, Path: path, @@ -97,19 +97,27 @@ func (b *Backend) State(name string) (state.State, error) { }, } + // If we're not locking, disable it + if !b.lock { + stateMgr = &state.LockDisabled{Inner: stateMgr} + } + + // Get the locker, which we know always exists + stateMgrLocker := stateMgr.(state.Locker) + // Grab a lock, we use this to write an empty state if one doesn't // exist already. We have to write an empty state as a sentinel value // so States() knows it exists. lockInfo := state.NewLockInfo() lockInfo.Operation = "init" - lockId, err := stateMgr.Lock(lockInfo) + lockId, err := stateMgrLocker.Lock(lockInfo) if err != nil { return nil, fmt.Errorf("failed to lock state in Consul: %s", err) } // Local helper function so we can call it multiple places lockUnlock := func(parent error) error { - if err := stateMgr.Unlock(lockId); err != nil { + if err := stateMgrLocker.Unlock(lockId); err != nil { return fmt.Errorf(strings.TrimSpace(errStateUnlock), lockId, err) } diff --git a/backend/remote-state/consul/backend_test.go b/backend/remote-state/consul/backend_test.go index 14133d44f..64f3a0eb3 100644 --- a/backend/remote-state/consul/backend_test.go +++ b/backend/remote-state/consul/backend_test.go @@ -38,14 +38,44 @@ func TestBackend(t *testing.T) { srv := newConsulTestServer(t) defer srv.Stop() - // Get the backend - b := backend.TestBackendConfig(t, New(), map[string]interface{}{ + path := fmt.Sprintf("tf-unit/%s", time.Now().String()) + + // Get the backend. We need two to test locking. + b1 := backend.TestBackendConfig(t, New(), map[string]interface{}{ "address": srv.HTTPAddr, - "path": fmt.Sprintf("tf-unit/%s", time.Now().String()), + "path": path, + }) + + b2 := backend.TestBackendConfig(t, New(), map[string]interface{}{ + "address": srv.HTTPAddr, + "path": path, }) // Test - backend.TestBackend(t, b) + backend.TestBackend(t, b1, b2) +} + +func TestBackend_lockDisabled(t *testing.T) { + srv := newConsulTestServer(t) + defer srv.Stop() + + path := fmt.Sprintf("tf-unit/%s", time.Now().String()) + + // Get the backend. We need two to test locking. + b1 := backend.TestBackendConfig(t, New(), map[string]interface{}{ + "address": srv.HTTPAddr, + "path": path, + "lock": false, + }) + + b2 := backend.TestBackendConfig(t, New(), map[string]interface{}{ + "address": srv.HTTPAddr, + "path": path + "different", // Diff so locking test would fail if it was locking + "lock": false, + }) + + // Test + backend.TestBackend(t, b1, b2) } func TestBackend_gzip(t *testing.T) { diff --git a/backend/testing.go b/backend/testing.go index 5298131cf..7039de5ad 100644 --- a/backend/testing.go +++ b/backend/testing.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" ) @@ -40,8 +41,15 @@ func TestBackendConfig(t *testing.T, b Backend, c map[string]interface{}) Backen // assumed to already be configured. This will test state functionality. // If the backend reports it doesn't support multi-state by returning the // error ErrNamedStatesNotSupported, then it will not test that. -func TestBackend(t *testing.T, b Backend) { - testBackendStates(t, b) +// +// If you want to test locking, two backends must be given. If b2 is nil, +// then state lockign won't be tested. +func TestBackend(t *testing.T, b1, b2 Backend) { + testBackendStates(t, b1) + + if b2 != nil { + testBackendStateLock(t, b1, b2) + } } func testBackendStates(t *testing.T, b Backend) { @@ -138,3 +146,77 @@ func testBackendStates(t *testing.T, b Backend) { } } } + +func testBackendStateLock(t *testing.T, b1, b2 Backend) { + // Get the default state for each + b1StateMgr, err := b1.State(DefaultStateName) + if err != nil { + t.Fatalf("error: %s", err) + } + if err := b1StateMgr.RefreshState(); err != nil { + t.Fatalf("bad: %s", err) + } + + // Fast exit if this doesn't support locking at all + if _, ok := b1StateMgr.(state.Locker); !ok { + t.Logf("TestBackend: backend %T doesn't support state locking, not testing", b1) + return + } + + t.Logf("TestBackend: testing state locking for %T", b1) + + b2StateMgr, err := b2.State(DefaultStateName) + if err != nil { + t.Fatalf("error: %s", err) + } + if err := b2StateMgr.RefreshState(); err != nil { + t.Fatalf("bad: %s", err) + } + + // Reassign so its obvious whats happening + lockerA := b1StateMgr.(state.Locker) + lockerB := b2StateMgr.(state.Locker) + + infoA := state.NewLockInfo() + infoA.Operation = "test" + infoA.Who = "clientA" + + infoB := state.NewLockInfo() + infoB.Operation = "test" + infoB.Who = "clientB" + + lockIDA, err := lockerA.Lock(infoA) + if err != nil { + t.Fatal("unable to get initial lock:", err) + } + + // If the lock ID is blank, assume locking is disabled + if lockIDA == "" { + t.Logf("TestBackend: %T: empty string returned for lock, assuming disabled", b1) + return + } + + _, err = lockerB.Lock(infoB) + if err == nil { + lockerA.Unlock(lockIDA) + t.Fatal("client B obtained lock while held by client A") + } + + if err := lockerA.Unlock(lockIDA); err != nil { + t.Fatal("error unlocking client A", err) + } + + lockIDB, err := lockerB.Lock(infoB) + if err != nil { + t.Fatal("unable to obtain lock from client B") + } + + if lockIDB == lockIDA { + t.Fatalf("duplicate lock IDs: %q", lockIDB) + } + + if err = lockerB.Unlock(lockIDB); err != nil { + t.Fatal("error unlocking client B:", err) + } + +} diff --git a/state/lock.go b/state/lock.go new file mode 100644 index 000000000..b3a03b3ef --- /dev/null +++ b/state/lock.go @@ -0,0 +1,38 @@ +package state + +import ( + "github.com/hashicorp/terraform/terraform" +) + +// LockDisabled implements State and Locker but disables state locking. +// If State doesn't support locking, this is a no-op. This is useful for +// easily disabling locking of an existing state or for tests. +type LockDisabled struct { + // We can't embed State directly since Go dislikes that a field is + // State and State interface has a method State + Inner State +} + +func (s *LockDisabled) State() *terraform.State { + return s.Inner.State() +} + +func (s *LockDisabled) WriteState(v *terraform.State) error { + return s.Inner.WriteState(v) +} + +func (s *LockDisabled) RefreshState() error { + return s.Inner.RefreshState() +} + +func (s *LockDisabled) PersistState() error { + return s.Inner.PersistState() +} + +func (s *LockDisabled) Lock(info *LockInfo) (string, error) { + return "", nil +} + +func (s *LockDisabled) Unlock(id string) error { + return nil +} diff --git a/state/lock_test.go b/state/lock_test.go new file mode 100644 index 000000000..d7246ac9d --- /dev/null +++ b/state/lock_test.go @@ -0,0 +1,10 @@ +package state + +import ( + "testing" +) + +func TestLockDisabled_impl(t *testing.T) { + var _ State = new(LockDisabled) + var _ Locker = new(LockDisabled) +} diff --git a/website/source/docs/backends/types/consul.html.md b/website/source/docs/backends/types/consul.html.md index 24d45fe14..f03e6dcc0 100644 --- a/website/source/docs/backends/types/consul.html.md +++ b/website/source/docs/backends/types/consul.html.md @@ -54,3 +54,4 @@ The following configuration options / environment variables are supported: * `http_auth` / `CONSUL_HTTP_AUTH` - (Optional) HTTP Basic Authentication credentials to be used when communicating with Consul, in the format of either `user` or `user:pass`. * `gzip` - (Optional) `true` to compress the state data using gzip, or `false` (the default) to leave it uncompressed. + * `lock` - (Optional) `false` to disable locking. This defaults to true, but will require session permissions with Consul to perform locking.