Refactor mirror feature

This commit is contained in:
Manuel Alejandro de Brito Fontes 2020-02-04 23:06:07 -03:00
parent b9e944a8a6
commit b3146354d4
14 changed files with 178 additions and 128 deletions

View file

@ -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,

View file

@ -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:

View file

@ -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)

View file

@ -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)

View file

@ -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
}

View file

@ -17,7 +17,6 @@ limitations under the License.
package mirror
import (
"fmt"
"reflect"
"testing"
@ -29,41 +28,28 @@ 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: "",
}},
}
@ -71,6 +57,7 @@ func TestParse(t *testing.T) {
ObjectMeta: meta_v1.ObjectMeta{
Name: "foo",
Namespace: api.NamespaceDefault,
UID: "c89a5111-b2e9-4af8-be19-c2a4a924c256",
},
Spec: networking.IngressSpec{},
}
@ -78,7 +65,6 @@ func TestParse(t *testing.T) {
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)
}

View file

@ -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
}

View file

@ -17,6 +17,7 @@ limitations under the License.
package parser
import (
"net/url"
"testing"
api "k8s.io/api/core/v1"
@ -93,8 +94,8 @@ func TestGetStringAnnotation(t *testing.T) {
{"valid - B", "string", " B", "B", false},
{"empty", "string", " ", "", true},
{"valid multiline", "string", `
rewrite (?i)/arcgis/rest/services/Utilities/Geometry/GeometryServer(.*)$ /arcgis/rest/services/Utilities/Geometry/GeometryServer$1 break;
rewrite (?i)/arcgis/services/Utilities/Geometry/GeometryServer(.*)$ /arcgis/services/Utilities/Geometry/GeometryServer$1 break;
rewrite (?i)/arcgis/rest/services/Utilities/Geometry/GeometryServer(.*)$ /arcgis/rest/services/Utilities/Geometry/GeometryServer$1 break;
rewrite (?i)/arcgis/services/Utilities/Geometry/GeometryServer(.*)$ /arcgis/services/Utilities/Geometry/GeometryServer$1 break;
`, `
rewrite (?i)/arcgis/rest/services/Utilities/Geometry/GeometryServer(.*)$ /arcgis/rest/services/Utilities/Geometry/GeometryServer$1 break;
rewrite (?i)/arcgis/services/Utilities/Geometry/GeometryServer(.*)$ /arcgis/services/Utilities/Geometry/GeometryServer$1 break;
@ -162,3 +163,40 @@ func TestGetIntAnnotation(t *testing.T) {
delete(data, test.field)
}
}
func TestStringToURL(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 := StringToURL(test.url)
if test.expErr {
if err == nil {
t.Fatalf("%v: expected error but none returned", test.title)
}
if err.Error() != 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)
}
}
}

View file

@ -30,6 +30,7 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/ingress-nginx/internal/ingress/annotations/authreq"
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/internal/ingress/controller/config"
ing_net "k8s.io/ingress-nginx/internal/net"
"k8s.io/ingress-nginx/internal/runtime"
@ -211,7 +212,7 @@ func ReadConfig(src map[string]string) config.Configuration {
if val, ok := conf[globalAuthURL]; ok {
delete(conf, globalAuthURL)
authURL, message := authreq.ParseStringToURL(val)
authURL, message := parser.StringToURL(val)
if authURL == nil {
klog.Warningf("Global auth location denied - %v.", message)
} else {
@ -235,7 +236,7 @@ func ReadConfig(src map[string]string) config.Configuration {
if val, ok := conf[globalAuthSignin]; ok {
delete(conf, globalAuthSignin)
signinURL, _ := authreq.ParseStringToURL(val)
signinURL, _ := parser.StringToURL(val)
if signinURL == nil {
klog.Warningf("Global auth location denied - %v.", "global-auth-signin setting is undefined and will not be set")
} else {

View file

@ -180,6 +180,7 @@ var (
"buildOpentracingForLocation": buildOpentracingForLocation,
"shouldLoadOpentracingModule": shouldLoadOpentracingModule,
"buildModSecurityForLocation": buildModSecurityForLocation,
"buildMirrorLocations": buildMirrorLocations,
}
)
@ -1377,3 +1378,29 @@ modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf;
return buffer.String()
}
func buildMirrorLocations(locs []*ingress.Location) string {
var buffer bytes.Buffer
mapped := sets.String{}
for _, loc := range locs {
if loc.Mirror.Source == "" || loc.Mirror.Target == "" {
continue
}
if mapped.Has(loc.Mirror.Source) {
continue
}
mapped.Insert(loc.Mirror.Source)
buffer.WriteString(fmt.Sprintf(`location = %v {
internal;
proxy_pass %v;
}
`, loc.Mirror.Source, loc.Mirror.Target))
}
return buffer.String()
}

View file

@ -440,18 +440,14 @@ func (l1 *Location) Equal(l2 *Location) bool {
return false
}
if l1.Mirror.URI != l2.Mirror.URI {
return false
}
if l1.Mirror.RequestBody != l2.Mirror.RequestBody {
return false
}
if !l1.Opentracing.Equal(&l2.Opentracing) {
return false
}
if !l1.Mirror.Equal(&l2.Mirror) {
return false
}
return true
}

View file

@ -798,7 +798,7 @@ stream {
{{ if not (empty $server.CertificateAuth.CRLFileName) }}
# PEM sha: {{ $server.CertificateAuth.CRLSHA }}
ssl_crl {{ $server.CertificateAuth.CRLFileName }};
ssl_crl {{ $server.CertificateAuth.CRLFileName }};
{{ end }}
{{ if not (empty $server.CertificateAuth.ErrorPage)}}
@ -832,6 +832,7 @@ stream {
{{ template "CUSTOM_ERRORS" (buildCustomErrorDeps $errorLocation.UpstreamName $errorLocation.Codes $all.EnableMetrics) }}
{{ end }}
{{ buildMirrorLocations $server.Locations }}
{{ $enforceRegex := enforceRegexModifier $server.Locations }}
{{ range $location := $server.Locations }}
@ -965,8 +966,8 @@ stream {
{{ buildOpentracingForLocation $all.Cfg.EnableOpentracing $location }}
{{ if $location.Mirror.URI }}
mirror {{ $location.Mirror.URI }};
{{ if $location.Mirror.Source }}
mirror {{ $location.Mirror.Source }};
mirror_request_body {{ $location.Mirror.RequestBody }};
{{ end }}

View file

@ -17,6 +17,7 @@ limitations under the License.
package annotations
import (
"fmt"
"strings"
. "github.com/onsi/ginkgo"
@ -35,32 +36,50 @@ var _ = framework.IngressNginxDescribe("Annotations - Mirror", func() {
AfterEach(func() {
})
It("should set mirror-uri to /mirror", func() {
It("should set mirror-target to http://localhost/mirror", func() {
annotations := map[string]string{
"nginx.ingress.kubernetes.io/mirror-uri": "/mirror",
"nginx.ingress.kubernetes.io/mirror-target": "http://localhost/mirror",
}
ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
f.EnsureIngress(ing)
ing = f.EnsureIngress(ing)
f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, "mirror /mirror;") && strings.Contains(server, "mirror_request_body on;")
return strings.Contains(server, fmt.Sprintf("mirror /_mirror-%v;", ing.UID)) &&
strings.Contains(server, "mirror_request_body on;")
})
})
It("should set mirror-target to https://test.env.com/$request_uri", func() {
annotations := map[string]string{
"nginx.ingress.kubernetes.io/mirror-target": "https://test.env.com/$request_uri",
}
ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
ing = f.EnsureIngress(ing)
f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, fmt.Sprintf("mirror /_mirror-%v;", ing.UID)) &&
strings.Contains(server, "mirror_request_body on;") &&
strings.Contains(server, "proxy_pass https://test.env.com/$request_uri;")
})
})
It("should disable mirror-request-body", func() {
annotations := map[string]string{
"nginx.ingress.kubernetes.io/mirror-uri": "/mirror",
"nginx.ingress.kubernetes.io/mirror-uri": "http://localhost/mirror",
"nginx.ingress.kubernetes.io/mirror-request-body": "off",
}
ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
f.EnsureIngress(ing)
ing = f.EnsureIngress(ing)
f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, "mirror /mirror;") && strings.Contains(server, "mirror_request_body off;")
return strings.Contains(server, fmt.Sprintf("mirror /_mirror-%v;", ing.UID)) &&
strings.Contains(server, "mirror_request_body off;")
})
})
})

View file

@ -306,7 +306,7 @@ func newDeployment(name, namespace, image string, port int32, replicas int32, co
volumeMounts []corev1.VolumeMount, volumes []corev1.Volume) *appsv1.Deployment {
probe := &corev1.Probe{
InitialDelaySeconds: 1,
PeriodSeconds: 5,
PeriodSeconds: 10,
SuccessThreshold: 1,
TimeoutSeconds: 1,
Handler: corev1.Handler{