From 0a1f9156dc3d471faf0a9c2165c870a28e8951a1 Mon Sep 17 00:00:00 2001
From: David Glasser <glasser@davidglasser.net>
Date: Tue, 13 Jun 2017 13:58:31 -0700
Subject: [PATCH] Fix resource.UniqueId to be properly ordered over multiple
 runs

The timestamp prefix added in #8249 was removed in #10152 to ensure that
returned IDs really are properly ordered.  However, this meant that IDs were no
longer ordered over multiple invocations of terraform, which was the main
motivation for adding the timestamp in the first place.  This commit does a
hybrid: timestamp-plus-incrementing-counter instead of just incrementing counter
or timestamp-plus-random.
---
 helper/resource/id.go      | 33 +++++++++++++++++----------------
 helper/resource/id_test.go | 33 +++++++++++++++++++++++++++++----
 2 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/helper/resource/id.go b/helper/resource/id.go
index 629582b3a..1cde67c1a 100644
--- a/helper/resource/id.go
+++ b/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)
 }
diff --git a/helper/resource/id_test.go b/helper/resource/id_test.go
index f2f10b2ed..7e5f27bfb 100644
--- a/helper/resource/id_test.go
+++ b/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)
+	}
 }