mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: replace moby/moby namesgenerator with internal implementation (#21377)
Replace the external moby/moby/pkg/namesgenerator dependency with an
internal implementation using gofakeit/v7. The moby package has ~25k
unique name combinations, and with its retry parameter only adds a
random digit 0-9, giving ~250k possibilities. In parallel tests, this
has led to collisions (flakes).
The new internal API at coderd/util/namesgenerator eliminates the
external dependnecy and offers functions with explicit uniqueness
guarantees. This PR also consolidates fragmented name generation in a
few places to use the new package.
| Old (moby/moby) | New |
|-------------------------------------|------------------------|
| namesgenerator.GetRandomName(0) | NameWith("_") |
| namesgenerator.GetRandomName(>0) | NameDigitWith("_") |
| testutil.GetRandomName(t) | UniqueName() |
| testutil.GetRandomNameHyphenated(t) | UniqueNameWith("-") |
namesgenerator package API:
- NameWith(delim): random name, not unique
- NameDigitWith(delim): random name with 1-9 suffix, not unique
- UniqueName(): guaranteed unique via atomic counter
- UniqueNameWith(delim): unique with custom delimiter
Names continue to be docker style `[adjective][delim][surname]`. Unique
names are truncated to 32 characters (preserving the numeric suffix) to
fit common name length limits in Coder.
Related test flakes:
https://github.com/coder/internal/issues/1212
https://github.com/coder/internal/issues/118
https://github.com/coder/internal/issues/1068
This commit is contained in:
+2
-2
@@ -9,7 +9,6 @@ import (
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/google/uuid"
|
||||
"github.com/moby/moby/pkg/namesgenerator"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"cdr.dev/slog/v3"
|
||||
@@ -22,6 +21,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/rbac"
|
||||
"github.com/coder/coder/v2/coderd/rbac/policy"
|
||||
"github.com/coder/coder/v2/coderd/telemetry"
|
||||
"github.com/coder/coder/v2/coderd/util/namesgenerator"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
)
|
||||
|
||||
@@ -101,7 +101,7 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
}
|
||||
|
||||
tokenName := namesgenerator.GetRandomName(1)
|
||||
tokenName := namesgenerator.NameDigitWith("_")
|
||||
|
||||
if len(createToken.TokenName) != 0 {
|
||||
tokenName = createToken.TokenName
|
||||
|
||||
@@ -11,7 +11,6 @@ import (
|
||||
"testing"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/moby/moby/pkg/namesgenerator"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"golang.org/x/xerrors"
|
||||
@@ -22,6 +21,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/rbac"
|
||||
"github.com/coder/coder/v2/coderd/rbac/policy"
|
||||
"github.com/coder/coder/v2/coderd/rbac/regosql"
|
||||
"github.com/coder/coder/v2/coderd/util/namesgenerator"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/cryptorand"
|
||||
)
|
||||
@@ -439,10 +439,10 @@ func RandomRBACObject() rbac.Object {
|
||||
OrgID: uuid.NewString(),
|
||||
Type: randomRBACType(),
|
||||
ACLUserList: map[string][]policy.Action{
|
||||
namesgenerator.GetRandomName(1): {RandomRBACAction()},
|
||||
namesgenerator.UniqueName(): {RandomRBACAction()},
|
||||
},
|
||||
ACLGroupList: map[string][]policy.Action{
|
||||
namesgenerator.GetRandomName(1): {RandomRBACAction()},
|
||||
namesgenerator.UniqueName(): {RandomRBACAction()},
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -471,7 +471,7 @@ func RandomRBACSubject() rbac.Subject {
|
||||
return rbac.Subject{
|
||||
ID: uuid.NewString(),
|
||||
Roles: rbac.RoleIdentifiers{rbac.RoleMember()},
|
||||
Groups: []string{namesgenerator.GetRandomName(1)},
|
||||
Groups: []string{namesgenerator.UniqueName()},
|
||||
Scope: rbac.ScopeAll,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -30,17 +30,17 @@ import (
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
"unicode"
|
||||
|
||||
"cloud.google.com/go/compute/metadata"
|
||||
"github.com/fullsailor/pkcs7"
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/golang-jwt/jwt/v4"
|
||||
"github.com/google/uuid"
|
||||
"github.com/moby/moby/pkg/namesgenerator"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"golang.org/x/text/cases"
|
||||
"golang.org/x/text/language"
|
||||
"golang.org/x/xerrors"
|
||||
"google.golang.org/api/idtoken"
|
||||
"google.golang.org/api/option"
|
||||
@@ -80,6 +80,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/schedule"
|
||||
"github.com/coder/coder/v2/coderd/telemetry"
|
||||
"github.com/coder/coder/v2/coderd/updatecheck"
|
||||
"github.com/coder/coder/v2/coderd/util/namesgenerator"
|
||||
"github.com/coder/coder/v2/coderd/util/ptr"
|
||||
"github.com/coder/coder/v2/coderd/webpush"
|
||||
"github.com/coder/coder/v2/coderd/workspaceapps"
|
||||
@@ -792,7 +793,7 @@ func AuthzUserSubject(user codersdk.User, orgID uuid.UUID) rbac.Subject {
|
||||
|
||||
func createAnotherUserRetry(t testing.TB, client *codersdk.Client, organizationIDs []uuid.UUID, retries int, roles []rbac.RoleIdentifier, mutators ...func(r *codersdk.CreateUserRequestWithOrgs)) (*codersdk.Client, codersdk.User) {
|
||||
req := codersdk.CreateUserRequestWithOrgs{
|
||||
Email: namesgenerator.GetRandomName(10) + "@coder.com",
|
||||
Email: namesgenerator.UniqueName() + "@coder.com",
|
||||
Username: RandomUsername(t),
|
||||
Name: RandomName(t),
|
||||
Password: "SomeSecurePassword!",
|
||||
@@ -1556,37 +1557,15 @@ func NewAzureInstanceIdentity(t testing.TB, instanceID string) (x509.VerifyOptio
|
||||
}
|
||||
}
|
||||
|
||||
func RandomUsername(t testing.TB) string {
|
||||
suffix, err := cryptorand.String(3)
|
||||
require.NoError(t, err)
|
||||
suffix = "-" + suffix
|
||||
n := strings.ReplaceAll(namesgenerator.GetRandomName(10), "_", "-") + suffix
|
||||
if len(n) > 32 {
|
||||
n = n[:32-len(suffix)] + suffix
|
||||
}
|
||||
return n
|
||||
func RandomUsername(_ testing.TB) string {
|
||||
return namesgenerator.UniqueNameWith("-")
|
||||
}
|
||||
|
||||
func RandomName(t testing.TB) string {
|
||||
var sb strings.Builder
|
||||
var err error
|
||||
ss := strings.Split(namesgenerator.GetRandomName(10), "_")
|
||||
for si, s := range ss {
|
||||
for ri, r := range s {
|
||||
if ri == 0 {
|
||||
_, err = sb.WriteRune(unicode.ToTitle(r))
|
||||
require.NoError(t, err)
|
||||
} else {
|
||||
_, err = sb.WriteRune(r)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
}
|
||||
if si < len(ss)-1 {
|
||||
_, err = sb.WriteRune(' ')
|
||||
require.NoError(t, err)
|
||||
}
|
||||
}
|
||||
return sb.String()
|
||||
// RandomName returns a random name in title case (e.g. "Happy Einstein").
|
||||
func RandomName(_ testing.TB) string {
|
||||
return cases.Title(language.English).String(
|
||||
namesgenerator.NameWith(" "),
|
||||
)
|
||||
}
|
||||
|
||||
// Used to easily create an HTTP transport!
|
||||
|
||||
@@ -1,8 +1,11 @@
|
||||
package coderdtest_test
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
"unicode"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.uber.org/goleak"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/coderdtest"
|
||||
@@ -28,3 +31,22 @@ func TestNew(t *testing.T) {
|
||||
_, _ = coderdtest.NewGoogleInstanceIdentity(t, "example", false)
|
||||
_, _ = coderdtest.NewAWSInstanceIdentity(t, "an-instance")
|
||||
}
|
||||
|
||||
func TestRandomName(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for range 10 {
|
||||
name := coderdtest.RandomName(t)
|
||||
|
||||
require.NotEmpty(t, name, "name should not be empty")
|
||||
require.NotContains(t, name, "_", "name should not contain underscores")
|
||||
|
||||
// Should be title cased (e.g., "Happy Einstein").
|
||||
words := strings.Split(name, " ")
|
||||
require.Len(t, words, 2, "name should have exactly two words")
|
||||
for _, word := range words {
|
||||
firstRune := []rune(word)[0]
|
||||
require.True(t, unicode.IsUpper(firstRune), "word %q should start with uppercase letter", word)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,11 +12,11 @@ import (
|
||||
|
||||
"github.com/anthropics/anthropic-sdk-go"
|
||||
anthropicoption "github.com/anthropics/anthropic-sdk-go/option"
|
||||
"github.com/moby/moby/pkg/namesgenerator"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"cdr.dev/slog/v3"
|
||||
"github.com/coder/aisdk-go"
|
||||
"github.com/coder/coder/v2/coderd/util/namesgenerator"
|
||||
strutil "github.com/coder/coder/v2/coderd/util/strings"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
)
|
||||
@@ -125,10 +125,7 @@ func generateFallback() TaskName {
|
||||
// We have a 32 character limit for the name.
|
||||
// We have a 5 character suffix `-ffff`.
|
||||
// This leaves us with 27 characters for the name.
|
||||
//
|
||||
// `namesgenerator.GetRandomName(0)` can generate names
|
||||
// up to 27 characters, but we truncate defensively.
|
||||
name := strings.ReplaceAll(namesgenerator.GetRandomName(0), "_", "-")
|
||||
name := namesgenerator.NameWith("-")
|
||||
name = name[:min(len(name), 27)]
|
||||
name = strings.TrimSuffix(name, "-")
|
||||
|
||||
|
||||
@@ -16,7 +16,6 @@ import (
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/google/uuid"
|
||||
"github.com/moby/moby/pkg/namesgenerator"
|
||||
"github.com/sqlc-dev/pqtype"
|
||||
"github.com/zclconf/go-cty/cty"
|
||||
"golang.org/x/xerrors"
|
||||
@@ -37,6 +36,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/rbac"
|
||||
"github.com/coder/coder/v2/coderd/rbac/policy"
|
||||
"github.com/coder/coder/v2/coderd/tracing"
|
||||
"github.com/coder/coder/v2/coderd/util/namesgenerator"
|
||||
"github.com/coder/coder/v2/coderd/util/ptr"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/examples"
|
||||
@@ -1699,7 +1699,7 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht
|
||||
}
|
||||
|
||||
if req.Name == "" {
|
||||
req.Name = namesgenerator.GetRandomName(1)
|
||||
req.Name = namesgenerator.NameDigitWith("_")
|
||||
}
|
||||
|
||||
err = tx.InsertTemplateVersion(ctx, database.InsertTemplateVersionParams{
|
||||
|
||||
+2
-2
@@ -19,7 +19,6 @@ import (
|
||||
"github.com/go-jose/go-jose/v4/jwt"
|
||||
"github.com/google/go-github/v43/github"
|
||||
"github.com/google/uuid"
|
||||
"github.com/moby/moby/pkg/namesgenerator"
|
||||
"golang.org/x/oauth2"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
@@ -41,6 +40,7 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/render"
|
||||
"github.com/coder/coder/v2/coderd/telemetry"
|
||||
"github.com/coder/coder/v2/coderd/userpassword"
|
||||
"github.com/coder/coder/v2/coderd/util/namesgenerator"
|
||||
"github.com/coder/coder/v2/coderd/util/ptr"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/cryptorand"
|
||||
@@ -1723,7 +1723,7 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
|
||||
validUsername bool
|
||||
)
|
||||
for i := 0; i < 10; i++ {
|
||||
alternate := fmt.Sprintf("%s-%s", original, namesgenerator.GetRandomName(1))
|
||||
alternate := fmt.Sprintf("%s-%s", original, namesgenerator.NameDigitWith("_"))
|
||||
|
||||
params.Username = codersdk.UsernameFrom(alternate)
|
||||
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
// Package namesgenerator generates random names.
|
||||
//
|
||||
// This package provides functions for generating random names in the format
|
||||
// "adjective_surname" with various options for delimiters and uniqueness.
|
||||
//
|
||||
// For identifiers that must be unique within a process, use UniqueName or
|
||||
// UniqueNameWith. For display purposes where uniqueness is not required,
|
||||
// use NameWith.
|
||||
package namesgenerator
|
||||
|
||||
import (
|
||||
"math/rand/v2"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
|
||||
"github.com/brianvoe/gofakeit/v7"
|
||||
)
|
||||
|
||||
// maxNameLen is the maximum length for names. Many places in Coder have a 32
|
||||
// character limit for names (e.g. usernames, workspace names).
|
||||
const maxNameLen = 32
|
||||
|
||||
// counter provides unique suffixes for UniqueName functions.
|
||||
var counter atomic.Int64
|
||||
|
||||
// NameWith returns a random name with a custom delimiter.
|
||||
// Names are not guaranteed to be unique.
|
||||
func NameWith(delim string) string {
|
||||
const seed = 0 // gofakeit will use a random crypto seed.
|
||||
faker := gofakeit.New(seed)
|
||||
adjective := strings.ToLower(faker.AdjectiveDescriptive())
|
||||
last := strings.ToLower(faker.LastName())
|
||||
return adjective + delim + last
|
||||
}
|
||||
|
||||
// NameDigitWith returns a random name with a single random digit suffix (1-9),
|
||||
// in the format "[adjective][delim][surname][digit]" e.g. "happy_smith9".
|
||||
// Provides some collision resistance while keeping names short and clean.
|
||||
// Not guaranteed to be unique.
|
||||
func NameDigitWith(delim string) string {
|
||||
const (
|
||||
minDigit = 1
|
||||
maxDigit = 9
|
||||
)
|
||||
//nolint:gosec // The random digit doesn't need to be cryptographically secure.
|
||||
return NameWith(delim) + strconv.Itoa(rand.IntN(maxDigit-minDigit+1))
|
||||
}
|
||||
|
||||
// UniqueName returns a random name with a monotonically increasing suffix,
|
||||
// guaranteeing uniqueness within the process. The name is truncated to 32
|
||||
// characters if necessary, preserving the numeric suffix.
|
||||
func UniqueName() string {
|
||||
return UniqueNameWith("_")
|
||||
}
|
||||
|
||||
// UniqueNameWith returns a unique name with a custom delimiter.
|
||||
// See UniqueName for details on uniqueness guarantees.
|
||||
func UniqueNameWith(delim string) string {
|
||||
name := NameWith(delim) + strconv.FormatInt(counter.Add(1), 10)
|
||||
return truncate(name, maxNameLen)
|
||||
}
|
||||
|
||||
// truncate truncates a name to maxLen characters. It assumes the name ends with
|
||||
// a numeric suffix and preserves it, truncating the base name portion instead.
|
||||
func truncate(name string, maxLen int) string {
|
||||
if len(name) <= maxLen {
|
||||
return name
|
||||
}
|
||||
// Find where the numeric suffix starts.
|
||||
suffixStart := len(name)
|
||||
for suffixStart > 0 && name[suffixStart-1] >= '0' && name[suffixStart-1] <= '9' {
|
||||
suffixStart--
|
||||
}
|
||||
base := name[:suffixStart]
|
||||
suffix := name[suffixStart:]
|
||||
truncateAt := maxLen - len(suffix)
|
||||
if truncateAt <= 0 {
|
||||
return strconv.Itoa(maxLen) // Fallback, shouldn't happen in practice.
|
||||
}
|
||||
return base[:truncateAt] + suffix
|
||||
}
|
||||
@@ -0,0 +1,103 @@
|
||||
package namesgenerator
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
"unicode"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestTruncate(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
input string
|
||||
maxLen int
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "no truncation needed",
|
||||
input: "foo1",
|
||||
maxLen: 10,
|
||||
want: "foo1",
|
||||
},
|
||||
{
|
||||
name: "exact fit",
|
||||
input: "foo1",
|
||||
maxLen: 4,
|
||||
want: "foo1",
|
||||
},
|
||||
{
|
||||
name: "truncate base",
|
||||
input: "foobar42",
|
||||
maxLen: 5,
|
||||
want: "foo42",
|
||||
},
|
||||
{
|
||||
name: "truncate more",
|
||||
input: "foobar3",
|
||||
maxLen: 3,
|
||||
want: "fo3",
|
||||
},
|
||||
{
|
||||
name: "long suffix",
|
||||
input: "foo123456",
|
||||
maxLen: 8,
|
||||
want: "fo123456",
|
||||
},
|
||||
{
|
||||
name: "realistic name",
|
||||
input: "condescending_proskuriakova999999",
|
||||
maxLen: 32,
|
||||
want: "condescending_proskuriakov999999",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
got := truncate(tt.input, tt.maxLen)
|
||||
assert.Equal(t, tt.want, got)
|
||||
assert.LessOrEqual(t, len(got), tt.maxLen)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestUniqueNameLength(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Generate many names to exercise the truncation logic.
|
||||
const iter = 10000
|
||||
for range iter {
|
||||
name := UniqueName()
|
||||
assert.LessOrEqual(t, len(name), maxNameLen)
|
||||
assert.Contains(t, name, "_")
|
||||
assert.Equal(t, name, strings.ToLower(name))
|
||||
verifyNoWhitespace(t, name)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUniqueNameWithLength(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Generate many names with hyphen delimiter.
|
||||
const iter = 10000
|
||||
for range iter {
|
||||
name := UniqueNameWith("-")
|
||||
assert.LessOrEqual(t, len(name), maxNameLen)
|
||||
assert.Contains(t, name, "-")
|
||||
assert.Equal(t, name, strings.ToLower(name))
|
||||
verifyNoWhitespace(t, name)
|
||||
}
|
||||
}
|
||||
|
||||
func verifyNoWhitespace(t *testing.T, s string) {
|
||||
t.Helper()
|
||||
for _, r := range s {
|
||||
if unicode.IsSpace(r) {
|
||||
t.Fatalf("found whitespace in string %q: %v", s, r)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user