diff --git a/cmd/tofu/main_test.go b/cmd/tofu/main_test.go index ac2b990871..45cac2292b 100644 --- a/cmd/tofu/main_test.go +++ b/cmd/tofu/main_test.go @@ -337,7 +337,6 @@ func TestMkConfigDir_new(t *testing.T) { func TestMkConfigDir_exists(t *testing.T) { tmpConfigDir := filepath.Join(t.TempDir(), ".terraform.d") - os.Mkdir(tmpConfigDir, os.ModePerm) err := mkConfigDir(tmpConfigDir) if err != nil { diff --git a/internal/backend/local/hook_state_test.go b/internal/backend/local/hook_state_test.go index 8144fbe478..3ae87332a9 100644 --- a/internal/backend/local/hook_state_test.go +++ b/internal/backend/local/hook_state_test.go @@ -66,14 +66,20 @@ func TestStateHookStopping(t *testing.T) { // We'll now force lastPersist to be long enough ago that persisting // should be due on the next call. hook.intermediatePersist.LastPersist = time.Now().Add(-5 * time.Hour) - hook.PostStateUpdate(s) + _, err = hook.PostStateUpdate(s) + if err != nil { + t.Fatalf("unexpected error from PostStateUpdate: %s", err) + } if is.Written == nil || !is.Written.Equal(s) { t.Fatalf("mismatching state written") } if is.Persisted == nil || !is.Persisted.Equal(s) { t.Fatalf("mismatching state persisted") } - hook.PostStateUpdate(s) + _, err = hook.PostStateUpdate(s) + if err != nil { + t.Fatalf("unexpected error from PostStateUpdate: %s", err) + } if is.Written == nil || !is.Written.Equal(s) { t.Fatalf("mismatching state written") } @@ -108,12 +114,18 @@ func TestStateHookStopping(t *testing.T) { } is.Persisted = nil - hook.PostStateUpdate(s) + _, err = hook.PostStateUpdate(s) + if err != nil { + t.Fatalf("unexpected error from PostStateUpdate: %s", err) + } if is.Persisted == nil || !is.Persisted.Equal(s) { t.Fatalf("mismatching state persisted") } is.Persisted = nil - hook.PostStateUpdate(s) + _, err = hook.PostStateUpdate(s) + if err != nil { + t.Fatalf("unexpected error from PostStateUpdate: %s", err) + } if is.Persisted == nil || !is.Persisted.Equal(s) { t.Fatalf("mismatching state persisted") } @@ -165,14 +177,20 @@ func TestStateHookCustomPersistRule(t *testing.T) { // We'll now force lastPersist to be long enough ago that persisting // should be due on the next call. hook.intermediatePersist.LastPersist = time.Now().Add(-5 * time.Hour) - hook.PostStateUpdate(s) + _, err = hook.PostStateUpdate(s) + if err != nil { + t.Fatalf("unexpected error from PostStateUpdate: %s", err) + } if is.Written == nil || !is.Written.Equal(s) { t.Fatalf("mismatching state written") } if is.Persisted != nil { t.Fatalf("has a persisted state, but shouldn't") } - hook.PostStateUpdate(s) + _, err = hook.PostStateUpdate(s) + if err != nil { + t.Fatalf("unexpected error from PostStateUpdate: %s", err) + } if is.Written == nil || !is.Written.Equal(s) { t.Fatalf("mismatching state written") } @@ -212,12 +230,18 @@ func TestStateHookCustomPersistRule(t *testing.T) { } is.Persisted = nil - hook.PostStateUpdate(s) + _, err = hook.PostStateUpdate(s) + if err != nil { + t.Fatalf("unexpected error from PostStateUpdate: %s", err) + } if is.Persisted == nil || !is.Persisted.Equal(s) { t.Fatalf("mismatching state persisted") } is.Persisted = nil - hook.PostStateUpdate(s) + _, err = hook.PostStateUpdate(s) + if err != nil { + t.Fatalf("unexpected error from PostStateUpdate: %s", err) + } if is.Persisted == nil || !is.Persisted.Equal(s) { t.Fatalf("mismatching state persisted") } diff --git a/internal/backend/remote-state/azure/arm_client.go b/internal/backend/remote-state/azure/arm_client.go index cbce664a97..0217531735 100644 --- a/internal/backend/remote-state/azure/arm_client.go +++ b/internal/backend/remote-state/azure/arm_client.go @@ -29,8 +29,6 @@ type ArmClient struct { // These Clients are only initialized if an Access Key isn't provided groupsClient *resources.GroupsClient storageAccountsClient *armStorage.AccountsClient - containersClient *containers.Client - blobsClient *blobs.Client // azureAdStorageAuth is only here if we're using AzureAD Authentication but is an Authorizer for Storage azureAdStorageAuth *autorest.Authorizer diff --git a/internal/backend/remote-state/azure/backend_state.go b/internal/backend/remote-state/azure/backend_state.go index f19ef77c81..10031dda8d 100644 --- a/internal/backend/remote-state/azure/backend_state.go +++ b/internal/backend/remote-state/azure/backend_state.go @@ -142,10 +142,6 @@ func (b *Backend) StateMgr(ctx context.Context, name string) (statemgr.Full, err return stateMgr, nil } -func (b *Backend) client() *RemoteClient { - return &RemoteClient{} -} - func (b *Backend) path(name string) string { if name == backend.DefaultStateName { return b.keyName diff --git a/internal/backend/remote-state/azure/client.go b/internal/backend/remote-state/azure/client.go index 074cc4f484..1f5baa9928 100644 --- a/internal/backend/remote-state/azure/client.go +++ b/internal/backend/remote-state/azure/client.go @@ -22,7 +22,6 @@ import ( ) const ( - leaseHeader = "x-ms-lease-id" // Must be lower case lockInfoMetaKey = "terraformlockid" ) diff --git a/internal/backend/remote-state/azure/helpers_test.go b/internal/backend/remote-state/azure/helpers_test.go index ab6efc6ad0..649d1e4af5 100644 --- a/internal/backend/remote-state/azure/helpers_test.go +++ b/internal/backend/remote-state/azure/helpers_test.go @@ -44,15 +44,6 @@ func testAccAzureBackendRunningInAzure(t *testing.T) { } } -// these kind of tests can only run when within GitHub Actions (e.g. OIDC) -func testAccAzureBackendRunningInGitHubActions(t *testing.T) { - testAccAzureBackend(t) - - if os.Getenv("TF_RUNNING_IN_GITHUB_ACTIONS") == "" { - t.Skip("Skipping test since not running in GitHub Actions") - } -} - func buildTestClient(t *testing.T, res resourceNames) *ArmClient { subscriptionID := os.Getenv("ARM_SUBSCRIPTION_ID") tenantID := os.Getenv("ARM_TENANT_ID") diff --git a/internal/backend/remote-state/consul/client.go b/internal/backend/remote-state/consul/client.go index d1998b72a5..86ca2aa0bd 100644 --- a/internal/backend/remote-state/consul/client.go +++ b/internal/backend/remote-state/consul/client.go @@ -181,7 +181,7 @@ func (c *RemoteClient) Put(data []byte) error { // the user. We may end up with dangling chunks but there is no way // to be sure we won't. path := strings.TrimRight(c.Path, "/") + fmt.Sprintf("/tfstate.%s/", hash) - kv.DeleteTree(path, nil) + _, _ = kv.DeleteTree(path, nil) } } @@ -307,11 +307,17 @@ func (c *RemoteClient) Delete() error { } _, err = kv.Delete(c.Path, nil) + if err != nil { + return err + } // If there were chunks we need to remove them if chunked { path := strings.TrimRight(c.Path, "/") + fmt.Sprintf("/tfstate.%s/", hash) - kv.DeleteTree(path, nil) + _, err = kv.DeleteTree(path, nil) + if err != nil { + return err + } } return err @@ -539,7 +545,8 @@ func (c *RemoteClient) createSession() (string, error) { log.Println("[INFO] created consul lock session", id) // keep the session renewed - go session.RenewPeriodic(lockSessionTTL, id, nil, ctx.Done()) + // there's not really any good way of propagating errors from this function, so we ignore them + go session.RenewPeriodic(lockSessionTTL, id, nil, ctx.Done()) //nolint:errcheck return id, nil } @@ -574,8 +581,14 @@ func (c *RemoteClient) unlock(id string) error { } // We ignore the errors that may happen during cleanup kv := c.Client.KV() - kv.Delete(c.lockPath()+lockSuffix, nil) - kv.Delete(c.lockPath()+lockInfoSuffix, nil) + _, err = kv.Delete(c.lockPath()+lockSuffix, nil) + if err != nil { + log.Printf("[ERROR] could not delete lock @ %s: %s\n", c.lockPath()+lockSuffix, err) + } + _, err = kv.Delete(c.lockPath()+lockInfoSuffix, nil) + if err != nil { + log.Printf("[ERROR] could not delete lock info @ %s: %s\n", c.lockPath()+lockInfoSuffix, err) + } return nil } @@ -618,7 +631,10 @@ func (c *RemoteClient) unlock(id string) error { // This is only cleanup, and will fail if the lock was immediately taken by // another client, so we don't report an error to the user here. - c.consulLock.Destroy() + err := c.consulLock.Destroy() + if err != nil { + log.Printf("[ERROR] could not destroy consul lock: %s\n", err) + } return errs } @@ -644,7 +660,10 @@ func uncompressState(data []byte) ([]byte, error) { if err != nil { return nil, err } - b.ReadFrom(gz) + _, err = b.ReadFrom(gz) + if err != nil { + return nil, err + } if err := gz.Close(); err != nil { return nil, err } diff --git a/internal/backend/remote-state/consul/client_test.go b/internal/backend/remote-state/consul/client_test.go index 40b0bb1b76..f5c6227a9e 100644 --- a/internal/backend/remote-state/consul/client_test.go +++ b/internal/backend/remote-state/consul/client_test.go @@ -444,7 +444,7 @@ func TestConsul_lostLockConnection(t *testing.T) { for i := 0; i < 3; i++ { dialed := conns.dialedDone() // kill any open connections - conns.Kill() + conns.Kill(t) // wait for a new connection to be dialed, and kill it again <-dialed } @@ -493,12 +493,15 @@ func (u *unreliableConns) dialedDone() chan struct{} { // Kill these with a deadline, just to make sure we don't end up with any EOFs // that get ignored. -func (u *unreliableConns) Kill() { +func (u *unreliableConns) Kill(t *testing.T) { u.Lock() defer u.Unlock() for _, conn := range u.conns { - conn.(*net.TCPConn).SetDeadline(time.Now()) + err := conn.(*net.TCPConn).SetDeadline(time.Now()) + if err != nil { + t.Fatal("failed to kill connection:", err) + } } u.conns = nil } diff --git a/internal/backend/remote-state/cos/client.go b/internal/backend/remote-state/cos/client.go index 19b04688c9..e96f37508b 100644 --- a/internal/backend/remote-state/cos/client.go +++ b/internal/backend/remote-state/cos/client.go @@ -10,6 +10,7 @@ import ( "context" "crypto/md5" "encoding/json" + "errors" "fmt" "io" "log" @@ -84,30 +85,39 @@ func (c *remoteClient) Lock(info *statemgr.LockInfo) (string, error) { if err != nil { return "", c.lockError(err) } - defer c.cosUnlock(c.bucket, c.lockFile) + // Local helper function so we can call it multiple places + lockUnlock := func(parent error) error { + if err := c.cosUnlock(c.bucket, c.lockFile); err != nil { + return errors.Join( + fmt.Errorf("error unlocking cos state: %w", c.lockError(err)), + parent, + ) + } + return parent + } exists, _, _, err := c.getObject(c.lockFile) if err != nil { - return "", c.lockError(err) + return "", lockUnlock(c.lockError(err)) } if exists { - return "", c.lockError(fmt.Errorf("lock file %s exists", c.lockFile)) + return "", lockUnlock(c.lockError(fmt.Errorf("lock file %s exists", c.lockFile))) } info.Path = c.lockFile data, err := json.Marshal(info) if err != nil { - return "", c.lockError(err) + return "", lockUnlock(c.lockError(err)) } check := fmt.Sprintf("%x", md5.Sum(data)) err = c.putObject(c.lockFile, data) if err != nil { - return "", c.lockError(err) + return "", lockUnlock(c.lockError(err)) } - return check, nil + return check, lockUnlock(nil) } // Unlock unlock remote state file @@ -330,7 +340,10 @@ func (c *remoteClient) deleteBucket(recursive bool) error { return fmt.Errorf("failed to empty bucket %v: %w", c.bucket, err) } for _, v := range obs { - c.deleteObject(v.Key) + err := c.deleteObject(v.Key) + if err != nil { + return fmt.Errorf("failed to delete object with key %s: %w", v.Key, err) + } } } diff --git a/internal/backend/remote-state/http/client_test.go b/internal/backend/remote-state/http/client_test.go index 5f50001718..60d4b93983 100644 --- a/internal/backend/remote-state/http/client_test.go +++ b/internal/backend/remote-state/http/client_test.go @@ -137,11 +137,16 @@ type testHTTPHandler struct { func (h *testHTTPHandler) Handle(w http.ResponseWriter, r *http.Request) { switch r.Method { case "GET": - w.Write(h.Data) + if _, err := w.Write(h.Data); err != nil { + w.WriteHeader(500) + return + } + w.WriteHeader(200) case "PUT": buf := new(bytes.Buffer) if _, err := io.Copy(buf, r.Body); err != nil { w.WriteHeader(500) + return } w.WriteHeader(201) h.Data = buf.Bytes() @@ -149,6 +154,7 @@ func (h *testHTTPHandler) Handle(w http.ResponseWriter, r *http.Request) { buf := new(bytes.Buffer) if _, err := io.Copy(buf, r.Body); err != nil { w.WriteHeader(500) + return } h.Data = buf.Bytes() case "LOCK": @@ -164,7 +170,8 @@ func (h *testHTTPHandler) Handle(w http.ResponseWriter, r *http.Request) { w.WriteHeader(200) default: w.WriteHeader(500) - w.Write([]byte(fmt.Sprintf("Unknown method: %s", r.Method))) + // this is already returning a 500, no need for further error checking + _, _ = fmt.Fprintf(w, "Unknown method: %s", r.Method) } } @@ -172,7 +179,11 @@ func (h *testHTTPHandler) Handle(w http.ResponseWriter, r *http.Request) { func (h *testHTTPHandler) HandleWebDAV(w http.ResponseWriter, r *http.Request) { switch r.Method { case "GET": - w.Write(h.Data) + if _, err := w.Write(h.Data); err != nil { + w.WriteHeader(500) + return + } + w.WriteHeader(200) case "PUT": buf := new(bytes.Buffer) if _, err := io.Copy(buf, r.Body); err != nil { @@ -190,7 +201,8 @@ func (h *testHTTPHandler) HandleWebDAV(w http.ResponseWriter, r *http.Request) { w.WriteHeader(200) default: w.WriteHeader(500) - w.Write([]byte(fmt.Sprintf("Unknown method: %s", r.Method))) + // this is already returning a 500, no need for further error checking + _, _ = fmt.Fprintf(w, "Unknown method: %s", r.Method) } } diff --git a/internal/backend/remote-state/inmem/backend.go b/internal/backend/remote-state/inmem/backend.go index 956a8b93a4..f5553cc857 100644 --- a/internal/backend/remote-state/inmem/backend.go +++ b/internal/backend/remote-state/inmem/backend.go @@ -80,13 +80,16 @@ func (b *Backend) configure(ctx context.Context) error { // set the default client lock info per the test config data := schema.FromContextBackendConfig(ctx) - if v, ok := data.GetOk("lock_id"); ok && v.(string) != "" { + _, hasDefaultLock := locks.m[backend.DefaultStateName] + if v, ok := data.GetOk("lock_id"); ok && v.(string) != "" && !hasDefaultLock { info := statemgr.NewLockInfo() info.ID = v.(string) info.Operation = "test" info.Info = "test config" - locks.lock(backend.DefaultStateName, info) + if _, err := locks.lock(backend.DefaultStateName, info); err != nil { + return err + } } return nil @@ -140,17 +143,34 @@ func (b *Backend) StateMgr(_ context.Context, name string) (statemgr.Full, error if err != nil { return nil, fmt.Errorf("failed to lock inmem state: %w", err) } - defer s.Unlock(lockID) + + // Local helper function so we can call it multiple places + lockUnlock := func(parent error) error { + if err := s.Unlock(lockID); err != nil { + return errors.Join( + fmt.Errorf("error unlocking inmem state: %w", err), + parent, + ) + } + return parent + } // If we have no state, we have to create an empty state if v := s.State(); v == nil { if err := s.WriteState(statespkg.NewState()); err != nil { + err = lockUnlock(err) return nil, err } if err := s.PersistState(nil); err != nil { + err = lockUnlock(err) return nil, err } } + + // Unlock, the state should now be initialized + if err := lockUnlock(nil); err != nil { + return nil, err + } } return s, nil diff --git a/internal/backend/remote-state/kubernetes/client.go b/internal/backend/remote-state/kubernetes/client.go index 3ca58c7b0e..ccc9d2196e 100644 --- a/internal/backend/remote-state/kubernetes/client.go +++ b/internal/backend/remote-state/kubernetes/client.go @@ -371,7 +371,10 @@ func uncompressState(data string) ([]byte, error) { if err != nil { return nil, err } - b.ReadFrom(gz) + _, err = b.ReadFrom(gz) + if err != nil { + return nil, err + } if err := gz.Close(); err != nil { return nil, err } diff --git a/internal/backend/remote-state/oss/backend.go b/internal/backend/remote-state/oss/backend.go index 52af9165ea..51cfbf14fc 100644 --- a/internal/backend/remote-state/oss/backend.go +++ b/internal/backend/remote-state/oss/backend.go @@ -472,7 +472,10 @@ func getAssumeRoleAK(accessKey, secretKey, stsToken, region, roleArn, sessionNam return "", "", "", err } if stsEndpoint != "" { - endpoints.AddEndpointMapping(region, "STS", stsEndpoint) + err = endpoints.AddEndpointMapping(region, "STS", stsEndpoint) + if err != nil { + return "", "", "", err + } } response, err := client.AssumeRole(request) if err != nil { diff --git a/internal/backend/remote/backend.go b/internal/backend/remote/backend.go index 6370de5343..bc6e0b5f6d 100644 --- a/internal/backend/remote/backend.go +++ b/internal/backend/remote/backend.go @@ -983,15 +983,6 @@ func generalError(msg string, err error) error { } } -func checkConstraintsWarning(err error) tfdiags.Diagnostic { - return tfdiags.Sourceless( - tfdiags.Warning, - fmt.Sprintf("Failed to check version constraints: %v", err), - "Checking version constraints is considered optional, but this is an"+ - "unexpected error which should be reported.", - ) -} - // The newline in this error is to make it look good in the CLI! const initialRetryError = ` [reset][yellow]There was an error connecting to the remote backend. Please do not exit diff --git a/internal/backend/remote/backend_plan.go b/internal/backend/remote/backend_plan.go index 6aa1f5e5d8..c9559e7d80 100644 --- a/internal/backend/remote/backend_plan.go +++ b/internal/backend/remote/backend_plan.go @@ -373,7 +373,9 @@ in order to capture the filesystem context the remote workspace expects: log.Printf("[ERROR] error searching process ID: %v", err) return } - p.Signal(syscall.SIGINT) + if err := p.Signal(syscall.SIGINT); err != nil { + log.Printf("[ERROR] error sending interrupt signal: %v", err) + } } } }() diff --git a/internal/backend/remote/backend_state_test.go b/internal/backend/remote/backend_state_test.go index 050c581224..4a11eb7935 100644 --- a/internal/backend/remote/backend_state_test.go +++ b/internal/backend/remote/backend_state_test.go @@ -53,7 +53,10 @@ func TestRemoteClient_Put_withRunID(t *testing.T) { // Create a new empty state. sf := statefile.New(states.NewState(), "", 0) var buf bytes.Buffer - statefile.Write(sf, &buf, encryption.StateEncryptionDisabled()) + err := statefile.Write(sf, &buf, encryption.StateEncryptionDisabled()) + if err != nil { + t.Fatalf("error writing to statefile, got %v", err) + } // Store the new state to verify (this will be done // by the mock that is used) that the run ID is set. diff --git a/internal/backend/remote/testing.go b/internal/backend/remote/testing.go index c20f7f0612..e48dac48f9 100644 --- a/internal/backend/remote/testing.go +++ b/internal/backend/remote/testing.go @@ -216,22 +216,28 @@ func testServer(t *testing.T) *httptest.Server { // Respond to service discovery calls. mux.HandleFunc("/well-known/terraform.json", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - io.WriteString(w, `{ + _, err := io.WriteString(w, `{ "state.v2": "/api/v2/", "tfe.v2.1": "/api/v2/", "versions.v1": "/v1/versions/" }`) + if err != nil { + w.WriteHeader(500) + } }) // Respond to service version constraints calls. mux.HandleFunc("/v1/versions/", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - io.WriteString(w, fmt.Sprintf(`{ + _, err := io.WriteString(w, fmt.Sprintf(`{ "service": "%s", "product": "terraform", "minimum": "0.1.0", "maximum": "10.0.0" }`, path.Base(r.URL.Path))) + if err != nil { + w.WriteHeader(500) + } }) // Respond to pings to get the API version header. @@ -243,7 +249,7 @@ func testServer(t *testing.T) *httptest.Server { // Respond to the initial query to read the hashicorp org entitlements. mux.HandleFunc("/api/v2/organizations/hashicorp/entitlement-set", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/vnd.api+json") - io.WriteString(w, `{ + _, err := io.WriteString(w, `{ "data": { "id": "org-GExadygjSbKP8hsY", "type": "entitlement-sets", @@ -257,12 +263,15 @@ func testServer(t *testing.T) *httptest.Server { } } }`) + if err != nil { + w.WriteHeader(500) + } }) // Respond to the initial query to read the no-operations org entitlements. mux.HandleFunc("/api/v2/organizations/no-operations/entitlement-set", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/vnd.api+json") - io.WriteString(w, `{ + _, err := io.WriteString(w, `{ "data": { "id": "org-ufxa3y8jSbKP8hsT", "type": "entitlement-sets", @@ -276,13 +285,16 @@ func testServer(t *testing.T) *httptest.Server { } } }`) + if err != nil { + w.WriteHeader(500) + } }) // All tests that are assumed to pass will use the hashicorp organization, // so for all other organization requests we will return a 404. mux.HandleFunc("/api/v2/organizations/", func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(404) - io.WriteString(w, `{ + _, err := io.WriteString(w, `{ "errors": [ { "status": "404", @@ -290,6 +302,9 @@ func testServer(t *testing.T) *httptest.Server { } ] }`) + if err != nil { + w.WriteHeader(500) + } }) return httptest.NewServer(mux) diff --git a/internal/backend/testing.go b/internal/backend/testing.go index 2974a2ed56..b1dfd71273 100644 --- a/internal/backend/testing.go +++ b/internal/backend/testing.go @@ -424,7 +424,7 @@ func testLocksInWorkspace(t *testing.T, b1, b2 Backend, testForceUnlock bool, wo _, err = lockerB.Lock(infoB) if err == nil { - lockerA.Unlock(lockIDA) + _ = lockerA.Unlock(lockIDA) // test already failed, no need to check err further t.Fatal("client B obtained lock while held by client A") } diff --git a/internal/cloud/backend_plan.go b/internal/cloud/backend_plan.go index 5285b24ebe..87f5236df9 100644 --- a/internal/cloud/backend_plan.go +++ b/internal/cloud/backend_plan.go @@ -324,7 +324,9 @@ in order to capture the filesystem context the remote workspace expects: log.Printf("[ERROR] error searching process ID: %v", err) return } - p.Signal(syscall.SIGINT) + if err := p.Signal(syscall.SIGINT); err != nil { + log.Printf("[ERROR] error sending interrupt signal: %v", err) + } } } }() diff --git a/internal/cloud/backend_taskStage_policyEvaluation_test.go b/internal/cloud/backend_taskStage_policyEvaluation_test.go index b024cb96c6..2c904597da 100644 --- a/internal/cloud/backend_taskStage_policyEvaluation_test.go +++ b/internal/cloud/backend_taskStage_policyEvaluation_test.go @@ -84,7 +84,7 @@ func TestCloud_runTaskStageWithPolicyEvaluation(t *testing.T) { trs := policyEvaluationSummarizer{ cloud: b, } - c.context.Poll(0, 0, func(i int) (bool, error) { + err := c.context.Poll(0, 0, func(i int) (bool, error) { cont, _, _ := trs.Summarize(c.context, c.writer, c.taskStage()) if cont { return true, nil @@ -98,5 +98,8 @@ func TestCloud_runTaskStageWithPolicyEvaluation(t *testing.T) { } return false, nil }) + if err != nil { + t.Fatalf("Error while polling: %v", err) + } } } diff --git a/internal/cloud/backend_taskStage_taskResults_test.go b/internal/cloud/backend_taskStage_taskResults_test.go index 78bcafbeaf..926f98f83c 100644 --- a/internal/cloud/backend_taskStage_taskResults_test.go +++ b/internal/cloud/backend_taskStage_taskResults_test.go @@ -155,7 +155,7 @@ func TestCloud_runTasksWithTaskResults(t *testing.T) { trs := taskResultSummarizer{ cloud: b, } - c.context.Poll(0, 0, func(i int) (bool, error) { + err := c.context.Poll(0, 0, func(i int) (bool, error) { cont, _, _ := trs.Summarize(c.context, c.writer, c.taskStage()) if cont { return true, nil @@ -169,5 +169,8 @@ func TestCloud_runTasksWithTaskResults(t *testing.T) { } return false, nil }) + if err != nil { + t.Fatalf("Error while polling: %v", err) + } } } diff --git a/internal/cloud/e2e/main_test.go b/internal/cloud/e2e/main_test.go index dc7fc4fb01..c976b363c7 100644 --- a/internal/cloud/e2e/main_test.go +++ b/internal/cloud/e2e/main_test.go @@ -136,7 +136,10 @@ func testRunner(t *testing.T, cases testCases, orgCount int, tfEnvFlags ...strin if lenInput > 0 { for i := 0; i < lenInput; i++ { input := tfCmd.userInput[i] - exp.SendLine(input) + _, err := exp.SendLine(input) + if err != nil { + subtest.Fatal(err) + } // use the index to find the corresponding // output that matches the input. if lenInputOutput-1 >= i { @@ -183,6 +186,13 @@ func setTfeClient() { } } +func chdirOCF(dir string) { + if err := os.Chdir(dir); err != nil { + fmt.Printf("Could not change directories: %v\n", err) + os.Exit(1) + } +} + func setupBinary() func() { log.Println("Setting up terraform binary") tmpTerraformBinaryDir, err := os.MkdirTemp("", "terraform-test") @@ -192,9 +202,9 @@ func setupBinary() func() { } log.Println(tmpTerraformBinaryDir) currentDir, err := os.Getwd() - defer os.Chdir(currentDir) + defer chdirOCF(currentDir) if err != nil { - fmt.Printf("Could not change directories: %v\n", err) + fmt.Printf("Could not get current directory: %v\n", err) os.Exit(1) } // Getting top level dir @@ -204,10 +214,7 @@ func setupBinary() func() { topLevel := len(dirPaths) - 3 topDir := strings.Join(dirPaths[0:topLevel], "/") - if err := os.Chdir(topDir); err != nil { - fmt.Printf("Could not change directories: %v\n", err) - os.Exit(1) - } + chdirOCF(topDir) cmd := exec.Command( "go", diff --git a/internal/cloud/state_test.go b/internal/cloud/state_test.go index 676e4c52c2..e3981e92a4 100644 --- a/internal/cloud/state_test.go +++ b/internal/cloud/state_test.go @@ -182,7 +182,7 @@ func TestCloudLocks(t *testing.T) { _, err = lockerB.Lock(infoB) if err == nil { - lockerA.Unlock(lockIDA) + _ = lockerA.Unlock(lockIDA) // test already failed, no need to check err further t.Fatal("client B obtained lock while held by client A") } if _, ok := err.(*statemgr.LockError); !ok { @@ -335,12 +335,15 @@ func TestState_PersistState(t *testing.T) { if err != nil { t.Fatal(err) } - cloudState.WriteState(states.BuildState(func(s *states.SyncState) { + err = cloudState.WriteState(states.BuildState(func(s *states.SyncState) { s.SetOutputValue( addrs.OutputValue{Name: "boop"}.Absolute(addrs.RootModuleInstance), cty.StringVal("beep"), false, "", ) })) + if err != nil { + t.Fatal(err) + } err = cloudState.PersistState(nil) if err != nil { diff --git a/internal/cloud/testing.go b/internal/cloud/testing.go index 3fa991a4c3..21cc54c1d0 100644 --- a/internal/cloud/testing.go +++ b/internal/cloud/testing.go @@ -40,10 +40,6 @@ import ( "github.com/opentofu/opentofu/internal/tofu" ) -const ( - testCred = "test-auth-token" -) - var ( tfeHost = "app.terraform.io" credsSrc = svcauth.StaticCredentialsSource(map[svchost.Hostname]svcauth.HostCredentials{ @@ -424,12 +420,18 @@ func testServerWithSnapshotsEnabled(t *testing.T, enabled bool) *httptest.Server fakeState := states.NewState() fakeStateFile := statefile.New(fakeState, "boop", 1) var buf bytes.Buffer - statefile.Write(fakeStateFile, &buf, encryption.StateEncryptionDisabled()) + err := statefile.Write(fakeStateFile, &buf, encryption.StateEncryptionDisabled()) + if err != nil { + t.Fatal(err) + } respBody := buf.Bytes() w.Header().Set("content-type", "application/json") w.Header().Set("content-length", strconv.FormatInt(int64(len(respBody)), 10)) w.WriteHeader(http.StatusOK) - w.Write(respBody) + _, err = w.Write(respBody) + if err != nil { + t.Fatal(err) + } return } @@ -477,7 +479,10 @@ func testServerWithSnapshotsEnabled(t *testing.T, enabled bool) *httptest.Server } w.WriteHeader(http.StatusOK) - w.Write(fakeBodyRaw) + _, err = w.Write(fakeBodyRaw) + if err != nil { + t.Fatal(err) + } })) serverURL = server.URL return server @@ -490,20 +495,26 @@ var testDefaultRequestHandlers = map[string]func(http.ResponseWriter, *http.Requ // Respond to service discovery calls. "/well-known/terraform.json": func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - io.WriteString(w, `{ + _, err := io.WriteString(w, `{ "tfe.v2": "/api/v2/", }`) + if err != nil { + w.WriteHeader(500) + } }, // Respond to service version constraints calls. "/v1/versions/": func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - io.WriteString(w, fmt.Sprintf(`{ + _, err := io.WriteString(w, fmt.Sprintf(`{ "service": "%s", "product": "terraform", "minimum": "0.1.0", "maximum": "10.0.0" }`, path.Base(r.URL.Path))) + if err != nil { + w.WriteHeader(500) + } }, // Respond to pings to get the API version header. @@ -515,7 +526,7 @@ var testDefaultRequestHandlers = map[string]func(http.ResponseWriter, *http.Requ // Respond to the initial query to read the hashicorp org entitlements. "/api/v2/organizations/hashicorp/entitlement-set": func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/vnd.api+json") - io.WriteString(w, `{ + _, err := io.WriteString(w, `{ "data": { "id": "org-GExadygjSbKP8hsY", "type": "entitlement-sets", @@ -529,12 +540,15 @@ var testDefaultRequestHandlers = map[string]func(http.ResponseWriter, *http.Requ } } }`) + if err != nil { + w.WriteHeader(500) + } }, // Respond to the initial query to read the no-operations org entitlements. "/api/v2/organizations/no-operations/entitlement-set": func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/vnd.api+json") - io.WriteString(w, `{ + _, err := io.WriteString(w, `{ "data": { "id": "org-ufxa3y8jSbKP8hsT", "type": "entitlement-sets", @@ -548,13 +562,16 @@ var testDefaultRequestHandlers = map[string]func(http.ResponseWriter, *http.Requ } } }`) + if err != nil { + w.WriteHeader(500) + } }, // All tests that are assumed to pass will use the hashicorp organization, // so for all other organization requests we will return a 404. "/api/v2/organizations/": func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(404) - io.WriteString(w, `{ + _, err := io.WriteString(w, `{ "errors": [ { "status": "404", @@ -562,6 +579,9 @@ var testDefaultRequestHandlers = map[string]func(http.ResponseWriter, *http.Requ } ] }`) + if err != nil { + w.WriteHeader(500) + } }, } diff --git a/internal/dag/dag.go b/internal/dag/dag.go index 8f85df8217..43ee9f3f48 100644 --- a/internal/dag/dag.go +++ b/internal/dag/dag.go @@ -105,7 +105,8 @@ func (g *AcyclicGraph) TransitiveReduction() { for _, u := range g.Vertices() { uTargets := g.downEdgesNoCopy(u) - g.DepthFirstWalk(g.downEdgesNoCopy(u), func(v Vertex, d int) error { + // err is always returned as nil + _ = g.DepthFirstWalk(g.downEdgesNoCopy(u), func(v Vertex, d int) error { shared := uTargets.Intersection(g.downEdgesNoCopy(v)) for _, vPrime := range shared { g.RemoveEdge(BasicEdge(u, vPrime)) diff --git a/internal/dag/dag_test.go b/internal/dag/dag_test.go index 6d5fe1ac62..aac821b1af 100644 --- a/internal/dag/dag_test.go +++ b/internal/dag/dag_test.go @@ -460,7 +460,8 @@ func TestAcyclicGraphWalkOrder(t *testing.T) { t.Run("DepthFirst", func(t *testing.T) { var visits []vertexAtDepth - g.walk(depthFirst|downOrder, true, start, func(v Vertex, d int) error { + // err will always be nil + _ = g.walk(depthFirst|downOrder, true, start, func(v Vertex, d int) error { visits = append(visits, vertexAtDepth{v, d}) return nil @@ -474,7 +475,8 @@ func TestAcyclicGraphWalkOrder(t *testing.T) { }) t.Run("ReverseDepthFirst", func(t *testing.T) { var visits []vertexAtDepth - g.walk(depthFirst|upOrder, true, reverse, func(v Vertex, d int) error { + // err will always be nil + _ = g.walk(depthFirst|upOrder, true, reverse, func(v Vertex, d int) error { visits = append(visits, vertexAtDepth{v, d}) return nil @@ -488,7 +490,8 @@ func TestAcyclicGraphWalkOrder(t *testing.T) { }) t.Run("BreadthFirst", func(t *testing.T) { var visits []vertexAtDepth - g.walk(breadthFirst|downOrder, true, start, func(v Vertex, d int) error { + // err will always be nil + _ = g.walk(breadthFirst|downOrder, true, start, func(v Vertex, d int) error { visits = append(visits, vertexAtDepth{v, d}) return nil @@ -502,7 +505,8 @@ func TestAcyclicGraphWalkOrder(t *testing.T) { }) t.Run("ReverseBreadthFirst", func(t *testing.T) { var visits []vertexAtDepth - g.walk(breadthFirst|upOrder, true, reverse, func(v Vertex, d int) error { + // err will always be nil + _ = g.walk(breadthFirst|upOrder, true, reverse, func(v Vertex, d int) error { visits = append(visits, vertexAtDepth{v, d}) return nil diff --git a/internal/dag/dot.go b/internal/dag/dot.go index 58a6a24c61..7a56a16416 100644 --- a/internal/dag/dot.go +++ b/internal/dag/dot.go @@ -57,15 +57,15 @@ func (g *marshalGraph) Dot(opts *DotOpts) []byte { } var w indentWriter - w.WriteString("digraph {\n") + w.writeString("digraph {\n") w.Indent() // some dot defaults - w.WriteString(`compound = "true"` + "\n") - w.WriteString(`newrank = "true"` + "\n") + w.writeString(`compound = "true"` + "\n") + w.writeString(`newrank = "true"` + "\n") // the top level graph is written as the first subgraph - w.WriteString(`subgraph "root" {` + "\n") + w.writeString(`subgraph "root" {` + "\n") g.writeBody(opts, &w) // cluster isn't really used other than for naming purposes in some graphs @@ -80,7 +80,7 @@ func (g *marshalGraph) Dot(opts *DotOpts) []byte { } w.Unindent() - w.WriteString("}\n") + w.writeString("}\n") return w.Bytes() } @@ -152,7 +152,8 @@ func (g *marshalGraph) writeSubgraph(sg *marshalGraph, opts *DotOpts, depth int, name = "cluster_" + name sg.Attrs["label"] = sg.Name } - w.WriteString(fmt.Sprintf("subgraph %q {\n", name)) + // writing to the buffer does not produce an error + _, _ = fmt.Fprintf(w, "subgraph %q {\n", name) sg.writeBody(opts, w) for _, sg := range sg.Subgraphs { @@ -164,7 +165,7 @@ func (g *marshalGraph) writeBody(opts *DotOpts, w *indentWriter) { w.Indent() for _, as := range attrStrings(g.Attrs) { - w.WriteString(as + "\n") + w.writeString(as + "\n") } // list of Vertices that aren't to be included in the dot output @@ -176,7 +177,7 @@ func (g *marshalGraph) writeBody(opts *DotOpts, w *indentWriter) { continue } - w.Write(v.dot(g, opts)) + _, _ = w.Write(v.dot(g, opts)) } var dotEdges []string @@ -219,11 +220,11 @@ func (g *marshalGraph) writeBody(opts *DotOpts, w *indentWriter) { sort.Strings(dotEdges) for _, e := range dotEdges { - w.WriteString(e + "\n") + w.writeString(e + "\n") } w.Unindent() - w.WriteString("}\n") + w.writeString("}\n") } func writeAttrs(buf *bytes.Buffer, attrs map[string]string) { @@ -273,6 +274,12 @@ func (w *indentWriter) Write(b []byte) (int, error) { return w.Buffer.Write(b) } +// writeString is a helper function to write a string to the indentWriter without the need for handling errors. +// the errors are ignored here because writing to a bytes.Buffer should never fail +func (w *indentWriter) writeString(s string) { + _, _ = w.WriteString(s) +} + func (w *indentWriter) WriteString(s string) (int, error) { w.indent() return w.Buffer.WriteString(s) diff --git a/internal/grpcwrap/provisioner.go b/internal/grpcwrap/provisioner.go index b314f63ecf..86de2ec630 100644 --- a/internal/grpcwrap/provisioner.go +++ b/internal/grpcwrap/provisioner.go @@ -74,15 +74,13 @@ func (p *provisioner) ProvisionResource(req *tfplugin5.ProvisionResource_Request configVal, err := decodeDynamicValue(req.Config, ty) if err != nil { srvResp.Diagnostics = convert.AppendProtoDiag(srvResp.Diagnostics, err) - srv.Send(srvResp) - return nil + return srv.Send(srvResp) } connVal, err := decodeDynamicValue(req.Connection, shared.ConnectionBlockSupersetSchema.ImpliedType()) if err != nil { srvResp.Diagnostics = convert.AppendProtoDiag(srvResp.Diagnostics, err) - srv.Send(srvResp) - return nil + return srv.Send(srvResp) } resp := p.provisioner.ProvisionResource(provisioners.ProvisionResourceRequest{ @@ -92,8 +90,7 @@ func (p *provisioner) ProvisionResource(req *tfplugin5.ProvisionResource_Request }) srvResp.Diagnostics = convert.AppendProtoDiag(srvResp.Diagnostics, resp.Diagnostics) - srv.Send(srvResp) - return nil + return srv.Send(srvResp) } func (p *provisioner) Stop(context.Context, *tfplugin5.Stop_Request) (*tfplugin5.Stop_Response, error) { diff --git a/internal/initwd/from_module.go b/internal/initwd/from_module.go index 604259b5de..41aa7b3ee3 100644 --- a/internal/initwd/from_module.go +++ b/internal/initwd/from_module.go @@ -340,7 +340,7 @@ func DirFromModule(ctx context.Context, loader *configload.Loader, rootDir, modu continue } - err = os.MkdirAll(instPath, os.ModePerm) + err := os.MkdirAll(instPath, os.ModePerm) if err != nil { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, @@ -353,7 +353,7 @@ func DirFromModule(ctx context.Context, loader *configload.Loader, rootDir, modu // We copy rather than "rename" here because renaming between directories // can be tricky in edge-cases like network filesystems, etc. log.Printf("[TRACE] copying new module %s from %s to %s", newKey, record.Dir, instPath) - err := copy.CopyDir(instPath, tempPath) + err = copy.CopyDir(instPath, tempPath) if err != nil { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, @@ -377,7 +377,7 @@ func DirFromModule(ctx context.Context, loader *configload.Loader, rootDir, modu hooks.Install(newRecord.Key, newRecord.Version, newRecord.Dir) } - retManifest.WriteSnapshotToDir(modulesDir) + err = retManifest.WriteSnapshotToDir(modulesDir) if err != nil { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, diff --git a/internal/initwd/from_module_test.go b/internal/initwd/from_module_test.go index 4dcd64b3eb..b5692472a2 100644 --- a/internal/initwd/from_module_test.go +++ b/internal/initwd/from_module_test.go @@ -302,7 +302,10 @@ func TestDirFromModule_rel_submodules(t *testing.T) { t.Fatalf("failed to switch to temp dir %s: %s", tmpDir, err) } t.Cleanup(func() { - os.Chdir(oldDir) + err := os.Chdir(oldDir) + if err != nil { + t.Logf("error running chdir to %s: %s", oldDir, err) + } // Trigger garbage collection to ensure that all open file handles are closed. // This prevents TempDir RemoveAll cleanup errors on Windows. if runtime.GOOS == "windows" { diff --git a/internal/plans/objchange/objchange.go b/internal/plans/objchange/objchange.go index f3f0c7d734..a756720c50 100644 --- a/internal/plans/objchange/objchange.go +++ b/internal/plans/objchange/objchange.go @@ -405,7 +405,8 @@ func optionalValueNotComputable(schema *configschema.Attribute, val cty.Value) b } foundNonComputedAttr := false - cty.Walk(val, func(path cty.Path, v cty.Value) (bool, error) { + // err is always nil + _ = cty.Walk(val, func(path cty.Path, v cty.Value) (bool, error) { if v.IsNull() { return true, nil } @@ -439,7 +440,8 @@ func validPriorFromConfig(schema nestedSchema, prior, config cty.Value) bool { stop := errors.New("stop") valid := true - cty.Walk(prior, func(path cty.Path, priorV cty.Value) (bool, error) { + // err is always nil or `stop` + _ = cty.Walk(prior, func(path cty.Path, priorV cty.Value) (bool, error) { configV, err := path.Apply(config) if err != nil { // most likely dynamic objects with different types diff --git a/internal/registry/test/mock_registry.go b/internal/registry/test/mock_registry.go index b4bec85409..f54b996a8b 100644 --- a/internal/registry/test/mock_registry.go +++ b/internal/registry/test/mock_registry.go @@ -236,7 +236,11 @@ func mockRegHandler(config map[uint8]struct{}) http.Handler { return } w.Header().Set("Content-Type", "application/json") - w.Write(js) + _, err = w.Write(js) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } } mux.Handle("/v1/modules/", @@ -257,7 +261,10 @@ func mockRegHandler(config map[uint8]struct{}) http.Handler { mux.HandleFunc("/.well-known/terraform.json", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - io.WriteString(w, `{"modules.v1":"http://localhost/v1/modules/", "providers.v1":"http://localhost/v1/providers/"}`) + _, err := io.WriteString(w, `{"modules.v1":"http://localhost/v1/modules/", "providers.v1":"http://localhost/v1/providers/"}`) + if err != nil { + w.WriteHeader(500) + } }) return mux }