Skip to content

Commit

Permalink
Prepare code base for Go 1.24 update. (#29412)
Browse files Browse the repository at this point in the history
* Fix "t.Fatal from a non-test goroutine" errors in cache_test.go

 - t.Fatal(f) should not be called within a Go routine based on it's documentation and only from the main test's thread.
 - In 1.24 this seems to cause build failures

* Address all "non-constant format string errors" from go vet

 - Within 1.24 these now cause test builds to fail

…" from go vet
  • Loading branch information
stevendpclark authored Jan 27, 2025
1 parent 5ff8a3d commit 9456671
Show file tree
Hide file tree
Showing 23 changed files with 69 additions and 64 deletions.
3 changes: 2 additions & 1 deletion api/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package api
import (
"bytes"
"encoding/json"
"errors"
"fmt"
"io"
"reflect"
Expand Down Expand Up @@ -380,7 +381,7 @@ func ParseSecret(r io.Reader) (*Secret, error) {
if err := json.Unmarshal(errBytes, &errStrArray); err != nil {
return nil, err
}
return nil, fmt.Errorf(strings.Join(errStrArray, " "))
return nil, errors.New(strings.Join(errStrArray, " "))
}

// if any raw data is present in resp.Body, add it to secret
Expand Down
4 changes: 2 additions & 2 deletions builtin/logical/pki/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2358,7 +2358,7 @@ func runTestSignVerbatim(t *testing.T, keyType string) {
t.Fatal(err)
}
if resp != nil && resp.IsError() {
t.Fatalf(resp.Error().Error())
t.Fatal(resp.Error().Error())
}
if resp.Data == nil || resp.Data["certificate"] == nil {
t.Fatal("did not get expected data")
Expand Down Expand Up @@ -2398,7 +2398,7 @@ func runTestSignVerbatim(t *testing.T, keyType string) {
t.Fatal(err)
}
if resp != nil && resp.IsError() {
t.Fatalf(resp.Error().Error())
t.Fatal(resp.Error().Error())
}
if resp.Data == nil || resp.Data["certificate"] == nil {
t.Fatal("did not get expected data")
Expand Down
3 changes: 2 additions & 1 deletion builtin/logical/pki/chain_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package pki
import (
"bytes"
"crypto/x509"
"errors"
"fmt"
"sort"

Expand Down Expand Up @@ -439,7 +440,7 @@ func (sc *storageContext) rebuildIssuersChains(referenceCert *issuing.IssuerEntr
}
}
if len(msg) > 0 {
return fmt.Errorf(msg)
return errors.New(msg)
}

// Finally, write all issuers to disk.
Expand Down
2 changes: 1 addition & 1 deletion builtin/logical/transit/path_hmac_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func TestTransit_HMAC(t *testing.T) {
t.Fatalf("error validating hmac: %s", errStr)
}
if resp.Data["valid"].(bool) == false {
t.Fatalf(fmt.Sprintf("error validating hmac;\nreq:\n%#v\nresp:\n%#v", *req, *resp))
t.Fatalf("error validating hmac;\nreq:\n%#v\nresp:\n%#v", *req, *resp)
}
}
req.Path = strings.ReplaceAll(req.Path, "hmac", "verify")
Expand Down
10 changes: 6 additions & 4 deletions command/agentproxyshared/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,17 +226,19 @@ func TestCache_ConcurrentRequests(t *testing.T) {
"key": key,
})
if err != nil {
t.Fatal(err)
t.Error(err.Error())
return
}
secret, err := testClient.Logical().Read(key)
if err != nil {
t.Fatal(err)
t.Error(err.Error())
return
}
if secret == nil || secret.Data["key"].(string) != key {
t.Fatal(fmt.Sprintf("failed to read value for key: %q", key))
t.Errorf("failed to read value for key: %q", key)
return
}
}(i)

}
wg.Wait()
}
Expand Down
4 changes: 2 additions & 2 deletions command/debug_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ func TestDebugCommand_InsecureUmask(t *testing.T) {
// check permissions of the parent debug directory
err = isValidFilePermissions(fs)
if err != nil {
t.Fatalf(err.Error())
t.Fatal(err.Error())
}

// check permissions of the files within the parent directory
Expand All @@ -768,7 +768,7 @@ func TestDebugCommand_InsecureUmask(t *testing.T) {
err = filepath.Walk(bundlePath, func(path string, info os.FileInfo, err error) error {
err = isValidFilePermissions(info)
if err != nil {
t.Fatalf(err.Error())
t.Fatal(err.Error())
}
return nil
})
Expand Down
3 changes: 2 additions & 1 deletion command/operator_diagnose_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package command

import (
"context"
"errors"
"fmt"
"io/ioutil"
"os"
Expand Down Expand Up @@ -733,7 +734,7 @@ func compareResult(exp *diagnose.Result, act *diagnose.Result) error {
for _, c := range act.Children {
errStrings = append(errStrings, fmt.Sprintf("%+v", c))
}
return fmt.Errorf(strings.Join(errStrings, ","))
return errors.New(strings.Join(errStrings, ","))
}

if len(exp.Children) > 0 {
Expand Down
17 changes: 8 additions & 9 deletions command/server/config_custom_response_headers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package server

import (
"fmt"
"testing"

"github.com/go-test/deep"
Expand Down Expand Up @@ -65,7 +64,7 @@ func TestCustomResponseHeadersConfigs(t *testing.T) {
t.Fatalf("Error encountered when loading config %+v", err)
}
if diff := deep.Equal(expectedCustomResponseHeader, config.Listeners[0].CustomResponseHeaders); diff != nil {
t.Fatalf(fmt.Sprintf("parsed custom headers do not match the expected ones, difference: %v", diff))
t.Fatalf("parsed custom headers do not match the expected ones, difference: %v", diff)
}
}

Expand All @@ -84,29 +83,29 @@ func TestCustomResponseHeadersConfigsMultipleListeners(t *testing.T) {
t.Fatalf("Error encountered when loading config %+v", err)
}
if diff := deep.Equal(expectedCustomResponseHeader, config.Listeners[0].CustomResponseHeaders); diff != nil {
t.Fatalf(fmt.Sprintf("parsed custom headers do not match the expected ones, difference: %v", diff))
t.Fatalf("parsed custom headers do not match the expected ones, difference: %v", diff)
}

if diff := deep.Equal(expectedCustomResponseHeader, config.Listeners[1].CustomResponseHeaders); diff == nil {
t.Fatalf(fmt.Sprintf("parsed custom headers do not match the expected ones, difference: %v", diff))
t.Fatalf("parsed custom headers do not match the expected ones, difference: %v", diff)
}
if diff := deep.Equal(expectedCustomResponseHeader["default"], config.Listeners[1].CustomResponseHeaders["default"]); diff != nil {
t.Fatalf(fmt.Sprintf("parsed custom headers do not match the expected ones, difference: %v", diff))
t.Fatalf("parsed custom headers do not match the expected ones, difference: %v", diff)
}

if diff := deep.Equal(expectedCustomResponseHeader, config.Listeners[2].CustomResponseHeaders); diff == nil {
t.Fatalf(fmt.Sprintf("parsed custom headers do not match the expected ones, difference: %v", diff))
t.Fatalf("parsed custom headers do not match the expected ones, difference: %v", diff)
}

if diff := deep.Equal(defaultSTS, config.Listeners[2].CustomResponseHeaders["default"]); diff != nil {
t.Fatalf(fmt.Sprintf("parsed custom headers do not match the expected ones, difference: %v", diff))
t.Fatalf("parsed custom headers do not match the expected ones, difference: %v", diff)
}

if diff := deep.Equal(expectedCustomResponseHeader, config.Listeners[3].CustomResponseHeaders); diff == nil {
t.Fatalf(fmt.Sprintf("parsed custom headers do not match the expected ones, difference: %v", diff))
t.Fatalf("parsed custom headers do not match the expected ones, difference: %v", diff)
}

if diff := deep.Equal(defaultSTS, config.Listeners[3].CustomResponseHeaders["default"]); diff != nil {
t.Fatalf(fmt.Sprintf("parsed custom headers do not match the expected ones, difference: %v", diff))
t.Fatalf("parsed custom headers do not match the expected ones, difference: %v", diff)
}
}
4 changes: 2 additions & 2 deletions helper/osutil/fileinfo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ func TestCheckPathInfo(t *testing.T) {

err = checkPathInfo(info, "testFile", tc.uid, int(tc.permissions))
if tc.expectError && err == nil {
t.Errorf("invalid result. expected error")
t.Error("invalid result. expected error")
}
if !tc.expectError && err != nil {
t.Errorf(err.Error())
t.Error(err.Error())
}

err = os.RemoveAll("testFile")
Expand Down
4 changes: 2 additions & 2 deletions sdk/database/dbplugin/v5/grpc_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,12 @@ func (g *gRPCServer) NewUser(ctx context.Context, req *proto.NewUserRequest) (*p

func (g *gRPCServer) UpdateUser(ctx context.Context, req *proto.UpdateUserRequest) (*proto.UpdateUserResponse, error) {
if req.GetUsername() == "" {
return &proto.UpdateUserResponse{}, status.Errorf(codes.InvalidArgument, "no username provided")
return &proto.UpdateUserResponse{}, status.Error(codes.InvalidArgument, "no username provided")
}

dbReq, err := getUpdateUserRequest(req)
if err != nil {
return &proto.UpdateUserResponse{}, status.Errorf(codes.InvalidArgument, err.Error())
return &proto.UpdateUserResponse{}, status.Error(codes.InvalidArgument, err.Error())
}

impl, err := g.getDatabase(ctx)
Expand Down
17 changes: 9 additions & 8 deletions sdk/helper/certutil/certutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"encoding/asn1"
"encoding/json"
"encoding/pem"
"errors"
"fmt"
"math/big"
mathrand "math/rand"
Expand Down Expand Up @@ -58,7 +59,7 @@ func TestCertBundleConversion(t *testing.T) {
err = compareCertBundleToParsedCertBundle(cbut, pcbut)
if err != nil {
t.Logf("Error occurred with bundle %d in test array (index %d).\n", i+1, i)
t.Errorf(err.Error())
t.Error(err.Error())
}

cbut, err := pcbut.ToCertBundle()
Expand All @@ -68,7 +69,7 @@ func TestCertBundleConversion(t *testing.T) {

err = compareCertBundleToParsedCertBundle(cbut, pcbut)
if err != nil {
t.Fatalf(err.Error())
t.Fatal(err.Error())
}
}
}
Expand Down Expand Up @@ -132,7 +133,7 @@ func TestCertBundleParsing(t *testing.T) {
err = compareCertBundleToParsedCertBundle(cbut, pcbut)
if err != nil {
t.Logf("Error occurred with bundle %d in test array (index %d).\n", i+1, i)
t.Fatalf(err.Error())
t.Fatal(err.Error())
}

dataMap := structs.New(cbut).Map()
Expand All @@ -144,7 +145,7 @@ func TestCertBundleParsing(t *testing.T) {
err = compareCertBundleToParsedCertBundle(cbut, pcbut)
if err != nil {
t.Logf("Error occurred with bundle %d in test array (index %d).\n", i+1, i)
t.Fatalf(err.Error())
t.Fatal(err.Error())
}

pcbut, err = ParsePEMBundle(cbut.ToPEMBundle())
Expand All @@ -155,14 +156,14 @@ func TestCertBundleParsing(t *testing.T) {
err = compareCertBundleToParsedCertBundle(cbut, pcbut)
if err != nil {
t.Logf("Error occurred with bundle %d in test array (index %d).\n", i+1, i)
t.Fatalf(err.Error())
t.Fatal(err.Error())
}
}
}

func compareCertBundleToParsedCertBundle(cbut *CertBundle, pcbut *ParsedCertBundle) error {
if cbut == nil {
return fmt.Errorf("got nil bundle")
return errors.New("got nil bundle")
}
if pcbut == nil {
return fmt.Errorf("got nil parsed bundle")
Expand Down Expand Up @@ -281,7 +282,7 @@ func TestCSRBundleConversion(t *testing.T) {

err = compareCSRBundleToParsedCSRBundle(csrbut, pcsrbut)
if err != nil {
t.Fatalf(err.Error())
t.Fatal(err.Error())
}

csrbut, err = pcsrbut.ToCSRBundle()
Expand All @@ -291,7 +292,7 @@ func TestCSRBundleConversion(t *testing.T) {

err = compareCSRBundleToParsedCSRBundle(csrbut, pcsrbut)
if err != nil {
t.Fatalf(err.Error())
t.Fatal(err.Error())
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/helper/certutil/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func GetHexFormatted(buf []byte, sep string) string {
var ret bytes.Buffer
for _, cur := range buf {
if ret.Len() > 0 {
fmt.Fprintf(&ret, sep)
fmt.Fprint(&ret, sep)
}
fmt.Fprintf(&ret, "%02x", cur)
}
Expand Down
4 changes: 2 additions & 2 deletions sdk/helper/keysutil/lock_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,12 +277,12 @@ func (lm *LockManager) BackupPolicy(ctx context.Context, storage logical.Storage
return "", err
}
if p == nil {
return "", fmt.Errorf(fmt.Sprintf("key %q not found", name))
return "", fmt.Errorf("key %q not found", name)
}
}

if atomic.LoadUint32(&p.deleted) == 1 {
return "", fmt.Errorf(fmt.Sprintf("key %q not found", name))
return "", fmt.Errorf("key %q not found", name)
}

backup, err := p.Backup(ctx, storage)
Expand Down
3 changes: 1 addition & 2 deletions sdk/plugin/grpc_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,8 +459,7 @@ func (s *gRPCSystemViewServer) GenerateIdentityToken(ctx context.Context, req *p
TTL: time.Duration(req.GetTTL()) * time.Second,
})
if err != nil {
return &pb.GenerateIdentityTokenResponse{}, status.Errorf(codes.Internal,
err.Error())
return &pb.GenerateIdentityTokenResponse{}, status.Error(codes.Internal, err.Error())
}

return &pb.GenerateIdentityTokenResponse{
Expand Down
12 changes: 6 additions & 6 deletions vault/activity_log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1329,7 +1329,7 @@ func TestActivityLog_loadCurrentClientSegment(t *testing.T) {
for _, tc := range testCases {
data, err := proto.Marshal(tc.entities)
if err != nil {
t.Fatalf(err.Error())
t.Fatal(err.Error())
}
WriteToStorage(t, core, ActivityLogPrefix+tc.path, data)
}
Expand Down Expand Up @@ -1442,7 +1442,7 @@ func TestActivityLog_loadPriorEntitySegment(t *testing.T) {
for _, tc := range testCases {
data, err := proto.Marshal(tc.entities)
if err != nil {
t.Fatalf(err.Error())
t.Fatal(err.Error())
}
WriteToStorage(t, core, ActivityLogPrefix+tc.path, data)
}
Expand Down Expand Up @@ -1488,7 +1488,7 @@ func TestActivityLog_loadTokenCount(t *testing.T) {

data, err := proto.Marshal(tokenCount)
if err != nil {
t.Fatalf(err.Error())
t.Fatal(err.Error())
}

testCases := []struct {
Expand Down Expand Up @@ -1622,7 +1622,7 @@ func setupActivityRecordsInStorage(t *testing.T, base time.Time, includeEntities
Clients: []*activity.EntityRecord{entityRecord},
})
if err != nil {
t.Fatalf(err.Error())
t.Fatal(err.Error())
}
if i == 0 {
WriteToStorage(t, core, ActivityLogPrefix+"entity/"+fmt.Sprint(monthsAgo.Unix())+"/0", entityData)
Expand All @@ -1648,7 +1648,7 @@ func setupActivityRecordsInStorage(t *testing.T, base time.Time, includeEntities

tokenData, err := proto.Marshal(tokenCount)
if err != nil {
t.Fatalf(err.Error())
t.Fatal(err.Error())
}

WriteToStorage(t, core, ActivityLogPrefix+"directtokens/"+fmt.Sprint(base.Unix())+"/0", tokenData)
Expand Down Expand Up @@ -4064,7 +4064,7 @@ func TestActivityLog_partialMonthClientCountWithMultipleMountPaths(t *testing.T)
Clients: []*activity.EntityRecord{entityRecord},
})
if err != nil {
t.Fatalf(err.Error())
t.Fatal(err.Error())
}
storagePath := fmt.Sprintf("%sentity/%d/%d", ActivityLogPrefix, timeutil.StartOfMonth(now).Unix(), i)
WriteToStorage(t, core, storagePath, entityData)
Expand Down
2 changes: 1 addition & 1 deletion vault/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ func TestCore_HasVaultVersion(t *testing.T) {
upgradeTime := versionEntry.TimestampInstalled

if upgradeTime.After(time.Now()) || upgradeTime.Before(time.Now().Add(-1*time.Hour)) {
t.Fatalf("upgrade time isn't within reasonable bounds of new core initialization. " +
t.Fatal("upgrade time isn't within reasonable bounds of new core initialization. " +
fmt.Sprintf("time is: %+v, upgrade time is %+v", time.Now(), upgradeTime))
}
}
Expand Down
4 changes: 2 additions & 2 deletions vault/diagnose/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"sync"
"time"

wordwrap "github.com/mitchellh/go-wordwrap"
"github.com/mitchellh/go-wordwrap"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/codes"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
Expand Down Expand Up @@ -118,7 +118,7 @@ func (t *TelemetryCollector) OnStart(_ context.Context, s sdktrace.ReadWriteSpan
defer t.mu.Unlock()
t.spans[s.SpanContext().SpanID()] = s
if isMainSection(s) {
fmt.Fprintf(t.ui, status_unknown+s.Name())
fmt.Fprint(t.ui, status_unknown+s.Name())
}
}

Expand Down
Loading

0 comments on commit 9456671

Please sign in to comment.