SC-7143 endpoint lookup and error handling #104
Conversation
| if err = s.checkEndpoint(peer); err != nil { | ||
| log.Warn().Err(err).Msg("could not fetch endpoint from remote peer") | ||
| return nil, fmt.Errorf("could not fetch endpoint from remote peer: %s", err) | ||
| } |
There was a problem hiding this comment.
@bbengfort @DanielSollis This might cause some issues when people get new certificates, I suppose we could just reboot the rVASPs but for the long term do we need to just have the rVASPs always do the lookup or manually flush the cache periodically?
| # Parse the pem certificate into a base64 string | ||
| certs = pem.parse_file(args.cert) | ||
| encoded = base64.b64encode(certs[0].as_bytes()) | ||
| template['signing_certificates'] = [{'data': encoded.decode('ascii')}] |
There was a problem hiding this comment.
This adds the signing certificate to the rVASP GDS record so that when we're testing in docker compose the peers lookup is able to retrieve it.
| for n in names: | ||
| if n in record['common_name']: | ||
| record['common_name'] = n | ||
| break |
There was a problem hiding this comment.
This was a bit awkward, but for docker compose testing we need the common names of the rVASPs to match the dns names that docker compose gives them (which are the service names in the .yaml). So I had to write this script to make sure the rVASP common names are "alice", "bob", "evil" since the peers cache is indexed by common name. Note that for deployment we just use the real common names (e.g., "api.alice.vaspbot.net") so this script and the other fixture generation script is only used to bootstrap testing.
pkg/rvasp/peer.go
Outdated
|
|
||
| // checkEndpoint ensures that the peer has an endpoint to connect to, and performs a | ||
| // lookup against the directory service if it does not. | ||
| func (s *Server) checkEndpoint(peer *peers.Peer) (err error) { |
There was a problem hiding this comment.
I would rename this something like "resolveEndpoint", "checkEndpoint" doesn't really communicate that the endpoint might also be set.
There was a problem hiding this comment.
That's a great point, I was struggling to think of a good name which communicates the behavior but I think resolveEndpoint is a bit better.
This PR fixes SC-7143 and SC-7144. The rVASPs now check for existence of the endpoint on the peer before trying to do key exchanges or transfers, and use the peers cache instead of always performing the GDS lookups. This also cleans up a lot of the error handling by returning errors in the envelope instead of in the gRPC error.