Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/user-guide/nginx-configuration/annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down Expand Up @@ -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`:
Expand Down
40 changes: 32 additions & 8 deletions internal/ingress/annotations/authreq/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,15 @@ import (
type Config struct {
URL string `json:"url"`
// Host contains the hostname defined in the URL
Host string `json:"host"`
SigninURL string `json:"signinUrl"`
Method string `json:"method"`
ResponseHeaders []string `json:"responseHeaders,omitempty"`
RequestRedirect string `json:"requestRedirect"`
AuthSnippet string `json:"authSnippet"`
AuthCacheKey string `json:"authCacheKey"`
AuthCacheDuration []string `json:"authCacheDuration"`
Host string `json:"host"`
SigninURL string `json:"signinUrl"`
Method string `json:"method"`
ResponseHeaders []string `json:"responseHeaders,omitempty"`
RequestRedirect string `json:"requestRedirect"`
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
Expand Down Expand Up @@ -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{
Expand All @@ -217,6 +240,7 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
AuthSnippet: authSnippet,
AuthCacheKey: authCacheKey,
AuthCacheDuration: authCacheDuration,
ProxySetHeaders: proxySetHeaders,
}, nil
}

Expand Down
52 changes: 52 additions & 0 deletions internal/ingress/annotations/authreq/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
19 changes: 10 additions & 9 deletions internal/ingress/controller/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -820,12 +820,13 @@ type ListenPorts struct {
type GlobalExternalAuth struct {
URL string `json:"url"`
// Host contains the hostname defined in the URL
Host string `json:"host"`
SigninURL string `json:"signinUrl"`
Method string `json:"method"`
ResponseHeaders []string `json:"responseHeaders,omitempty"`
RequestRedirect string `json:"requestRedirect"`
AuthSnippet string `json:"authSnippet"`
AuthCacheKey string `json:"authCacheKey"`
AuthCacheDuration []string `json:"authCacheDuration"`
Host string `json:"host"`
SigninURL string `json:"signinUrl"`
Method string `json:"method"`
ResponseHeaders []string `json:"responseHeaders,omitempty"`
RequestRedirect string `json:"requestRedirect"`
AuthSnippet string `json:"authSnippet"`
AuthCacheKey string `json:"authCacheKey"`
AuthCacheDuration []string `json:"authCacheDuration"`
ProxySetHeaders map[string]string `json:"proxySetHeaders,omitempty"`
}
14 changes: 14 additions & 0 deletions internal/ingress/controller/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ var (
"buildAuthLocation": buildAuthLocation,
"shouldApplyGlobalAuth": shouldApplyGlobalAuth,
"buildAuthResponseHeaders": buildAuthResponseHeaders,
"buildAuthProxySetHeaders": buildAuthProxySetHeaders,
"buildProxyPass": buildProxyPass,
"filterRateLimits": filterRateLimits,
"buildRateLimitZones": buildRateLimitZones,
Expand Down Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions internal/ingress/controller/template/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
16 changes: 11 additions & 5 deletions internal/ingress/resolver/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,23 @@ limitations under the License.
package resolver

import (
"errors"

apiv1 "k8s.io/api/core/v1"

"k8s.io/ingress-nginx/internal/ingress/defaults"
)

// Mock implements the Resolver interface
type Mock struct {
ConfigMaps map[string]*apiv1.ConfigMap
}

// GetDefaultBackend returns the backend that must be used as default
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
Expand All @@ -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")
}
4 changes: 4 additions & 0 deletions rootfs/etc/nginx/template/nginx.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
21 changes: 21 additions & 0 deletions test/e2e/annotations/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
25 changes: 21 additions & 4 deletions test/e2e/framework/framework.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,14 +261,18 @@ 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")
}

config, err := f.KubeClientSet.
CoreV1().
ConfigMaps(f.Namespace).
Get("nginx-configuration", metav1.GetOptions{})
Get(name, metav1.GetOptions{})
if err != nil {
return nil, err
}
Expand All @@ -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")

Expand All @@ -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()
Expand Down