Skip to content

Commit

Permalink
Get rid of math/rand in favor of math/rand/v2 (#49294)
Browse files Browse the repository at this point in the history
* Get rid of math/rand in favor of math/rand/v2

* Update retryutils to use rand.N

* Update e ref
  • Loading branch information
espadolini committed Nov 21, 2024
1 parent 2a2a0ae commit 0ce99ba
Show file tree
Hide file tree
Showing 31 changed files with 94 additions and 112 deletions.
2 changes: 2 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ linters-settings:
deny:
- pkg: io/ioutil
desc: 'use "io" or "os" packages instead'
- pkg: math/rand$
desc: 'use "math/rand/v2" or "crypto/rand" instead'
- pkg: github.com/golang/protobuf
desc: 'use "google.golang.org/protobuf"'
- pkg: github.com/hashicorp/go-uuid
Expand Down
6 changes: 3 additions & 3 deletions api/utils/retryutils/jitter.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func FullJitter(d time.Duration) time.Duration {
return 0
}

return time.Duration(rand.Int64N(int64(d)))
return rand.N(d)
}

// HalfJitter is a jitter on the range [d/2, d). This is a large range and most
Expand All @@ -92,7 +92,7 @@ func HalfJitter(d time.Duration) time.Duration {
return d
}

return d - frac + time.Duration(rand.Int64N(int64(frac)))
return d - frac + rand.N(frac)
}

// SeventhJitter returns a jitter on the range [6d/7, d). Prefer smaller jitters
Expand All @@ -108,5 +108,5 @@ func SeventhJitter(d time.Duration) time.Duration {
return d
}

