Split out typed config from legacy command-line switches; refactor ingress commands and fix tests

2025-07-27 00:49:57 +00:00 · 2020-10-20 09:29:13 -05:00
parent eaf03305bd
commit ca4887fb19
6 changed files with 326 additions and 347 deletions
--- a/ingress/ingress.go
+++ b/ingress/ingress.go
@@ -13,7 +13,6 @@ var (
 	ErrNoIngressRules             = errors.New("No ingress rules were specified in the config file")
 	errLastRuleNotCatchAll        = errors.New("The last ingress rule must match all hostnames (i.e. it must be missing, or must be \"*\")")
 	errBadWildcard                = errors.New("Hostname patterns can have at most one wildcard character (\"*\") and it can only be used for subdomains, e.g. \"*.example.com\"")
-	errNoIngressRulesMatch        = errors.New("The URL didn't match any ingress rules")
 	ErrURLIncompatibleWithIngress = errors.New("You can't set the --url flag (or $TUNNEL_URL) when using multiple-origin ingress rules")
 )

@@ -166,14 +165,3 @@ func ParseIngress(ing UnvalidatedIngress) (Ingress, error) {
 	}
 	return ing.validate()
 }
-
-// RuleCommand checks which ingress rule matches the given request URL.
-func RuleCommand(ing Ingress, requestURL *url.URL) error {
-	if requestURL.Hostname() == "" {
-		return fmt.Errorf("%s is malformed and doesn't have a hostname", requestURL)
-	}
-	i := ing.FindMatchingRule(requestURL.Hostname(), requestURL.Path)
-	fmt.Printf("Matched rule #%d\n", i+1)
-	fmt.Println(ing.Rules[i].MultiLineString())
-	return nil
-}
--- a/ingress/ingress_test.go
+++ b/ingress/ingress_test.go
@@ -2,185 +2,165 @@ package ingress

 import (
 	"net/url"
-	//"reflect"
 	"regexp"
 	"testing"

+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gopkg.in/yaml.v2"
 )

