Fix violations of non-constant format strings linter

Depends on gravitational/teleport.e#6006. Now that we are compliant, the ignore rule was removed from the golangci-lint config to prevent future regressions.
gravitational · Feb 4, 2025 · 6830eec · 6830eec
1 parent 5ae0088
commit 6830eec
Show file tree

Hide file tree

Showing 137 changed files with 339 additions and 353 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -55,9 +55,6 @@ issues:
     - path: provider/provider.go # integrations/terraform
       linters: [staticcheck]
       text: 'grpc.WithReturnConnectionError is deprecated'
-    - linters: [govet]
-      path-except: ^e/
-      text: 'non-constant format string in call to github.com/gravitational/trace.'
     # BlockUntilContext should indeed be favored, this exception exists because
     # at this time there are too many offenders.
     - linters: [staticcheck]

diff --git a/api/breaker/breaker.go b/api/breaker/breaker.go
@@ -345,7 +345,7 @@ func (c *CircuitBreaker) beforeExecution() (uint64, error) {
 		c.cfg.OnExecute(false, StateTripped)
 
 		if c.cfg.TrippedErrorMessage != "" {
-			return generation, trace.ConnectionProblem(nil, c.cfg.TrippedErrorMessage)
+			return generation, trace.ConnectionProblem(nil, "%s", c.cfg.TrippedErrorMessage)
 		}
 
 		return generation, trace.Wrap(ErrStateTripped)

diff --git a/api/types/duration.go b/api/types/duration.go
@@ -59,7 +59,7 @@ func (d *Duration) UnmarshalJSON(data []byte) error {
 	}
 	out, err := parseDuration(stringVar)
 	if err != nil {
-		return trace.BadParameter(err.Error())
+		return trace.BadParameter("%s", err.Error())
 	}
 	*d = out
 	return nil
@@ -83,7 +83,7 @@ func (d *Duration) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	}
 	out, err := parseDuration(stringVar)
 	if err != nil {
-		return trace.BadParameter(err.Error())
+		return trace.BadParameter("%s", err.Error())
 	}
 	*d = out
 	return nil
