From eab282f61782a2c0cd6772954200e02ab2bb6e27 Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Wed, 18 Feb 2026 18:51:43 +0000 Subject: [PATCH] test: Add `http` backend locking test (#38192) * test: Update TestHTTPBackend so it can be created with pre-existing lock info inside it. This will be used for tests that want to assert the lock info returned with lock errors. * test: Add a test asserting data returned in lock errors from the http backend. * chore: Replace use of `io/ioutil` in http backend * chore: Update changelog entry to link to the original PR https://github.com/hashicorp/terraform/pull/38144 * test: Improve test code comments --- .changes/v1.15/BUG FIXES-20260210-113930.yaml | 4 +- internal/backend/remote-state/http/client.go | 3 +- .../backend/remote-state/http/client_test.go | 54 +++++++++++++++++++ .../backend/remote-state/http/test_backend.go | 18 +++++++ 4 files changed, 76 insertions(+), 3 deletions(-) diff --git a/.changes/v1.15/BUG FIXES-20260210-113930.yaml b/.changes/v1.15/BUG FIXES-20260210-113930.yaml index 5e3428f3f5..d42670a8e5 100644 --- a/.changes/v1.15/BUG FIXES-20260210-113930.yaml +++ b/.changes/v1.15/BUG FIXES-20260210-113930.yaml @@ -1,3 +1,5 @@ kind: BUG FIXES -body: 'backend: Return conflicting lock info from HTTP backend instead of the lock that failed to be taken' +body: 'backend/http: Return conflicting lock info from HTTP backend instead of the lock that failed to be taken' time: 2026-02-10T11:39:30.00000-08:00 +custom: + Issue: "38144" \ No newline at end of file diff --git a/internal/backend/remote-state/http/client.go b/internal/backend/remote-state/http/client.go index 1236b018bd..d4ab321735 100644 --- a/internal/backend/remote-state/http/client.go +++ b/internal/backend/remote-state/http/client.go @@ -10,7 +10,6 @@ import ( "encoding/json" "fmt" "io" - "io/ioutil" "net/http" "net/url" @@ -102,7 +101,7 @@ func (c *httpClient) Lock(info *statemgr.LockInfo) (string, error) { return "", fmt.Errorf("HTTP remote state endpoint invalid auth") case http.StatusConflict, http.StatusLocked: defer resp.Body.Close() - body, err := ioutil.ReadAll(resp.Body) + body, err := io.ReadAll(resp.Body) if err != nil { return "", &statemgr.LockError{ Err: fmt.Errorf("HTTP remote state already locked, failed to read body"), diff --git a/internal/backend/remote-state/http/client_test.go b/internal/backend/remote-state/http/client_test.go index daf79de62f..cdd639596e 100644 --- a/internal/backend/remote-state/http/client_test.go +++ b/internal/backend/remote-state/http/client_test.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/go-retryablehttp" "github.com/hashicorp/terraform/internal/states/remote" + "github.com/hashicorp/terraform/internal/states/statemgr" ) func TestHTTPClient_impl(t *testing.T) { @@ -93,6 +94,59 @@ func TestHTTPClient(t *testing.T) { remote.TestClient(t, client) } +// Make assertions about the data returned in lock errors +func TestHTTPClient_lockErrors(t *testing.T) { + // Create a test HTTP backend that's already locked and contains + // data about the current lock. + testOperation := "test-setup-lock" + testWho := "i-am-the-one-who-locks" + handler := new(TestHTTPBackend) + handler.Locked = true + handler.LockInfo = &statemgr.LockInfo{ + Operation: testOperation, + Who: testWho, + } + ts := httptest.NewServer(http.HandlerFunc(handler.Handle)) + defer ts.Close() + + url, err := url.Parse(ts.URL) + if err != nil { + t.Fatalf("Parse: %s", err) + } + + // Test locking when the test server is set up to already be locked. + var locker statemgr.Locker = &httpClient{ + URL: url, + UpdateMethod: "PUT", + LockURL: url, + LockMethod: "LOCK", + UnlockURL: url, + UnlockMethod: "UNLOCK", + Client: retryablehttp.NewClient(), + } + + // Attempt to acquire a new lock with the data below + info := statemgr.NewLockInfo() + info.Operation = "can-i-get-a-lock?" + info.Who = "client-that-wants-the-lock" + _, err = locker.Lock(info) + + // Assert expected outcome: an error mentioning the pre-existing lock. + if err == nil { + t.Fatal("test client obtained lock while the server was locked by another client") + } + lockErr, ok := err.(*statemgr.LockError) + if !ok { + t.Errorf("expected a LockError, but was %t: %s", err, err) + } + if lockErr.Info.Operation != testOperation { + t.Errorf("expected lock info operation %q, but was %q", testOperation, lockErr.Info.Operation) + } + if lockErr.Info.Who != testWho { + t.Errorf("expected lock held by %q, but was %q", testWho, lockErr.Info.Who) + } +} + type TestBrokenHTTPBackend struct { lastRequestWasBroken bool handler *TestHTTPBackend diff --git a/internal/backend/remote-state/http/test_backend.go b/internal/backend/remote-state/http/test_backend.go index 8082c7271c..2a287aad3d 100644 --- a/internal/backend/remote-state/http/test_backend.go +++ b/internal/backend/remote-state/http/test_backend.go @@ -4,10 +4,13 @@ package http import ( "bytes" + "encoding/json" "fmt" "io" "net/http" "reflect" + + "github.com/hashicorp/terraform/internal/states/statemgr" ) type TestRequestHandleFunc func(w http.ResponseWriter, r *http.Request) @@ -16,6 +19,10 @@ type TestHTTPBackend struct { Data []byte Locked bool + // LockInfo is set by the calling test code and is not + // set when tests use the Lock method on an http backend. + LockInfo *statemgr.LockInfo + methodFuncs map[string]TestRequestHandleFunc methodCalls map[string]int } @@ -46,6 +53,17 @@ func (h *TestHTTPBackend) Handle(w http.ResponseWriter, r *http.Request) { case "LOCK": if h.Locked { w.WriteHeader(423) + if h.LockInfo != nil { + // Write lock info to response, but only if + // the test http backend server with lock info present. + jsonLockInfo, err := json.Marshal(h.LockInfo) + if err != nil { + w.WriteHeader(500) + w.Write([]byte("Failed to marshal lock info in test http backend server: " + err.Error())) + return + } + w.Write(jsonLockInfo) + } } else { h.Locked = true }