Fix resource.UniqueId to be properly ordered over multiple runs

helper/resource/id.go

@@ -1,21 +1,17 @@
 package resource
 
 import (
-	"crypto/rand"
 	"fmt"
-	"math/big"
+	"strings"
 	"sync"
+	"time"
 )
 
 const UniqueIdPrefix = `terraform-`
 
-// idCounter is a randomly seeded monotonic counter for generating ordered
-// unique ids.  It uses a big.Int so we can easily increment a long numeric
-// string.  The max possible hex value here with 12 random bytes is
-// "01000000000000000000000000", so there's no chance of rollover during
-// operation.
+// idCounter is a monotonic counter for generating ordered unique ids.
 var idMutex sync.Mutex
-var idCounter = big.NewInt(0).SetBytes(randomBytes(12))
+var idCounter uint32
 
 // Helper for a resource to generate a unique identifier w/ default prefix
 func UniqueId() string {
@@ -25,15 +21,20 @@ func UniqueId() string {
 // Helper for a resource to generate a unique identifier w/ given prefix
 //
 // After the prefix, the ID consists of an incrementing 26 digit value (to match
-// previous timestamp output).
+// previous timestamp output).  After the prefix, the ID consists of a timestamp
+// and an incrementing 8 hex digit value The timestamp means that multiple IDs
+// created with the same prefix will sort in the order of their creation, even
+// across multiple terraform executions, as long as the clock is not turned back
+// between calls, and as long as any given terraform execution generates fewer
+// than 4 billion IDs.
 func PrefixedUniqueId(prefix string) string {
+	// Be precise to 4 digits of fractional seconds, but remove the dot before the
+	// fractional seconds.
+	timestamp := strings.Replace(
+		time.Now().UTC().Format("20060102150405.0000"), ".", "", 1)
+
 	idMutex.Lock()
 	defer idMutex.Unlock()
-	return fmt.Sprintf("%s%026x", prefix, idCounter.Add(idCounter, big.NewInt(1)))
-}
-
-func randomBytes(n int) []byte {
-	b := make([]byte, n)
-	rand.Read(b)
-	return b
+	idCounter++
+	return fmt.Sprintf("%s%s%08x", prefix, timestamp, idCounter)
 }

helper/resource/id_test.go

@@ -4,11 +4,19 @@ import (
 	"regexp"
 	"strings"
 	"testing"
+	"time"
 )
 
+var allDigits = regexp.MustCompile(`^\d+$`)
 var allHex = regexp.MustCompile(`^[a-f0-9]+$`)
 
 func TestUniqueId(t *testing.T) {
+	split := func(rest string) (timestamp, increment string) {
+		return rest[:18], rest[18:]
+	}
+
+	const prefix = "terraform-"
+
 	iterations := 10000
 	ids := make(map[string]struct{})
 	var id, lastId string
@@ -19,18 +27,24 @@ func TestUniqueId(t *testing.T) {
 			t.Fatalf("Got duplicated id! %s", id)
 		}
 
-		if !strings.HasPrefix(id, "terraform-") {
+		if !strings.HasPrefix(id, prefix) {
 			t.Fatalf("Unique ID didn't have terraform- prefix! %s", id)
 		}
 
-		rest := strings.TrimPrefix(id, "terraform-")
+		rest := strings.TrimPrefix(id, prefix)
 
 		if len(rest) != 26 {
 			t.Fatalf("Post-prefix part has wrong length! %s", rest)
 		}
 
-		if !allHex.MatchString(rest) {
-			t.Fatalf("Random part not all hex! %s", rest)
+		timestamp, increment := split(rest)
+
+		if !allDigits.MatchString(timestamp) {
+			t.Fatalf("Timestamp not all digits! %s", timestamp)
+		}
+
+		if !allHex.MatchString(increment) {
+			t.Fatalf("Increment part not all hex! %s", increment)
 		}
 
 		if lastId != "" && lastId >= id {
@@ -40,4 +54,15 @@ func TestUniqueId(t *testing.T) {
 		ids[id] = struct{}{}
 		lastId = id
 	}
+
+	id1 := UniqueId()
+	time.Sleep(time.Millisecond)
+	id2 := UniqueId()
+	timestamp1, _ := split(strings.TrimPrefix(id1, prefix))
+	timestamp2, _ := split(strings.TrimPrefix(id2, prefix))
+
+	if timestamp1 == timestamp2 {
+		t.Fatalf("Timestamp part should update at least once a millisecond %s %s",
+			id1, id2)
+	}
 }