-//func Test_parseIngress(t *testing.T) {
-//	localhost8000, err := url.Parse("https://localhost:8000")
-//	require.NoError(t, err)
-//	localhost8001, err := url.Parse("https://localhost:8001")
-//	require.NoError(t, err)
-//	type args struct {
-//		rawYAML string
-//	}
-//	tests := []struct {
-//		name    string
-//		args    args
-//		want    Ingress
-//		wantErr bool
-//	}{
-//		{
-//			name:    "Empty file",
-//			args:    args{rawYAML: ""},
-//			wantErr: true,
-//		},
-//		{
-//			name: "Multiple rules",
-//			args: args{rawYAML: `
-//ingress:
-//  - hostname: tunnel1.example.com
-//    service: https://localhost:8000
-//  - hostname: "*"
-//    service: https://localhost:8001
-//`},
-//			want: Ingress{Rules: []Rule{
-//				{
-//					Hostname: "tunnel1.example.com",
-//					Service:  localhost8000,
-//				},
-//				{
-//					Hostname: "*",
-//					Service:  localhost8001,
-//				},
-//			}},
-//		},
-//		{
-//			name: "Extra keys",
-//			args: args{rawYAML: `
-//ingress:
-//  - hostname: "*"
-//    service: https://localhost:8000
-//extraKey: extraValue
-//`},
-//			want: Ingress{Rules: []Rule{
-//				{
-//					Hostname: "*",
-//					Service:  localhost8000,
-//				},
-//			}},
-//		},
-//		{
-//			name: "Hostname can be omitted",
-//			args: args{rawYAML: `
-//ingress:
-//  - service: https://localhost:8000
-//`},
-//			want: Ingress{Rules: []Rule{
-//				{
-//					Service: localhost8000,
-//				},
-//			}},
-//		},
-//		{
-//			name: "Invalid service",
-//			args: args{rawYAML: `
-//ingress:
-//  - hostname: "*"
-//    service: https://local host:8000
-//`},
-//			wantErr: true,
-//		},
-//		{
-//			name: "Invalid YAML",
-//			args: args{rawYAML: `
-//key: "value
-//`},
-//			wantErr: true,
-//		},
-//		{
-//			name: "Last rule isn't catchall",
-//			args: args{rawYAML: `
-//ingress:
-//  - hostname: example.com
-//    service: https://localhost:8000
-//`},
-//			wantErr: true,
-//		},
-//		{
-//			name: "First rule is catchall",
-//			args: args{rawYAML: `
-//ingress:
-//  - service: https://localhost:8000
-//  - hostname: example.com
-//    service: https://localhost:8000
-//`},
-//			wantErr: true,
-//		},
-//		{
-//			name: "Catch-all rule can't have a path",
-//			args: args{rawYAML: `
-//ingress:
-//  - service: https://localhost:8001
-//    path: /subpath1/(.*)/subpath2
-//`},
-//			wantErr: true,
-//		},
-//		{
-//			name: "Invalid regex",
-//			args: args{rawYAML: `
-//ingress:
-//  - hostname: example.com
-//    service: https://localhost:8000
-//    path: "*/subpath2"
-//  - service: https://localhost:8001
-//`},
-//			wantErr: true,
-//		},
-//		{
-//			name: "Service must have a scheme",
-//			args: args{rawYAML: `
-//ingress:
-//  - service: localhost:8000
-//`},
-//			wantErr: true,
-//		},
-//		{
-//			name: "Wildcard not at start",
-//			args: args{rawYAML: `
-//ingress:
-//  - hostname: "test.*.example.com"
-//    service: https://localhost:8000
-//`},
-//			wantErr: true,
-//		},
-//		{
-//			name: "Can't use --url",
-//			args: args{rawYAML: `
-//url: localhost:8080
-//ingress:
-//  - hostname: "*.example.com"
-//    service: https://localhost:8000
-//`},
-//			wantErr: true,
-//		},
-//		{
-//			name: "Service can't have a path",
-//			args: args{rawYAML: `
-//ingress:
-//  - service: https://localhost:8000/static/
-//`},
-//			wantErr: true,
-//		},
-//	}
-//	for _, tt := range tests {
-//		t.Run(tt.name, func(t *testing.T) {
-//			got, err := ParseIngress([]byte(tt.args.rawYAML))
-//			if (err != nil) != tt.wantErr {
-//				t.Errorf("ParseIngress() error = %v, wantErr %v", err, tt.wantErr)
-//				return
-//			}
-//			if !reflect.DeepEqual(got, tt.want) {
-//				t.Errorf("ParseIngress() = %v, want %v", got, tt.want)
-//			}
-//		})
-//	}
-//}
+func Test_parseIngress(t *testing.T) {
+	localhost8000 := MustParseURL(t, "https://localhost:8000")
+	localhost8001 := MustParseURL(t, "https://localhost:8001")
+	type args struct {
+		rawYAML string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    Ingress
+		wantErr bool
+	}{
+		{
+			name:    "Empty file",
+			args:    args{rawYAML: ""},
+			wantErr: true,
+		},
+		{
+			name: "Multiple rules",
+			args: args{rawYAML: `
+ingress:
+ - hostname: tunnel1.example.com
+   service: https://localhost:8000
+ - hostname: "*"
+   service: https://localhost:8001
+`},
+			want: Ingress{Rules: []Rule{
+				{
+					Hostname: "tunnel1.example.com",
+					Service:  localhost8000,
+				},
+				{
+					Hostname: "*",
+					Service:  localhost8001,
+				},
+			}},
+		},
+		{
+			name: "Extra keys",
+			args: args{rawYAML: `
+ingress:
+ - hostname: "*"
+   service: https://localhost:8000
+extraKey: extraValue
+`},
+			want: Ingress{Rules: []Rule{
+				{
+					Hostname: "*",
+					Service:  localhost8000,
+				},
+			}},
+		},
+		{
+			name: "Hostname can be omitted",
+			args: args{rawYAML: `
+ingress:
+ - service: https://localhost:8000
+`},
+			want: Ingress{Rules: []Rule{
+				{
+					Service: localhost8000,
+				},
+			}},
+		},
+		{
+			name: "Invalid service",
+			args: args{rawYAML: `
+ingress:
+ - hostname: "*"
+   service: https://local host:8000
+`},
+			wantErr: true,
+		},
+		{
+			name: "Last rule isn't catchall",
+			args: args{rawYAML: `
+ingress:
+ - hostname: example.com
+   service: https://localhost:8000
+`},
+			wantErr: true,
+		},
+		{
+			name: "First rule is catchall",
+			args: args{rawYAML: `
+ingress:
+ - service: https://localhost:8000
+ - hostname: example.com
+   service: https://localhost:8000
+`},
+			wantErr: true,
+		},
+		{
+			name: "Catch-all rule can't have a path",
+			args: args{rawYAML: `
+ingress:
+ - service: https://localhost:8001
+   path: /subpath1/(.*)/subpath2
+`},
+			wantErr: true,
+		},
+		{
+			name: "Invalid regex",
+			args: args{rawYAML: `
+ingress:
+ - hostname: example.com
+   service: https://localhost:8000
+   path: "*/subpath2"
+ - service: https://localhost:8001
+`},
+			wantErr: true,
+		},
+		{
+			name: "Service must have a scheme",
+			args: args{rawYAML: `
+ingress:
+ - service: localhost:8000
+`},
+			wantErr: true,
+		},
+		{
+			name: "Wildcard not at start",
+			args: args{rawYAML: `
+ingress:
+ - hostname: "test.*.example.com"
+   service: https://localhost:8000
+`},
+			wantErr: true,
+		},
+		{
+			name: "Service can't have a path",
+			args: args{rawYAML: `
+ingress:
+ - service: https://localhost:8000/static/
+`},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := ParseIngress(MustReadIngress(tt.args.rawYAML))
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ParseIngress() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}

-func MustParse(t *testing.T, rawURL string) *url.URL {
+func MustParseURL(t *testing.T, rawURL string) *url.URL {
 	u, err := url.Parse(rawURL)
 	require.NoError(t, err)
 	return u
@@ -207,7 +187,7 @@ func Test_rule_matches(t *testing.T) {
 				Hostname: "example.com",
 			},
 			args: args{
-				requestURL: MustParse(t, "https://example.com"),
+				requestURL: MustParseURL(t, "https://example.com"),
 			},
 			want: true,
 		},
@@ -217,7 +197,7 @@ func Test_rule_matches(t *testing.T) {
 				Hostname: "*",
 			},
 			args: args{
-				requestURL: MustParse(t, "https://example.com"),
+				requestURL: MustParseURL(t, "https://example.com"),
 			},
 			want: true,
 		},
@@ -227,7 +207,7 @@ func Test_rule_matches(t *testing.T) {
 				Hostname: "example.com",
 			},
 			args: args{
-				requestURL: MustParse(t, "https://foo.bar"),
+				requestURL: MustParseURL(t, "https://foo.bar"),
 			},
 			want: false,
 		},
@@ -237,7 +217,7 @@ func Test_rule_matches(t *testing.T) {
 				Hostname: "*.example.com",
 			},
 			args: args{
-				requestURL: MustParse(t, "https://adam.example.com"),
+				requestURL: MustParseURL(t, "https://adam.example.com"),
 			},
 			want: true,
 		},
