From 7b507095f464d6ddc7f07a2f9c2a3ad8c0240c65 Mon Sep 17 00:00:00 2001 From: Fernando Diaz Date: Tue, 29 Jan 2019 00:21:08 -0600 Subject: [PATCH] Increase Unit Test Coverage for Templates Increases the Coverage for nginx ingress template functions. The majority of the added unit tests are for checking the invalid type handling. --- .../ingress/controller/template/template.go | 5 +- .../controller/template/template_test.go | 312 +++++++++++++++++- 2 files changed, 304 insertions(+), 13 deletions(-) diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index 5386b26c5..9c1e0f8f2 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -230,9 +230,6 @@ func buildLuaSharedDictionaries(s interface{}, disableLuaRestyWAF bool) string { } } - if len(out) == 0 { - return "" - } return strings.Join(out, ";\n\r") + ";" } @@ -840,7 +837,7 @@ func buildOpentracing(input interface{}) string { if cfg.ZipkinCollectorHost != "" { buf.WriteString("opentracing_load_tracer /usr/local/lib/libzipkin_opentracing.so /etc/nginx/opentracing.json;") } else if cfg.JaegerCollectorHost != "" { - buf.WriteString("opentracing_load_tracer /usr/local/lib/libjaegertracing_plugin.so /etc/nginx/opentracing.json;") + buf.WriteString("opentracing_load_tracer /usr/local/lib/libjaegertracing_plugin.so /etc/nginx/opentracing.json;") } buf.WriteString("\r\n") diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index a1fbf642f..fd8032c56 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -22,6 +22,7 @@ import ( "os" "path" "reflect" + "sort" "strings" "testing" @@ -33,7 +34,9 @@ import ( "k8s.io/ingress-nginx/internal/file" "k8s.io/ingress-nginx/internal/ingress" "k8s.io/ingress-nginx/internal/ingress/annotations/authreq" + "k8s.io/ingress-nginx/internal/ingress/annotations/influxdb" "k8s.io/ingress-nginx/internal/ingress/annotations/luarestywaf" + "k8s.io/ingress-nginx/internal/ingress/annotations/ratelimit" "k8s.io/ingress-nginx/internal/ingress/annotations/rewrite" "k8s.io/ingress-nginx/internal/ingress/controller/config" ) @@ -163,6 +166,14 @@ proxy_pass http://upstream_balancer;`, ) func TestBuildLuaSharedDictionaries(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := "" + actual := buildLuaSharedDictionaries(invalidType, true) + + if !reflect.DeepEqual(expected, actual) { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + servers := []*ingress.Server{ { Hostname: "foo.bar", @@ -209,6 +220,14 @@ func TestFormatIP(t *testing.T) { } func TestBuildLocation(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := "/" + actual := buildLocation(invalidType, true) + + if !reflect.DeepEqual(expected, actual) { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + for k, tc := range tmplFuncTestcases { loc := &ingress.Location{ Path: tc.Path, @@ -263,6 +282,14 @@ func TestBuildProxyPass(t *testing.T) { } func TestBuildAuthLocation(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := "" + actual := buildAuthLocation(invalidType) + + if !reflect.DeepEqual(expected, actual) { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + authURL := "foo.com/auth" loc := &ingress.Location{ @@ -275,7 +302,7 @@ func TestBuildAuthLocation(t *testing.T) { str := buildAuthLocation(loc) encodedAuthURL := strings.Replace(base64.URLEncoding.EncodeToString([]byte(loc.Path)), "=", "", -1) - expected := fmt.Sprintf("/_external-auth-%v", encodedAuthURL) + expected = fmt.Sprintf("/_external-auth-%v", encodedAuthURL) if str != expected { t.Errorf("Expected \n'%v'\nbut returned \n'%v'", expected, str) @@ -283,11 +310,19 @@ func TestBuildAuthLocation(t *testing.T) { } func TestBuildAuthResponseHeaders(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := []string{} + actual := buildAuthResponseHeaders(invalidType) + + if !reflect.DeepEqual(expected, actual) { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + loc := &ingress.Location{ ExternalAuth: authreq.Config{ResponseHeaders: []string{"h1", "H-With-Caps-And-Dashes"}}, } headers := buildAuthResponseHeaders(loc) - expected := []string{ + expected = []string{ "auth_request_set $authHeader0 $upstream_http_h1;", "proxy_set_header 'h1' $authHeader0;", "auth_request_set $authHeader1 $upstream_http_h_with_caps_and_dashes;", @@ -378,6 +413,14 @@ func BenchmarkTemplateWithData(b *testing.B) { } func TestBuildDenyVariable(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := "" + actual := buildDenyVariable(invalidType) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + a := buildDenyVariable("host1.example.com_/.well-known/acme-challenge") b := buildDenyVariable("host1.example.com_/.well-known/acme-challenge") if !reflect.DeepEqual(a, b) { @@ -416,6 +459,14 @@ func TestBuildByteSize(t *testing.T) { } func TestIsLocationAllowed(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := false + actual := isLocationAllowed(invalidType) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + loc := ingress.Location{ Denied: nil, } @@ -427,23 +478,47 @@ func TestIsLocationAllowed(t *testing.T) { } func TestBuildForwardedFor(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := "" + actual := buildForwardedFor(invalidType) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + inputStr := "X-Forwarded-For" - outputStr := buildForwardedFor(inputStr) + expected = "$http_x_forwarded_for" + actual = buildForwardedFor(inputStr) - validStr := "$http_x_forwarded_for" - - if outputStr != validStr { - t.Errorf("Expected '%v' but returned '%v'", validStr, outputStr) + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) } } func TestBuildResolversForLua(t *testing.T) { + ipOne := net.ParseIP("192.0.0.1") ipTwo := net.ParseIP("2001:db8:1234:0000:0000:0000:0000:0000") ipList := []net.IP{ipOne, ipTwo} - expected := "\"192.0.0.1\", \"2001:db8:1234::\"" - actual := buildResolversForLua(ipList, false) + invalidType := &ingress.Ingress{} + expected := "" + actual := buildResolversForLua(invalidType, false) + + // Invalid Type for []net.IP + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + + actual = buildResolversForLua(ipList, invalidType) + + // Invalid Type for bool + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + + expected = "\"192.0.0.1\", \"2001:db8:1234::\"" + actual = buildResolversForLua(ipList, false) if expected != actual { t.Errorf("Expected '%v' but returned '%v'", expected, actual) @@ -462,6 +537,22 @@ func TestBuildResolvers(t *testing.T) { ipTwo := net.ParseIP("2001:db8:1234:0000:0000:0000:0000:0000") ipList := []net.IP{ipOne, ipTwo} + invalidType := &ingress.Ingress{} + expected := "" + actual := buildResolvers(invalidType, false) + + // Invalid Type for []net.IP + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + + actual = buildResolvers(ipList, invalidType) + + // Invalid Type for bool + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + validResolver := "resolver 192.0.0.1 [2001:db8:1234::] valid=30s;" resolver := buildResolvers(ipList, false) @@ -478,6 +569,14 @@ func TestBuildResolvers(t *testing.T) { } func TestBuildNextUpstream(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := "" + actual := buildNextUpstream(invalidType, "") + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + cases := map[string]struct { NextUpstream string NonIdempotent bool @@ -516,6 +615,14 @@ func TestBuildNextUpstream(t *testing.T) { } func TestBuildRateLimit(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := []string{} + actual := buildRateLimit(invalidType) + + if !reflect.DeepEqual(expected, actual) { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + loc := &ingress.Location{} loc.RateLimit.Connections.Name = "con" @@ -547,9 +654,45 @@ func TestBuildRateLimit(t *testing.T) { t.Errorf("Expected '%v' but returned '%v'", validLimits, limits) } } + + // Invalid limit + limits = buildRateLimit(&ingress.Ingress{}) + if !reflect.DeepEqual(expected, limits) { + t.Errorf("Expected '%v' but returned '%v'", expected, limits) + } +} + +// TODO: Needs more tests +func TestBuildRateLimitZones(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := []string{} + actual := buildRateLimitZones(invalidType) + + if !reflect.DeepEqual(expected, actual) { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } +} + +// TODO: Needs more tests +func TestFilterRateLimits(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := []ratelimit.Config{} + actual := filterRateLimits(invalidType) + + if !reflect.DeepEqual(expected, actual) { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } } func TestBuildAuthSignURL(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := "" + actual := buildAuthSignURL(invalidType) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + cases := map[string]struct { Input, Output string }{ @@ -566,6 +709,13 @@ func TestBuildAuthSignURL(t *testing.T) { } func TestIsLocationInLocationList(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := false + actual := isLocationInLocationList(invalidType, "") + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } testCases := []struct { location *ingress.Location @@ -589,6 +739,14 @@ func TestIsLocationInLocationList(t *testing.T) { } func TestBuildUpstreamName(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := "" + actual := buildUpstreamName(invalidType) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + defaultBackend := "upstream-name" defaultHost := "example.com" @@ -744,3 +902,139 @@ func TestGetIngressInformation(t *testing.T) { t.Errorf("Expected %v, but got %v", expected, info) } } + +func TestCollectCustomErrorsPerServer(t *testing.T) { + invalidType := &ingress.Ingress{} + customErrors := collectCustomErrorsPerServer(invalidType) + if customErrors != nil { + t.Errorf("Expected %v but returned %v", nil, customErrors) + } + + server := &ingress.Server{ + Locations: []*ingress.Location{ + {CustomHTTPErrors: []int{404, 405}}, + {CustomHTTPErrors: []int{404, 500}}, + }, + } + + expected := []int{404, 405, 500} + uniqueCodes := collectCustomErrorsPerServer(server) + sort.Ints(uniqueCodes) + + if !reflect.DeepEqual(expected, uniqueCodes) { + t.Errorf("Expected '%v' but got '%v'", expected, uniqueCodes) + } +} + +func TestProxySetHeader(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := "proxy_set_header" + actual := proxySetHeader(invalidType) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + + grpcBackend := &ingress.Location{ + BackendProtocol: "GRPC", + } + + expected = "grpc_set_header" + actual = proxySetHeader(grpcBackend) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } +} + +func TestBuildInfluxDB(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := "" + actual := buildInfluxDB(invalidType) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + + cfg := influxdb.Config{ + InfluxDBEnabled: true, + InfluxDBServerName: "ok.com", + InfluxDBHost: "host.com", + InfluxDBPort: "5252", + InfluxDBMeasurement: "ok", + } + expected = "influxdb server_name=ok.com host=host.com port=5252 measurement=ok enabled=true;" + actual = buildInfluxDB(cfg) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } +} + +func TestBuildOpenTracing(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := "" + actual := buildOpentracing(invalidType) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + + cfgJaeger := config.Configuration{ + EnableOpentracing: true, + JaegerCollectorHost: "jaeger-host.com", + } + expected = "opentracing_load_tracer /usr/local/lib/libjaegertracing_plugin.so /etc/nginx/opentracing.json;\r\n" + actual = buildOpentracing(cfgJaeger) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + + cfgZipkin := config.Configuration{ + EnableOpentracing: true, + ZipkinCollectorHost: "zipkin-host.com", + } + expected = "opentracing_load_tracer /usr/local/lib/libzipkin_opentracing.so /etc/nginx/opentracing.json;\r\n" + actual = buildOpentracing(cfgZipkin) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + +} + +func TestEnforceRegexModifier(t *testing.T) { + invalidType := &ingress.Ingress{} + expected := false + actual := enforceRegexModifier(invalidType) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + + locs := []*ingress.Location{ + { + Rewrite: rewrite.Config{ + Target: "/alright", + UseRegex: true, + }, + Path: "/ok", + }, + } + expected = true + actual = enforceRegexModifier(locs) + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } +} + +func TestStripLocationModifer(t *testing.T) { + expected := "ok.com" + actual := stripLocationModifer("~*ok.com") + + if expected != actual { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } +}