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.
This commit is contained in:
Dan Upton 2022-11-09 14:54:57 +00:00 committed by GitHub
parent 9cb7311abc
commit 7b2d08d461
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 4 additions and 26 deletions

View File

@ -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,

View File

@ -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
}

View File

@ -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
}

View File

@ -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,
)

View File

@ -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