@@ -189,7 +189,7 @@ func parseDuration(s string) (Duration, error) {
 		return 0, nil
 	}
 	if s == "" {
-		return 0, trace.BadParameter("time: invalid duration " + orig)
+		return 0, trace.BadParameter("time: invalid duration %q", orig)
 	}
 	for s != "" {
 		var (
@@ -201,13 +201,13 @@ func parseDuration(s string) (Duration, error) {
 
 		// The next character must be [0-9.]
 		if !(s[0] == '.' || '0' <= s[0] && s[0] <= '9') {
-			return 0, trace.BadParameter("time: invalid duration " + orig)
+			return 0, trace.BadParameter("time: invalid duration %q", orig)
 		}
 		// Consume [0-9]*
 		pl := len(s)
 		v, s, err = leadingInt(s)
 		if err != nil {
-			return 0, trace.BadParameter("time: invalid duration " + orig)
+			return 0, trace.BadParameter("time: invalid duration %q", orig)
 		}
 		pre := pl != len(s) // whether we consumed anything before a period
 
@@ -221,7 +221,7 @@ func parseDuration(s string) (Duration, error) {
 		}
 		if !pre && !post {
 			// no digits (e.g. ".s" or "-.s")
-			return 0, trace.BadParameter("time: invalid duration " + orig)
+			return 0, trace.BadParameter("time: invalid duration %q", orig)
 		}
 
 		// Consume unit.
@@ -233,17 +233,17 @@ func parseDuration(s string) (Duration, error) {
 			}
 		}
 		if i == 0 {
-			return 0, trace.BadParameter("time: missing unit in duration " + orig)
+			return 0, trace.BadParameter("time: missing unit in duration %q", orig)
 		}
 		u := s[:i]
 		s = s[i:]
 		unit, ok := unitMap[u]
 		if !ok {
-			return 0, trace.BadParameter("time: unknown unit " + " in duration " + orig)
+			return 0, trace.BadParameter("time: unknown unit in duration %q", orig)
 		}
 		if v > (1<<63-1)/unit {
 			// overflow
-			return 0, trace.BadParameter("time: invalid duration " + orig)
+			return 0, trace.BadParameter("time: invalid duration %q", orig)
 		}
 		v *= unit
 		if f > 0 {
@@ -252,13 +252,13 @@ func parseDuration(s string) (Duration, error) {
 			v += int64(float64(f) * (float64(unit) / scale))
 			if v < 0 {
 				// overflow
-				return 0, trace.BadParameter("time: invalid duration " + orig)
+				return 0, trace.BadParameter("time: invalid duration %q", orig)
 			}
 		}
 		d += v
 		if d < 0 {
 			// overflow
-			return 0, trace.BadParameter("time: invalid duration " + orig)
+			return 0, trace.BadParameter("time: invalid duration %q", orig)
 		}
 	}
 

diff --git a/api/utils/keys/policy.go b/api/utils/keys/policy.go
@@ -14,7 +14,6 @@ limitations under the License.
 package keys
 
 import (
-	"fmt"
 	"regexp"
 
 	"github.com/gravitational/trace"
@@ -164,7 +163,7 @@ var privateKeyPolicyErrRegex = regexp.MustCompile(`private key policy not (met|s
 
 func NewPrivateKeyPolicyError(p PrivateKeyPolicy) error {
 	// TODO(Joerger): Replace with "private key policy not satisfied" in 16.0.0
-	return trace.BadParameter(fmt.Sprintf("private key policy not met: %s", p))
+	return trace.BadParameter("private key policy not met: %s", p)
 }
 
 // ParsePrivateKeyPolicyError checks if the given error is a private key policy

diff --git a/api/utils/retryutils/retry.go b/api/utils/retryutils/retry.go
@@ -195,7 +195,7 @@ func (r *Linear) For(ctx context.Context, retryFn func() error) error {
 		case <-r.After():
 			r.Inc()
 		case <-ctx.Done():
-			return trace.LimitExceeded(ctx.Err().Error())
+			return trace.LimitExceeded("%s", ctx.Err().Error())
 		}
 	}
 }

diff --git a/api/utils/sshutils/conn.go b/api/utils/sshutils/conn.go
@@ -19,7 +19,6 @@ package sshutils
 import (
 	"bytes"
 	"encoding/json"
-	"fmt"
 	"io"
 
 	"github.com/gravitational/trace"
@@ -68,10 +67,11 @@ func ConnectProxyTransport(sconn ssh.Conn, req *DialReq, exclusive bool) (conn *
 		// passed to us via stderr.
 		errMessageBytes, _ := io.ReadAll(channel.Stderr())
 		errMessage := string(bytes.TrimSpace(errMessageBytes))
-		if len(errMessage) == 0 {
-			errMessage = fmt.Sprintf("failed connecting to %v [%v]", req.Address, req.ServerID)
+		if len(errMessage) > 0 {
+			return nil, false, trace.Errorf("%s", errMessage)
 		}
-		return nil, false, trace.Errorf(errMessage)
+
+		return nil, false, trace.Errorf("failed connecting to %v [%v]", req.Address, req.ServerID)
 	}
 
 	if exclusive {

diff --git a/api/utils/tlsutils/tlsutils.go b/api/utils/tlsutils/tlsutils.go
@@ -36,7 +36,7 @@ func ParseCertificatePEM(bytes []byte) (*x509.Certificate, error) {
 	}
 	cert, err := x509.ParseCertificate(block.Bytes)
 	if err != nil {
-		return nil, trace.BadParameter(err.Error())
+		return nil, trace.BadParameter("%s", err.Error())
 	}
 	return cert, nil
 }

diff --git a/e b/e
diff --git a/integrations/access/datadog/client.go b/integrations/access/datadog/client.go
@@ -131,14 +131,12 @@ func onAfterDatadogResponse(sink common.StatusSink) resty.ResponseMiddleware {
 		}
 
 		if resp.IsError() {
-			var details string
 			switch result := resp.Error().(type) {
 			case *ErrorResult:
-				details = fmt.Sprintf("http error code=%v, errors=[%v]", resp.StatusCode(), strings.Join(result.Errors, ", "))
+				return trace.Errorf("http error code=%v, errors=[%v]", resp.StatusCode(), strings.Join(result.Errors, ", "))
 			default:
-				details = fmt.Sprintf("unknown error result %#v", result)
+				return trace.Errorf("unknown error result %#v", result)
 			}
-			return trace.Errorf(details)
 		}
 		return nil
 	}

diff --git a/integrations/access/pagerduty/client.go b/integrations/access/pagerduty/client.go
@@ -125,23 +125,24 @@ func onAfterPagerDutyResponse(sink common.StatusSink) resty.ResponseMiddleware {
 			log.ErrorContext(ctx, "Error while emitting PagerDuty plugin status", "error", err)
 		}
 
+		var errorFn func(string, ...interface{}) error = trace.Errorf
+		if status.GetCode() == types.PluginStatusCode_UNAUTHORIZED {
+			errorFn = func(msg string, args ...interface{}) error {
+				return trace.AccessDenied(msg, args...)
+			}
+		}
+
 		if resp.IsError() {
-			var details string
 			switch result := resp.Error().(type) {
 			case *ErrorResult:
 				// Do we have a formatted PagerDuty API error response? We set
 				// an empty `ErrorResult` in the pre-request hook, and if the
 				// HTTP server returns an error, the `resty` middleware will
 				// attempt to unmarshal the error response into it.
-				details = fmt.Sprintf("http error code=%v, err_code=%v, message=%v, errors=[%v]", resp.StatusCode(), result.Code, result.Message, strings.Join(result.Errors, ", "))
+				return errorFn("http error code=%v, err_code=%v, message=%v, errors=[%v]", resp.StatusCode(), result.Code, result.Message, strings.Join(result.Errors, ", "))
 			default:
-				details = fmt.Sprintf("unknown error result %#v", result)
-			}
-
-			if status.GetCode() == types.PluginStatusCode_UNAUTHORIZED {
-				return trace.AccessDenied(details)
+				return errorFn("unknown error result %#v", result)
 			}
-			return trace.Errorf(details)
 		}
 		return nil
 	}

diff --git a/integrations/terraform/provider/credentials.go b/integrations/terraform/provider/credentials.go
@@ -499,7 +499,7 @@ func (CredentialsFromNativeMachineID) Credentials(ctx context.Context, config pr
 	}
 
 	if apitypes.JoinMethod(joinMethod) == apitypes.JoinMethodToken {
-		return nil, trace.BadParameter(`the secret token join method ('token') is not supported for native Machine ID joining.
+		return nil, trace.BadParameter("%s", `the secret token join method ('token') is not supported for native Machine ID joining.
 
 Secret tokens are single use and the Terraform provider does not save the certificates it obtained, so the token join method can only be used once.
 If you want to run the Terraform provider in the CI (GitHub Actions, GitlabCI, Circle CI) or in a supported runtime (AWS, GCP, Azure, Kubernetes, machine with a TPM)