Merge pull request #4550 from Shopify/upstream-auth-proxy-set-headers

Add support for configmap of headers for auth-url per ingress

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

@@ -30,6 +30,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
 |[nginx.ingress.kubernetes.io/auth-url](#external-authentication)|string|
 |[nginx.ingress.kubernetes.io/auth-cache-key](#external-authentication)|string|
 |[nginx.ingress.kubernetes.io/auth-cache-duration](#external-authentication)|string|
+|[nginx.ingress.kubernetes.io/auth-proxy-set-headers](#external-authentication)|string|
 |[nginx.ingress.kubernetes.io/auth-snippet](#external-authentication)|string|
 |[nginx.ingress.kubernetes.io/enable-global-auth](#external-authentication)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/backend-protocol](#backend-protocol)|string|HTTP,HTTPS,GRPC,GRPCS,AJP|
@@ -414,6 +415,8 @@ Additionally it is possible to set:
   `<SignIn_URL>` to specify the location of the error page.
 * `nginx.ingress.kubernetes.io/auth-response-headers`:
   `<Response_Header_1, ..., Response_Header_n>` to specify headers to pass to backend once authentication request completes.
+* `nginx.ingress.kubernetes.io/auth-proxy-set-headers`:
+  `<ConfigMap>` the name of a ConfigMap that specifies headers to pass to the authentication service
 * `nginx.ingress.kubernetes.io/auth-request-redirect`:
   `<Request_Redirect_URL>`  to specify the X-Auth-Request-Redirect header value.
 * `nginx.ingress.kubernetes.io/auth-cache-key`:

internal/ingress/annotations/authreq/main.go

@@ -44,6 +44,7 @@ type Config struct {
 	AuthSnippet       string            `json:"authSnippet"`
 	AuthCacheKey      string            `json:"authCacheKey"`
 	AuthCacheDuration []string          `json:"authCacheDuration"`
+	ProxySetHeaders   map[string]string `json:"proxySetHeaders",omitempty`
 }
 
 // DefaultCacheDuration is the fallback value if no cache duration is provided
@@ -205,6 +206,28 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
 		}
 	}
 
+	proxySetHeaderMap, err := parser.GetStringAnnotation("auth-proxy-set-headers", ing)
+	if err != nil {
+		klog.V(3).Infof("auth-set-proxy-headers annotation is undefined and will not be set")
+	}
+
+	var proxySetHeaders map[string]string
+
+	if proxySetHeaderMap != "" {
+		proxySetHeadersMapContents, err := a.r.GetConfigMap(proxySetHeaderMap)
+		if err != nil {
+			return nil, ing_errors.NewLocationDenied(fmt.Sprintf("unable to find configMap %q", proxySetHeaderMap))
+		}
+
+		for header, value := range proxySetHeadersMapContents.Data {
+			if !ValidHeader(header) || !ValidHeader(value) {
+				return nil, ing_errors.NewLocationDenied("invalid proxy-set-headers in configmap")
+			}
+		}
+
+		proxySetHeaders = proxySetHeadersMapContents.Data
+	}
+
 	requestRedirect, _ := parser.GetStringAnnotation("auth-request-redirect", ing)
 
 	return &Config{
@@ -217,6 +240,7 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
 		AuthSnippet:       authSnippet,
 		AuthCacheKey:      authCacheKey,
 		AuthCacheDuration: authCacheDuration,
+		ProxySetHeaders:   proxySetHeaders,
 	}, nil
 }
 

internal/ingress/annotations/authreq/main_test.go

@@ -298,5 +298,57 @@ func TestParseStringToCacheDurations(t *testing.T) {
 			t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.expectedDurations, dur)
 		}
 	}
-
+}
+
+func TestProxySetHeaders(t *testing.T) {
+	ing := buildIngress()
+
+	data := map[string]string{}
+	ing.SetAnnotations(data)
+
+	tests := []struct {
+		title   string
+		url     string
+		headers map[string]string
+		expErr  bool
+	}{
+		{"single header", "http://goog.url", map[string]string{"header": "h1"}, false},
+		{"no header map", "http://goog.url", nil, true},
+		{"header with spaces", "http://goog.url", map[string]string{"header": "bad value"}, true},
+		{"header with other bad symbols", "http://goog.url", map[string]string{"header": "bad+value"}, true},
+	}
+
+	for _, test := range tests {
+		data[parser.GetAnnotationWithPrefix("auth-url")] = test.url
+		data[parser.GetAnnotationWithPrefix("auth-proxy-set-headers")] = "proxy-headers-map"
+		data[parser.GetAnnotationWithPrefix("auth-method")] = "GET"
+
+		configMapResolver := &resolver.Mock{
+			ConfigMaps: map[string]*api.ConfigMap{},
+		}
+
+		if test.headers != nil {
+			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 {
+				t.Errorf("expected error but retuned nil")
+			}
+			continue
+		}
+
+		t.Log(i)
+		u, ok := i.(*Config)
+		if !ok {
+			t.Errorf("%v: expected an External type", test.title)
+			continue
+		}
+
+		if !reflect.DeepEqual(u.ProxySetHeaders, test.headers) {
+			t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.headers, u.ProxySetHeaders)
+		}
+	}
 }

