Address review feedback from @jefferai

helper/policyutil/policyutil.go

@@ -59,7 +59,7 @@ func SanitizePolicies(policies []string, addDefault bool) []string {
 	return strutil.RemoveDuplicates(policies)
 }
 
-// ComparePolicies checks whether the given policy sets are equivalent, as in,
+// EquivalentPolicies checks whether the given policy sets are equivalent, as in,
 // they contain the same values. The benefit of this method is that it leaves
 // the "default" policy out of its comparisons as it may be added later by core
 // after a set of policies has been saved by a backend.

helper/strutil/strutil.go

@@ -58,3 +58,49 @@ func RemoveDuplicates(items []string) []string {
 	sort.Strings(items)
 	return items
 }
+
+// EquivalentSlices checks whether the given string sets are equivalent, as in,
+// they contain the same values.
+func EquivalentSlices(a, b []string) bool {
+	if a == nil && b == nil {
+		return true
+	}
+
+	if a == nil || b == nil {
+		return false
+	}
+
+	// First we'll build maps to ensure unique values
+	mapA := map[string]bool{}
+	mapB := map[string]bool{}
+	for _, keyA := range a {
+		mapA[keyA] = true
+	}
+	for _, keyB := range b {
+		mapB[keyB] = true
+	}
+
+	// Now we'll build our checking slices
+	var sortedA, sortedB []string
+	for keyA, _ := range mapA {
+		sortedA = append(sortedA, keyA)
+	}
+	for keyB, _ := range mapB {
+		sortedB = append(sortedB, keyB)
+	}
+	sort.Strings(sortedA)
+	sort.Strings(sortedB)
+
+	// Finally, compare
+	if len(sortedA) != len(sortedB) {
+		return false
+	}
+
+	for i := range sortedA {
+		if sortedA[i] != sortedB[i] {
+			return false
+		}
+	}
+
+	return true
+}

helper/strutil/strutil_test.go

@@ -2,6 +2,19 @@ package strutil
 
 import "testing"
 
+func TestStrutil_EquivalentSlices(t *testing.T) {
+	slice1 := []string{"test1", "test2", "test3"}
+	slice2 := []string{"test1", "test2", "test3"}
+	if !EquivalentSlices(slice1, slice2) {
+		t.Fatalf("bad: expected a match")
+	}
+
+	slice2 = append(slice2, "test4")
+	if EquivalentSlices(slice1, slice2) {
+		t.Fatalf("bad: expected a mismatch")
+	}
+}
+
 func TestStrListContains(t *testing.T) {
 	haystack := []string{
 		"dev",

physical/consul.go

@@ -470,7 +470,7 @@ shutdown:
 				go func() {
 					defer atomic.CompareAndSwapInt64(&serviceRegLock, 1, 0)
 					for !shutdown {
-						serviceID, err := c.reconcileConsul(activeFunc, sealedFunc)
+						serviceID, err := c.reconcileConsul(registeredServiceID, activeFunc, sealedFunc)
 						if err != nil {
 							c.logger.Printf("[WARN]: physical/consul: reconcile unable to talk with Consul backend: %v", err)
 							time.Sleep(consulRetryInterval)
@@ -535,7 +535,7 @@ func (c *ConsulBackend) serviceID() string {
 // without any locks held and can be run concurrently, therefore no changes
 // to ConsulBackend can be made in this method (i.e. wtb const receiver for
 // compiler enforced safety).
-func (c *ConsulBackend) reconcileConsul(activeFunc activeFunction, sealedFunc sealedFunction) (serviceID string, err error) {
+func (c *ConsulBackend) reconcileConsul(registeredServiceID string, activeFunc activeFunction, sealedFunc sealedFunction) (serviceID string, err error) {
 	// Query vault Core for its current state
 	active := activeFunc()
 	sealed := sealedFunc()
@@ -558,7 +558,19 @@ func (c *ConsulBackend) reconcileConsul(activeFunc activeFunction, sealedFunc se
 
 	tags := c.fetchServiceTags(active)
 
-	if currentVaultService != nil {
+	var reregister bool
+
+	switch {
+	case currentVaultService == nil, registeredServiceID == "":
+		reregister = true
+	default:
+		switch {
+		case !strutil.EquivalentSlices(currentVaultService.ServiceTags, tags):
+			reregister = true
+		}
+	}
+
+	if !reregister {
 		// When re-registration is not required, return a valid serviceID
 		// to avoid registration in the next cycle.
 		return serviceID, nil