diff --git a/api/adc/redaction_test.go b/api/adc/redaction_test.go new file mode 100644 index 00000000..283efc0b --- /dev/null +++ b/api/adc/redaction_test.go @@ -0,0 +1,78 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package adc + +import ( + "bytes" + "encoding/json" + "testing" + + "github.com/go-logr/logr" + "github.com/go-logr/zapr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" +) + +// bufferLogger builds a logger identical to the production one (zapr + zap +// console encoder) but writing into buf, so we can assert on real log output. +func bufferLogger(buf *bytes.Buffer) logr.Logger { + core := zapcore.NewCore( + zapcore.NewConsoleEncoder(zap.NewDevelopmentEncoderConfig()), + zapcore.AddSync(buf), + zapcore.DebugLevel, + ) + return zapr.NewLogger(zap.New(core)) +} + +const secretPluginValue = "SUPER-SECRET-KAFKA-PASSWORD" + +func secretPluginMap() map[string]any { + return map[string]any{ + "kafka-logger": map[string]any{"sasl_config": map[string]any{"password": secretPluginValue}}, + "http-logger": map[string]any{"uri": "http://logs.example"}, + } +} + +// Plugin config is arbitrary user JSON that routinely carries credentials, so +// no plugin map may reach the log sink whole. +func TestPluginMapsMarshalLogEmitNamesOnly(t *testing.T) { + for name, value := range map[string]any{ + "plugins": Plugins(secretPluginMap()), + "globalRules": GlobalRule(secretPluginMap()), + "pluginMetadata": PluginMetadata(secretPluginMap()), + } { + t.Run(name, func(t *testing.T) { + var buf bytes.Buffer + bufferLogger(&buf).V(1).Info("site", name, value) + out := buf.String() + + assert.NotContains(t, out, secretPluginValue, "plugin config leaked into logs") + assert.Contains(t, out, "kafka-logger", "plugin name should survive for debugging") + assert.Contains(t, out, "http-logger") + }) + } +} + +// MarshalLog must not change what goes on the wire to the data plane. +func TestPluginMapsMarshalJSONUnaffected(t *testing.T) { + b, err := json.Marshal(Plugins(secretPluginMap())) + require.NoError(t, err) + assert.Contains(t, string(b), secretPluginValue) +} diff --git a/api/adc/types.go b/api/adc/types.go index f7adae1c..c7c67019 100644 --- a/api/adc/types.go +++ b/api/adc/types.go @@ -22,6 +22,7 @@ import ( "encoding/json" "errors" "fmt" + "sort" "strconv" "strings" "time" @@ -86,6 +87,11 @@ func (g *GlobalRule) DeepCopy() GlobalRule { return GlobalRule(copied) } +// MarshalLog implements logr.Marshaler. See Plugins.MarshalLog. +func (g GlobalRule) MarshalLog() any { + return pluginNames(g) +} + // +k8s:deepcopy-gen=true type GlobalRuleItem struct { Metadata `json:",inline" yaml:",inline"` @@ -101,6 +107,11 @@ func (p *PluginMetadata) DeepCopy() PluginMetadata { return PluginMetadata(copied) } +// MarshalLog implements logr.Marshaler. See Plugins.MarshalLog. +func (p PluginMetadata) MarshalLog() any { + return pluginNames(p) +} + // +k8s:deepcopy-gen=true type ConsumerGroup struct { Metadata `json:",inline" yaml:",inline"` @@ -400,6 +411,23 @@ func (p Plugins) DeepCopy() Plugins { return out } +// MarshalLog implements logr.Marshaler so logging a plugin map emits only the +// plugin names. Plugin config is arbitrary user JSON and routinely carries +// credentials (kafka SASL passwords, logger tokens, OIDC client secrets). +// It affects logging only, not the JSON sent to the data plane. +func (p Plugins) MarshalLog() any { + return pluginNames(p) +} + +func pluginNames(p map[string]any) []string { + names := make([]string, 0, len(p)) + for name := range p { + names = append(names, name) + } + sort.Strings(names) + return names +} + // UpstreamNode is the node in upstream type UpstreamNode struct { Host string `json:"host" yaml:"host"` diff --git a/internal/adc/translator/gateway.go b/internal/adc/translator/gateway.go index fd4d4dff..03739329 100644 --- a/internal/adc/translator/gateway.go +++ b/internal/adc/translator/gateway.go @@ -395,10 +395,10 @@ func (t *Translator) fillPluginMetadataFromGatewayProxy(pluginMetadata adctypes. for pluginName, plugin := range gatewayProxy.Spec.PluginMetadata { var pluginConfig map[string]any if err := json.Unmarshal(plugin.Raw, &pluginConfig); err != nil { - t.Log.Error(err, "gateway proxy plugin_metadata unmarshal failed", "plugin", pluginName, "config", string(plugin.Raw)) + t.Log.Error(err, "gateway proxy plugin_metadata unmarshal failed", "plugin", pluginName) continue } - t.Log.V(1).Info("fill plugin_metadata for gateway proxy", "plugin", pluginName, "config", pluginConfig) + t.Log.V(1).Info("fill plugin_metadata for gateway proxy", "plugin", pluginName) pluginMetadata[pluginName] = pluginConfig } } diff --git a/internal/controller/consumer_controller.go b/internal/controller/consumer_controller.go index f3fb80a9..230255a2 100644 --- a/internal/controller/consumer_controller.go +++ b/internal/controller/consumer_controller.go @@ -232,7 +232,7 @@ func (r *ConsumerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c rk := utils.NamespacedNameKind(consumer) if err := ProcessGatewayProxy(r.Client, r.Log, tctx, gateway, rk); err != nil { - r.Log.Error(err, "failed to process gateway proxy", "gateway", gateway) + r.Log.Error(err, "failed to process gateway proxy", "gateway", utils.NamespacedName(gateway)) statusErr = err } diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index 8e0153bc..e34c2c40 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -174,7 +174,7 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } if err := r.Provider.Delete(ctx, hr); err != nil { - r.Log.Error(err, "failed to delete httproute", "httproute", hr) + r.Log.Error(err, "failed to delete httproute", "httproute", utils.NamespacedName(hr)) return ctrl.Result{}, err } return ctrl.Result{}, nil @@ -217,7 +217,7 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( APIVersion: gatewayv1.GroupVersion.String(), } if err := r.Provider.Delete(ctx, hr); err != nil { - r.Log.Error(err, "failed to delete httproute", "httproute", hr) + r.Log.Error(err, "failed to delete httproute", "httproute", utils.NamespacedName(hr)) return ctrl.Result{}, err } return ctrl.Result{}, nil