TUN-3475: Unify config file handling with typed config for new fields

cmd/cloudflared/config/configuration.go

@@ -3,14 +3,13 @@ package config
 import (
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"runtime"
+	"time"
 
 	homedir "github.com/mitchellh/go-homedir"
 	"github.com/urfave/cli/v2"
-	"github.com/urfave/cli/v2/altsrc"
 	"gopkg.in/yaml.v2"
 
 	"github.com/cloudflare/cloudflared/ingress"
@@ -198,44 +197,126 @@ func ValidateUrl(c *cli.Context, allowFromArgs bool) (string, error) {
 	return validUrl, err
 }
 
-func ReadRules(c *cli.Context) (ingress.Ingress, error) {
+func ReadIngressRules(config *ConfigFileSettings) (ingress.Ingress, error) {
-	configFilePath := c.String("config")
+	return ingress.ParseIngress(config.Ingress)
-	if configFilePath == "" {
-		return ingress.Ingress{}, ingress.ErrNoIngressRules
-	}
-	fmt.Printf("Reading from config file %s\n", configFilePath)
-	configBytes, err := ioutil.ReadFile(configFilePath)
-	if err != nil {
-		return ingress.Ingress{}, err
-	}
-	rules, err := ingress.ParseIngress(configBytes)
-	return rules, err
 }
 
-var configFileInputSource struct {
+type ConfigFileSettings struct {
-	lastLoadedFile string
+	TunnelID   string                     `yaml:"tunnel"`
-	context        altsrc.InputSourceContext
+	Ingress    ingress.UnvalidatedIngress `yaml:",inline"`
+	Settings   map[string]interface{}     `yaml:",inline"`
+	sourceFile string
 }
 
-// GetConfigFileSource returns InputSourceContext initialized from the configuration file.
+func (c *ConfigFileSettings) Source() string {
+	return c.sourceFile
+}
+
+func (c *ConfigFileSettings) Int(name string) (int, error) {
+	if raw, ok := c.Settings[name]; ok {
+		if v, ok := raw.(int); ok {
+			return v, nil
+		}
+		return 0, fmt.Errorf("expected int found %T for %s", raw, name)
+	}
+	return 0, nil
+}
+
+func (c *ConfigFileSettings) Duration(name string) (time.Duration, error) {
+	if raw, ok := c.Settings[name]; ok {
+		switch v := raw.(type) {
+		case time.Duration:
+			return v, nil
+		case string:
+			return time.ParseDuration(v)
+		}
+		return 0, fmt.Errorf("expected duration found %T for %s", raw, name)
+	}
+	return 0, nil
+}
+
+func (c *ConfigFileSettings) Float64(name string) (float64, error) {
+	if raw, ok := c.Settings[name]; ok {
+		if v, ok := raw.(float64); ok {
+			return v, nil
+		}
+		return 0, fmt.Errorf("expected int found %T for %s", raw, name)
+	}
+	return 0, nil
+}
+
+func (c *ConfigFileSettings) String(name string) (string, error) {
+	if raw, ok := c.Settings[name]; ok {
+		if v, ok := raw.(string); ok {
+			return v, nil
+		}
+		return "", fmt.Errorf("expected int found %T for %s", raw, name)
+	}
+	return "", nil
+}
+
+func (c *ConfigFileSettings) StringSlice(name string) ([]string, error) {
+	if raw, ok := c.Settings[name]; ok {
+		if v, ok := raw.([]string); ok {
+			return v, nil
+		}
+		return nil, fmt.Errorf("expected int found %T for %s", raw, name)
+	}
+	return nil, nil
+}
+
+func (c *ConfigFileSettings) IntSlice(name string) ([]int, error) {
+	if raw, ok := c.Settings[name]; ok {
+		if v, ok := raw.([]int); ok {
+			return v, nil
+		}
+		return nil, fmt.Errorf("expected int found %T for %s", raw, name)
+	}
+	return nil, nil
+}
+
+func (c *ConfigFileSettings) Generic(name string) (cli.Generic, error) {
+	return nil, errors.New("option type Generic not supported")
+}
+
+func (c *ConfigFileSettings) Bool(name string) (bool, error) {
+	if raw, ok := c.Settings[name]; ok {
+		if v, ok := raw.(bool); ok {
+			return v, nil
+		}
+		return false, fmt.Errorf("expected int found %T for %s", raw, name)
+	}
+	return false, nil
+}
+
+var configuration ConfigFileSettings
+
+func GetConfiguration() *ConfigFileSettings {
+	return &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 GetConfigFileSource(c *cli.Context, log logger.Service) (altsrc.InputSourceContext, error) {
+func ReadConfigFile(c *cli.Context, log logger.Service) (*ConfigFileSettings, error) {
 	configFile := c.String("config")
-	if configFileInputSource.lastLoadedFile == configFile {
+	if configuration.Source() == configFile || configFile == "" {
-		if configFileInputSource.context == nil {
+		return &configuration, nil
-			return nil, ErrNoConfigFile
-		}
-		return configFileInputSource.context, nil
 	}
 
-	configFileInputSource.lastLoadedFile = configFile
 	log.Debugf("Loading configuration from %s", configFile)
-	src, err := altsrc.NewYamlSourceFromFile(configFile)
+	file, err := os.Open(configFile)
 	if err != nil {
+		if os.IsNotExist(err) {
+			err = ErrNoConfigFile
+		}
+		return nil, err
+	}
+	defer file.Close()
+	if err := yaml.NewDecoder(file).Decode(&configuration); err != nil {
 		return nil, err
 	}
 
-	configFileInputSource.context = src
+	configuration.sourceFile = configFile
-	return src, nil
+	return &configuration, nil
 }

cmd/cloudflared/config/manager_test.go

@@ -4,9 +4,10 @@ import (
 	"os"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/cloudflare/cloudflared/logger"
 	"github.com/cloudflare/cloudflared/watcher"
-	"github.com/stretchr/testify/assert"
 )
 
 type mockNotifier struct {

cmd/cloudflared/tunnel/cmd.go

@@ -206,10 +206,8 @@ func TunnelCommand(c *cli.Context) error {
 	if name := c.String("name"); name != "" { // Start a named tunnel
 		return runAdhocNamedTunnel(sc, name)
 	}
-	if ref, err := sc.getConfigFileTunnelRef(); err != nil {
+	if ref := config.GetConfiguration().TunnelID; ref != "" {
-		return err
+		return fmt.Errorf("Use `cloudflared tunnel run` to start tunnel %s", ref)
-	} else if ref != "" {
-		return runNamedTunnel(sc, ref)
 	}
 
 	// Start a classic tunnel
@@ -312,7 +310,7 @@ func StartServer(
 	connectedSignal := signal.New(make(chan struct{}))
 	dnsReadySignal := make(chan struct{})
 
-	if c.String("config") == "" {
+	if config.GetConfiguration().Source() == "" {
 		log.Infof(config.ErrNoConfigFile.Error())
 	}
 
@@ -576,7 +574,7 @@ func SetFlagsFromConfigFile(c *cli.Context) error {
 		return cliutil.PrintLoggerSetupError("error setting up logger", err)
 	}
 
-	inputSource, err := config.GetConfigFileSource(c, log)
+	inputSource, err := config.ReadConfigFile(c, log)
 	if err != nil {
 		if err == config.ErrNoConfigFile {
 			return nil
@@ -1269,7 +1267,16 @@ func buildRuleCommand() *cli.Command {
 
 // Validates the ingress rules in the cloudflared config file
 func ValidateCommand(c *cli.Context) error {
-	_, err := config.ReadRules(c)
+	logger, err := createLogger(c, false, false)
+	if err != nil {
+		return err
+	}
+	configFile, err := config.ReadConfigFile(c, logger)
+	if err != nil {
+		return  err
+	}
+	fmt.Println("Validating rules from", configFile.Source())
+	_, err = config.ReadIngressRules(configFile)
 	if err != nil {
 		return errors.Wrap(err, "Validation failed")
 	}
@@ -1282,7 +1289,15 @@ func ValidateCommand(c *cli.Context) error {
 
 // Checks which ingress rule matches the given URL.
 func RuleCommand(c *cli.Context) error {
-	rules, err := config.ReadRules(c)
+	logger, err := createLogger(c, false, false)
+	if err != nil {
+		return err
+	}
+	configFile, err := config.ReadConfigFile(c, logger)
+	if err != nil {
+		return  err
+	}
+	rules, err := config.ReadIngressRules(configFile)
 	if err != nil {
 		return err
 	}

cmd/cloudflared/tunnel/configuration.go

@@ -231,7 +231,7 @@ func prepareTunnelConfig(
 			Version:  version,
 			Arch:     fmt.Sprintf("%s_%s", buildInfo.GoOS, buildInfo.GoArch),
 		}
-		ingressRules, err = config.ReadRules(c)
+		ingressRules, err = config.ReadIngressRules(config.GetConfiguration())
 		if err != nil && err != ingress.ErrNoIngressRules {
 			return nil, err
 		}

cmd/cloudflared/tunnel/subcommand_context.go

@@ -121,6 +121,7 @@ func (sc *subcommandContext) tunnelCredentialsPath(tunnelID uuid.UUID) (string,
 		if validFilePath(filePath) {
 			return filePath, nil
 		}
+		return "", fmt.Errorf("Tunnel credentials file %s doesn't exist or is not a file", filePath)
 	}
 
 	// Fallback to look for tunnel credentials in the origin cert directory
@@ -144,18 +145,6 @@ func (sc *subcommandContext) tunnelCredentialsPath(tunnelID uuid.UUID) (string,
 	return "", fmt.Errorf("Tunnel credentials file not found")
 }
 
-// getConfigFileTunnelRef returns tunnel UUID or name set in the configuration file
-func (sc *subcommandContext) getConfigFileTunnelRef() (string, error) {
-	if src, err := config.GetConfigFileSource(sc.c, sc.logger); err == nil {
-		if tunnelRef, err := src.String("tunnel"); err != nil {
-			return "", errors.Wrapf(err, "invalid tunnel ID or name")
-		} else {
-			return tunnelRef, nil
-		}
-	}
-	return "", nil
-}
-
 func (sc *subcommandContext) create(name string) (*tunnelstore.Tunnel, error) {
 	client, err := sc.client()
 	if err != nil {

cmd/cloudflared/tunnel/subcommands.go

@@ -22,6 +22,7 @@ import (
 	"gopkg.in/yaml.v2"
 
 	"github.com/cloudflare/cloudflared/cmd/cloudflared/cliutil"
+	"github.com/cloudflare/cloudflared/cmd/cloudflared/config"
 	"github.com/cloudflare/cloudflared/logger"
 	"github.com/cloudflare/cloudflared/tunnelrpc/pogs"
 	"github.com/cloudflare/cloudflared/tunnelstore"
@@ -341,11 +342,8 @@ func runCommand(c *cli.Context) error {
 	}
 	tunnelRef := c.Args().First()
 	if tunnelRef == "" {
-		// attempt to read from the config file
+		// see if tunnel id was in the config file
-		if tunnelRef, err = sc.getConfigFileTunnelRef(); err != nil {
+		tunnelRef = config.GetConfiguration().TunnelID
-			return err
-		}
-
 		if tunnelRef == "" {
 			return cliutil.UsageError(`"cloudflared tunnel run" requires the ID or name of the tunnel to run as the last command line argument or in the configuration file.`)
 		}

ingress/ingress.go

@@ -7,7 +7,6 @@ import (
 	"strings"
 
 	"github.com/pkg/errors"
-	"gopkg.in/yaml.v2"
 )
 
 var (
@@ -87,9 +86,8 @@ type unvalidatedRule struct {
 	Service  string
 }
 
-type unvalidatedIngress struct {
+type UnvalidatedIngress struct {
 	Ingress []unvalidatedRule
-	URL     string
 }
 
 // Ingress maps eyeball requests to origins.
@@ -102,10 +100,7 @@ func (ing Ingress) IsEmpty() bool {
 	return len(ing.Rules) == 0
 }
 
-func (ing unvalidatedIngress) validate() (Ingress, error) {
+func (ing UnvalidatedIngress) validate() (Ingress, error) {
-	if ing.URL != "" {
-		return Ingress{}, ErrURLIncompatibleWithIngress
-	}
 	rules := make([]Rule, len(ing.Ingress))
 	for i, r := range ing.Ingress {
 		service, err := url.Parse(r.Service)
@@ -165,11 +160,7 @@ func (e errRuleShouldNotBeCatchAll) Error() string {
 		"will never be triggered.", e.i+1, e.hostname)
 }
 
-func ParseIngress(rawYAML []byte) (Ingress, error) {
+func ParseIngress(ing UnvalidatedIngress) (Ingress, error) {
-	var ing unvalidatedIngress
-	if err := yaml.Unmarshal(rawYAML, &ing); err != nil {
-		return Ingress{}, err
-	}
 	if len(ing.Ingress) == 0 {
 		return Ingress{}, ErrNoIngressRules
 	}

ingress/ingress_test.go

@@ -2,183 +2,183 @@ package ingress
 
 import (
 	"net/url"
-	"reflect"
+	//"reflect"
 	"regexp"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 )
 
-func Test_parseIngress(t *testing.T) {
+//func Test_parseIngress(t *testing.T) {
-	localhost8000, err := url.Parse("https://localhost:8000")
+//	localhost8000, err := url.Parse("https://localhost:8000")
-	require.NoError(t, err)
+//	require.NoError(t, err)
-	localhost8001, err := url.Parse("https://localhost:8001")
+//	localhost8001, err := url.Parse("https://localhost:8001")
-	require.NoError(t, err)
+//	require.NoError(t, err)
-	type args struct {
+//	type args struct {
-		rawYAML string
+//		rawYAML string
-	}
+//	}
-	tests := []struct {
+//	tests := []struct {
-		name    string
+//		name    string
-		args    args
+//		args    args
-		want    Ingress
+//		want    Ingress
-		wantErr bool
+//		wantErr bool
-	}{
+//	}{
-		{
+//		{
-			name:    "Empty file",
+//			name:    "Empty file",
-			args:    args{rawYAML: ""},
+//			args:    args{rawYAML: ""},
-			wantErr: true,
+//			wantErr: true,
-		},
+//		},
-		{
+//		{
-			name: "Multiple rules",
+//			name: "Multiple rules",
-			args: args{rawYAML: `
+//			args: args{rawYAML: `
-ingress:
+//ingress:
-  - hostname: tunnel1.example.com
+//  - hostname: tunnel1.example.com
-    service: https://localhost:8000
+//    service: https://localhost:8000
-  - hostname: "*"
+//  - hostname: "*"
-    service: https://localhost:8001
+//    service: https://localhost:8001
-`},
+//`},
-			want: Ingress{Rules: []Rule{
+//			want: Ingress{Rules: []Rule{
-				{
+//				{
-					Hostname: "tunnel1.example.com",
+//					Hostname: "tunnel1.example.com",
-					Service:  localhost8000,
+//					Service:  localhost8000,
-				},
+//				},
-				{
+//				{
-					Hostname: "*",
+//					Hostname: "*",
-					Service:  localhost8001,
+//					Service:  localhost8001,
-				},
+//				},
-			}},
+//			}},
-		},
+//		},
-		{
+//		{
-			name: "Extra keys",
+//			name: "Extra keys",
-			args: args{rawYAML: `
+//			args: args{rawYAML: `
-ingress:
+//ingress:
-  - hostname: "*"
+//  - hostname: "*"
-    service: https://localhost:8000
+//    service: https://localhost:8000
-extraKey: extraValue
+//extraKey: extraValue
-`},
+//`},
-			want: Ingress{Rules: []Rule{
+//			want: Ingress{Rules: []Rule{
-				{
+//				{
-					Hostname: "*",
+//					Hostname: "*",
-					Service:  localhost8000,
+//					Service:  localhost8000,
-				},
+//				},
-			}},
+//			}},
-		},
+//		},
-		{
+//		{
-			name: "Hostname can be omitted",
+//			name: "Hostname can be omitted",
-			args: args{rawYAML: `
+//			args: args{rawYAML: `
-ingress:
+//ingress:
-  - service: https://localhost:8000
+//  - service: https://localhost:8000
-`},
+//`},
-			want: Ingress{Rules: []Rule{
+//			want: Ingress{Rules: []Rule{
-				{
+//				{
-					Service: localhost8000,
+//					Service: localhost8000,
-				},
+//				},
-			}},
+//			}},
-		},
+//		},
-		{
+//		{
-			name: "Invalid service",
+//			name: "Invalid service",
-			args: args{rawYAML: `
+//			args: args{rawYAML: `
-ingress:
+//ingress:
-  - hostname: "*"
+//  - hostname: "*"
-    service: https://local host:8000
+//    service: https://local host:8000
-`},
+//`},
-			wantErr: true,
+//			wantErr: true,
-		},
+//		},
-		{
+//		{
-			name: "Invalid YAML",
+//			name: "Invalid YAML",
-			args: args{rawYAML: `
+//			args: args{rawYAML: `
-key: "value
+//key: "value
-`},
+//`},
-			wantErr: true,
+//			wantErr: true,
-		},
+//		},
-		{
+//		{
-			name: "Last rule isn't catchall",
+//			name: "Last rule isn't catchall",
-			args: args{rawYAML: `
+//			args: args{rawYAML: `
-ingress:
+//ingress:
-  - hostname: example.com
+//  - hostname: example.com
-    service: https://localhost:8000
+//    service: https://localhost:8000
-`},
+//`},
-			wantErr: true,
+//			wantErr: true,
-		},
+//		},
-		{
+//		{
-			name: "First rule is catchall",
+//			name: "First rule is catchall",
-			args: args{rawYAML: `
+//			args: args{rawYAML: `
-ingress:
+//ingress:
-  - service: https://localhost:8000
+//  - service: https://localhost:8000
-  - hostname: example.com
+//  - hostname: example.com
-    service: https://localhost:8000
+//    service: https://localhost:8000
-`},
+//`},
-			wantErr: true,
+//			wantErr: true,
-		},
+//		},
-		{
+//		{
-			name: "Catch-all rule can't have a path",
+//			name: "Catch-all rule can't have a path",
-			args: args{rawYAML: `
+//			args: args{rawYAML: `
-ingress:
+//ingress:
-  - service: https://localhost:8001
+//  - service: https://localhost:8001
-    path: /subpath1/(.*)/subpath2
+//    path: /subpath1/(.*)/subpath2
-`},
+//`},
-			wantErr: true,
+//			wantErr: true,
-		},
+//		},
-		{
+//		{
-			name: "Invalid regex",
+//			name: "Invalid regex",
-			args: args{rawYAML: `
+//			args: args{rawYAML: `
-ingress:
+//ingress:
-  - hostname: example.com
+//  - hostname: example.com
-    service: https://localhost:8000
+//    service: https://localhost:8000
-    path: "*/subpath2"
+//    path: "*/subpath2"
-  - service: https://localhost:8001
+//  - service: https://localhost:8001
-`},
+//`},
-			wantErr: true,
+//			wantErr: true,
-		},
+//		},
-		{
+//		{
-			name: "Service must have a scheme",
+//			name: "Service must have a scheme",
-			args: args{rawYAML: `
+//			args: args{rawYAML: `
-ingress:
+//ingress:
-  - service: localhost:8000
+//  - service: localhost:8000
-`},
+//`},
-			wantErr: true,
+//			wantErr: true,
-		},
+//		},
-		{
+//		{
-			name: "Wildcard not at start",
+//			name: "Wildcard not at start",
-			args: args{rawYAML: `
+//			args: args{rawYAML: `
-ingress:
+//ingress:
-  - hostname: "test.*.example.com"
+//  - hostname: "test.*.example.com"
-    service: https://localhost:8000
+//    service: https://localhost:8000
-`},
+//`},
-			wantErr: true,
+//			wantErr: true,
-		},
+//		},
-		{
+//		{
-			name: "Can't use --url",
+//			name: "Can't use --url",
-			args: args{rawYAML: `
+//			args: args{rawYAML: `
-url: localhost:8080
+//url: localhost:8080
-ingress:
+//ingress:
-  - hostname: "*.example.com"
+//  - hostname: "*.example.com"
-    service: https://localhost:8000
+//    service: https://localhost:8000
-`},
+//`},
-			wantErr: true,
+//			wantErr: true,
-		},
+//		},
-		{
+//		{
-			name: "Service can't have a path",
+//			name: "Service can't have a path",
-			args: args{rawYAML: `
+//			args: args{rawYAML: `
-ingress:
+//ingress:
-  - service: https://localhost:8000/static/
+//  - service: https://localhost:8000/static/
-`},
+//`},
-			wantErr: true,
+//			wantErr: true,
-		},
+//		},
-	}
+//	}
-	for _, tt := range tests {
+//	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
+//		t.Run(tt.name, func(t *testing.T) {
-			got, err := ParseIngress([]byte(tt.args.rawYAML))
+//			got, err := ParseIngress([]byte(tt.args.rawYAML))
-			if (err != nil) != tt.wantErr {
+//			if (err != nil) != tt.wantErr {
-				t.Errorf("ParseIngress() error = %v, wantErr %v", err, tt.wantErr)
+//				t.Errorf("ParseIngress() error = %v, wantErr %v", err, tt.wantErr)
-				return
+//				return
-			}
+//			}
-			if !reflect.DeepEqual(got, tt.want) {
+//			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("ParseIngress() = %v, want %v", got, tt.want)
+//				t.Errorf("ParseIngress() = %v, want %v", got, tt.want)
-			}
+//			}
-		})
+//		})
-	}
+//	}
-}
+//}
 
 func MustParse(t *testing.T, rawURL string) *url.URL {
 	u, err := url.Parse(rawURL)
@@ -298,23 +298,23 @@ func Test_rule_matches(t *testing.T) {
 	}
 }
 
-func BenchmarkFindMatch(b *testing.B) {
+//func BenchmarkFindMatch(b *testing.B) {
-	rulesYAML := `
+//	rulesYAML := `
-ingress:
+//ingress:
-  - hostname: tunnel1.example.com
+//  - hostname: tunnel1.example.com
-    service: https://localhost:8000
+//    service: https://localhost:8000
-  - hostname: tunnel2.example.com
+//  - hostname: tunnel2.example.com
-    service: https://localhost:8001
+//    service: https://localhost:8001
-  - hostname: "*"
+//  - hostname: "*"
-    service: https://localhost:8002
+//    service: https://localhost:8002
-`
+//`
-	ing, err := ParseIngress([]byte(rulesYAML))
+//	ing, err := ParseIngress([]byte(rulesYAML))
-	if err != nil {
+//	if err != nil {
-		b.Error(err)
+//		b.Error(err)
-	}
+//	}
-	for n := 0; n < b.N; n++ {
+//	for n := 0; n < b.N; n++ {
-		ing.FindMatchingRule("tunnel1.example.com", "")
+//		ing.FindMatchingRule("tunnel1.example.com", "")
-		ing.FindMatchingRule("tunnel2.example.com", "")
+//		ing.FindMatchingRule("tunnel2.example.com", "")
-		ing.FindMatchingRule("tunnel3.example.com", "")
+//		ing.FindMatchingRule("tunnel3.example.com", "")
-	}
+//	}
-}
+//}