From 4bd17766a9fba97379e68e13149ee2c46adc96a7 Mon Sep 17 00:00:00 2001 From: Adam Chalmers Date: Thu, 6 May 2021 15:10:47 -0500 Subject: [PATCH] TUN-4359: Warn about unused keys in 'tunnel ingress validate' --- cmd/cloudflared/cliutil/handler.go | 27 +++++++++++++------ cmd/cloudflared/linux_service.go | 2 +- cmd/cloudflared/tunnel/cmd.go | 2 +- cmd/cloudflared/tunnel/ingress_subcommands.go | 9 +++++-- config/configuration.go | 25 ++++++++++++----- ingress/ingress_test.go | 4 +-- ingress/origin_request_config.go | 4 +-- 7 files changed, 50 insertions(+), 23 deletions(-) diff --git a/cmd/cloudflared/cliutil/handler.go b/cmd/cloudflared/cliutil/handler.go index 59e99c53..47c8af87 100644 --- a/cmd/cloudflared/cliutil/handler.go +++ b/cmd/cloudflared/cliutil/handler.go @@ -13,27 +13,38 @@ func Action(actionFunc cli.ActionFunc) cli.ActionFunc { } func ConfiguredAction(actionFunc cli.ActionFunc) cli.ActionFunc { + // Adapt actionFunc to the type signature required by ConfiguredActionWithWarnings + f := func(context *cli.Context, _ string) error { + return actionFunc(context) + } + + return ConfiguredActionWithWarnings(f) +} + +// Just like ConfiguredAction, but accepts a second parameter with configuration warnings. +func ConfiguredActionWithWarnings(actionFunc func(*cli.Context, string) error) cli.ActionFunc { return WithErrorHandler(func(c *cli.Context) error { - if err := setFlagsFromConfigFile(c); err != nil { + warnings, err := setFlagsFromConfigFile(c) + if err != nil { return err } - return actionFunc(c) + return actionFunc(c, warnings) }) } -func setFlagsFromConfigFile(c *cli.Context) error { +func setFlagsFromConfigFile(c *cli.Context) (configWarnings string, err error) { const errorExitCode = 1 log := logger.CreateLoggerFromContext(c, logger.EnableTerminalLog) - inputSource, err := config.ReadConfigFile(c, log) + inputSource, warnings, err := config.ReadConfigFile(c, log) if err != nil { if err == config.ErrNoConfigFile { - return nil + return "", nil } - return cli.Exit(err, errorExitCode) + return "", cli.Exit(err, errorExitCode) } if err := altsrc.ApplyInputSource(c, inputSource); err != nil { - return cli.Exit(err, errorExitCode) + return "", cli.Exit(err, errorExitCode) } - return nil + return warnings, nil } diff --git a/cmd/cloudflared/linux_service.go b/cmd/cloudflared/linux_service.go index bade05eb..d70c42ba 100644 --- a/cmd/cloudflared/linux_service.go +++ b/cmd/cloudflared/linux_service.go @@ -238,7 +238,7 @@ func installLinuxService(c *cli.Context) error { "--origincert", serviceConfigDir + "/" + serviceCredentialFile, } } else { - src, err := config.ReadConfigFile(c, log) + src, _, err := config.ReadConfigFile(c, log) if err != nil { return err } diff --git a/cmd/cloudflared/tunnel/cmd.go b/cmd/cloudflared/tunnel/cmd.go index 5f1f2a13..bdabcd0b 100644 --- a/cmd/cloudflared/tunnel/cmd.go +++ b/cmd/cloudflared/tunnel/cmd.go @@ -699,7 +699,7 @@ func configureProxyFlags(shouldHide bool) []cli.Flag { Hidden: shouldHide, }), altsrc.NewDurationFlag(&cli.DurationFlag{ - Name: ingress.ProxyTCPKeepAlive, + Name: ingress.ProxyTCPKeepAliveFlag, Usage: "HTTP proxy TCP keepalive duration", Value: time.Second * 30, Hidden: shouldHide, diff --git a/cmd/cloudflared/tunnel/ingress_subcommands.go b/cmd/cloudflared/tunnel/ingress_subcommands.go index 68e159d2..22a5944b 100644 --- a/cmd/cloudflared/tunnel/ingress_subcommands.go +++ b/cmd/cloudflared/tunnel/ingress_subcommands.go @@ -45,7 +45,7 @@ func buildIngressSubcommand() *cli.Command { func buildValidateIngressCommand() *cli.Command { return &cli.Command{ Name: "validate", - Action: cliutil.ConfiguredAction(validateIngressCommand), + Action: cliutil.ConfiguredActionWithWarnings(validateIngressCommand), Usage: "Validate the ingress configuration ", UsageText: "cloudflared tunnel [--config FILEPATH] ingress validate", Description: "Validates the configuration file, ensuring your ingress rules are OK.", @@ -68,7 +68,7 @@ func buildTestURLCommand() *cli.Command { } // validateIngressCommand check the syntax of the ingress rules in the cloudflared config file -func validateIngressCommand(c *cli.Context) error { +func validateIngressCommand(c *cli.Context, warnings string) error { conf := config.GetConfiguration() if conf.Source() == "" { fmt.Println("No configuration file was found. Please create one, or use the --config flag to specify its filepath. You can use the help command to learn more about configuration files") @@ -81,6 +81,11 @@ func validateIngressCommand(c *cli.Context) error { if c.IsSet("url") { return ingress.ErrURLIncompatibleWithIngress } + if warnings != "" { + fmt.Println("Warning: unused keys detected in your config file. Here is a list of unused keys:") + fmt.Println(warnings) + return nil + } fmt.Println("OK") return nil } diff --git a/config/configuration.go b/config/configuration.go index 4b9d8d25..961de6c6 100644 --- a/config/configuration.go +++ b/config/configuration.go @@ -358,13 +358,13 @@ func GetConfiguration() *Configuration { // ReadConfigFile returns InputSourceContext initialized from the configuration file. // On repeat calls returns with the same file, returns without reading the file again; however, // if value of "config" flag changes, will read the new config file -func ReadConfigFile(c *cli.Context, log *zerolog.Logger) (*configFileSettings, error) { +func ReadConfigFile(c *cli.Context, log *zerolog.Logger) (settings *configFileSettings, warnings string, err error) { configFile := c.String("config") if configuration.Source() == configFile || configFile == "" { if configuration.Source() == "" { - return nil, ErrNoConfigFile + return nil, "", ErrNoConfigFile } - return &configuration, nil + return &configuration, "", nil } log.Debug().Msgf("Loading configuration from %s", configFile) @@ -373,16 +373,27 @@ func ReadConfigFile(c *cli.Context, log *zerolog.Logger) (*configFileSettings, e if os.IsNotExist(err) { err = ErrNoConfigFile } - return nil, err + return nil, "", err } defer file.Close() if err := yaml.NewDecoder(file).Decode(&configuration); err != nil { if err == io.EOF { log.Error().Msgf("Configuration file %s was empty", configFile) - return &configuration, nil + return &configuration, "", nil } - return nil, errors.Wrap(err, "error parsing YAML in config file at "+configFile) + return nil, "", errors.Wrap(err, "error parsing YAML in config file at "+configFile) } configuration.sourceFile = configFile - return &configuration, nil + + // Parse it again, with strict mode, to find warnings. + if file, err := os.Open(configFile); err == nil { + decoder := yaml.NewDecoder(file) + decoder.SetStrict(true) + var unusedConfig configFileSettings + if err := decoder.Decode(&unusedConfig); err != nil { + warnings = err.Error() + } + } + + return &configuration, warnings, nil } diff --git a/ingress/ingress_test.go b/ingress/ingress_test.go index 12eb8ed6..7867d2b5 100644 --- a/ingress/ingress_test.go +++ b/ingress/ingress_test.go @@ -402,7 +402,7 @@ func TestSingleOriginSetsConfig(t *testing.T) { flagSet.Bool("hello-world", true, "") flagSet.Duration(ProxyConnectTimeoutFlag, time.Second, "") flagSet.Duration(ProxyTLSTimeoutFlag, time.Second, "") - flagSet.Duration(ProxyTCPKeepAlive, time.Second, "") + flagSet.Duration(ProxyTCPKeepAliveFlag, time.Second, "") flagSet.Bool(ProxyNoHappyEyeballsFlag, true, "") flagSet.Int(ProxyKeepAliveConnectionsFlag, 10, "") flagSet.Duration(ProxyKeepAliveTimeoutFlag, time.Second, "") @@ -423,7 +423,7 @@ func TestSingleOriginSetsConfig(t *testing.T) { require.NoError(t, err) err = cliCtx.Set(ProxyTLSTimeoutFlag, "1s") require.NoError(t, err) - err = cliCtx.Set(ProxyTCPKeepAlive, "1s") + err = cliCtx.Set(ProxyTCPKeepAliveFlag, "1s") require.NoError(t, err) err = cliCtx.Set(ProxyNoHappyEyeballsFlag, "true") require.NoError(t, err) diff --git a/ingress/origin_request_config.go b/ingress/origin_request_config.go index f8f06d2d..37e3ebbc 100644 --- a/ingress/origin_request_config.go +++ b/ingress/origin_request_config.go @@ -22,7 +22,7 @@ const ( Socks5Flag = "socks5" ProxyConnectTimeoutFlag = "proxy-connect-timeout" ProxyTLSTimeoutFlag = "proxy-tls-timeout" - ProxyTCPKeepAlive = "proxy-tcp-keepalive" + ProxyTCPKeepAliveFlag = "proxy-tcp-keepalive" ProxyNoHappyEyeballsFlag = "proxy-no-happy-eyeballs" ProxyKeepAliveConnectionsFlag = "proxy-keepalive-connections" ProxyKeepAliveTimeoutFlag = "proxy-keepalive-timeout" @@ -60,7 +60,7 @@ func originRequestFromSingeRule(c *cli.Context) OriginRequestConfig { if flag := ProxyTLSTimeoutFlag; c.IsSet(flag) { tlsTimeout = c.Duration(flag) } - if flag := ProxyTCPKeepAlive; c.IsSet(flag) { + if flag := ProxyTCPKeepAliveFlag; c.IsSet(flag) { tcpKeepAlive = c.Duration(flag) } if flag := ProxyNoHappyEyeballsFlag; c.IsSet(flag) {