api: use errors.New not fmt.Errorf when error doesn't have format. (#14027)

* api: use errors.New not fmt.Errorf when error doesn't have format.

* semgrep: add rule to catch fmt.Errorf use without formatting.

.semgrep/api_errorf.yml

@@ -0,0 +1,11 @@
+rules:
+  - id: "fmt_errorf_unformatted_use"
+    patterns:
+      - pattern: fmt.Errorf("...")
+    message: "Use of fmt.Errorf without formatting. Please use errors.New"
+    languages:
+      - "go"
+    severity: "WARNING"
+    paths:
+      include:
+        - "./api/*"

api/acl.go

@@ -1,7 +1,7 @@
 package api
 
 import (
-	"fmt"
+	"errors"
 	"time"
 )
 
@@ -28,7 +28,7 @@ func (a *ACLPolicies) List(q *QueryOptions) ([]*ACLPolicyListStub, *QueryMeta, e
 // Upsert is used to create or update a policy
 func (a *ACLPolicies) Upsert(policy *ACLPolicy, q *WriteOptions) (*WriteMeta, error) {
 	if policy == nil || policy.Name == "" {
-		return nil, fmt.Errorf("missing policy name")
+		return nil, errors.New("missing policy name")
 	}
 	wm, err := a.client.write("/v1/acl/policy/"+policy.Name, policy, nil, q)
 	if err != nil {
@@ -40,7 +40,7 @@ func (a *ACLPolicies) Upsert(policy *ACLPolicy, q *WriteOptions) (*WriteMeta, er
 // Delete is used to delete a policy
 func (a *ACLPolicies) Delete(policyName string, q *WriteOptions) (*WriteMeta, error) {
 	if policyName == "" {
-		return nil, fmt.Errorf("missing policy name")
+		return nil, errors.New("missing policy name")
 	}
 	wm, err := a.client.delete("/v1/acl/policy/"+policyName, nil, nil, q)
 	if err != nil {
@@ -52,7 +52,7 @@ func (a *ACLPolicies) Delete(policyName string, q *WriteOptions) (*WriteMeta, er
 // Info is used to query a specific policy
 func (a *ACLPolicies) Info(policyName string, q *QueryOptions) (*ACLPolicy, *QueryMeta, error) {
 	if policyName == "" {
-		return nil, nil, fmt.Errorf("missing policy name")
+		return nil, nil, errors.New("missing policy name")
 	}
 	var resp ACLPolicy
 	wm, err := a.client.query("/v1/acl/policy/"+policyName, &resp, q)
@@ -113,7 +113,7 @@ func (a *ACLTokens) List(q *QueryOptions) ([]*ACLTokenListStub, *QueryMeta, erro
 // Create is used to create a token
 func (a *ACLTokens) Create(token *ACLToken, q *WriteOptions) (*ACLToken, *WriteMeta, error) {
 	if token.AccessorID != "" {
-		return nil, nil, fmt.Errorf("cannot specify Accessor ID")
+		return nil, nil, errors.New("cannot specify Accessor ID")
 	}
 	var resp ACLToken
 	wm, err := a.client.write("/v1/acl/token", token, &resp, q)
@@ -126,7 +126,7 @@ func (a *ACLTokens) Create(token *ACLToken, q *WriteOptions) (*ACLToken, *WriteM
 // Update is used to update an existing token
 func (a *ACLTokens) Update(token *ACLToken, q *WriteOptions) (*ACLToken, *WriteMeta, error) {
 	if token.AccessorID == "" {
-		return nil, nil, fmt.Errorf("missing accessor ID")
+		return nil, nil, errors.New("missing accessor ID")
 	}
 	var resp ACLToken
 	wm, err := a.client.write("/v1/acl/token/"+token.AccessorID,
@@ -140,7 +140,7 @@ func (a *ACLTokens) Update(token *ACLToken, q *WriteOptions) (*ACLToken, *WriteM
 // Delete is used to delete a token
 func (a *ACLTokens) Delete(accessorID string, q *WriteOptions) (*WriteMeta, error) {
 	if accessorID == "" {
-		return nil, fmt.Errorf("missing accessor ID")
+		return nil, errors.New("missing accessor ID")
 	}
 	wm, err := a.client.delete("/v1/acl/token/"+accessorID, nil, nil, q)
 	if err != nil {
@@ -152,7 +152,7 @@ func (a *ACLTokens) Delete(accessorID string, q *WriteOptions) (*WriteMeta, erro
 // Info is used to query a token
 func (a *ACLTokens) Info(accessorID string, q *QueryOptions) (*ACLToken, *QueryMeta, error) {
 	if accessorID == "" {
-		return nil, nil, fmt.Errorf("missing accessor ID")
+		return nil, nil, errors.New("missing accessor ID")
 	}
 	var resp ACLToken
 	wm, err := a.client.query("/v1/acl/token/"+accessorID, &resp, q)
@@ -180,7 +180,7 @@ func (a *ACLTokens) UpsertOneTimeToken(q *WriteOptions) (*OneTimeToken, *WriteMe
 		return nil, nil, err
 	}
 	if resp == nil {
-		return nil, nil, fmt.Errorf("no one-time token returned")
+		return nil, nil, errors.New("no one-time token returned")
 	}
 	return resp.OneTimeToken, wm, nil
 }
@@ -188,7 +188,7 @@ func (a *ACLTokens) UpsertOneTimeToken(q *WriteOptions) (*OneTimeToken, *WriteMe
 // ExchangeOneTimeToken is used to create a one-time token
 func (a *ACLTokens) ExchangeOneTimeToken(secret string, q *WriteOptions) (*ACLToken, *WriteMeta, error) {
 	if secret == "" {
-		return nil, nil, fmt.Errorf("missing secret ID")
+		return nil, nil, errors.New("missing secret ID")
 	}
 	req := &OneTimeTokenExchangeRequest{OneTimeSecretID: secret}
 	var resp *OneTimeTokenExchangeResponse
@@ -197,7 +197,7 @@ func (a *ACLTokens) ExchangeOneTimeToken(secret string, q *WriteOptions) (*ACLTo
 		return nil, nil, err
 	}
 	if resp == nil {
-		return nil, nil, fmt.Errorf("no ACL token returned")
+		return nil, nil, errors.New("no ACL token returned")
 	}
 	return resp.Token, wm, nil
 }

api/allocations.go

@@ -2,7 +2,7 @@ package api
 
 import (
 	"context"
-	"fmt"
+	"errors"
 	"io"
 	"sort"
 	"strings"
@@ -12,7 +12,7 @@ import (
 var (
 	// NodeDownErr marks an operation as not able to complete since the node is
 	// down.
-	NodeDownErr = fmt.Errorf("node down")
+	NodeDownErr = errors.New("node down")
 )
 
 const (
@@ -106,8 +106,7 @@ func (a *Allocations) Exec(ctx context.Context,
 
 func (a *Allocations) Stats(alloc *Allocation, q *QueryOptions) (*AllocResourceUsage, error) {
 	var resp AllocResourceUsage
-	path := fmt.Sprintf("/v1/client/allocation/%s/stats", alloc.ID)
-	_, err := a.client.query(path, &resp, q)
+	_, err := a.client.query("/v1/client/allocation/"+alloc.ID+"/stats", &resp, q)
 	return &resp, err
 }
 

api/api.go

@@ -345,9 +345,9 @@ func DefaultConfig() *Config {
 // otherwise, returns the same client
 func cloneWithTimeout(httpClient *http.Client, t time.Duration) (*http.Client, error) {
 	if httpClient == nil {
-		return nil, fmt.Errorf("nil HTTP client")
+		return nil, errors.New("nil HTTP client")
 	} else if httpClient.Transport == nil {
-		return nil, fmt.Errorf("nil HTTP client transport")
+		return nil, errors.New("nil HTTP client transport")
 	}
 
 	if t.Nanoseconds() < 0 {
@@ -398,7 +398,7 @@ func ConfigureTLS(httpClient *http.Client, tlsConfig *TLSConfig) error {
 		return nil
 	}
 	if httpClient == nil {
-		return fmt.Errorf("config HTTP Client must be set")
+		return errors.New("config HTTP Client must be set")
 	}
 
 	var clientCert tls.Certificate
@@ -412,7 +412,7 @@ func ConfigureTLS(httpClient *http.Client, tlsConfig *TLSConfig) error {
 			}
 			foundClientCert = true
 		} else {
-			return fmt.Errorf("Both client cert and client key must be provided")
+			return errors.New("Both client cert and client key must be provided")
 		}
 	} else if len(tlsConfig.ClientCertPEM) != 0 || len(tlsConfig.ClientKeyPEM) != 0 {
 		if len(tlsConfig.ClientCertPEM) != 0 && len(tlsConfig.ClientKeyPEM) != 0 {
@@ -423,7 +423,7 @@ func ConfigureTLS(httpClient *http.Client, tlsConfig *TLSConfig) error {
 			}
 			foundClientCert = true
 		} else {
-			return fmt.Errorf("Both client cert and client key must be provided")
+			return errors.New("Both client cert and client key must be provided")
 		}
 	}
 
@@ -849,7 +849,7 @@ func (c *Client) websocket(endpoint string, q *QueryOptions) (*websocket.Conn, *
 
 	transport, ok := c.httpClient.Transport.(*http.Transport)
 	if !ok {
-		return nil, nil, fmt.Errorf("unsupported transport")
+		return nil, nil, errors.New("unsupported transport")
 	}
 	dialer := websocket.Dialer{
 		ReadBufferSize:   4096,

api/api_test.go

@@ -501,7 +501,7 @@ func TestCloneHttpClient(t *testing.T) {
 	client := defaultHttpClient()
 	originalTransport := client.Transport.(*http.Transport)
 	originalTransport.Proxy = func(*http.Request) (*url.URL, error) {
-		return nil, fmt.Errorf("stub function")
+		return nil, errors.New("stub function")
 	}
 
 	t.Run("closing with negative timeout", func(t *testing.T) {

api/fs_test.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
 	"reflect"
@@ -47,7 +48,7 @@ func TestFS_Logs(t *testing.T) {
 			return false, fmt.Errorf("node not ready: %s", nodes[0].Status)
 		}
 		if _, ok := nodes[0].Drivers["mock_driver"]; !ok {
-			return false, fmt.Errorf("mock_driver not ready")
+			return false, errors.New("mock_driver not ready")
 		}
 		return true, nil
 	}, func(err error) {
@@ -279,7 +280,7 @@ func TestFS_FrameReader_Error(t *testing.T) {
 	r.SetUnblockTime(10 * time.Millisecond)
 
 	// Send an error
-	expected := fmt.Errorf("test error")
+	expected := errors.New("test error")
 	errCh <- expected
 
 	// Read a little

api/internal/testutil/freeport/freeport.go

@@ -3,7 +3,7 @@
 package freeport
 
 import (
-	"fmt"
+	"errors"
 	"math/rand"
 	"net"
 	"sync"
@@ -110,7 +110,7 @@ func Free(n int) (ports []int, err error) {
 	defer mu.Unlock()
 
 	if n > blockSize-1 {
-		return nil, fmt.Errorf("freeport: block size too small")
+		return nil, errors.New("freeport: block size too small")
 	}
 
 	// Reserve a port block

api/ioutil.go

@@ -5,13 +5,13 @@ import (
 	"crypto/sha256"
 	"crypto/sha512"
 	"encoding/base64"
-	"fmt"
+	"errors"
 	"hash"
 	"io"
 	"strings"
 )
 
-var errMismatchChecksum = fmt.Errorf("mismatch checksum")
+var errMismatchChecksum = errors.New("mismatch checksum")
 
 // checksumValidatingReader is a wrapper reader that validates
 // the checksum of the underlying reader.
@@ -38,7 +38,7 @@ type checksumValidatingReader struct {
 func newChecksumValidatingReader(r io.ReadCloser, digest string) (io.ReadCloser, error) {
 	parts := strings.SplitN(digest, "=", 2)
 	if len(parts) != 2 {
-		return nil, fmt.Errorf("invalid digest format")
+		return nil, errors.New("invalid digest format")
 	}
 
 	algo := parts[0]

api/ioutil_test.go

@@ -5,7 +5,7 @@ import (
 	"crypto/sha256"
 	"crypto/sha512"
 	"encoding/base64"
-	"fmt"
+	"errors"
 	"hash"
 	"io"
 	"io/ioutil"
@@ -73,7 +73,7 @@ func TestChecksumValidatingReader_PropagatesError(t *testing.T) {
 	defer pr.Close()
 	defer pw.Close()
 
-	expectedErr := fmt.Errorf("some error")
+	expectedErr := errors.New("some error")
 
 	go func() {
 		pw.Write([]byte("some input"))

api/jobs.go

@@ -1,6 +1,7 @@
 package api
 
 import (
+	"errors"
 	"fmt"
 	"net/url"
 	"sort"
@@ -400,7 +401,7 @@ func (j *Jobs) Plan(job *Job, diff bool, q *WriteOptions) (*JobPlanResponse, *Wr
 
 func (j *Jobs) PlanOpts(job *Job, opts *PlanOptions, q *WriteOptions) (*JobPlanResponse, *WriteMeta, error) {
 	if job == nil {
-		return nil, nil, fmt.Errorf("must pass non-nil job")
+		return nil, nil, errors.New("must pass non-nil job")
 	}
 
 	// Setup the request

api/sentinel.go

@@ -1,6 +1,8 @@
 package api
 
-import "fmt"
+import (
+	"errors"
+)
 
 // SentinelPolicies is used to query the Sentinel Policy endpoints.
 type SentinelPolicies struct {
@@ -25,7 +27,7 @@ func (a *SentinelPolicies) List(q *QueryOptions) ([]*SentinelPolicyListStub, *Qu
 // Upsert is used to create or update a policy
 func (a *SentinelPolicies) Upsert(policy *SentinelPolicy, q *WriteOptions) (*WriteMeta, error) {
 	if policy == nil || policy.Name == "" {
-		return nil, fmt.Errorf("missing policy name")
+		return nil, errors.New("missing policy name")
 	}
 	wm, err := a.client.write("/v1/sentinel/policy/"+policy.Name, policy, nil, q)
 	if err != nil {
@@ -37,7 +39,7 @@ func (a *SentinelPolicies) Upsert(policy *SentinelPolicy, q *WriteOptions) (*Wri
 // Delete is used to delete a policy
 func (a *SentinelPolicies) Delete(policyName string, q *WriteOptions) (*WriteMeta, error) {
 	if policyName == "" {
-		return nil, fmt.Errorf("missing policy name")
+		return nil, errors.New("missing policy name")
 	}
 	wm, err := a.client.delete("/v1/sentinel/policy/"+policyName, nil, nil, q)
 	if err != nil {
@@ -49,7 +51,7 @@ func (a *SentinelPolicies) Delete(policyName string, q *WriteOptions) (*WriteMet
 // Info is used to query a specific policy
 func (a *SentinelPolicies) Info(policyName string, q *QueryOptions) (*SentinelPolicy, *QueryMeta, error) {
 	if policyName == "" {
-		return nil, nil, fmt.Errorf("missing policy name")
+		return nil, nil, errors.New("missing policy name")
 	}
 	var resp SentinelPolicy
 	wm, err := a.client.query("/v1/sentinel/policy/"+policyName, &resp, q)