TUN-4359: Warn about unused keys in 'tunnel ingress validate'

Author: adamchalmers

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
 }

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
 		}

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,

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
 }

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
 }

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)

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) {