From d0c2f88aba38c5722d80ba72e3039c13d23b9dbf Mon Sep 17 00:00:00 2001 From: Paul Banks Date: Thu, 7 Jun 2018 12:26:11 +0100 Subject: [PATCH] More misc comment cleanup --- connect/resolver.go | 10 +++++----- connect/service.go | 24 +++++++++++++++++++----- connect/service_test.go | 6 ------ connect/tls.go | 11 ++++++++--- 4 files changed, 32 insertions(+), 19 deletions(-) diff --git a/connect/resolver.go b/connect/resolver.go index 47b006388..b65bada06 100644 --- a/connect/resolver.go +++ b/connect/resolver.go @@ -28,16 +28,16 @@ type Resolver interface { Resolve(ctx context.Context) (addr string, certURI connect.CertURI, err error) } -// StaticResolver is a statically defined resolver. This can be used to connect -// to an known-Connect endpoint without performing service discovery. +// StaticResolver is a statically defined resolver. This can be used to Dial a +// known Connect endpoint without performing service discovery. type StaticResolver struct { // Addr is the network address (including port) of the instance. It must be - // the connect-enabled mTLS server and may be a proxy in front of the actual + // the connect-enabled mTLS listener and may be a proxy in front of the actual // target service process. It is a string in any valid form for passing - // directly to `net.Dial("tcp", addr)`. + // directly to net.Dial("tcp", addr). Addr string - // CertURL is the _identity_ we expect the server to present in it's TLS + // CertURL is the identity we expect the server to present in it's TLS // certificate. It must be an exact URI string match or the connection will be // rejected. CertURI connect.CertURI diff --git a/connect/service.go b/connect/service.go index 4db4ce8ca..5c6dd2ced 100644 --- a/connect/service.go +++ b/connect/service.go @@ -20,8 +20,6 @@ import ( // This can represent a service that only is a server, only is a client, or // both. // -// TODO(banks): API for monitoring status of certs from app -// // TODO(banks): Agent implicit health checks based on knowing which certs are // available should prevent clients being routed until the agent knows the // service has been delivered valid certificates. Once built, document that here @@ -137,6 +135,14 @@ func NewDevServiceWithTLSConfig(serviceName string, logger *log.Logger, // to usable certificates due to not being initially setup yet or a prolonged // error during renewal. The listener will be able to accept connections again // once connectivity is restored provided the client's Token is valid. +// +// To prevent routing traffic to the app instance while it's certificates are +// invalid or not populated yet you may use Ready in a health check endpoint +// and/or ReadyWait during startup before starting the TLS listener. The latter +// only prevents connections during initial bootstrap (including permission +// issues where certs can never be issued due to bad credentials) but won't +// handle the case that certificates expire and an error prevents timely +// renewal. func (s *Service) ServerTLSConfig() *tls.Config { return s.tlsCfg.Get(newServerSideVerifier(s.client, s.service)) } @@ -148,6 +154,10 @@ func (s *Service) ServerTLSConfig() *tls.Config { // depending on the Resolver implementation. // // Timeout can be managed via the Context. +// +// Calls to Dial made before the Service has loaded certificates from the agent +// will fail. You can prevent this by using Ready or ReadyWait in app during +// startup. func (s *Service) Dial(ctx context.Context, resolver Resolver) (net.Conn, error) { addr, certURI, err := resolver.Resolve(ctx) if err != nil { @@ -289,9 +299,13 @@ func (s *Service) Ready() bool { } // ReadyWait returns a chan that is closed when the the Service becomes ready -// for use. Note that if the Service is ready when it is called it returns a nil -// chan. Ready means that it has root and leaf certificates configured which we -// assume are valid. +// for use for the first time. Note that if the Service is ready when it is +// called it returns a nil chan. Ready means that it has root and leaf +// certificates configured which we assume are valid. The service may +// subsequently stop being "ready" if it's certificates expire or are revoked +// and an error prevents new ones being loaded but this method will not stop +// returning a nil chan in that case. It is only useful for initial startup. For +// ongoing health Ready() should be used. func (s *Service) ReadyWait() <-chan struct{} { return s.tlsCfg.ReadyWait() } diff --git a/connect/service_test.go b/connect/service_test.go index ba4a9b0a0..c284b9c6e 100644 --- a/connect/service_test.go +++ b/connect/service_test.go @@ -215,12 +215,6 @@ func TestService_HTTPClient(t *testing.T) { }() <-testSvr.Listening - // TODO(banks): this will talk http2 on both client and server. I hit some - // compatibility issues when testing though need to make sure that the http - // server with our TLSConfig can actually support HTTP/1.1 as well. Could make - // this a table test with all 4 permutations of client/server http version - // support. - // Still get connection refused some times so retry on those retry.Run(t, func(r *retry.R) { // Hook the service resolver to avoid needing full agent setup. diff --git a/connect/tls.go b/connect/tls.go index db96eb3de..4ca448371 100644 --- a/connect/tls.go +++ b/connect/tls.go @@ -337,9 +337,14 @@ func (cfg *dynamicTLSConfig) Ready() bool { return cfg.leaf != nil && cfg.roots != nil } -// ReadyWait returns a chan that is closed when the the tlsConfig becomes Ready -// for use. Note that if the config is ready when it is called it returns a nil -// chan. +// ReadyWait returns a chan that is closed when the the Service becomes ready +// for use for the first time. Note that if the Service is ready when it is +// called it returns a nil chan. Ready means that it has root and leaf +// certificates configured which we assume are valid. The service may +// subsequently stop being "ready" if it's certificates expire or are revoked +// and an error prevents new ones being loaded but this method will not stop +// returning a nil chan in that case. It is only useful for initial startup. For +// ongoing health Ready() should be used. func (cfg *dynamicTLSConfig) ReadyWait() <-chan struct{} { return cfg.readyCh }