Merge pull request #14143 from hashicorp/cleanup-slice-sets-3

cleanup: more cleanup of slices that are really sets

go.mod

@@ -64,7 +64,7 @@ require (
 	github.com/hashicorp/go-plugin v1.4.3
 	github.com/hashicorp/go-secure-stdlib/listenerutil v0.1.4
 	github.com/hashicorp/go-secure-stdlib/strutil v0.1.2
-	github.com/hashicorp/go-set v0.1.2
+	github.com/hashicorp/go-set v0.1.4
 	github.com/hashicorp/go-sockaddr v1.0.2
 	github.com/hashicorp/go-syslog v1.0.0
 	github.com/hashicorp/go-uuid v1.0.2

go.sum

@@ -760,8 +760,8 @@ github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 h1:kes8mmyCpxJsI7FTwtzRqEy9
 github.com/hashicorp/go-secure-stdlib/strutil v0.1.2/go.mod h1:Gou2R9+il93BqX25LAKCLuM+y9U2T4hlwvT1yprcna4=
 github.com/hashicorp/go-secure-stdlib/tlsutil v0.1.1 h1:Yc026VyMyIpq1UWRnakHRG01U8fJm+nEfEmjoAb00n8=
 github.com/hashicorp/go-secure-stdlib/tlsutil v0.1.1/go.mod h1:l8slYwnJA26yBz+ErHpp2IRCLr0vuOMGBORIz4rRiAs=
-github.com/hashicorp/go-set v0.1.2 h1:WqFkeT32zKiD/l7zwO1RLF4YwctJwp6IByML0LLa0os=
-github.com/hashicorp/go-set v0.1.2/go.mod h1:0jTQeDo6GKX0WMFUV4IicFkxXo9DuoRnUODngpsoYCk=
+github.com/hashicorp/go-set v0.1.4 h1:LADVZzgPFLNI20TII1l75bnBI0LTnxpoCOPNjozNpi8=
+github.com/hashicorp/go-set v0.1.4/go.mod h1:XFMEKCP3rGoZUBvdYwC9k2YVDj8PsMU/B0ITuYkl8IA=
 github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt+ozgdvDeDU=
 github.com/hashicorp/go-sockaddr v1.0.2 h1:ztczhD1jLxIRjVejw8gFomI1BQZOe2WoVOu0SyteCQc=
 github.com/hashicorp/go-sockaddr v1.0.2/go.mod h1:rB4wwRAUzs07qva3c5SdrY/NEtAUjGlgmH/UkBUC97A=

helper/funcs.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	multierror "github.com/hashicorp/go-multierror"
+	"github.com/hashicorp/go-set"
 	"github.com/hashicorp/hcl/hcl/ast"
 	"golang.org/x/exp/constraints"
 )
@@ -683,3 +684,31 @@ OUTER:
 	}
 	return true
 }
+
+// SliceSetEq returns true if slices a and b contain the same elements (in no
+// particular order), using '==' for comparison.
+//
+// Note: for pointers, consider implementing an Equals method and using
+// ElementsEquals instead.
+func SliceSetEq[T comparable](a, b []T) bool {
+	lenA, lenB := len(a), len(b)
+	if lenA != lenB {
+		return false
+	}
+
+	if lenA > 10 {
+		// avoid quadratic comparisons over large input
+		return set.From(a).EqualSlice(b)
+	}
+
+OUTER:
+	for _, item := range a {
+		for _, other := range b {
+			if item == other {
+				continue OUTER
+			}
+		}
+		return false
+	}
+	return true
+}

helper/funcs_test.go

@@ -593,3 +593,37 @@ func Test_ElementsEquals(t *testing.T) {
 		must.False(t, ElementsEquals(b, a))
 	})
 }
+
+func Test_SliceSetEq(t *testing.T) {
+	t.Run("empty", func(t *testing.T) {
+		a := make([]int, 0)
+		b := make([]int, 0)
+		must.True(t, SliceSetEq(a, b))
+	})
+
+	t.Run("subset small", func(t *testing.T) {
+		a := []int{1, 2, 3, 4, 5}
+		b := []int{1, 2, 3}
+		must.False(t, SliceSetEq(a, b))
+		must.False(t, SliceSetEq(b, a))
+	})
+
+	t.Run("subset large", func(t *testing.T) {
+		a := []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}
+		b := []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}
+		must.False(t, SliceSetEq(a, b))
+		must.False(t, SliceSetEq(b, a))
+	})
+
+	t.Run("same small", func(t *testing.T) {
+		a := []int{1, 2, 3, 4, 5}
+		b := []int{1, 2, 3, 4, 5}
+		must.True(t, SliceSetEq(a, b))
+	})
+
+	t.Run("same large", func(t *testing.T) {
+		a := []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}
+		b := []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}
+		must.True(t, SliceSetEq(a, b))
+	})
+}

nomad/structs/services.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/go-multierror"
+	"github.com/hashicorp/go-set"
 	"github.com/hashicorp/nomad/helper"
 	"github.com/hashicorp/nomad/helper/args"
 	"github.com/hashicorp/nomad/helper/pointer"
@@ -1363,29 +1364,13 @@ func (p *ConsulProxy) Copy() *ConsulProxy {
 		return nil
 	}
 
