Refactor mirror feature

cmd/plugin/lints/ingress.go

@@ -66,6 +66,7 @@ func GetIngressLints() []IngressLint {
 		removedAnnotation("add-base-url", 3174, "0.22.0"),
 		removedAnnotation("base-url-scheme", 3174, "0.22.0"),
 		removedAnnotation("session-cookie-hash", 3743, "0.24.0"),
+		removedAnnotation("mirror-uri", 5015, "0.28.1"),
 		{
 			message: "The rewrite-target annotation value does not reference a capture group",
 			issue:   3174,

docs/user-guide/nginx-configuration/annotations.md

@@ -118,8 +118,8 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
 |[nginx.ingress.kubernetes.io/enable-owasp-core-rules](#modsecurity)|bool|
 |[nginx.ingress.kubernetes.io/modsecurity-transaction-id](#modsecurity)|string|
 |[nginx.ingress.kubernetes.io/modsecurity-snippet](#modsecurity)|string|
-|[nginx.ingress.kubernetes.io/mirror-uri](#mirror)|string|
 |[nginx.ingress.kubernetes.io/mirror-request-body](#mirror)|string|
+|[nginx.ingress.kubernetes.io/mirror-target](#mirror)|string|
 
 ### Canary
 
@@ -869,19 +869,10 @@ nginx.ingress.kubernetes.io/satisfy: "any"
 
 Enables a request to be mirrored to a mirror backend. Responses by mirror backends are ignored. This feature is useful, to see how requests will react in "test" backends.
 
-You can mirror a request to the `/mirror` path on your ingress, by applying the below:
+The mirror backend can be set by applying:
 
 ```yaml
-nginx.ingress.kubernetes.io/mirror-uri: "/mirror"
-```
-
-The mirror path can be defined as a separate ingress resource:
-
-```
-location = /mirror {
-    internal;
-    proxy_pass http://test_backend;
-}
+nginx.ingress.kubernetes.io/mirror-target: https://test.env.com/$request_uri
 ```
 
 By default the request-body is sent to the mirror backend, but can be turned off by applying:

internal/ingress/annotations/authreq/main.go

@@ -18,7 +18,6 @@ package authreq
 
 import (
 	"fmt"
-	"net/url"
 	"regexp"
 	"strings"
 
@@ -159,9 +158,9 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
 		return nil, err
 	}
 
-	authURL, message := ParseStringToURL(urlString)
-	if authURL == nil {
-		return nil, ing_errors.NewLocationDenied(message)
+	authURL, err := parser.StringToURL(urlString)
+	if err != nil {
+		return nil, ing_errors.InvalidContent{Name: err.Error()}
 	}
 
 	authMethod, _ := parser.GetStringAnnotation("auth-method", ing)
@@ -244,25 +243,6 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
 	}, nil
 }
 
-// ParseStringToURL parses the provided string into URL and returns error
-// message in case of failure
-func ParseStringToURL(input string) (*url.URL, string) {
-
-	parsedURL, err := url.Parse(input)
-	if err != nil {
-		return nil, fmt.Sprintf("%v is not a valid URL: %v", input, err)
-	}
-	if parsedURL.Scheme == "" {
-		return nil, "url scheme is empty."
-	} else if parsedURL.Host == "" {
-		return nil, "url host is empty."
-	} else if strings.Contains(parsedURL.Host, "..") {
-		return nil, "invalid url host."
-	}
-	return parsedURL, ""
-
-}
-
 // ParseStringToCacheDurations parses and validates the provided string
 // into a list of cache durations.
 // It will always return at least one duration (the default duration)

internal/ingress/annotations/authreq/main_test.go

@@ -18,7 +18,6 @@ package authreq
 
 import (
 	"fmt"
-	"net/url"
 	"reflect"
 	"testing"
 
@@ -173,7 +172,6 @@ func TestHeaderAnnotations(t *testing.T) {
 			continue
 		}
 
-		t.Log(i)
 		u, ok := i.(*Config)
 		if !ok {
 			t.Errorf("%v: expected an External type", test.title)
@@ -221,7 +219,6 @@ func TestCacheDurationAnnotations(t *testing.T) {
 			continue
 		}
 
-		t.Log(i)
 		u, ok := i.(*Config)
 		if !ok {
 			t.Errorf("%v: expected an External type", test.title)
@@ -234,41 +231,6 @@ func TestCacheDurationAnnotations(t *testing.T) {
 	}
 }
 
-func TestParseStringToURL(t *testing.T) {
-	validURL := "http://bar.foo.com/external-auth"
-	validParsedURL, _ := url.Parse(validURL)
-
-	tests := []struct {
-		title   string
-		url     string
-		message string
-		parsed  *url.URL
-		expErr  bool
-	}{
-		{"empty", "", "url scheme is empty.", nil, true},
-		{"no scheme", "bar", "url scheme is empty.", nil, true},
-		{"invalid host", "http://", "url host is empty.", nil, true},
-		{"invalid host (multiple dots)", "http://foo..bar.com", "invalid url host.", nil, true},
-		{"valid URL", validURL, "", validParsedURL, false},
-	}
-
-	for _, test := range tests {
-
-		i, err := ParseStringToURL(test.url)
-		if test.expErr {
-			if err != test.message {
-				t.Errorf("%v: expected error \"%v\" but \"%v\" was returned", test.title, test.message, err)
-			}
-			continue
-		}
-
-		if i.String() != test.parsed.String() {
-			t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.parsed, i)
-		}
-	}
-
-}
-
 func TestParseStringToCacheDurations(t *testing.T) {
 
 	tests := []struct {
@@ -331,7 +293,6 @@ func TestProxySetHeaders(t *testing.T) {
 			configMapResolver.ConfigMaps["proxy-headers-map"] = &api.ConfigMap{Data: test.headers}
 		}
 
-		t.Log(configMapResolver)
 		i, err := NewParser(configMapResolver).Parse(ing)
 		if test.expErr {
 			if err == nil {
@@ -340,7 +301,6 @@ func TestProxySetHeaders(t *testing.T) {
 			continue
 		}
 
-		t.Log(i)
 		u, ok := i.(*Config)
 		if !ok {
 			t.Errorf("%v: expected an External type", test.title)

internal/ingress/annotations/mirror/main.go

@@ -17,6 +17,8 @@ limitations under the License.
 package mirror
 
 import (
+	"fmt"
+
 	networking "k8s.io/api/networking/v1beta1"
 
 	"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
@@ -25,8 +27,34 @@ import (
 
 // Config returns the mirror to use in a given location
 type Config struct {
-	URI         string `json:"uri"`
+	Source      string `json:"source"`
 	RequestBody string `json:"requestBody"`
+	Target      string `json:"target"`
+}
+
+// Equal tests for equality between two Configuration types
+func (m1 *Config) Equal(m2 *Config) bool {
+	if m1 == m2 {
+		return true
+	}
+
+	if m1 == nil || m2 == nil {
+		return false
+	}
+
+	if m1.Source != m2.Source {
+		return false
+	}
+
+	if m1.RequestBody != m2.RequestBody {
+		return false
+	}
+
+	if m1.Target != m2.Target {
+		return false
+	}
+
+	return true
 }
 
 type mirror struct {
@@ -41,18 +69,20 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation {
 // ParseAnnotations parses the annotations contained in the ingress
 // rule used to configure mirror
 func (a mirror) Parse(ing *networking.Ingress) (interface{}, error) {
-	config := &Config{}
-	var err error
-
-	config.URI, err = parser.GetStringAnnotation("mirror-uri", ing)
-	if err != nil {
-		config.URI = ""
+	config := &Config{
+		Source: fmt.Sprintf("/_mirror-%v", ing.UID),
 	}
 
+	var err error
 	config.RequestBody, err = parser.GetStringAnnotation("mirror-request-body", ing)
 	if err != nil || config.RequestBody != "off" {
 		config.RequestBody = "on"
 	}
 
+	config.Target, err = parser.GetStringAnnotation("mirror-target", ing)
+	if err != nil {
+		config.Target = ""
+	}
+
 	return config, nil
 }

internal/ingress/annotations/mirror/main_test.go

@@ -17,7 +17,6 @@ limitations under the License.
 package mirror
 
 import (
-	"fmt"
 	"reflect"
 	"testing"
 
@@ -29,56 +28,43 @@ import (
 )
 
 func TestParse(t *testing.T) {
-	uri := parser.GetAnnotationWithPrefix("mirror-uri")
 	requestBody := parser.GetAnnotationWithPrefix("mirror-request-body")
+	backendURL := parser.GetAnnotationWithPrefix("mirror-target")
 
 	ap := NewParser(&resolver.Mock{})
 	if ap == nil {
 		t.Fatalf("expected a parser.IngressAnnotation but returned nil")
 	}
 
+	ngxURI := "/_mirror-c89a5111-b2e9-4af8-be19-c2a4a924c256"
 	testCases := []struct {
 		annotations map[string]string
 		expected    *Config
 	}{
-		{map[string]string{uri: "/mirror", requestBody: ""}, &Config{
-			URI:         "/mirror",
+		{map[string]string{backendURL: "https://test.env.com/$request_uri"}, &Config{
+			Source:      ngxURI,
 			RequestBody: "on",
+			Target:      "https://test.env.com/$request_uri",
 		}},
-		{map[string]string{uri: "/mirror", requestBody: "off"}, &Config{
-			URI:         "/mirror",
+		{map[string]string{requestBody: "off"}, &Config{
+			Source:      ngxURI,
 			RequestBody: "off",
-		}},
-		{map[string]string{uri: "", requestBody: "ahh"}, &Config{
-			URI:         "",
-			RequestBody: "on",
-		}},
-		{map[string]string{uri: "", requestBody: ""}, &Config{
-			URI:         "",
-			RequestBody: "on",
-		}},
-		{map[string]string{}, &Config{
-			URI:         "",
-			RequestBody: "on",
-		}},
-		{nil, &Config{
-			URI:         "",
-			RequestBody: "on",
+			Target:      "",
 		}},
 	}
 
 	ing := &networking.Ingress{
 		ObjectMeta: meta_v1.ObjectMeta{
 			Name:      "foo",
 			Namespace: api.NamespaceDefault,
+			UID:       "c89a5111-b2e9-4af8-be19-c2a4a924c256",
 		},
 		Spec: networking.IngressSpec{},
 	}
 
 	for _, testCase := range testCases {
 		ing.SetAnnotations(testCase.annotations)
 		result, _ := ap.Parse(ing)
-		fmt.Printf("%t", result)
 		if !reflect.DeepEqual(result, testCase.expected) {
 			t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, result, testCase.annotations)
 		}

internal/ingress/annotations/parser/main.go

@@ -18,6 +18,7 @@ package parser
 
 import (
 	"fmt"
+	"net/url"
 	"strconv"
 	"strings"
 
@@ -152,3 +153,22 @@ func AnnotationsReferencesConfigmap(ing *networking.Ingress) bool {
 
 	return false
 }
+
+// StringToURL parses the provided string into URL and returns error
+// message in case of failure
+func StringToURL(input string) (*url.URL, error) {
+	parsedURL, err := url.Parse(input)
+	if err != nil {
+		return nil, fmt.Errorf("%v is not a valid URL: %v", input, err)
+	}
+
+	if parsedURL.Scheme == "" {
+		return nil, fmt.Errorf("url scheme is empty")
+	} else if parsedURL.Host == "" {
+		return nil, fmt.Errorf("url host is empty")
+	} else if strings.Contains(parsedURL.Host, "..") {
+		return nil, fmt.Errorf("invalid url host")
+	}
+
+	return parsedURL, nil
+}