From c5f8d6fa5228c3f7f70c6a31600d71a90e958378 Mon Sep 17 00:00:00 2001 From: eshitachandwani Date: Sat, 21 Dec 2024 01:39:50 +0530 Subject: [PATCH] address comments --- internal/proxyattributes/proxyattributes.go | 6 ++---- .../delegatingresolver/delegatingresolver.go | 14 +++++++------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/internal/proxyattributes/proxyattributes.go b/internal/proxyattributes/proxyattributes.go index 21a3e6153409..d25d33efd373 100644 --- a/internal/proxyattributes/proxyattributes.go +++ b/internal/proxyattributes/proxyattributes.go @@ -31,15 +31,13 @@ type keyType string const proxyOptionsKey = keyType("grpc.resolver.delegatingresolver.proxyOptions") // Options holds the proxy connection details needed during the CONNECT -// handshake. It includes the user information and the connect address. +// handshake. type Options struct { User url.Userinfo ConnectAddr string } -// Set returns a copy of addr with attributes containing the provided user -// and connect address, which are needed during the CONNECT handshake for a -// proxy connection. +// Set returns a copy of addr with opts set in its attributes. func Set(addr resolver.Address, opts Options) resolver.Address { addr.Attributes = addr.Attributes.WithValue(proxyOptionsKey, opts) return addr diff --git a/internal/resolver/delegatingresolver/delegatingresolver.go b/internal/resolver/delegatingresolver/delegatingresolver.go index d5a8e1cc4aa8..81984cbc5584 100644 --- a/internal/resolver/delegatingresolver/delegatingresolver.go +++ b/internal/resolver/delegatingresolver/delegatingresolver.go @@ -188,12 +188,6 @@ func (r *delegatingResolver) updateClientConnStateLocked() error { return nil } - // Update curState to include target resolver's state information, such as - // the service config, provided by the target resolver. This ensures - // curState contains all necessary information when passed to UpdateState. - // The state update is only sent after both the target and proxy resolvers - // have sent their updates, and curState has been updated with the combined - // addresses. curState := *r.targetResolverState // If multiple resolved proxy addresses are present, we send only the // unresolved proxy host and let net.Dial handle the proxy host name @@ -242,6 +236,7 @@ func (r *delegatingResolver) updateClientConnStateLocked() error { } endpoints = append(endpoints, resolver.Endpoint{Addresses: addrs}) } + // Use the targetResolverState for its service config and attributes contents. curState.Addresses = addresses curState.Endpoints = endpoints return r.cc.UpdateState(curState) @@ -259,12 +254,17 @@ func (r *delegatingResolver) updateProxyResolverState(state resolver.State) erro logger.Infof("Addresses received from proxy resolver: %s", state.Addresses) } if len(state.Endpoints) > 0 { + + // We expect exactly one address per endpoint because the proxy + // resolver uses "dns" resolution. r.proxyAddrs = make([]resolver.Address, 0, len(state.Endpoints)) for _, endpoint := range state.Endpoints { r.proxyAddrs = append(r.proxyAddrs, endpoint.Addresses...) } - } else { + } else if state.Addresses != nil { r.proxyAddrs = state.Addresses + } else { + r.proxyAddrs = []resolver.Address{} // ensure proxyAddrs is non-nil to indicate an update has been received } err := r.updateClientConnStateLocked() // Another possible approach was to block until updates are received from