From 81ac9dc24e0c47031d27cc19c59ab32167817e2a Mon Sep 17 00:00:00 2001 From: Travis Raines Date: Wed, 16 Dec 2020 13:45:41 -0800 Subject: [PATCH] feat(parser) debug log config on update failure Add a new --log-failed-config flag. When enabled, if the reconciliation loop encounters an error, perform a diff against the last known good configuration and write it to a temporary file. At log level debug, dump a copy of the target configuration and a diff against the last known good configuration (if any) to disk. By default, redact certificate keys and credential secrets from the dump. The --log-sensitive-fields flag dumps these in the clear. These dumps can be used for offline troubleshooting outside the original Kubernetes environment. They are compatible with decK, and provide a way to inspect sync failures outside the controller in cases where the controller's error logging is ambiguous. --- cli/ingress-controller/flags.go | 6 ++-- cli/ingress-controller/main.go | 3 ++ go.mod | 1 + internal/ingress/controller/controller.go | 13 ++++++- internal/ingress/controller/kong.go | 35 +++++++++++++++++++ .../controller/parser/kongstate/consumer.go | 19 ++++++++++ .../controller/parser/kongstate/kongstate.go | 10 ++++++ 7 files changed, 84 insertions(+), 3 deletions(-) diff --git a/cli/ingress-controller/flags.go b/cli/ingress-controller/flags.go index 402dbc06c7..e4c289034e 100644 --- a/cli/ingress-controller/flags.go +++ b/cli/ingress-controller/flags.go @@ -77,8 +77,9 @@ type cliConfig struct { EnableReverseSync bool // Logging - LogLevel string - LogFormat string + LogLevel string + LogFormat string + LogSensitiveConfig bool // k8s connection details APIServerHost string @@ -310,6 +311,7 @@ func parseFlags() (cliConfig, error) { // Logging config.LogLevel = viper.GetString("log-level") config.LogFormat = viper.GetString("log-format") + config.LogSensitiveConfig = viper.GetBool("log-sensitive-config") // k8s connection details config.APIServerHost = viper.GetString("apiserver-host") diff --git a/cli/ingress-controller/main.go b/cli/ingress-controller/main.go index 192eb7d093..667e0d44c0 100644 --- a/cli/ingress-controller/main.go +++ b/cli/ingress-controller/main.go @@ -102,6 +102,9 @@ func controllerConfigFromCLIConfig(cliConfig cliConfig) controller.Configuration UpdateStatus: cliConfig.UpdateStatus, UpdateStatusOnShutdown: cliConfig.UpdateStatusOnShutdown, ElectionID: cliConfig.ElectionID, + + LogSensitiveConfig: cliConfig.LogSensitiveConfig, + LogLevel: cliConfig.LogLevel, } } diff --git a/go.mod b/go.mod index 11d50ca5aa..2660f27fd9 100644 --- a/go.mod +++ b/go.mod @@ -20,6 +20,7 @@ require ( github.com/stretchr/testify v1.5.1 github.com/tidwall/gjson v1.2.1 github.com/tidwall/match v1.0.1 // indirect + github.com/yudai/gojsondiff v1.0.0 golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208 gopkg.in/go-playground/assert.v1 v1.2.1 // indirect gopkg.in/go-playground/pool.v3 v3.1.1 diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 7fee5017f2..a3b4a557d9 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -19,6 +19,7 @@ package controller import ( "context" "fmt" + "io/ioutil" "sync" "sync/atomic" "time" @@ -90,7 +91,9 @@ type Configuration struct { EnableKnativeIngressSupport bool - Logger logrus.FieldLogger + Logger logrus.FieldLogger + LogSensitiveConfig bool + LogLevel string } // sync collects all the pieces required to assemble the configuration file and @@ -223,6 +226,7 @@ type KongController struct { backgroundGroup errgroup.Group runningConfigHash []byte + lastConfig []byte isShuttingDown uint32 @@ -231,12 +235,19 @@ type KongController struct { PluginSchemaStore PluginSchemaStore Logger logrus.FieldLogger + + tmpDir string } // Start starts a new master process running in foreground, blocking until the next call to // Stop. func (n *KongController) Start() { n.Logger.Debugf("startin up controller") + var err error + n.tmpDir, err = ioutil.TempDir("", "controller") + if err != nil { + panic(fmt.Errorf("failed to create a temporary working directory: %v", err)) + } ctx, cancel := context.WithCancel(context.Background()) defer cancel() diff --git a/internal/ingress/controller/kong.go b/internal/ingress/controller/kong.go index d6473298eb..ba03153bed 100644 --- a/internal/ingress/controller/kong.go +++ b/internal/ingress/controller/kong.go @@ -22,6 +22,7 @@ import ( "crypto/sha256" "encoding/json" "fmt" + "io/ioutil" "net/http" "reflect" "sort" @@ -36,8 +37,28 @@ import ( "github.com/kong/go-kong/kong" "github.com/kong/kubernetes-ingress-controller/internal/ingress/controller/parser/kongstate" "github.com/kong/kubernetes-ingress-controller/internal/ingress/utils" + "github.com/yudai/gojsondiff" + "github.com/yudai/gojsondiff/formatter" ) +func getDiff(a []byte, b []byte) (string, error) { + differ := gojsondiff.New() + d, err := differ.Compare(a, b) + if err != nil { + return "", err + } + var rightObject map[string]interface{} + err = json.Unmarshal(b, &rightObject) + if err != nil { + return "", err + } + + formatter := formatter.NewAsciiFormatter(rightObject, + formatter.AsciiFormatterConfig{}) + diffString, err := formatter.Format(d) + return diffString, err +} + // OnUpdate is called periodically by syncQueue to keep the configuration in sync. // returning nil implies the synchronization finished correctly. // Returning an error means requeue the update. @@ -72,10 +93,24 @@ func (n *KongController) OnUpdate(ctx context.Context, state *kongstate.KongStat } else { err = n.onUpdateDBMode(targetContent) } + var target []byte + if n.cfg.LogSensitiveConfig { + target, _ = json.Marshal(targetContent) + } else { + state.Sanitize() + sanitizedContent := n.toDeckContent(ctx, state) + target, _ = json.Marshal(sanitizedContent) + } if err != nil { + if n.cfg.LogLevel == "debug" { + diff, _ := getDiff(target, n.lastConfig) + _ = ioutil.WriteFile(n.tmpDir+"/target.json", target, 0600) + _ = ioutil.WriteFile(n.tmpDir+"/diff.json", []byte(diff), 0600) + } return err } n.runningConfigHash = shaSum + n.lastConfig = target n.Logger.Info("successfully synced configuration to kong") return nil } diff --git a/internal/ingress/controller/parser/kongstate/consumer.go b/internal/ingress/controller/parser/kongstate/consumer.go index f7622520a8..c7bd49c82c 100644 --- a/internal/ingress/controller/parser/kongstate/consumer.go +++ b/internal/ingress/controller/parser/kongstate/consumer.go @@ -24,6 +24,25 @@ type Consumer struct { K8sKongConsumer configurationv1.KongConsumer } +func (c *Consumer) SanitizeCredentials() { + redacted := "REDACTED" + for _, auth := range c.KeyAuths { + auth.Key = &redacted + } + for _, auth := range c.HMACAuths { + auth.Secret = &redacted + } + for _, auth := range c.JWTAuths { + auth.Secret = &redacted + } + for _, auth := range c.BasicAuths { + auth.Password = &redacted + } + for _, auth := range c.Oauth2Creds { + auth.ClientSecret = &redacted + } +} + func (c *Consumer) SetCredential(log logrus.FieldLogger, credType string, credConfig interface{}) error { switch credType { case "key-auth", "keyauth_credential": diff --git a/internal/ingress/controller/parser/kongstate/kongstate.go b/internal/ingress/controller/parser/kongstate/kongstate.go index 7ee8b1429d..47ff83fabd 100644 --- a/internal/ingress/controller/parser/kongstate/kongstate.go +++ b/internal/ingress/controller/parser/kongstate/kongstate.go @@ -302,6 +302,16 @@ func (ks *KongState) FillPlugins(log logrus.FieldLogger, s store.Storer) { ks.Plugins = buildPlugins(log, s, ks.getPluginRelations()) } +func (ks *KongState) Sanitize() { + redacted := "REDACTED" + for _, cert := range ks.Certificates { + cert.Certificate.Key = &redacted + } + for _, consumer := range ks.Consumers { + consumer.SanitizeCredentials() + } +} + var supportedCreds = sets.NewString( "acl", "basic-auth",