internal/ingress/controller/config/config.go

@@ -645,7 +645,7 @@ func NewDefault() Configuration {
 	defNginxStatusIpv4Whitelist = append(defNginxStatusIpv4Whitelist, "127.0.0.1")
 	defNginxStatusIpv6Whitelist = append(defNginxStatusIpv6Whitelist, "::1")
 	defProxyDeadlineDuration := time.Duration(5) * time.Second
-	defGlobalExternalAuth := GlobalExternalAuth{"", "", "", "", append(defResponseHeaders, ""), "", "", "", []string{}}
+	defGlobalExternalAuth := GlobalExternalAuth{"", "", "", "", append(defResponseHeaders, ""), "", "", "", []string{}, map[string]string{}}
 
 	cfg := Configuration{
 		AllowBackendServerHeader:         false,
@@ -828,4 +828,5 @@ type GlobalExternalAuth struct {
 	AuthSnippet       string            `json:"authSnippet"`
 	AuthCacheKey      string            `json:"authCacheKey"`
 	AuthCacheDuration []string          `json:"authCacheDuration"`
+	ProxySetHeaders   map[string]string `json:"proxySetHeaders,omitempty"`
 }

internal/ingress/controller/template/template.go

@@ -141,6 +141,7 @@ var (
 		"buildAuthLocation":               buildAuthLocation,
 		"shouldApplyGlobalAuth":           shouldApplyGlobalAuth,
 		"buildAuthResponseHeaders":        buildAuthResponseHeaders,
+		"buildAuthProxySetHeaders":        buildAuthProxySetHeaders,
 		"buildProxyPass":                  buildProxyPass,
 		"filterRateLimits":                filterRateLimits,
 		"buildRateLimitZones":             buildRateLimitZones,
@@ -463,6 +464,19 @@ func buildAuthResponseHeaders(headers []string) []string {
 	return res
 }
 
+func buildAuthProxySetHeaders(headers map[string]string) []string {
+	res := []string{}
+
+	if len(headers) == 0 {
+		return res
+	}
+
+	for name, value := range headers {
+		res = append(res, fmt.Sprintf("proxy_set_header '%v' '%v';", name, value))
+	}
+	return res
+}
+
 // buildProxyPass produces the proxy pass string, if the ingress has redirects
 // (specified through the nginx.ingress.kubernetes.io/rewrite-target annotation)
 // If the annotation nginx.ingress.kubernetes.io/add-base-url:"true" is specified it will

internal/ingress/controller/template/template_test.go

@@ -450,6 +450,23 @@ func TestBuildAuthResponseHeaders(t *testing.T) {
 	}
 }
 
+func TestBuildAuthProxySetHeaders(t *testing.T) {
+	proxySetHeaders := map[string]string{
+		"header1": "value1",
+		"header2": "value2",
+	}
+	expected := []string{
+		"proxy_set_header 'header1' 'value1';",
+		"proxy_set_header 'header2' 'value2';",
+	}
+
+	headers := buildAuthProxySetHeaders(proxySetHeaders)
+
+	if !reflect.DeepEqual(expected, headers) {
+		t.Errorf("Expected \n'%v'\nbut returned \n'%v'", expected, headers)
+	}
+}
+
 func TestTemplateWithData(t *testing.T) {
 	pwd, _ := os.Getwd()
 	f, err := os.Open(path.Join(pwd, "../../../../test/data/config.json"))

internal/ingress/resolver/mock.go

@@ -17,6 +17,8 @@ limitations under the License.
 package resolver
 
 import (
+	"errors"
+
 	apiv1 "k8s.io/api/core/v1"
 
 	"k8s.io/ingress-nginx/internal/ingress/defaults"
@@ -24,6 +26,7 @@ import (
 
 // Mock implements the Resolver interface
 type Mock struct {
+	ConfigMaps map[string]*apiv1.ConfigMap
 }
 
 // GetDefaultBackend returns the backend that must be used as default
@@ -31,11 +34,6 @@ func (m Mock) GetDefaultBackend() defaults.Backend {
 	return defaults.Backend{}
 }
 
-// GetConfigMap searches for configmap containing the namespace and name usting the character /
-func (m Mock) GetConfigMap(string) (*apiv1.ConfigMap, error) {
-	return nil, nil
-}
-
 // GetSecret searches for secrets contenating the namespace and name using a the character /
 func (m Mock) GetSecret(string) (*apiv1.Secret, error) {
 	return nil, nil
@@ -52,3 +50,11 @@ func (m Mock) GetAuthCertificate(string) (*AuthSSLCert, error) {
 func (m Mock) GetService(string) (*apiv1.Service, error) {
 	return nil, nil
 }
+
+// GetConfigMap searches for configMaps contenating the namespace and name using a the character /
+func (m Mock) GetConfigMap(name string) (*apiv1.ConfigMap, error) {
+	if v, ok := m.ConfigMaps[name]; ok {
+		return v, nil
+	}
+	return nil, errors.New("no configmap")
+}

rootfs/etc/nginx/template/nginx.tmpl

@@ -935,6 +935,10 @@ stream {
             proxy_set_header ssl-client-issuer-dn   $ssl_client_i_dn;
             {{ end }}
 
+            {{- range $line := buildAuthProxySetHeaders $externalAuth.ProxySetHeaders}}
+            {{ $line }}
+            {{- end }}
+
             {{ if not (empty $externalAuth.AuthSnippet) }}
             {{ $externalAuth.AuthSnippet }}
             {{ end }}

test/e2e/annotations/auth.go

@@ -291,6 +291,27 @@ var _ = framework.IngressNginxDescribe("Annotations - Auth", func() {
 			})
 	})
 
+	It(`should set "proxy_set_header 'My-Custom-Header' '42';" when auth-headers are set`, func() {
+		host := "auth"
+
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/auth-url":               "http://foo.bar/basic-auth/user/password",
+			"nginx.ingress.kubernetes.io/auth-proxy-set-headers": f.Namespace + "/auth-headers",
+		}
+
+		f.CreateConfigMap("auth-headers", map[string]string{
+			"My-Custom-Header": "42",
+		})
+
+		ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, &annotations)
+		f.EnsureIngress(ing)
+
+		f.WaitForNginxServer(host,
+			func(server string) bool {
+				return Expect(server).Should(ContainSubstring(`proxy_set_header 'My-Custom-Header' '42';`))
+			})
+	})
+
 	It(`should set cache_key when external auth cache is configured`, func() {
 		host := "auth"
 

test/e2e/framework/framework.go

@@ -261,6 +261,10 @@ func (f *Framework) matchNginxConditions(name string, matcher func(cfg string) b
 }
 
 func (f *Framework) getNginxConfigMap() (*v1.ConfigMap, error) {
+	return f.getConfigMap("nginx-configuration")
+}
+
+func (f *Framework) getConfigMap(name string) (*v1.ConfigMap, error) {
 	if f.KubeClientSet == nil {
 		return nil, fmt.Errorf("KubeClientSet not initialized")
 	}
@@ -268,7 +272,7 @@ func (f *Framework) getNginxConfigMap() (*v1.ConfigMap, error) {
 	config, err := f.KubeClientSet.
 		CoreV1().
 		ConfigMaps(f.Namespace).
-		Get("nginx-configuration", metav1.GetOptions{})
+		Get(name, metav1.GetOptions{})
 	if err != nil {
 		return nil, err
 	}
@@ -291,9 +295,11 @@ func (f *Framework) GetNginxConfigMapData() (map[string]string, error) {
 
 // SetNginxConfigMapData sets ingress-nginx's nginx-configuration configMap data
 func (f *Framework) SetNginxConfigMapData(cmData map[string]string) {
-	// Needs to do a Get and Set, Update will not take just the Data field
-	// or a configMap that is not the very last revision
-	config, err := f.getNginxConfigMap()
+	f.SetConfigMapData("nginx-configuration", cmData)
+}
+
+func (f *Framework) SetConfigMapData(name string, cmData map[string]string) {
+	config, err := f.getConfigMap(name)
 	Expect(err).NotTo(HaveOccurred())
 	Expect(config).NotTo(BeNil(), "expected a configmap but none returned")
 
@@ -308,6 +314,17 @@ func (f *Framework) SetNginxConfigMapData(cmData map[string]string) {
 	time.Sleep(5 * time.Second)
 }
 
+func (f *Framework) CreateConfigMap(name string, data map[string]string) {
+	_, err := f.KubeClientSet.CoreV1().ConfigMaps(f.Namespace).Create(&v1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: f.Namespace,
+		},
+		Data: data,
+	})
+	Expect(err).NotTo(HaveOccurred(), "failed to create configMap")
+}
+
 // UpdateNginxConfigMapData updates single field in ingress-nginx's nginx-configuration map data
 func (f *Framework) UpdateNginxConfigMapData(key string, value string) {
 	config, err := f.GetNginxConfigMapData()