-	newP := &ConsulProxy{
+	return &ConsulProxy{
 		LocalServiceAddress: p.LocalServiceAddress,
 		LocalServicePort:    p.LocalServicePort,
 		Expose:              p.Expose.Copy(),
+		Upstreams:           slices.Clone(p.Upstreams),
+		Config:              helper.CopyMap(p.Config),
 	}
-
-	if n := len(p.Upstreams); n > 0 {
-		newP.Upstreams = make([]ConsulUpstream, n)
-
-		for i := range p.Upstreams {
-			newP.Upstreams[i] = *p.Upstreams[i].Copy()
-		}
-	}
-
-	if n := len(p.Config); n > 0 {
-		newP.Config = make(map[string]interface{}, n)
-
-		for k, v := range p.Config {
-			newP.Config[k] = v
-		}
-	}
-
-	return newP
 }
 
 // opaqueMapsEqual compares map[string]interface{} commonly used for opaque
@@ -1492,61 +1477,16 @@ type ConsulUpstream struct {
 	MeshGateway ConsulMeshGateway
 }
 
-func upstreamsEquals(a, b []ConsulUpstream) bool {
-	if len(a) != len(b) {
-		return false
-	}
-
-LOOP: // order does not matter
-	for _, upA := range a {
-		for _, upB := range b {
-			if upA.Equals(&upB) {
-				continue LOOP
-			}
-		}
-		return false
-	}
-	return true
-}
-
-// Copy the stanza recursively. Returns nil if u is nil.
-func (u *ConsulUpstream) Copy() *ConsulUpstream {
-	if u == nil {
-		return nil
-	}
-
-	return &ConsulUpstream{
-		DestinationName:      u.DestinationName,
-		DestinationNamespace: u.DestinationNamespace,
-		LocalBindPort:        u.LocalBindPort,
-		Datacenter:           u.Datacenter,
-		LocalBindAddress:     u.LocalBindAddress,
-		MeshGateway:          u.MeshGateway.Copy(),
-	}
-}
-
 // Equals returns true if the structs are recursively equal.
 func (u *ConsulUpstream) Equals(o *ConsulUpstream) bool {
 	if u == nil || o == nil {
 		return u == o
 	}
+	return *u == *o
+}
 
-	switch {
-	case u.DestinationName != o.DestinationName:
-		return false
-	case u.DestinationNamespace != o.DestinationNamespace:
-		return false
-	case u.LocalBindPort != o.LocalBindPort:
-		return false
-	case u.Datacenter != o.Datacenter:
-		return false
-	case u.LocalBindAddress != o.LocalBindAddress:
-		return false
-	case !u.MeshGateway.Equals(o.MeshGateway):
-		return false
-	}
-
-	return true
+func upstreamsEquals(a, b []ConsulUpstream) bool {
+	return set.From(a).Equal(set.From(b))
 }
 
 // ConsulExposeConfig represents a Consul Connect expose jobspec stanza.
@@ -1562,21 +1502,8 @@ type ConsulExposePath struct {
 	ListenerPort  string
 }
 
-func exposePathsEqual(pathsA, pathsB []ConsulExposePath) bool {
-	if len(pathsA) != len(pathsB) {
-		return false
-	}
-
-LOOP: // order does not matter
-	for _, pathA := range pathsA {
-		for _, pathB := range pathsB {
-			if pathA == pathB {
-				continue LOOP
-			}
-		}
-		return false
-	}
-	return true
+func exposePathsEqual(a, b []ConsulExposePath) bool {
+	return helper.SliceSetEq(a, b)
 }
 
 // Copy the stanza. Returns nil if e is nil.
@@ -2041,21 +1968,8 @@ func (l *ConsulIngressListener) Validate() error {
 	return nil
 }
 
-func ingressServicesEqual(servicesA, servicesB []*ConsulIngressService) bool {
-	if len(servicesA) != len(servicesB) {
-		return false
-	}
-
-COMPARE: // order does not matter
-	for _, serviceA := range servicesA {
-		for _, serviceB := range servicesB {
-			if serviceA.Equals(serviceB) {
-				continue COMPARE
-			}
-		}
-		return false
-	}
-	return true
+func ingressServicesEqual(a, b []*ConsulIngressService) bool {
+	return helper.ElementsEquals(a, b)
 }
 
 // ConsulIngressConfigEntry represents the Consul Configuration Entry type for
@@ -2116,21 +2030,8 @@ func (e *ConsulIngressConfigEntry) Validate() error {
 	return nil
 }
 
-func ingressListenersEqual(listenersA, listenersB []*ConsulIngressListener) bool {
-	if len(listenersA) != len(listenersB) {
-		return false
-	}
-
-COMPARE: // order does not matter
-	for _, listenerA := range listenersA {
-		for _, listenerB := range listenersB {
-			if listenerA.Equals(listenerB) {
-				continue COMPARE
-			}
-		}
-		return false
-	}
-	return true
+func ingressListenersEqual(a, b []*ConsulIngressListener) bool {
+	return helper.ElementsEquals(a, b)
 }
 
 type ConsulLinkedService struct {
@@ -2205,21 +2106,8 @@ func (s *ConsulLinkedService) Validate() error {
 	return nil
 }
 
-func linkedServicesEqual(servicesA, servicesB []*ConsulLinkedService) bool {
-	if len(servicesA) != len(servicesB) {
-		return false
-	}
-
-COMPARE: // order does not matter
-	for _, serviceA := range servicesA {
-		for _, serviceB := range servicesB {
-			if serviceA.Equals(serviceB) {
-				continue COMPARE
-			}
-		}
-		return false
-	}
-	return true
+func linkedServicesEqual(a, b []*ConsulLinkedService) bool {
+	return helper.ElementsEquals(a, b)
 }
 
 type ConsulTerminatingConfigEntry struct {