Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
eshitachandwani committed Dec 20, 2024
1 parent 69253c7 commit c5f8d6f
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 11 deletions.
6 changes: 2 additions & 4 deletions internal/proxyattributes/proxyattributes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions internal/resolver/delegatingresolver/delegatingresolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down

0 comments on commit c5f8d6f

Please sign in to comment.