Implement human-readable random identifier generation (#653)

* Allow human-readable identifier generation

* Apply suggestions from code review

* Use pet names for random identifiers

* fixup! Allow human-readable identifier generation

* fixup! Use pet names for random identifiers

Author: 0x2b3bfa0

cmd/create/create.go

@@ -102,13 +102,9 @@ func (o *Options) Run(cmd *cobra.Command, args []string, cloud *common.Cloud) er
 		cfg.Spot = common.Spot(common.SpotEnabled)
 	}
 
-	id := common.NewRandomIdentifier()
-
-	if o.Name != "" {
-		id = common.NewIdentifier(o.Name)
-		if identifier, err := common.ParseIdentifier(o.Name); err == nil {
-			id = identifier
-		}
+	id := common.NewRandomIdentifier(o.Name)
+	if identifier, err := common.ParseIdentifier(o.Name); err == nil {
+		id = identifier
 	}
 
 	ctx, cancel := context.WithTimeout(context.Background(), cloud.Timeouts.Create)

go.mod

@@ -23,6 +23,7 @@ require (
 	github.com/brianvoe/gofakeit/v6 v6.9.0
 	github.com/cloudflare/gokey v0.1.0
 	github.com/docker/go-units v0.4.0
+	github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0 // indirect
 	github.com/gobwas/glob v0.2.3
 	github.com/google/go-github/v42 v42.0.0
 	github.com/google/go-github/v45 v45.2.0

go.sum

@@ -325,6 +325,8 @@ github.com/dropbox/dropbox-sdk-go-unofficial/v6 v6.0.3 h1:h71/Ky9+298V45NSkxjhFv
 github.com/dropbox/dropbox-sdk-go-unofficial/v6 v6.0.3/go.mod h1:rSS3kM9XMzSQ6pw91Qgd6yB5jdt70N4OdtrAf74As5M=
 github.com/dustin/go-humanize v0.0.0-20180421182945-02af3965c54e/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
+github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0 h1:90Ly+6UfUypEF6vvvW5rQIv9opIL8CbmW9FT20LDQoY=
+github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0/go.mod h1:V+Qd57rJe8gd4eiGzZyg4h54VLHmYVVw54iMnlAMrF8=
 github.com/dvyukov/go-fuzz v0.0.0-20200318091601-be3528f3a813/go.mod h1:11Gm+ccJnvAhCNLlf5+cS9KjtbaD5I5zaZpFMsTHWTw=
 github.com/elazarl/go-bindata-assetfs v1.0.0/go.mod h1:v+YaWX3bdea5J/mo8dSETolEo7R71Vk1u8bnjau5yw4=
 github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153 h1:yUdfgN0XgIJw7foRItutHYUIhlcKzcSf5vDpdhQAKTc=

iterative/resource_task.go

@@ -374,16 +374,16 @@ func resourceTaskBuild(ctx context.Context, d *schema.ResourceData, m interface{
 			if newId, err := common.ParseIdentifier(name); err == nil {
 				id = newId
 			} else {
-				id = common.NewIdentifier(name)
+				id = common.NewDeterministicIdentifier(name)
 			}
 		} else if name := os.Getenv("GITHUB_RUN_ID"); name != "" {
-			id = common.NewIdentifier(name)
+			id = common.NewDeterministicIdentifier(name)
 		} else if name := os.Getenv("CI_PIPELINE_ID"); name != "" {
-			id = common.NewIdentifier(name)
+			id = common.NewDeterministicIdentifier(name)
 		} else if name := os.Getenv("BITBUCKET_STEP_TRIGGERER_UUID"); name != "" {
-			id = common.NewIdentifier(name)
+			id = common.NewDeterministicIdentifier(name)
 		} else {
-			id = common.NewRandomIdentifier()
+			id = common.NewRandomIdentifier("")
 		}
 	}
 

task/common/identifier.go

@@ -10,40 +10,56 @@ import (
 	"crypto/sha256"
 
 	"github.com/aohorodnyk/uid"
+	"github.com/dustinkirkland/golang-petname"
 )
 
-type Identifier string
+type Identifier struct {
+	name string
+	salt string
+}
 
 var ErrWrongIdentifier = errors.New("wrong identifier")
 
 const (
 	maximumLongLength = 50
 	shortLength       = 16
+	nameLength        = maximumLongLength-shortLength-uint32(len("tpi---"))
 )
 
 func ParseIdentifier(identifier string) (Identifier, error) {
 	re := regexp.MustCompile(`(?s)^tpi-([a-z0-9]+(?:[a-z0-9-]*[a-z0-9])?)-([a-z0-9]+)-([a-z0-9]+)$`)
 
 	if match := re.FindStringSubmatch(string(identifier)); len(match) > 0 && hash(match[1]+match[2], shortLength/2) == match[3] {
-		return Identifier(match[1]), nil
+		return Identifier{name: match[1], salt: match[2]}, nil
 	}
 
-	return Identifier(""), ErrWrongIdentifier
+	return Identifier{}, ErrWrongIdentifier
 }
 
-func NewIdentifier(identifier string) Identifier {
-	return Identifier(identifier)
+// NewDeterministicIdentifier returns a new deterministic Identifier, using the
+// provided name as a seed. Repeated calls to this function are guaranteed to
+// generate the same Identifier.
+func NewDeterministicIdentifier(name string) Identifier {
+	seed := normalize(name, nameLength)
+	return Identifier{name: name, salt: hash(seed, shortLength/2)}
 }
 
-func NewRandomIdentifier() Identifier {
-	return NewIdentifier(uid.NewProvider36Size(8).MustGenerate().String())
+// NewRandomIdentifier returns a new random Identifier. Repeated calls to this
+// function are guaranteed to generate different Identifiers, as long as there
+// are no collisions.
+func NewRandomIdentifier(name string) Identifier {
+	seed := uid.NewProvider36Size(8).MustGenerate().String()
+	if name == "" {
+		petname.NonDeterministicMode()
+		name = petname.Generate(3, "-")
+	}
+
+	return Identifier{name: name, salt: hash(seed, shortLength/2)}
 }
 
 func (i Identifier) Long() string {
-	name := normalize(string(i), maximumLongLength-shortLength-uint32(len("tpi---")))
-	digest := hash(name, shortLength/2)
-
-	return fmt.Sprintf("tpi-%s-%s-%s", name, digest, hash(name+digest, shortLength/2))
+	name := normalize(i.name, nameLength)
+	return fmt.Sprintf("tpi-%s-%s-%s", name, i.salt, hash(name+i.salt, shortLength/2))
 }
 
 func (i Identifier) Short() string {
@@ -52,9 +68,9 @@ func (i Identifier) Short() string {
 }
 
 // hash deterministically generates a Base36 digest of `size`
-// characters using `identifier` as the seed.
-func hash(identifier string, size uint8) string {
-	digest := sha256.Sum256([]byte(identifier))
+// characters using the provided seed.
+func hash(seed string, size uint8) string {
+	digest := sha256.Sum256([]byte(seed))
 	random := uid.NewRandCustom(bytes.NewReader(digest[:]))
 	encoder := uid.NewEncoderBase36()
 	provider := uid.NewProviderCustom(sha256.Size, random, encoder)

task/common/identifier_test.go

@@ -10,14 +10,14 @@ import (
 func TestIdentifier(t *testing.T) {
 	name := gofakeit.NewCrypto().Sentence(512)
 	t.Run("stability", func(t *testing.T) {
-		identifier := NewIdentifier(name)
+		identifier := NewDeterministicIdentifier(name)
 
 		require.Equal(t, identifier.Long(), identifier.Long())
 		require.Equal(t, identifier.Short(), identifier.Short())
 	})
 
-	t.Run("consistent", func(t *testing.T) {
-		identifier := NewIdentifier("5299fe10-79e9-4c3b-b15e-036e8e60ab6c")
+	t.Run("consistency", func(t *testing.T) {
+		identifier := NewDeterministicIdentifier("5299fe10-79e9-4c3b-b15e-036e8e60ab6c")
 		parsed, err := ParseIdentifier(identifier.Long())
 
 		require.NoError(t, err)
@@ -26,7 +26,7 @@ func TestIdentifier(t *testing.T) {
 	})
 
 	t.Run("homogeneity", func(t *testing.T) {
-		identifier := NewIdentifier(name)
+		identifier := NewDeterministicIdentifier(name)
 
 		long := identifier.Long()
 		short := identifier.Short()
@@ -39,13 +39,26 @@ func TestIdentifier(t *testing.T) {
 	})
 
 	t.Run("compatibility", func(t *testing.T) {
-		identifier := NewIdentifier("test")
+		identifier := NewDeterministicIdentifier("test")
 
 		require.Equal(t, "tpi-test-3z4xlzwq-3u0vweb4", identifier.Long())
 		require.Equal(t, "3z4xlzwq3u0vweb4", identifier.Short())
 
 		parsed, err := ParseIdentifier(identifier.Long())
-		require.Equal(t, parsed, identifier)
+		require.Equal(t, parsed.Long(), identifier.Long())
 		require.NoError(t, err)
 	})
+
+	t.Run("randomness", func(t *testing.T) {
+		name := "test"
+		
+		first := NewRandomIdentifier(name)
+		second := NewRandomIdentifier(name)
+
+		require.NotEqual(t, first.Long(), second.Long())
+		require.NotEqual(t, first.Short(), second.Short())
+
+		require.Contains(t, first.Long(), name)
+		require.Contains(t, second.Long(), name)
+	})
 }

task/task.go

@@ -2,7 +2,6 @@ package task
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"net"
 
@@ -31,10 +30,6 @@ func List(ctx context.Context, cloud common.Cloud) ([]common.Identifier, error)
 }
 
 func New(ctx context.Context, cloud common.Cloud, identifier common.Identifier, task common.Task) (Task, error) {
-	if identifier == "" {
-		return nil, errors.New("identifier must not be empty")
-	}
-
 	switch cloud.Provider {
 	case common.ProviderAWS:
 		return aws.New(ctx, cloud, identifier, task)

task/task_smoke_test.go

@@ -72,7 +72,7 @@ func TestTaskSmoke(t *testing.T) {
 				},
 			}
 
-			identifier := common.NewIdentifier(testName)
+			identifier := common.NewDeterministicIdentifier(testName)
 
 			task := common.Task{
 				Size: common.Size{