Use StructValue for logging struct values; Add checks to avoid panics…

… when passing nil pointers to StructValue
cloudfoundry · Sep 18, 2024 · 66da390 · 66da390
1 parent ff44964
commit 66da390
Show file tree

Hide file tree

Showing 7 changed files with 16 additions and 8 deletions.
diff --git a/config/config.go b/config/config.go
@@ -16,6 +16,8 @@ import (
 	"code.cloudfoundry.org/localip"
 	"go.step.sm/crypto/pemutil"
 	"gopkg.in/yaml.v3"
+
+	log "code.cloudfoundry.org/gorouter/logger"
 )
 
 const (
@@ -330,7 +332,7 @@ func checkClientCertificateMetadataRule(chain []*x509.Certificate, logger *slog.
 				return nil
 			}
 		}
-		logger.Warn("invalid-subject", slog.String("issuer", cert.Issuer.String()), slog.String("subject", cert.Subject.String()), slog.Any("allowed", rule.ValidSubjects))
+		logger.Warn("invalid-subject", slog.String("issuer", cert.Issuer.String()), slog.String("subject", cert.Subject.String()), slog.Any("allowed", log.StructValue(rule.ValidSubjects)))
 		return fmt.Errorf("subject not in the list of allowed subjects for CA Subject %q: %q", rule.CASubject, subject)
 	}
 	// this should never happen as the function is only called on successful client certificate verification as callback

diff --git a/logger/logger.go b/logger/logger.go
@@ -154,9 +154,14 @@ type StructWithLogValue struct {
 }
 
 func (r StructWithLogValue) LogValue() slog.Value {
+	if r.Value == nil || (reflect.ValueOf(r.Value).Kind() == reflect.Ptr && reflect.ValueOf(r.Value).IsNil()) {
+		return slog.GroupValue()
+	}
 	v := reflect.ValueOf(r.Value)
 	if v.Kind() == reflect.Interface || v.Kind() == reflect.Pointer {
 		v = v.Elem()
+	} else if v.Kind() != reflect.Struct {
+		return slog.GroupValue()
 	}
 	var values []slog.Attr
 	for i := 0; i < v.NumField(); i++ {

diff --git a/mbus/subscriber.go b/mbus/subscriber.go
@@ -243,7 +243,7 @@ func (s *Subscriber) registerEndpoint(msg *RegistryMessage) {
 	if err != nil {
 		s.logger.Error("Unable to register route",
 			log.ErrAttr(err),
-			slog.Any("message", msg),
+			slog.Any("message", log.StructValue(msg)),
 		)
 		return
 	}
@@ -258,7 +258,7 @@ func (s *Subscriber) unregisterEndpoint(msg *RegistryMessage) {
 	if err != nil {
 		s.logger.Error("Unable to unregister route",
 			log.ErrAttr(err),
-			slog.Any("message", msg),
+			slog.Any("message", log.StructValue(msg)),
 		)
 		return
 	}

diff --git a/proxy/round_tripper/proxy_round_tripper.go b/proxy/round_tripper/proxy_round_tripper.go
@@ -223,7 +223,7 @@ func (rt *roundTripper) RoundTrip(originalRequest *http.Request) (*http.Response
 		} else {
 			logger.Debug(
 				"route-service",
-				slog.Any("route-service-url", reqInfo.RouteServiceURL),
+				slog.Any("route-service-url", log.StructValue(reqInfo.RouteServiceURL)),
 				slog.Int("attempt", attempt),
 			)
 

diff --git a/registry/registry.go b/registry/registry.go
@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"code.cloudfoundry.org/gorouter/config"
+	log "code.cloudfoundry.org/gorouter/logger"
 	"code.cloudfoundry.org/gorouter/metrics"
 	"code.cloudfoundry.org/gorouter/registry/container"
 	"code.cloudfoundry.org/gorouter/route"
@@ -435,7 +436,7 @@ func buildSlogAttrs(uri route.Uri, endpoint *route.Endpoint) []any {
 		slog.String("instance_id", endpoint.PrivateInstanceId),
 		slog.String("server_cert_domain_san", endpoint.ServerCertDomainSAN),
 		slog.String("protocol", endpoint.Protocol),
-		slog.Any("modification_tag", endpoint.ModificationTag),
+		slog.Any("modification_tag", log.StructValue(endpoint.ModificationTag)),
 		isoSegField,
 		slog.Bool("isTLS", endpoint.IsTLS()),
 	}

diff --git a/route_fetcher/route_fetcher.go b/route_fetcher/route_fetcher.go
@@ -160,7 +160,7 @@ func (r *RouteFetcher) subscribeToEvents(token *oauth2.Token) error {
 			}
 			break
 		}
-		r.logger.Debug("received-event", slog.Any("event", event))
+		r.logger.Debug("received-event", slog.Any("event", log.StructValue(event)))
 		r.eventChannel <- event
 	}
 	return err

diff --git a/router/router.go b/router/router.go
@@ -322,7 +322,7 @@ func (r *Router) serveHTTPS(server *http.Server, errChan chan error) error {
 
 	r.tlsListener = tls.NewListener(listener, tlsConfig)
 
-	r.logger.Info("tls-listener-started", "address", log.StructValue(r.tlsListener.Addr()))
+	r.logger.Info("tls-listener-started", slog.Any("address", log.StructValue(r.tlsListener.Addr())))
 
 	go func() {
 		err := server.Serve(r.tlsListener)
@@ -366,7 +366,7 @@ func (r *Router) serveHTTP(server *http.Server, errChan chan error) error {
 		}
 	}
 
-	r.logger.Info("tcp-listener-started", "address", log.StructValue(r.listener.Addr()))
+	r.logger.Info("tcp-listener-started", slog.Any("address", log.StructValue(r.listener.Addr())))
 
 	go func() {
 		err := server.Serve(r.listener)