From acfdbb23a97d365fcf0be7958320ee3e9320878f Mon Sep 17 00:00:00 2001
From: Dan Upton <daniel@floppy.co>
Date: Wed, 9 Nov 2022 14:54:57 +0000
Subject: [PATCH] chore: remove unused argument from
 MergeNodeServiceWithCentralConfig (#15024)

Previously, the MergeNodeServiceWithCentralConfig method accepted a
ServiceSpecificRequest argument, of which only the Datacenter and
QueryOptions fields were used.

Digging a little deeper, it turns out these fields were only passed
down to the ComputeResolvedServiceConfig method (through the
ServiceConfigRequest struct) which didn't actually use them.

As such, not all call-sites passed a valid ServiceSpecificRequest
so it's safer to remove the argument altogether to prevent future
changes from depending on it.
---
 agent/configentry/merge_service_config.go             |  3 ---
 agent/consul/catalog_endpoint.go                      |  8 ++------
 agent/consul/health_endpoint.go                       |  2 +-
 .../services/dataplane/get_envoy_bootstrap_params.go  |  6 ------
 agent/proxycfg-sources/catalog/config_source.go       | 11 +----------
 5 files changed, 4 insertions(+), 26 deletions(-)

diff --git a/agent/configentry/merge_service_config.go b/agent/configentry/merge_service_config.go
index b131b7e7a..20e22720b 100644
--- a/agent/configentry/merge_service_config.go
+++ b/agent/configentry/merge_service_config.go
@@ -23,7 +23,6 @@ type StateStore interface {
 func MergeNodeServiceWithCentralConfig(
 	ws memdb.WatchSet,
 	state StateStore,
-	args *structs.ServiceSpecificRequest,
 	ns *structs.NodeService,
 	logger hclog.Logger) (uint64, *structs.NodeService, error) {
 
@@ -47,8 +46,6 @@ func MergeNodeServiceWithCentralConfig(
 
 	configReq := &structs.ServiceConfigRequest{
 		Name:           serviceName,
-		Datacenter:     args.Datacenter,
-		QueryOptions:   args.QueryOptions,
 		MeshGateway:    ns.Proxy.MeshGateway,
 		Mode:           ns.Proxy.Mode,
 		UpstreamIDs:    upstreams,
diff --git a/agent/consul/catalog_endpoint.go b/agent/consul/catalog_endpoint.go
index bf0492a7b..665977695 100644
--- a/agent/consul/catalog_endpoint.go
+++ b/agent/consul/catalog_endpoint.go
@@ -753,7 +753,7 @@ func (c *Catalog) ServiceNodes(args *structs.ServiceSpecificRequest, reply *stru
 					mergedsn := sn
 					ns := sn.ToNodeService()
 					if ns.IsSidecarProxy() || ns.IsGateway() {
-						cfgIndex, mergedns, err := configentry.MergeNodeServiceWithCentralConfig(ws, state, args, ns, c.logger)
+						cfgIndex, mergedns, err := configentry.MergeNodeServiceWithCentralConfig(ws, state, ns, c.logger)
 						if err != nil {
 							return err
 						}
@@ -957,11 +957,7 @@ func (c *Catalog) NodeServiceList(args *structs.NodeSpecificRequest, reply *stru
 				for _, ns := range services.Services {
 					mergedns := ns
 					if ns.IsSidecarProxy() || ns.IsGateway() {
-						serviceSpecificReq := structs.ServiceSpecificRequest{
-							Datacenter:   args.Datacenter,
-							QueryOptions: args.QueryOptions,
-						}
-						cfgIndex, mergedns, err = configentry.MergeNodeServiceWithCentralConfig(ws, state, &serviceSpecificReq, ns, c.logger)
+						cfgIndex, mergedns, err = configentry.MergeNodeServiceWithCentralConfig(ws, state, ns, c.logger)
 						if err != nil {
 							return err
 						}
diff --git a/agent/consul/health_endpoint.go b/agent/consul/health_endpoint.go
index 28823d958..855ffbfc0 100644
--- a/agent/consul/health_endpoint.go
+++ b/agent/consul/health_endpoint.go
@@ -257,7 +257,7 @@ func (h *Health) ServiceNodes(args *structs.ServiceSpecificRequest, reply *struc
 				for _, node := range resolvedNodes {
 					ns := node.Service
 					if ns.IsSidecarProxy() || ns.IsGateway() {
-						cfgIndex, mergedns, err := configentry.MergeNodeServiceWithCentralConfig(ws, state, args, ns, h.logger)
+						cfgIndex, mergedns, err := configentry.MergeNodeServiceWithCentralConfig(ws, state, ns, h.logger)
 						if err != nil {
 							return err
 						}
diff --git a/agent/grpc-external/services/dataplane/get_envoy_bootstrap_params.go b/agent/grpc-external/services/dataplane/get_envoy_bootstrap_params.go
index 08f53578f..5323dab05 100644
--- a/agent/grpc-external/services/dataplane/get_envoy_bootstrap_params.go
+++ b/agent/grpc-external/services/dataplane/get_envoy_bootstrap_params.go
@@ -74,15 +74,9 @@ func (s *Server) GetEnvoyBootstrapParams(ctx context.Context, req *pbdataplane.G
 		NodeId:      string(svc.ID),
 	}
 
-	// This is awkward because it's designed for different requests, but
-	// this fakes the ServiceSpecificRequest so that we can reuse code.
 	_, ns, err := configentry.MergeNodeServiceWithCentralConfig(
 		nil,
 		store,
-		&structs.ServiceSpecificRequest{
-			Datacenter:   s.Datacenter,
-			QueryOptions: options,
-		},
 		svc.ToNodeService(),
 		logger,
 	)
diff --git a/agent/proxycfg-sources/catalog/config_source.go b/agent/proxycfg-sources/catalog/config_source.go
index a6d60c328..0f86a3a37 100644
--- a/agent/proxycfg-sources/catalog/config_source.go
+++ b/agent/proxycfg-sources/catalog/config_source.go
@@ -132,16 +132,7 @@ func (m *ConfigSource) startSync(closeCh <-chan chan struct{}, proxyID proxycfg.
 			return nil, err
 		}
 
-		_, ns, err = configentry.MergeNodeServiceWithCentralConfig(
-			ws,
-			store,
-			// TODO(agentless): it doesn't seem like we actually *need* any of the
-			// values on this request struct - we should try to remove the parameter
-			// in case that changes in the future as this call-site isn't passing them.
-			&structs.ServiceSpecificRequest{},
-			ns,
-			logger,
-		)
+		_, ns, err = configentry.MergeNodeServiceWithCentralConfig(ws, store, ns, logger)
 		if err != nil {
 			logger.Error("failed to merge with central config", "error", err.Error())
 			return nil, err