return d - frac + time.Duration(rand.Int64N(int64(frac)))
return d - frac + rand.N(frac)
}
2 changes: 1 addition & 1 deletion e
Submodule e updated from a7c9a7 to 627ab5
5 changes: 5 additions & 0 deletions e_imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ import (
_ "github.com/elimity-com/scim/schema"
_ "github.com/ghodss/yaml"
_ "github.com/go-jose/go-jose/v3"
_ "github.com/go-jose/go-jose/v3/json"
_ "github.com/go-piv/piv-go/piv"
_ "github.com/gogo/protobuf/proto"
_ "github.com/google/go-attestation/attest"
Expand Down Expand Up @@ -156,6 +157,7 @@ import (
_ "gopkg.in/check.v1"
_ "k8s.io/apimachinery/pkg/util/yaml"

_ "github.com/gravitational/teleport/api"
_ "github.com/gravitational/teleport/api/accessrequest"
_ "github.com/gravitational/teleport/api/breaker"
_ "github.com/gravitational/teleport/api/client"
Expand All @@ -173,11 +175,13 @@ import (
_ "github.com/gravitational/teleport/api/gen/proto/go/teleport/devicetrust/v1"
_ "github.com/gravitational/teleport/api/gen/proto/go/teleport/externalauditstorage/v1"
_ "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1"
_ "github.com/gravitational/teleport/api/gen/proto/go/teleport/identitycenter/v1"
_ "github.com/gravitational/teleport/api/gen/proto/go/teleport/label/v1"
_ "github.com/gravitational/teleport/api/gen/proto/go/teleport/loginrule/v1"
_ "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1"
_ "github.com/gravitational/teleport/api/gen/proto/go/teleport/okta/v1"
_ "github.com/gravitational/teleport/api/gen/proto/go/teleport/plugins/v1"
_ "github.com/gravitational/teleport/api/gen/proto/go/teleport/provisioning/v1"
_ "github.com/gravitational/teleport/api/gen/proto/go/teleport/resourceusage/v1"
_ "github.com/gravitational/teleport/api/gen/proto/go/teleport/samlidp/v1"
_ "github.com/gravitational/teleport/api/gen/proto/go/teleport/scim/v1"
Expand All @@ -202,6 +206,7 @@ import (
_ "github.com/gravitational/teleport/api/types/secreports"
_ "github.com/gravitational/teleport/api/types/secreports/convert/v1"
_ "github.com/gravitational/teleport/api/types/trait"
_ "github.com/gravitational/teleport/api/types/trait/convert/v1"
_ "github.com/gravitational/teleport/api/types/userloginstate"
_ "github.com/gravitational/teleport/api/types/wrappers"
_ "github.com/gravitational/teleport/api/utils"
Expand Down
17 changes: 6 additions & 11 deletions examples/dynamoathenamigration/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"errors"
"fmt"
"io"
"math/rand"
"math/rand/v2"
"os"
"path/filepath"
"sort"
Expand All @@ -39,6 +39,7 @@ import (

apievents "github.com/gravitational/teleport/api/types/events"
"github.com/gravitational/teleport/api/utils/prompt"
"github.com/gravitational/teleport/api/utils/retryutils"
"github.com/gravitational/teleport/lib/utils"
)

Expand Down Expand Up @@ -190,7 +191,7 @@ func (m *mockEmitter) EmitAuditEvent(ctx context.Context, in apievents.AuditEven
// Without it, in rare cases after 50 events still some worker were not able
// to read message because of other processing it immediately.
select {
case <-time.After(time.Duration(rand.Intn(50)+50) * time.Microsecond):
case <-time.After(retryutils.HalfJitter(100 * time.Microsecond)):
return nil
case <-ctx.Done():
return ctx.Err()
Expand Down Expand Up @@ -695,8 +696,6 @@ func generateExportFilesFromLines(t *testing.T, lines []string) (file *os.File,
}

func generateExportFileOfSize(t *testing.T, wantSize int, minDate, maxDate string) (*os.File, int) {
rnd := rand.New(rand.NewSource(time.Now().UnixNano()))

f, err := os.CreateTemp(t.TempDir(), "*")
require.NoError(t, err)
require.NoError(t, err)
Expand All @@ -705,7 +704,7 @@ func generateExportFileOfSize(t *testing.T, wantSize int, minDate, maxDate strin
var noOfEvents, size int
for size < wantSize {
noOfEvents++
line := eventLineFromTime(randomTime(t, minDate, maxDate, rnd).Format(time.DateOnly))
line := eventLineFromTime(randomTime(t, minDate, maxDate).Format(time.DateOnly))
size += len(line)
_, err = zw.Write([]byte(line + "\n"))
require.NoError(t, err)
Expand All @@ -717,16 +716,12 @@ func generateExportFileOfSize(t *testing.T, wantSize int, minDate, maxDate strin
return f, noOfEvents
}

func randomTime(t *testing.T, minStr, maxStr string, rnd *rand.Rand) time.Time {
func randomTime(t *testing.T, minStr, maxStr string) time.Time {
min, err := time.Parse(time.DateOnly, minStr)
require.NoError(t, err)
max, err := time.Parse(time.DateOnly, maxStr)
require.NoError(t, err)
minUnix := min.Unix()
maxUnix := max.Unix()
delta := maxUnix - minUnix
sec := rnd.Int63n(delta) + minUnix
return time.Unix(sec, 0)
return min.Add(rand.N(max.Sub(min)))
}

func eventLineFromTime(eventTime string) string {
Expand Down
4 changes: 2 additions & 2 deletions examples/teleport-usage/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"fmt"
"log"
"math"
"math/rand"
"math/rand/v2"
"os"
"reflect"
"strconv"
Expand Down Expand Up @@ -334,7 +334,7 @@ func (a *adaptiveRateLimiter) wait(permits float64) {
durationToWait := time.Duration(permits / a.permitCapacity * float64(time.Second))
time.Sleep(durationToWait)

if rand.Intn(10) == 0 {
if rand.N(10) == 0 {
a.adjustUp()
}
}
Expand Down
4 changes: 2 additions & 2 deletions integrations/lib/backoff/backoff.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ package backoff

import (
"context"
"math/rand"
"math/rand/v2"
"time"

"github.com/gravitational/trace"
Expand Down Expand Up @@ -58,7 +58,7 @@ func NewDecorrWithMul(base, cap time.Duration, mul int64, clock clockwork.Clock)
}

func (backoff *decorr) Do(ctx context.Context) error {
backoff.sleep = backoff.base + rand.Int63n(backoff.sleep*backoff.mul-backoff.base)
backoff.sleep = backoff.base + rand.N(backoff.sleep*backoff.mul-backoff.base)
if backoff.sleep > backoff.cap {
backoff.sleep = backoff.cap
}
Expand Down
9 changes: 4 additions & 5 deletions integrations/operator/controllers/resources/testlib/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ package testlib

import (
"context"
"math/rand"
"math/rand/v2"
"os"
"path/filepath"
"runtime"
Expand Down Expand Up @@ -88,12 +88,11 @@ func deleteNamespaceForTest(t *testing.T, kc kclient.Client, ns *core.Namespace)
require.NoError(t, err)
}

var letterRunes = []rune("abcdefghijklmnopqrstuvwxyz1234567890")

func ValidRandomResourceName(prefix string) string {
b := make([]rune, 5)
const letters = "abcdefghijklmnopqrstuvwxyz1234567890"
b := make([]byte, 5)
for i := range b {
b[i] = letterRunes[rand.Intn(len(letterRunes))]
b[i] = letters[rand.N(len(letters))]
}
return prefix + string(b)
}
Expand Down
2 changes: 1 addition & 1 deletion lib/auth/accountrecovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"encoding/binary"
"encoding/hex"
"fmt"
"math/rand"
"math/rand/v2"
"net/netip"
"strings"
"testing"
Expand Down
13 changes: 6 additions & 7 deletions lib/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import (
"io"
"log/slog"
"math/big"
insecurerand "math/rand"
mathrand "math/rand/v2"
"net"
"os"
"regexp"
Expand Down Expand Up @@ -1337,8 +1337,7 @@ func (a *Server) runPeriodicOperations() {
// to avoid contention on the database in case if there are multiple
// auth servers running - so they don't compete trying
// to update the same resources.
r := insecurerand.New(insecurerand.NewSource(a.GetClock().Now().UnixNano()))
period := defaults.HighResPollingPeriod + time.Duration(r.Intn(int(defaults.HighResPollingPeriod/time.Second)))*time.Second
period := retryutils.HalfJitter(2 * defaults.HighResPollingPeriod)

ticker := interval.NewMulti(
a.GetClock(),
Expand Down Expand Up @@ -1433,7 +1432,7 @@ func (a *Server) runPeriodicOperations() {
case <-a.closeCtx.Done():
return
case <-remoteClustersRefresh.Next():
a.refreshRemoteClusters(a.closeCtx, r)
a.refreshRemoteClusters(a.closeCtx)
}
}
}()
Expand Down Expand Up @@ -1862,7 +1861,7 @@ var (
)

// refreshRemoteClusters updates connection status of all remote clusters.
func (a *Server) refreshRemoteClusters(ctx context.Context, rnd *insecurerand.Rand) {
func (a *Server) refreshRemoteClusters(ctx context.Context) {
remoteClusters, err := a.Services.GetRemoteClusters(ctx)
if err != nil {
log.WithError(err).Error("Failed to load remote clusters for status refresh")
Expand All @@ -1876,7 +1875,7 @@ func (a *Server) refreshRemoteClusters(ctx context.Context, rnd *insecurerand.Ra
}

// randomize the order to optimize for multiple auth servers running in parallel
rnd.Shuffle(len(remoteClusters), func(i, j int) {
mathrand.Shuffle(len(remoteClusters), func(i, j int) {
remoteClusters[i], remoteClusters[j] = remoteClusters[j], remoteClusters[i]
})

Expand Down Expand Up @@ -4828,7 +4827,7 @@ func (a *Server) PingInventory(ctx context.Context, req proto.InventoryPingReque
return proto.InventoryPingResponse{}, trace.NotFound("no control stream found for server %q", req.ServerID)
}

id := insecurerand.Uint64()
id := mathrand.Uint64()

if !req.ControlLog {
// this ping doesn't pass through the control log, so just execute it immediately.
Expand Down
8 changes: 4 additions & 4 deletions lib/auth/keystore/gcp_kms_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"crypto/x509/pkix"
"encoding/pem"
"math/big"
"math/rand"
mathrandv1 "math/rand" //nolint:depguard // only used for deterministic output
"net"
"strings"
"sync"
Expand Down Expand Up @@ -206,7 +206,7 @@ func (f *fakeGCPKMSServer) AsymmetricSign(ctx context.Context, req *kmspb.Asymme
return nil, trace.BadParameter("unsupported digest type %T", typedDigest)
}

testRand := rand.New(rand.NewSource(0))
testRand := mathrandv1.New(mathrandv1.NewSource(0))
sig, err := signer.Sign(testRand, digest, alg)
if err != nil {
return nil, trace.Wrap(err)
Expand Down Expand Up @@ -517,7 +517,7 @@ func TestGCPKMSKeystore(t *testing.T) {
Key: clientPrivKey.SSHPublicKey(),
CertType: ssh.HostCert,
}
err = cert.SignCert(rand.New(rand.NewSource(0)), sshSigner)
err = cert.SignCert(mathrandv1.New(mathrandv1.NewSource(0)), sshSigner)
if tc.expectSignError {
require.Error(t, err, "expected to get error signing SSH cert")
return
Expand Down Expand Up @@ -545,7 +545,7 @@ func TestGCPKMSKeystore(t *testing.T) {
},
}
_, err = x509.CreateCertificate(
rand.New(rand.NewSource(0)),
mathrandv1.New(mathrandv1.NewSource(0)),
template,
tlsCA.Cert,
clientPrivKey.Public(),
Expand Down
6 changes: 2 additions & 4 deletions lib/auth/kube_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
package auth

import (
"crypto/rand"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
"math/rand"
"testing"
"time"

Expand Down Expand Up @@ -112,9 +112,7 @@ func newTestCSR(subj pkix.Name) ([]byte, error) {
x509CSR := &x509.CertificateRequest{
Subject: subj,
}
// Use math/rand to avoid blocking on system entropy.
rng := rand.New(rand.NewSource(0))
derCSR, err := x509.CreateCertificateRequest(rng, x509CSR, priv)
derCSR, err := x509.CreateCertificateRequest(rand.Reader, x509CSR, priv)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion lib/auth/password_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"context"
"encoding/base32"
"fmt"
"math/rand"
"math/rand/v2"
"testing"
"time"

Expand Down
13 changes: 5 additions & 8 deletions lib/auth/trustedcluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ package auth
import (
"context"
"fmt"
insecurerand "math/rand"
"testing"
"time"

Expand All @@ -44,7 +43,6 @@ import (
func TestRemoteClusterStatus(t *testing.T) {
ctx := context.Background()
a := newTestAuthServer(ctx, t)
rnd := insecurerand.New(insecurerand.NewSource(a.GetClock().Now().UnixNano()))

rc, err := types.NewRemoteCluster("rc")
require.NoError(t, err)
Expand All @@ -53,7 +51,7 @@ func TestRemoteClusterStatus(t *testing.T) {

// This scenario deals with only one remote cluster, so it never hits the limit on status updates.
// TestRefreshRemoteClusters focuses on verifying the update limit logic.
a.refreshRemoteClusters(ctx, rnd)
a.refreshRemoteClusters(ctx)

wantRC := rc
// Initially, no tunnels exist and status should be "offline".
Expand Down Expand Up @@ -83,7 +81,7 @@ func TestRemoteClusterStatus(t *testing.T) {
require.NoError(t, err)
require.NoError(t, a.UpsertTunnelConnection(tc2))

a.refreshRemoteClusters(ctx, rnd)
a.refreshRemoteClusters(ctx)

// With active tunnels, the status is "online" and last_heartbeat is set to
// the latest tunnel heartbeat.
Expand All @@ -96,7 +94,7 @@ func TestRemoteClusterStatus(t *testing.T) {
// Delete the latest connection.
require.NoError(t, a.DeleteTunnelConnection(tc2.GetClusterName(), tc2.GetName()))

a.refreshRemoteClusters(ctx, rnd)
a.refreshRemoteClusters(ctx)

// The status should remain the same, since tc1 still exists.
// The last_heartbeat should remain the same, since tc1 has an older
Expand All @@ -109,7 +107,7 @@ func TestRemoteClusterStatus(t *testing.T) {
// Delete the remaining connection
require.NoError(t, a.DeleteTunnelConnection(tc1.GetClusterName(), tc1.GetName()))

a.refreshRemoteClusters(ctx, rnd)
a.refreshRemoteClusters(ctx)

// The status should switch to "offline".
// The last_heartbeat should remain the same.
Expand Down Expand Up @@ -162,7 +160,6 @@ func TestRefreshRemoteClusters(t *testing.T) {
require.LessOrEqual(t, tt.clustersNeedUpdate, tt.clustersTotal)

a := newTestAuthServer(ctx, t)
rnd := insecurerand.New(insecurerand.NewSource(a.GetClock().Now().UnixNano()))

allClusters := make(map[string]types.RemoteCluster)
for i := 0; i < tt.clustersTotal; i++ {
Expand All @@ -186,7 +183,7 @@ func TestRefreshRemoteClusters(t *testing.T) {
}
}

a.refreshRemoteClusters(ctx, rnd)
a.refreshRemoteClusters(ctx)

clusters, err := a.GetRemoteClusters(ctx)
require.NoError(t, err)
Expand Down
Loading

0 comments on commit 0ce99ba

Please sign in to comment.