Improve pod logs related to Supervisor TLS certificate problems

This commit is contained in:
Ryan Richard
2023-09-08 09:22:10 -07:00
parent 33311714e5
commit ce567c481b
5 changed files with 82 additions and 24 deletions

View File

@@ -15,6 +15,8 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
"go.pinniped.dev/internal/certauthority"
"go.pinniped.dev/internal/httputil/requestutil"
"go.pinniped.dev/internal/plog"
)
// contextKey type is unexported to prevent collisions.
@@ -41,6 +43,17 @@ func withBootstrapPaths(handler http.Handler, paths ...string) http.Handler {
isBootstrap, _ := req.Context().Value(bootstrapKey).(*atomic.Bool)
if isBootstrap != nil && isBootstrap.Load() && !bootstrapPaths.Has(req.URL.Path) {
// When a user-provided cert was not found for a request path which requires it,
// then emit a log statement visible at the default log level.
plog.Warning("error finding user-provided TLS cert to use for for incoming request",
"proto", req.Proto,
"method", req.Method,
"host", req.Host,
"requestSNIServerName", requestutil.SNIServerName(req),
"path", req.URL.Path,
"remoteAddr", req.RemoteAddr,
)
http.Error(w, "pinniped supervisor has invalid TLS serving certificate configuration", http.StatusInternalServerError)
return
}

View File

@@ -521,30 +521,35 @@ func runSupervisor(ctx context.Context, podInfo *downward.PodInfo, cfg *supervis
c := ptls.Default(nil)
c.GetCertificate = func(info *tls.ClientHelloInfo) (*tls.Certificate, error) {
cert := dynamicTLSCertProvider.GetTLSCert(strings.ToLower(info.ServerName))
foundServerNameCert := cert != nil
defaultCert := dynamicTLSCertProvider.GetDefaultTLSCert()
if plog.Enabled(plog.LevelTrace) { // minor CPU optimization as this is generally just noise
host, port, _ := net.SplitHostPort(info.Conn.LocalAddr().String()) // error is safe to ignore here
plog.Trace("GetCertificate called",
"info.ServerName", info.ServerName,
"foundSNICert", cert != nil,
"foundDefaultCert", defaultCert != nil,
"host", host,
"port", port,
)
}
if cert == nil {
if !foundServerNameCert {
cert = defaultCert
}
// If we still don't have a cert for the request at this point, then using the bootstrapping cert,
// but in that case also set the request to fail unless it is a health check request.
usingBootstrapCert := false
if cert == nil {
usingBootstrapCert = true
setIsBootstrapConn(info.Context()) // make this connection only work for bootstrap requests
cert = bootstrapCert
}
// Emit logs visible at a higher level of logging than the default. Using Info level so the user
// can safely configure a production Supervisor to show this message if they choose.
plog.Info("choosing TLS cert for incoming request",
"requestSNIServerName", info.ServerName,
"foundCertForSNIServerNameFromFederationDomain", foundServerNameCert,
"foundDefaultCertFromSecret", defaultCert != nil,
"defaultCertSecretName", cfg.NamesConfig.DefaultTLSCertificateSecret,
"servingBootstrapHealthzCert", usingBootstrapCert,
"requestLocalAddr", info.Conn.LocalAddr().String(),
"requestRemoteAddr", info.Conn.RemoteAddr().String(),
)
return cert, nil
}