Merge pull request #7583 from hashicorp/dnephin/id-printing

Fix printing of ID types

agent/agent_test.go

@@ -742,8 +742,7 @@ func TestAgent_CheckAliasRPC(t *testing.T) {
 		foundService := false
 		lookup := structs.NewServiceID("svcid1", structs.WildcardEnterpriseMeta())
 		for _, srv := range out.NodeServices.Services {
-			sid := srv.CompoundServiceID()
+			if lookup.Matches(srv.CompoundServiceID()) {
-			if lookup.Matches(&sid) {
 				foundService = true
 			}
 		}

agent/checks/alias.go

@@ -156,8 +156,7 @@ RETRY_CALL:
 		return false, err
 	}
 	for _, srv := range out.NodeServices.Services {
-		sid := srv.CompoundServiceID()
+		if serviceID.Matches(srv.CompoundServiceID()) {
-		if serviceID.Matches(&sid) {
 			return true, nil
 		}
 	}
@@ -250,8 +249,7 @@ func (c *CheckAlias) processChecks(checks []*structs.HealthCheck, CheckIfService
 		if c.Node != "" && c.Node != chk.Node {
 			continue
 		}
-		sid := chk.CompoundServiceID()
+		serviceMatch := c.ServiceID.Matches(chk.CompoundServiceID())
-		serviceMatch := c.ServiceID.Matches(&sid)
 		if chk.ServiceID != "" && !serviceMatch {
 			continue
 		}

agent/consul/discoverychain/compile.go

@@ -5,10 +5,11 @@ import (
 	"strings"
 	"time"
 
-	"github.com/hashicorp/consul/agent/connect"
-	"github.com/hashicorp/consul/agent/structs"
 	"github.com/mitchellh/hashstructure"
 	"github.com/mitchellh/mapstructure"
+
+	"github.com/hashicorp/consul/agent/connect"
+	"github.com/hashicorp/consul/agent/structs"
 )
 
 type CompileRequest struct {
@@ -721,7 +722,7 @@ func (c *compiler) getSplitterNode(sid structs.ServiceID) (*structs.DiscoveryGra
 		}
 
 		// Check to see if the split is eligible for additional splitting.
-		if !splitID.Matches(&sid) && split.ServiceSubset == "" {
+		if !splitID.Matches(sid) && split.ServiceSubset == "" {
 			nextNode, err := c.getSplitterNode(splitID)
 			if err != nil {
 				return nil, err

agent/consul/state/config_entry.go

@@ -4,11 +4,12 @@ import (
 	"errors"
 	"fmt"
 
+	memdb "github.com/hashicorp/go-memdb"
+
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/consul/discoverychain"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/lib"
-	memdb "github.com/hashicorp/go-memdb"
 )
 
 const (
@@ -473,7 +474,7 @@ func (s *Store) discoveryChainSourcesTxn(tx ReadTxn, ws memdb.WatchSet, dc strin
 			em := structs.EnterpriseMetaInitializer(t.Namespace)
 			candidate := structs.NewServiceName(t.Service, &em)
 
-			if !candidate.Matches(&destination) {
+			if !candidate.Matches(destination) {
 				continue
 			}
 			if idx > maxIdx {

agent/local/state.go

@@ -687,7 +687,7 @@ func (l *State) ChecksForService(serviceID structs.ServiceID, includeNodeChecks
 
 		if c.Check.ServiceID != "" {
 			sid := c.Check.CompoundServiceID()
-			if !serviceID.Matches(&sid) {
+			if !serviceID.Matches(sid) {
 				continue
 			}
 		} else if !includeNodeChecks {
@@ -1150,7 +1150,7 @@ func (l *State) deleteService(key structs.ServiceID) error {
 		for _, c := range l.checks {
 			if c.Deleted && c.Check != nil {
 				sid := c.Check.CompoundServiceID()
-				if sid.Matches(&key) {
+				if sid.Matches(key) {
 					l.pruneCheck(c.Check.CompoundCheckID())
 				}
 			}
@@ -1239,8 +1239,7 @@ func (l *State) syncService(key structs.ServiceID) error {
 		if c.Deleted || c.InSync {
 			continue
 		}
-		sid := c.Check.CompoundServiceID()
+		if !key.Matches(c.Check.CompoundServiceID()) {
-		if !key.Matches(&sid) {
 			continue
 		}
 		if st != l.checkToken(checkKey) {

agent/structs/config_entry.go

@@ -6,14 +6,15 @@ import (
 	"strings"
 	"time"
 
-	"github.com/hashicorp/consul/acl"
-	"github.com/hashicorp/consul/agent/cache"
-	"github.com/hashicorp/consul/lib"
-	"github.com/hashicorp/consul/lib/decode"
 	"github.com/hashicorp/go-msgpack/codec"
 	"github.com/hashicorp/go-multierror"
 	"github.com/mitchellh/hashstructure"
 	"github.com/mitchellh/mapstructure"
+
+	"github.com/hashicorp/consul/acl"
+	"github.com/hashicorp/consul/agent/cache"
+	"github.com/hashicorp/consul/lib"
+	"github.com/hashicorp/consul/lib/decode"
 )
 
 const (
@@ -599,7 +600,7 @@ type UpstreamConfigs []UpstreamConfig
 
 func (configs UpstreamConfigs) GetUpstreamConfig(sid ServiceID) (config map[string]interface{}, found bool) {
 	for _, usconf := range configs {
-		if usconf.Upstream.Matches(&sid) {
+		if usconf.Upstream.Matches(sid) {
 			return usconf.Config, true
 		}
 	}

agent/structs/config_entry_gateways.go

@@ -5,9 +5,10 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/miekg/dns"
+
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/lib/stringslice"
-	"github.com/miekg/dns"
 )
 
 // IngressGatewayConfigEntry manages the configuration for an ingress service
@@ -460,8 +461,8 @@ func (g *GatewayService) Addresses(defaultHosts []string) []string {
 }
 
 func (g *GatewayService) IsSame(o *GatewayService) bool {
-	return g.Gateway.Matches(&o.Gateway) &&
+	return g.Gateway.Matches(o.Gateway) &&
-		g.Service.Matches(&o.Service) &&
+		g.Service.Matches(o.Service) &&
 		g.GatewayKind == o.GatewayKind &&
 		g.Port == o.Port &&
 		g.Protocol == o.Protocol &&

agent/structs/structs.go

@@ -1754,7 +1754,7 @@ func NewCheckID(id types.CheckID, entMeta *EnterpriseMeta) CheckID {
 
 // StringHash is used mainly to populate part of the filename of a check
 // definition persisted on the local agent
-func (cid *CheckID) StringHash() string {
+func (cid CheckID) StringHash() string {
 	hasher := md5.New()
 	hasher.Write([]byte(cid.ID))
 	cid.EnterpriseMeta.addToHash(hasher, true)
@@ -1778,35 +1778,19 @@ func NewServiceID(id string, entMeta *EnterpriseMeta) ServiceID {
 	return sid
 }
 
-func (sid *ServiceID) Matches(other *ServiceID) bool {
+func (sid ServiceID) Matches(other ServiceID) bool {
-	if sid == nil && other == nil {
+	return sid.ID == other.ID && sid.EnterpriseMeta.Matches(&other.EnterpriseMeta)
-		return true
-	}
-
-	if sid == nil || other == nil || sid.ID != other.ID || !sid.EnterpriseMeta.Matches(&other.EnterpriseMeta) {
-		return false
-	}
-
-	return true
 }
 
 // StringHash is used mainly to populate part of the filename of a service
 // definition persisted on the local agent
-func (sid *ServiceID) StringHash() string {
+func (sid ServiceID) StringHash() string {
 	hasher := md5.New()
 	hasher.Write([]byte(sid.ID))
 	sid.EnterpriseMeta.addToHash(hasher, true)
 	return fmt.Sprintf("%x", hasher.Sum(nil))
 }
 
-func (sid *ServiceID) LessThan(other *ServiceID) bool {
-	if sid.EnterpriseMeta.LessThan(&other.EnterpriseMeta) {
-		return true
-	}
-
-	return sid.ID < other.ID
-}
-
 type IndexedNodes struct {
 	Nodes Nodes
 	QueryMeta
@@ -1837,30 +1821,14 @@ func NewServiceName(name string, entMeta *EnterpriseMeta) ServiceName {
 	return ret
 }
 
-func (n *ServiceName) Matches(o *ServiceName) bool {
+func (n ServiceName) Matches(o ServiceName) bool {
-	if n == nil && o == nil {
+	return n.Name == o.Name && n.EnterpriseMeta.Matches(&o.EnterpriseMeta)
-		return true
-	}
-
-	if n == nil || o == nil || n.Name != o.Name || !n.EnterpriseMeta.Matches(&o.EnterpriseMeta) {
-		return false
-	}
-
-	return true
 }
 
-func (n *ServiceName) ToServiceID() ServiceID {
+func (n ServiceName) ToServiceID() ServiceID {
 	return ServiceID{ID: n.Name, EnterpriseMeta: n.EnterpriseMeta}
 }
 
-func (n *ServiceName) LessThan(other *ServiceName) bool {
-	if n.EnterpriseMeta.LessThan(&other.EnterpriseMeta) {
-		return true
-	}
-
-	return n.Name < other.Name
-}
-
 type ServiceList []ServiceName
 
 type IndexedServiceList struct {

agent/structs/structs_oss.go

@@ -106,7 +106,7 @@ func ParseServiceIDString(input string) (string, *EnterpriseMeta) {
 	return input, DefaultEnterpriseMeta()
 }
 
-func (sid *ServiceID) String() string {
+func (sid ServiceID) String() string {
 	return sid.ID
 }
 
@@ -119,7 +119,7 @@ func ParseServiceNameString(input string) (string, *EnterpriseMeta) {
 	return input, DefaultEnterpriseMeta()
 }
 
-func (n *ServiceName) String() string {
+func (n ServiceName) String() string {
 	return n.Name
 }
 
@@ -128,7 +128,7 @@ func ServiceNameFromString(input string) ServiceName {
 	return ServiceName{Name: id}
 }
 
-func (cid *CheckID) String() string {
+func (cid CheckID) String() string {
 	return string(cid.ID)
 }
 

agent/structs/structs_oss_test.go

@@ -0,0 +1,41 @@
+package structs
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestServiceID_String(t *testing.T) {
+	t.Run("value", func(t *testing.T) {
+		sid := NewServiceID("the-id", &EnterpriseMeta{})
+		require.Equal(t, "the-id", fmt.Sprintf("%v", sid))
+	})
+	t.Run("pointer", func(t *testing.T) {
+		sid := NewServiceID("the-id", &EnterpriseMeta{})
+		require.Equal(t, "the-id", fmt.Sprintf("%v", &sid))
+	})
+}
+
+func TestCheckID_String(t *testing.T) {
+	t.Run("value", func(t *testing.T) {
+		cid := NewCheckID("the-id", &EnterpriseMeta{})
+		require.Equal(t, "the-id", fmt.Sprintf("%v", cid))
+	})
+	t.Run("pointer", func(t *testing.T) {
+		cid := NewCheckID("the-id", &EnterpriseMeta{})
+		require.Equal(t, "the-id", fmt.Sprintf("%v", &cid))
+	})
+}
+
+func TestServiceName_String(t *testing.T) {
+	t.Run("value", func(t *testing.T) {
+		sn := NewServiceName("the-id", &EnterpriseMeta{})
+		require.Equal(t, "the-id", fmt.Sprintf("%v", sn))
+	})
+	t.Run("pointer", func(t *testing.T) {
+		sn := NewServiceName("the-id", &EnterpriseMeta{})
+		require.Equal(t, "the-id", fmt.Sprintf("%v", &sn))
+	})
+}