TUN-3715: Only read config file once, right before invoking the command

CHANGES.md

@@ -1,5 +1,24 @@
 **Experimental**: This is a new format for release notes. The format and availability is subject to change.
 
+## UNRELEASED
+
+### Backward Incompatible Changes
+
+- none
+
+### New Features
+
+- none
+
+### Improvements
+
+- nonw
+
+### Bug Fixes
+
+- Don't look for configuration file in default paths when `--config FILE` flag is present after `tunnel` subcommand.
+
+
 ## 2021.3.0
 
 ### New Features

cmd/cloudflared/access/cmd.go

@@ -84,7 +84,7 @@ func Commands() []*cli.Command {
 			Subcommands: []*cli.Command{
 				{
 					Name:   "login",
-					Action: cliutil.ErrorHandler(login),
+					Action: cliutil.Action(login),
 					Usage:  "login <url of access application>",
 					Description: `The login subcommand initiates an authentication flow with your identity provider.
 					The subcommand will launch a browser. For headless systems, a url is provided.
@@ -100,7 +100,7 @@ func Commands() []*cli.Command {
 				},
 				{
 					Name:   "curl",
-					Action: cliutil.ErrorHandler(curl),
+					Action: cliutil.Action(curl),
 					Usage:  "curl [--allow-request, -ar] <url> [<curl args>...]",
 					Description: `The curl subcommand wraps curl and automatically injects the JWT into a cf-access-token
 					header when using curl to reach an application behind Access.`,
@@ -109,7 +109,7 @@ func Commands() []*cli.Command {
 				},
 				{
 					Name:        "token",
-					Action:      cliutil.ErrorHandler(generateToken),
+					Action:      cliutil.Action(generateToken),
 					Usage:       "token -app=<url of access application>",
 					ArgsUsage:   "url of Access application",
 					Description: `The token subcommand produces a JWT which can be used to authenticate requests.`,
@@ -121,7 +121,7 @@ func Commands() []*cli.Command {
 				},
 				{
 					Name:        "tcp",
-					Action:      cliutil.ErrorHandler(ssh),
+					Action:      cliutil.Action(ssh),
 					Aliases:     []string{"rdp", "ssh", "smb"},
 					Usage:       "",
 					ArgsUsage:   "",
@@ -175,7 +175,7 @@ func Commands() []*cli.Command {
 				},
 				{
 					Name:        "ssh-config",
-					Action:      cliutil.ErrorHandler(sshConfig),
+					Action:      cliutil.Action(sshConfig),
 					Usage:       "",
 					Description: `Prints an example configuration ~/.ssh/config`,
 					Flags: []cli.Flag{
@@ -191,7 +191,7 @@ func Commands() []*cli.Command {
 				},
 				{
 					Name:        "ssh-gen",
-					Action:      cliutil.ErrorHandler(sshGen),
+					Action:      cliutil.Action(sshGen),
 					Usage:       "",
 					Description: `Generates a short lived certificate for given hostname`,
 					Flags: []cli.Flag{

cmd/cloudflared/cliutil/deprecated.go

@@ -9,12 +9,12 @@ import (
 func RemovedCommand(name string) *cli.Command {
 	return &cli.Command{
 		Name: name,
-		Action: ErrorHandler(func(context *cli.Context) error {
+		Action: func(context *cli.Context) error {
 			return cli.Exit(
 				fmt.Sprintf("%s command is no longer supported by cloudflared. Consult Argo Tunnel documentation for possible alternative solutions.", name),
 				-1,
 			)
-		}),
+		},
 		Description: fmt.Sprintf("%s is deprecated", name),
 		Hidden: true,
 	}

cmd/cloudflared/cliutil/errors.go

@@ -2,6 +2,7 @@ package cliutil
 
 import (
 	"fmt"
+
 	"github.com/urfave/cli/v2"
 )
 
@@ -21,7 +22,7 @@ func UsageError(format string, args ...interface{}) error {
 }
 
 // Ensures exit with error code if actionFunc returns an error
-func ErrorHandler(actionFunc cli.ActionFunc) cli.ActionFunc {
+func WithErrorHandler(actionFunc cli.ActionFunc) cli.ActionFunc {
 	return func(ctx *cli.Context) error {
 		err := actionFunc(ctx)
 		if err != nil {

cmd/cloudflared/cliutil/handler.go

@@ -0,0 +1,53 @@
+package cliutil
+
+import (
+	"github.com/urfave/cli/v2"
+	"github.com/urfave/cli/v2/altsrc"
+
+	"github.com/cloudflare/cloudflared/config"
+	"github.com/cloudflare/cloudflared/logger"
+)
+
+func Action(actionFunc cli.ActionFunc) cli.ActionFunc {
+	return WithErrorHandler(func(c *cli.Context) error {
+		if err := setFlagsFromConfigFile(c); err != nil {
+			return err
+		}
+		return actionFunc(c)
+	})
+}
+
+func setFlagsFromConfigFile(c *cli.Context) error {
+	const errorExitCode = 1
+	log := logger.CreateLoggerFromContext(c, logger.EnableTerminalLog)
+	inputSource, err := config.ReadConfigFile(c, log)
+	if err != nil {
+		if err == config.ErrNoConfigFile {
+			return nil
+		}
+		return cli.Exit(err, errorExitCode)
+	}
+
+	if err := applyConfig(c, inputSource); err != nil {
+		return cli.Exit(err, errorExitCode)
+	}
+	return nil
+}
+
+func applyConfig(c *cli.Context, inputSource altsrc.InputSourceContext) error {
+	for _, context := range c.Lineage() {
+		if context.Command == nil {
+			// we've reached the placeholder root context not associated with the app
+			break
+		}
+		targetFlags := context.Command.Flags
+		if context.Command.Name == "" {
+			// commands that define child subcommands are executed as if they were an app
+			targetFlags = c.App.Flags
+		}
+		if err := altsrc.ApplyInputSourceValues(context, inputSource, targetFlags); err != nil {
+			return err
+		}
+	}
+	return nil
+}

cmd/cloudflared/linux_service.go

@@ -24,7 +24,7 @@ func runApp(app *cli.App, graceShutdownC chan struct{}) {
 			{
 				Name:   "install",
 				Usage:  "Install Argo Tunnel as a system service",
-				Action: cliutil.ErrorHandler(installLinuxService),
+				Action: cliutil.Action(installLinuxService),
 				Flags: []cli.Flag{
 					&cli.BoolFlag{
 						Name:  "legacy",
@@ -35,7 +35,7 @@ func runApp(app *cli.App, graceShutdownC chan struct{}) {
 			{
 				Name:   "uninstall",
 				Usage:  "Uninstall the Argo Tunnel service",
-				Action: cliutil.ErrorHandler(uninstallLinuxService),
+				Action: cliutil.Action(uninstallLinuxService),
 			},
 		},
 	})

cmd/cloudflared/macos_service.go

@@ -25,12 +25,12 @@ func runApp(app *cli.App, graceShutdownC chan struct{}) {
 			{
 				Name:   "install",
 				Usage:  "Install Argo Tunnel as an user launch agent",
-				Action: cliutil.ErrorHandler(installLaunchd),
+				Action: cliutil.Action(installLaunchd),
 			},
 			{
 				Name:   "uninstall",
 				Usage:  "Uninstall the Argo Tunnel launch agent",
-				Action: cliutil.ErrorHandler(uninstallLaunchd),
+				Action: cliutil.Action(uninstallLaunchd),
 			},
 		},
 	})

cmd/cloudflared/main.go

@@ -77,7 +77,6 @@ func main() {
 	See https://developers.cloudflare.com/argo-tunnel/ for more in-depth documentation.`
 	app.Flags = flags()
 	app.Action = action(graceShutdownC)
-	app.Before = tunnel.SetFlagsFromConfigFile
 	app.Commands = commands(cli.ShowVersion)
 
 	tunnel.Init(Version, graceShutdownC) // we need this to support the tunnel sub command...
@@ -90,7 +89,7 @@ func commands(version func(c *cli.Context)) []*cli.Command {
 	cmds := []*cli.Command{
 		{
 			Name:   "update",
-			Action: cliutil.ErrorHandler(updater.Update),
+			Action: cliutil.Action(updater.Update),
 			Usage:  "Update the agent if a new version exists",
 			Flags: []cli.Flag{
 				&cli.BoolFlag{
@@ -145,7 +144,7 @@ func isEmptyInvocation(c *cli.Context) bool {
 }
 
 func action(graceShutdownC chan struct{}) cli.ActionFunc {
-	return cliutil.ErrorHandler(func(c *cli.Context) (err error) {
+	return cliutil.Action(func(c *cli.Context) (err error) {
 		if isEmptyInvocation(c) {
 			return handleServiceMode(c, graceShutdownC)
 		}

cmd/cloudflared/proxydns/cmd.go

@@ -17,7 +17,8 @@ import (
 func Command(hidden bool) *cli.Command {
 	return &cli.Command{
 		Name:   "proxy-dns",
-		Action: cliutil.ErrorHandler(Run),
+		Action: cliutil.Action(Run),
+
 		Usage:  "Run a DNS over HTTPS proxy server.",
 		Flags: []cli.Flag{
 			&cli.StringFlag{

cmd/cloudflared/tunnel/cmd.go

@@ -7,7 +7,6 @@ import (
 	"io/ioutil"
 	"net/url"
 	"os"
-	"reflect"
 	"runtime/trace"
 	"strings"
 	"sync"
@@ -120,8 +119,7 @@ func Commands() []*cli.Command {
 func buildTunnelCommand(subcommands []*cli.Command) *cli.Command {
 	return &cli.Command{
 		Name:      "tunnel",
-		Action:    cliutil.ErrorHandler(TunnelCommand),
-		Before:    SetFlagsFromConfigFile,
+		Action:    cliutil.Action(TunnelCommand),
 		Category:  "Tunnel",
 		Usage:     "Make a locally-running web service accessible over the internet using Argo Tunnel.",
 		ArgsUsage: " ",
@@ -369,26 +367,6 @@ func StartServer(
 	return waitToShutdown(&wg, cancel, errC, graceShutdownC, c.Duration("grace-period"), log)
 }
 
-func SetFlagsFromConfigFile(c *cli.Context) error {
-	const exitCode = 1
-	log := logger.CreateLoggerFromContext(c, logger.EnableTerminalLog)
-	inputSource, err := config.ReadConfigFile(c, log)
-	if err != nil {
-		if err == config.ErrNoConfigFile {
-			return nil
-		}
-		return cli.Exit(err, exitCode)
-	}
-	targetFlags := c.Command.Flags
-	if c.Command.Name == "" {
-		targetFlags = c.App.Flags
-	}
-	if err := altsrc.ApplyInputSourceValues(c, inputSource, targetFlags); err != nil {
-		return cli.Exit(err, exitCode)
-	}
-	return nil
-}
-
 func waitToShutdown(wg *sync.WaitGroup,
 	cancelServerContext func(),
 	errC <-chan error,
@@ -477,33 +455,6 @@ func addPortIfMissing(uri *url.URL, port int) string {
 	return fmt.Sprintf("%s:%d", uri.Hostname(), port)
 }
 
-// appendFlags will append extra flags to a slice of flags.
-//
-// The cli package will panic if two flags exist with the same name,
-// so if extraFlags contains a flag that was already defined, modify the
-// original flags to use the extra version.
-func appendFlags(flags []cli.Flag, extraFlags ...cli.Flag) []cli.Flag {
-	for _, extra := range extraFlags {
-		var found bool
-
-		// Check if an extra flag overrides an existing flag.
-		for i, flag := range flags {
-			if reflect.DeepEqual(extra.Names(), flag.Names()) {
-				flags[i] = extra
-				found = true
-				break
-			}
-		}
-
-		// Append the extra flag if it has nothing to override.
-		if !found {
-			flags = append(flags, extra)
-		}
-	}
-
-	return flags
-}
-
 func tunnelFlags(shouldHide bool) []cli.Flag {
 	flags := configureCloudflaredFlags(shouldHide)
 	flags = append(flags, configureProxyFlags(shouldHide)...)
@@ -652,6 +603,7 @@ func tunnelFlags(shouldHide bool) []cli.Flag {
 			Aliases: []string{"n"},
 			EnvVars: []string{"TUNNEL_NAME"},
 			Usage:   "Stable name to identify the tunnel. Using this flag will create, route and run a tunnel. For production usage, execute each command separately",
+			Hidden:  shouldHide,
 		}),
 		altsrc.NewBoolFlag(&cli.BoolFlag{
 			Name:   uiFlag,

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.ErrorHandler(validateIngressCommand),
+		Action:      cliutil.Action(validateIngressCommand),
 		Usage:       "Validate the ingress configuration ",
 		UsageText:   "cloudflared tunnel [--config FILEPATH] ingress validate",
 		Description: "Validates the configuration file, ensuring your ingress rules are OK.",
@@ -55,7 +55,7 @@ func buildValidateIngressCommand() *cli.Command {
 func buildTestURLCommand() *cli.Command {
 	return &cli.Command{
 		Name:      "rule",
-		Action:    cliutil.ErrorHandler(testURLCommand),
+		Action:    cliutil.Action(testURLCommand),
 		Usage:     "Check which ingress rule matches a given request URL",
 		UsageText: "cloudflared tunnel [--config FILEPATH] ingress rule URL",
 		ArgsUsage: "URL",

cmd/cloudflared/tunnel/login.go

@@ -26,7 +26,7 @@ const (
 func buildLoginSubcommand(hidden bool) *cli.Command {
 	return &cli.Command{
 		Name:      "login",
-		Action:    cliutil.ErrorHandler(login),
+		Action:    cliutil.Action(login),
 		Usage:     "Generate a configuration file with your login details",
 		ArgsUsage: " ",
 		Flags: []cli.Flag{

cmd/cloudflared/tunnel/subcommands.go

@@ -119,7 +119,7 @@ var (
 func buildCreateCommand() *cli.Command {
 	return &cli.Command{
 		Name:      "create",
-		Action:    cliutil.ErrorHandler(createCommand),
+		Action:    cliutil.Action(createCommand),
 		Usage:     "Create a new tunnel with given name",
 		UsageText: "cloudflared tunnel [tunnel command options] create [subcommand options] NAME",
 		Description: `Creates a tunnel, registers it with Cloudflare edge and generates credential file used to run this tunnel.
@@ -190,7 +190,7 @@ func writeTunnelCredentials(
 func buildListCommand() *cli.Command {
 	return &cli.Command{
 		Name:        "list",
-		Action:      cliutil.ErrorHandler(listCommand),
+		Action:      cliutil.Action(listCommand),
 		Usage:       "List existing tunnels",
 		UsageText:   "cloudflared tunnel [tunnel command options] list [subcommand options]",
 		Description: "cloudflared tunnel list will display all active tunnels, their created time and associated connections. Use -d flag to include deleted tunnels. See the list of options to filter the list",
@@ -339,7 +339,7 @@ func fmtConnections(connections []tunnelstore.Connection, showRecentlyDisconnect
 func buildDeleteCommand() *cli.Command {
 	return &cli.Command{
 		Name:               "delete",
-		Action:             cliutil.ErrorHandler(deleteCommand),
+		Action:             cliutil.Action(deleteCommand),
 		Usage:              "Delete existing tunnel by UUID or name",
 		UsageText:          "cloudflared tunnel [tunnel command options] delete [subcommand options] TUNNEL",
 		Description:        "cloudflared tunnel delete will delete tunnels with the given tunnel UUIDs or names. A tunnel cannot be deleted if it has active connections. To delete the tunnel unconditionally, use -f flag.",
@@ -392,8 +392,7 @@ func buildRunCommand() *cli.Command {
 	flags = append(flags, configureProxyFlags(false)...)
 	return &cli.Command{
 		Name:      "run",
-		Action:    cliutil.ErrorHandler(runCommand),
-		Before:    SetFlagsFromConfigFile,
+		Action:    cliutil.Action(runCommand),
 		Usage:     "Proxy a local web server by running the given tunnel",
 		UsageText: "cloudflared tunnel [tunnel command options] run [subcommand options] [TUNNEL]",
 		Description: `Runs the tunnel identified by name or UUUD, creating highly available connections
@@ -445,7 +444,7 @@ func runNamedTunnel(sc *subcommandContext, tunnelRef string) error {
 func buildCleanupCommand() *cli.Command {
 	return &cli.Command{
 		Name:               "cleanup",
-		Action:             cliutil.ErrorHandler(cleanupCommand),
+		Action:             cliutil.Action(cleanupCommand),
 		Usage:              "Cleanup tunnel connections",
 		UsageText:          "cloudflared tunnel [tunnel command options] cleanup [subcommand options] TUNNEL",
 		Description:        "Delete connections for tunnels with the given UUIDs or names.",
@@ -474,7 +473,7 @@ func cleanupCommand(c *cli.Context) error {
 func buildRouteCommand() *cli.Command {
 	return &cli.Command{
 		Name:      "route",
-		Action:    cliutil.ErrorHandler(routeCommand),
+		Action:    cliutil.Action(routeCommand),
 		Usage:     "Define which traffic routed from Cloudflare edge to this tunnel: requests to a DNS hostname, to a Cloudflare Load Balancer, or traffic originating from Cloudflare WARP clients",
 		UsageText: "cloudflared tunnel [tunnel command options] route [subcommand options] [dns TUNNEL HOSTNAME]|[lb TUNNEL HOSTNAME LB-POOL]|[ip NETWORK TUNNEL]",
 		Description: `The route command defines how Cloudflare will proxy requests to this tunnel.

cmd/cloudflared/tunnel/teamnet_subcommands.go

@@ -25,7 +25,7 @@ Cloudflare WARP client. You can also build rules to determine who can reach cert
 		Subcommands: []*cli.Command{
 			{
 				Name:      "add",
-				Action:    cliutil.ErrorHandler(addRouteCommand),
+				Action:    cliutil.Action(addRouteCommand),
 				Usage:     "Add any new network to the routing table reachable via the tunnel",
 				UsageText: "cloudflared tunnel [--config FILEPATH] route ip add [CIDR] [TUNNEL] [COMMENT?]",
 				Description: `Adds any network route space (represented as a CIDR) to your routing table.
@@ -38,7 +38,7 @@ reachable from the tunnel.`,
 			{
 				Name:        "show",
 				Aliases:     []string{"list"},
-				Action:      cliutil.ErrorHandler(showRoutesCommand),
+				Action:      cliutil.Action(showRoutesCommand),
 				Usage:       "Show the routing table",
 				UsageText:   "cloudflared tunnel [--config FILEPATH] route ip show [flags]",
 				Description: `Shows your organization private routing table. You can use flags to filter the results.`,
@@ -46,7 +46,7 @@ reachable from the tunnel.`,
 			},
 			{
 				Name:        "delete",
-				Action:    cliutil.ErrorHandler(deleteRouteCommand),
+				Action:      cliutil.Action(deleteRouteCommand),
 				Usage:       "Delete a row from your organization's private routing table",
 				UsageText:   "cloudflared tunnel [--config FILEPATH] route ip delete [CIDR]",
 				Description: `Deletes the row for a given CIDR from your routing table. That portion
@@ -54,7 +54,7 @@ of your network will no longer be reachable by the WARP clients.`,
 			},
 			{
 				Name:      "get",
-				Action:    cliutil.ErrorHandler(getRouteByIPCommand),
+				Action:    cliutil.Action(getRouteByIPCommand),
 				Usage:     "Check which row of the routing table matches a given IP.",
 				UsageText: "cloudflared tunnel [--config FILEPATH] route ip get [IP]",
 				Description: `Checks which row of the routing table will be used to proxy a given IP.

cmd/cloudflared/windows_service.go

@@ -12,6 +12,9 @@ import (
 	"time"
 	"unsafe"
 
+	"github.com/pkg/errors"
+
+	"github.com/cloudflare/cloudflared/cmd/cloudflared/cliutil"
 	"github.com/cloudflare/cloudflared/logger"
 
 	"github.com/urfave/cli/v2"
@@ -48,12 +51,12 @@ func runApp(app *cli.App, graceShutdownC chan struct{}) {
 			{
 				Name:   "install",
 				Usage:  "Install Argo Tunnel as a Windows service",
-				Action: installWindowsService,
+				Action: cliutil.Action(installWindowsService),
 			},
 			{
 				Name:   "uninstall",
 				Usage:  "Uninstall the Argo Tunnel service",
-				Action: uninstallWindowsService,
+				Action: cliutil.Action(uninstallWindowsService),
 			},
 		},
 	})
@@ -177,35 +180,30 @@ func installWindowsService(c *cli.Context) error {
 	zeroLogger.Info().Msg("Installing Argo Tunnel Windows service")
 	exepath, err := os.Executable()
 	if err != nil {
-		zeroLogger.Err(err).Msg("Cannot find path name that start the process")
-		return err
+		return errors.Wrap(err, "Cannot find path name that start the process")
 	}
 	m, err := mgr.Connect()
 	if err != nil {
-		zeroLogger.Err(err).Msg("Cannot establish a connection to the service control manager")
-		return err
+		return errors.Wrap(err, "Cannot establish a connection to the service control manager")
 	}
 	defer m.Disconnect()
 	s, err := m.OpenService(windowsServiceName)
 	log := zeroLogger.With().Str(LogFieldWindowsServiceName, windowsServiceName).Logger()
 	if err == nil {
 		s.Close()
-		log.Err(err).Msg("service already exists")
-		return fmt.Errorf("service %s already exists", windowsServiceName)
+		return fmt.Errorf("Service %s already exists", windowsServiceName)
 	}
 	config := mgr.Config{StartType: mgr.StartAutomatic, DisplayName: windowsServiceDescription}
 	s, err = m.CreateService(windowsServiceName, exepath, config)
 	if err != nil {
-		log.Error().Msg("Cannot install service")
-		return err
+		return errors.Wrap(err, "Cannot install service")
 	}
 	defer s.Close()
 	log.Info().Msg("Argo Tunnel agent service is installed")
 	err = eventlog.InstallAsEventCreate(windowsServiceName, eventlog.Error|eventlog.Warning|eventlog.Info)
 	if err != nil {
 		s.Delete()
-		log.Err(err).Msg("Cannot install event logger")
-		return fmt.Errorf("SetupEventLogSource() failed: %s", err)
+		return errors.Wrap(err, "Cannot install event logger")
 	}
 	err = configRecoveryOption(s.Handle)
 	if err != nil {
@@ -223,26 +221,22 @@ func uninstallWindowsService(c *cli.Context) error {
 	log.Info().Msg("Uninstalling Argo Tunnel Windows Service")
 	m, err := mgr.Connect()
 	if err != nil {
-		log.Error().Msg("Cannot establish a connection to the service control manager")
-		return err
+		return errors.Wrap(err, "Cannot establish a connection to the service control manager")
 	}
 	defer m.Disconnect()
 	s, err := m.OpenService(windowsServiceName)
 	if err != nil {
-		log.Error().Msg("service is not installed")
-		return fmt.Errorf("service %s is not installed", windowsServiceName)
+		return fmt.Errorf("Service %s is not installed", windowsServiceName)
 	}
 	defer s.Close()
 	err = s.Delete()
 	if err != nil {
-		log.Error().Msg("Cannot delete service")
-		return err
+		return errors.Wrap(err, "Cannot delete service")
 	}
 	log.Info().Msg("Argo Tunnel agent service is uninstalled")
 	err = eventlog.Remove(windowsServiceName)
 	if err != nil {
-		log.Error().Msg("Cannot remove event logger")
-		return fmt.Errorf("RemoveEventLogSource() failed: %s", err)
+		return errors.Wrap(err, "Cannot remove event logger")
 	}
 	return nil
 }

origin/supervisor.go

@@ -30,12 +30,6 @@ const (
 	refreshAuthMaxBackoff = 10
 	// Waiting time before retrying a failed 'Authenticate' connection
 	refreshAuthRetryDuration = time.Second * 10
-	// Maximum time to make an Authenticate RPC
-	authTokenTimeout = time.Second * 30
-)
-
-var (
-	errEventDigestUnset = errors.New("event digest unset")
 )
 
 // Supervisor manages non-declarative tunnels. Establishes TCP connections with the edge, and

sshgen/sshgen.go

@@ -18,7 +18,6 @@ import (
 	homedir "github.com/mitchellh/go-homedir"
 	"github.com/pkg/errors"
 	gossh "golang.org/x/crypto/ssh"
-
 	"github.com/cloudflare/cloudflared/config"
 	cfpath "github.com/cloudflare/cloudflared/token"
 )