@@ -247,7 +227,7 @@ func Test_rule_matches(t *testing.T) {
 				Hostname: "*.example.com",
 			},
 			args: args{
-				requestURL: MustParse(t, "https://tunnel.com"),
+				requestURL: MustParseURL(t, "https://tunnel.com"),
 			},
 			want: false,
 		},
@@ -257,7 +237,7 @@ func Test_rule_matches(t *testing.T) {
 				Hostname: "*example.com",
 			},
 			args: args{
-				requestURL: MustParse(t, "https://www.example.com"),
+				requestURL: MustParseURL(t, "https://www.example.com"),
 			},
 			want: false,
 		},
@@ -267,7 +247,7 @@ func Test_rule_matches(t *testing.T) {
 				Hostname: "*.example.com",
 			},
 			args: args{
-				requestURL: MustParse(t, "https://adam.chalmers.example.com"),
+				requestURL: MustParseURL(t, "https://adam.chalmers.example.com"),
 			},
 			want: true,
 		},
@@ -278,7 +258,7 @@ func Test_rule_matches(t *testing.T) {
 				Path:     regexp.MustCompile("/static/.*\\.html"),
 			},
 			args: args{
-				requestURL: MustParse(t, "https://www.example.com/static/index.html"),
+				requestURL: MustParseURL(t, "https://www.example.com/static/index.html"),
 			},
 			want: true,
 		},
@@ -298,23 +278,33 @@ func Test_rule_matches(t *testing.T) {
 	}
 }

-//func BenchmarkFindMatch(b *testing.B) {
-//	rulesYAML := `
-//ingress:
-//  - hostname: tunnel1.example.com
-//    service: https://localhost:8000
-//  - hostname: tunnel2.example.com
-//    service: https://localhost:8001
-//  - hostname: "*"
-//    service: https://localhost:8002
-//`
-//	ing, err := ParseIngress([]byte(rulesYAML))
-//	if err != nil {
-//		b.Error(err)
-//	}
-//	for n := 0; n < b.N; n++ {
-//		ing.FindMatchingRule("tunnel1.example.com", "")
-//		ing.FindMatchingRule("tunnel2.example.com", "")
-//		ing.FindMatchingRule("tunnel3.example.com", "")
-//	}
-//}
+func BenchmarkFindMatch(b *testing.B) {
+	rulesYAML := `
+ingress:
+ - hostname: tunnel1.example.com
+   service: https://localhost:8000
+ - hostname: tunnel2.example.com
+   service: https://localhost:8001
+ - hostname: "*"
+   service: https://localhost:8002
+`
+
+	ing, err := ParseIngress(MustReadIngress(rulesYAML))
+	if err != nil {
+		b.Error(err)
+	}
+	for n := 0; n < b.N; n++ {
+		ing.FindMatchingRule("tunnel1.example.com", "")
+		ing.FindMatchingRule("tunnel2.example.com", "")
+		ing.FindMatchingRule("tunnel3.example.com", "")
+	}
+}
+
+func MustReadIngress(s string) UnvalidatedIngress {
+	var ing UnvalidatedIngress
+	err := yaml.Unmarshal([]byte(s), &ing)
+	if err != nil {
+		panic(err)
+	}
+	return ing
+}