Limit logging of warnings from plugins

Author: stephank

internal/caddy/convert.go

@@ -12,15 +12,25 @@ import (
 type Converter struct{}
 
 func (c Converter) ConvertToCaddyConfig(store *store.Store) (interface{}, error) {
-	cfg := converter.NewConfig()
+	plugins := converter.Plugins(store.Options.PluginsOrder)
+
+	defer func() {
+		for _, p := range plugins {
+			if f, ok := p.(converter.Finalizer); ok {
+				f.Finalize()
+			}
+		}
+	}()
 
-	for _, p := range converter.Plugins(store.Options.PluginsOrder) {
+	cfg := converter.NewConfig()
+	for _, p := range plugins {
 		if m, ok := p.(converter.GlobalMiddleware); ok {
 			err := m.GlobalHandler(cfg, store)
 			if err != nil {
 				return cfg, err
 			}
 		}
 	}
+
 	return cfg, nil
 }

internal/caddy/ingress/reverseproxy.go

@@ -8,17 +8,24 @@ import (
 	"github.com/caddyserver/caddy/v2/caddyconfig"
 	"github.com/caddyserver/caddy/v2/modules/caddyhttp"
 	"github.com/caddyserver/caddy/v2/modules/caddyhttp/reverseproxy"
+	"github.com/caddyserver/ingress/internal/controller"
 	"github.com/caddyserver/ingress/pkg/converter"
 )
 
-type ReverseProxyPlugin struct{}
+type ReverseProxyPlugin struct {
+	diags controller.Diagnostics
+}
 
 func (p ReverseProxyPlugin) IngressPlugin() converter.PluginInfo {
 	return converter.PluginInfo{
 		Name: "ingress.reverseproxy",
 		// Should always go last by default
 		Priority: -10,
-		New:      func() converter.Plugin { return new(ReverseProxyPlugin) },
+		New: func() converter.Plugin {
+			return &ReverseProxyPlugin{
+				diags: make(controller.Diagnostics),
+			}
+		},
 	}
 }
 
@@ -32,22 +39,22 @@ func (p ReverseProxyPlugin) IngressHandler(input converter.IngressMiddlewareInpu
 
 	serviceRef := path.Backend.Service
 	if serviceRef == nil {
-		logger.Warnf("Ingress %s/%s uses a non-service backend, which is not supported, and will be ignored", ing.Namespace, ing.Name)
+		p.diags.Warnf(logger, "Ingress %s/%s uses a non-service backend, which is not supported, and will be ignored", ing.Namespace, ing.Name)
 		return input.Route, nil
 	}
 
 	serviceName := fmt.Sprintf("%s/%s", ing.Namespace, serviceRef.Name)
 	service := input.Store.Service(serviceName)
 	if service == nil {
-		logger.Warnf("Ingress %s/%s references unknown service %s and will be ignored", ing.Namespace, ing.Name, serviceRef.Name)
+		p.diags.Warnf(logger, "Ingress %s/%s references unknown service %s and will be ignored", ing.Namespace, ing.Name, serviceRef.Name)
 		return input.Route, nil
 	}
 
 	var upstreams reverseproxy.UpstreamPool
 	if service.Spec.Type == "ExternalName" {
 		// Create a single upstream for type=ExternalName.
 		if serviceRef.Port.Number == 0 {
-			logger.Warnf("Ingress %s/%s references service %s with type=ExternalName and a named port, which is not supported, and will be ignored", ing.Namespace, ing.Name, service.Name)
+			p.diags.Warnf(logger, "Ingress %s/%s references service %s with type=ExternalName and a named port, which is not supported, and will be ignored", ing.Namespace, ing.Name, service.Name)
 			return input.Route, nil
 		}
 		upstreams = reverseproxy.UpstreamPool{
@@ -61,14 +68,14 @@ func (p ReverseProxyPlugin) IngressHandler(input converter.IngressMiddlewareInpu
 				(serviceRef.Port.Name != "" && port.Name == serviceRef.Port.Name) {
 				targetPort = int32(port.TargetPort.IntValue())
 				if targetPort == 0 {
-					logger.Warnf("Ingress %s/%s references service %s with a named target port, which is not supported, and will be ignored", ing.Namespace, ing.Name, service.Name)
+					p.diags.Warnf(logger, "Ingress %s/%s references service %s with a named target port, which is not supported, and will be ignored", ing.Namespace, ing.Name, service.Name)
 					return input.Route, nil
 				}
 				break
 			}
 		}
 		if targetPort == 0 {
-			logger.Warnf("Ingress %s/%s references an unknown port on service %s, and will be ignored", ing.Namespace, ing.Name, service.Name)
+			p.diags.Warnf(logger, "Ingress %s/%s references an unknown port on service %s, and will be ignored", ing.Namespace, ing.Name, service.Name)
 			return input.Route, nil
 		}
 
@@ -119,6 +126,10 @@ func (p ReverseProxyPlugin) IngressHandler(input converter.IngressMiddlewareInpu
 	return input.Route, nil
 }
 
+func (p ReverseProxyPlugin) Finalize() {
+	p.diags.Gc()
+}
+
 func formatDialAddr(host string, port int32) string {
 	if strings.Contains(host, ":") {
 		return fmt.Sprintf("[%s]:%d", host, port)
@@ -156,4 +167,5 @@ func init() {
 // Interface guards
 var (
 	_ = converter.IngressMiddleware(ReverseProxyPlugin{})
+	_ = converter.Finalizer(ReverseProxyPlugin{})
 )

internal/controller/diagnostics.go

@@ -0,0 +1,39 @@
+package controller
+
+import (
+	"fmt"
+	"time"
+
+	"go.uber.org/zap"
+)
+
+const diagnosticTimeout = 60 * time.Second
+
+func getCutoff() time.Time {
+	return time.Now().Add(diagnosticTimeout * -1)
+}
+
+// Diagnostics is a simple map tracking when a diagnostic was last logged.
+type Diagnostics map[string]time.Time
+
+// Gc removes entries from the map that have timed out.
+func (diags Diagnostics) Gc() {
+	cutoff := getCutoff()
+	for key, t := range diags {
+		if t.Compare(cutoff) < 0 {
+			delete(diags, key)
+		}
+	}
+}
+
+// Warnf formats the the given message and checks if it is present in the map.
+// If not, the message will be logged at warning level and added to the map.
+func (diags Diagnostics) Warnf(logger *zap.SugaredLogger, template string, args ...any) bool {
+	key := fmt.Sprintf(template, args...)
+	if t, ok := diags[key]; ok && t.Compare(getCutoff()) >= 0 {
+		return false
+	}
+	diags[key] = time.Now()
+	logger.Warnf(template, args...)
+	return true
+}

pkg/converter/converter.go

@@ -38,6 +38,12 @@ type IngressMiddleware interface {
 	IngressHandler(input IngressMiddlewareInput) (*caddyhttp.Route, error)
 }
 
+// Finalizer is an optional interface plugins can implement to perform finalization.
+// Note that Finalize may be called even if an earlier plugin caused an error.
+type Finalizer interface {
+	Finalize()
+}
+
 type Plugin interface {
 	IngressPlugin() PluginInfo
 }