diff --git a/internal/configs/annotations.go b/internal/configs/annotations.go index d9c2e526fc..ecd5c72178 100644 --- a/internal/configs/annotations.go +++ b/internal/configs/annotations.go @@ -213,6 +213,11 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool cfgParams.ProxyPassHeaders = proxyPassHeaders } + if proxySetHeaders, exists := GetMapKeyAsStringSlice(ingEx.Ingress.Annotations, "nginx.org/proxy-set-headers", ingEx.Ingress, ","); exists { + parsedHeaders := parseProxySetHeaders(proxySetHeaders) + cfgParams.ProxySetHeaders = parsedHeaders + } + if clientMaxBodySize, exists := ingEx.Ingress.Annotations["nginx.org/client-max-body-size"]; exists { cfgParams.ClientMaxBodySize = clientMaxBodySize } diff --git a/internal/configs/config_params.go b/internal/configs/config_params.go index 73b61cab1f..28a6654ce1 100644 --- a/internal/configs/config_params.go +++ b/internal/configs/config_params.go @@ -1,6 +1,7 @@ package configs import ( + "github.com/nginxinc/kubernetes-ingress/internal/configs/version2" "github.com/nginxinc/kubernetes-ingress/internal/nginx" conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1" ) @@ -70,6 +71,7 @@ type ConfigParams struct { ProxyHideHeaders []string ProxyMaxTempFileSize string ProxyPassHeaders []string + ProxySetHeaders []version2.Header ProxyProtocol bool ProxyReadTimeout string ProxySendTimeout string diff --git a/internal/configs/configmaps.go b/internal/configs/configmaps.go index fbcd63b68d..7b07b19532 100644 --- a/internal/configs/configmaps.go +++ b/internal/configs/configmaps.go @@ -3,6 +3,8 @@ package configs import ( "strings" + "github.com/nginxinc/kubernetes-ingress/internal/configs/version2" + "github.com/golang/glog" v1 "k8s.io/api/core/v1" @@ -66,6 +68,20 @@ func ParseConfigMap(cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasA cfgParams.ProxyPassHeaders = proxyPassHeaders } + if proxySetHeaders, exists := GetMapKeyAsStringSlice(cfgm.Data, "proxy-set-headers", cfgm, ","); exists { + var headers []version2.Header + for _, headerAndValue := range proxySetHeaders { + parts := strings.SplitN(headerAndValue, " ", 2) + name := strings.TrimSpace(parts[0]) + var value string + if len(parts) > 1 { + value = strings.TrimSpace(parts[1]) + } + headers = append(headers, version2.Header{Name: name, Value: value}) + } + cfgParams.ProxySetHeaders = headers + } + if clientMaxBodySize, exists := cfgm.Data["client-max-body-size"]; exists { cfgParams.ClientMaxBodySize = clientMaxBodySize } diff --git a/internal/configs/ingress.go b/internal/configs/ingress.go index 18e3a88b26..549f0888d5 100644 --- a/internal/configs/ingress.go +++ b/internal/configs/ingress.go @@ -475,6 +475,7 @@ func createLocation(path string, upstream version1.Upstream, cfg *ConfigParams, ProxyConnectTimeout: cfg.ProxyConnectTimeout, ProxyReadTimeout: cfg.ProxyReadTimeout, ProxySendTimeout: cfg.ProxySendTimeout, + ProxySetHeaders: cfg.ProxySetHeaders, ClientMaxBodySize: cfg.ClientMaxBodySize, Websocket: websocket, Rewrite: rewrite, diff --git a/internal/configs/parsing_helpers.go b/internal/configs/parsing_helpers.go index b48032e555..dc9645e9f3 100644 --- a/internal/configs/parsing_helpers.go +++ b/internal/configs/parsing_helpers.go @@ -7,6 +7,8 @@ import ( "strconv" "strings" + "github.com/nginxinc/kubernetes-ingress/internal/configs/version2" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -276,6 +278,20 @@ func ParseProxyBuffersSpec(s string) (string, error) { return "", errors.New("invalid proxy buffers string") } +// parseProxySetHeaders ensures that the string colon-separated list of headers and values +func parseProxySetHeaders(proxySetHeaders []string) []version2.Header { + var headers []version2.Header + for _, header := range proxySetHeaders { + parts := strings.SplitN(header, ":", 2) + if len(parts) == 1 { + headers = append(headers, version2.Header{Name: parts[0], Value: ""}) + } else { + headers = append(headers, version2.Header{Name: parts[0], Value: parts[1]}) + } + } + return headers +} + // ParsePortList ensures that the string is a comma-separated list of port numbers func ParsePortList(s string) ([]int, error) { var ports []int diff --git a/internal/configs/version1/config.go b/internal/configs/version1/config.go index bf376fe5be..c05edceb55 100644 --- a/internal/configs/version1/config.go +++ b/internal/configs/version1/config.go @@ -1,6 +1,9 @@ package version1 -import "github.com/nginxinc/kubernetes-ingress/internal/nginx" +import ( + "github.com/nginxinc/kubernetes-ingress/internal/configs/version2" + "github.com/nginxinc/kubernetes-ingress/internal/nginx" +) // UpstreamLabels describes the Prometheus labels for an NGINX upstream. type UpstreamLabels struct { @@ -166,6 +169,7 @@ type Location struct { ProxyConnectTimeout string ProxyReadTimeout string ProxySendTimeout string + ProxySetHeaders []version2.Header ClientMaxBodySize string Websocket bool Rewrite string diff --git a/internal/configs/version1/nginx-plus.ingress.tmpl b/internal/configs/version1/nginx-plus.ingress.tmpl index 68dbd13e4e..55b6439d24 100644 --- a/internal/configs/version1/nginx-plus.ingress.tmpl +++ b/internal/configs/version1/nginx-plus.ingress.tmpl @@ -260,9 +260,9 @@ server { proxy_set_header Upgrade $http_upgrade; proxy_set_header Connection $connection_upgrade; {{- else}} - {{- if $.Keepalive}}proxy_set_header Connection "";{{end}} + {{- if $.Keepalive}} + proxy_set_header Connection "";{{end}} {{- end}} - {{- if $location.LocationSnippets}} {{range $value := $location.LocationSnippets}} {{$value}}{{end}} @@ -285,6 +285,8 @@ server { proxy_read_timeout {{$location.ProxyReadTimeout}}; proxy_send_timeout {{$location.ProxySendTimeout}}; client_max_body_size {{$location.ClientMaxBodySize}}; + {{- $proxySetHeaders := generateProxySetHeaders $location $.Ingress.Annotations }} + {{$proxySetHeaders}} proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; diff --git a/internal/configs/version1/nginx.ingress.tmpl b/internal/configs/version1/nginx.ingress.tmpl index 9bc197e0a9..d018698a59 100644 --- a/internal/configs/version1/nginx.ingress.tmpl +++ b/internal/configs/version1/nginx.ingress.tmpl @@ -180,23 +180,23 @@ server { proxy_set_header Upgrade $http_upgrade; proxy_set_header Connection $connection_upgrade; {{- else}} - {{- if $.Keepalive}}proxy_set_header Connection "";{{end}} + {{- if $.Keepalive}} + proxy_set_header Connection "";{{end}} {{- end}} - {{- if $location.LocationSnippets}} {{range $value := $location.LocationSnippets}} {{$value}}{{end}} {{- end}} - {{- with $location.BasicAuth }} auth_basic {{ printf "%q" .Realm }}; auth_basic_user_file {{ .Secret }}; {{- end }} - proxy_connect_timeout {{$location.ProxyConnectTimeout}}; proxy_read_timeout {{$location.ProxyReadTimeout}}; proxy_send_timeout {{$location.ProxySendTimeout}}; client_max_body_size {{$location.ClientMaxBodySize}}; + {{- $proxySetHeaders := generateProxySetHeaders $location $.Ingress.Annotations -}} + {{$proxySetHeaders}} proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; diff --git a/internal/configs/version1/template_helper.go b/internal/configs/version1/template_helper.go index b93d2e80bd..565fde4295 100644 --- a/internal/configs/version1/template_helper.go +++ b/internal/configs/version1/template_helper.go @@ -1,7 +1,9 @@ package version1 import ( + "errors" "fmt" + "regexp" "strings" "text/template" @@ -64,14 +66,131 @@ func makePathWithRegex(path, regexType string) string { } } +var setHeader = regexp.MustCompile("^[-A-Za-z0-9]+$") + +func validateProxySetHeader(header string) error { + header = strings.TrimSpace(header) + if !setHeader.MatchString(header) { + return errors.New("invalid header syntax - syntax must be header: value") + } + return nil +} + +func defaultHeaderValues(headerParts []string, headerName string) string { + headerValue := strings.TrimSpace(headerParts[0]) + headerValue = strings.ReplaceAll(headerValue, "-", "_") + headerValue = strings.ToLower(headerValue) + return fmt.Sprintf("\n\t\tproxy_set_header %s $http_%s;", headerName, headerValue) +} + +func headersGreaterThanOne(headerParts []string, headerName string) string { + headerValue := strings.TrimSpace(headerParts[1]) + return fmt.Sprintf("\n\t\tproxy_set_header %s %q;", headerName, headerValue) +} + +func splitHeaders(header string) ([]string, string) { + header = strings.TrimSpace(header) + headerParts := strings.SplitN(header, ":", 2) + headerName := strings.TrimSpace(headerParts[0]) + return headerParts, headerName +} + +// minionProxySetHeaders takes a location and a bool map +// and generates proxy_set_header headers for minions based on the nginx.org/proxy-set-headers annotation in a mergeable ingress. +// It returns a string containing the generated proxy headers and a map to verify that the header exists +func minionProxySetHeaders(loc *Location, minionHeaders map[string]bool) (string, map[string]bool, error) { + proxySetHeaders, ok := loc.MinionIngress.Annotations["nginx.org/proxy-set-headers"] + if !ok { + return "", nil, nil + } + var combinedMinions string + headers := strings.Split(proxySetHeaders, ",") + for _, header := range headers { + headerParts, headerName := splitHeaders(header) + err := validateProxySetHeader(headerName) + if err != nil { + return "", nil, err + } + if len(headerParts) > 1 { + output := headersGreaterThanOne(headerParts, headerName) + minionHeaders[headerName] = true + combinedMinions += output + } else { + output := defaultHeaderValues(headerParts, headerName) + combinedMinions += output + } + } + return combinedMinions, minionHeaders, nil +} + +// standardIngressOrMasterProxySetHeaders takes two strings, two bools and a bool map +// and generates proxy_set_header headers based on the nginx.org/proxy-set-headers annotation in a standard ingress. +// It returns a string containing the generated proxy headers for either standardIngress/NonMergeable or Master. +func standardIngressOrMasterProxySetHeaders(result string, minionHeaders map[string]bool, proxySetHeaders string, ok bool, isMergeable bool) (string, error) { + if !ok { + return "", nil + } + headers := strings.Split(proxySetHeaders, ",") + for _, header := range headers { + headerParts, headerName := splitHeaders(header) + if isMergeable { + if _, ok := minionHeaders[headerName]; ok { + continue + } + } + if err := validateProxySetHeader(headerName); err != nil { + return "", err + } + if len(headerParts) > 1 { + output := headersGreaterThanOne(headerParts, headerName) + result += output + } else { + output := defaultHeaderValues(headerParts, headerName) + result += output + } + } + return result, nil +} + +// generateProxySetHeaders takes a location and an ingress annotations map +// and generates proxy_set_header directives based on the nginx.org/proxy-set-headers annotation. +// It returns a string containing the generated Nginx configuration to the template. +func generateProxySetHeaders(loc *Location, ingressAnnotations map[string]string) (string, error) { + proxySetHeaders, ok := ingressAnnotations["nginx.org/proxy-set-headers"] + var ingressResult string + minionHeaders := make(map[string]bool) + isMergeable := loc.MinionIngress != nil + if !isMergeable { + ingressResult, err := standardIngressOrMasterProxySetHeaders(ingressResult, minionHeaders, proxySetHeaders, ok, isMergeable) + if err != nil { + return "", err + } + return ingressResult, nil + } + combinedHeaders, minionHeaders, err := minionProxySetHeaders(loc, minionHeaders) + if err != nil { + return "", err + } + proxySetHeaders, ok = ingressAnnotations["nginx.org/proxy-set-headers"] + if !ok { + return combinedHeaders, nil + } + combinedHeaders, err = standardIngressOrMasterProxySetHeaders(combinedHeaders, minionHeaders, proxySetHeaders, ok, isMergeable) + if err != nil { + return "", err + } + return combinedHeaders, nil +} + var helperFunctions = template.FuncMap{ - "split": split, - "trim": trim, - "contains": strings.Contains, - "hasPrefix": strings.HasPrefix, - "hasSuffix": strings.HasSuffix, - "toLower": strings.ToLower, - "toUpper": strings.ToUpper, - "makeLocationPath": makeLocationPath, - "makeSecretPath": commonhelpers.MakeSecretPath, + "split": split, + "trim": trim, + "contains": strings.Contains, + "hasPrefix": strings.HasPrefix, + "hasSuffix": strings.HasSuffix, + "toLower": strings.ToLower, + "toUpper": strings.ToUpper, + "makeLocationPath": makeLocationPath, + "makeSecretPath": commonhelpers.MakeSecretPath, + "generateProxySetHeaders": generateProxySetHeaders, } diff --git a/internal/configs/version1/template_helper_test.go b/internal/configs/version1/template_helper_test.go index 10a833c377..c93d0fc6d2 100644 --- a/internal/configs/version1/template_helper_test.go +++ b/internal/configs/version1/template_helper_test.go @@ -2,6 +2,7 @@ package version1 import ( "bytes" + "strings" "testing" "text/template" ) @@ -475,3 +476,314 @@ func newToUpperTemplate(t *testing.T) *template.Template { } return tmpl } + +func TestGenerateProxySetHeadersForValidHeadersInMaster(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + annotations map[string]string + wantProxyHeaders []string + }{ + { + name: "Header with Number", + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-ABC1", + }, + wantProxyHeaders: []string{ + "proxy_set_header X-Forwarded-ABC1 $http_x_forwarded_abc1;", + }, + }, + { + name: "One Header", + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-ABC", + }, + wantProxyHeaders: []string{ + "proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;", + }, + }, + { + name: "Two Headers", + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-ABC,BVC", + }, + wantProxyHeaders: []string{ + "proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;", + "proxy_set_header BVC $http_bvc;", + }, + }, + { + name: "Two Headers with One Value", + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-ABC,BVC: test", + }, + wantProxyHeaders: []string{ + "proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;", + `proxy_set_header BVC "test";`, + }, + }, + { + name: "Three Headers", + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-ABC,BVC,X-Forwarded-Test", + }, + wantProxyHeaders: []string{ + "proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;", + "proxy_set_header BVC $http_bvc;", + "proxy_set_header X-Forwarded-Test $http_x_forwarded_test;", + }, + }, + { + name: "Three Headers with Two Value", + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-ABC: abc,BVC: bat,X-Forwarded-Test", + }, + wantProxyHeaders: []string{ + `proxy_set_header X-Forwarded-ABC "abc";`, + `proxy_set_header BVC "bat";`, + "proxy_set_header X-Forwarded-Test $http_x_forwarded_test;", + }, + }, + { + name: "One Header with Two Value", + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-ABC: test test2", + }, + wantProxyHeaders: []string{ + `proxy_set_header X-Forwarded-ABC "test test2";`, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + generatedConfig, err := generateProxySetHeaders(&Location{Path: ""}, tc.annotations) + if err != nil { + t.Fatal(err) + } + if len(tc.wantProxyHeaders) != strings.Count(generatedConfig, "\n") { + t.Fatalf("expected %d config lines, got %d", len(tc.wantProxyHeaders), strings.Count(generatedConfig, "\n")) + } + + for _, line := range tc.wantProxyHeaders { + if !strings.Contains(generatedConfig, line) { + t.Errorf("expected line %q not found in generated config", line) + } + } + }) + } +} + +func TestGenerateProxySetHeadersForInvalidHeadersForErrorsInMaster(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + annotations map[string]string + }{ + { + name: "Headers With Special Characters", + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-ABC!,BVC§", + }, + }, + { + name: "Headers with invalid Format", + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-ABC test", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, err := generateProxySetHeaders(&Location{Path: ""}, tc.annotations) + if err == nil { + t.Error("expected an error, but got nil") + } + }) + } +} + +func TestGenerateProxySetHeadersForValidHeadersInMasterAndTwoMinions(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + masterAnnotations map[string]string + coffeeAnnotations map[string]string + teaAnnotations map[string]string + wantCoffeeHeaders []string + wantTeaHeaders []string + }{ + { + name: "One Master Header and a unique header in Coffee and Tea", + masterAnnotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-ABC", + }, + coffeeAnnotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-Coffee", + }, + teaAnnotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-Tea", + }, + wantCoffeeHeaders: []string{ + "proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;", + "proxy_set_header X-Forwarded-Coffee $http_x_forwarded_coffee;", + }, + wantTeaHeaders: []string{ + "proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;", + "proxy_set_header X-Forwarded-Tea $http_x_forwarded_tea;", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + generatedMasterConfig, err := generateProxySetHeaders(&Location{Path: ""}, tc.masterAnnotations) + if err != nil { + t.Fatal(err) + } + generatedCoffeeConfig, err := generateProxySetHeaders(&Location{Path: "coffee"}, tc.coffeeAnnotations) + if err != nil { + t.Fatal(err) + } + generatedTeaConfig, err := generateProxySetHeaders(&Location{Path: "tea"}, tc.teaAnnotations) + if err != nil { + t.Fatal(err) + } + + generatedCoffeeConfig = generatedMasterConfig + "\n" + generatedCoffeeConfig + generatedTeaConfig = generatedMasterConfig + "\n" + generatedTeaConfig + + for _, wantHeader := range tc.wantCoffeeHeaders { + if !strings.Contains(generatedCoffeeConfig, wantHeader) { + t.Errorf("expected header %q not found in generated coffee config", wantHeader) + } + } + + for _, wantHeader := range tc.wantTeaHeaders { + if !strings.Contains(generatedTeaConfig, wantHeader) { + t.Errorf("expected header %q not found in generated tea config", wantHeader) + } + } + }) + } +} + +func TestGenerateProxySetHeadersForValidHeadersInMinionOverrideMaster(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + masterAnnotations map[string]string + coffeeAnnotations map[string]string + teaAnnotations map[string]string + wantCoffeeHeaders []string + wantTeaHeaders []string + }{ + { + name: "Coffee Overrides Master and Master still in Tea", + masterAnnotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-ABC", + }, + coffeeAnnotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-ABC: coffee", + }, + wantCoffeeHeaders: []string{ + `proxy_set_header X-Forwarded-ABC "coffee"`, + }, + wantTeaHeaders: []string{ + "proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + generatedMasterConfig, err := generateProxySetHeaders(&Location{Path: ""}, tc.masterAnnotations) + if err != nil { + t.Fatal(err) + } + generatedCoffeeConfig, err := generateProxySetHeaders(&Location{Path: "coffee"}, tc.coffeeAnnotations) + if err != nil { + t.Fatal(err) + } + generatedTeaConfig, err := generateProxySetHeaders(&Location{Path: "tea"}, tc.teaAnnotations) + if err != nil { + t.Fatal(err) + } + + generatedCoffeeConfig = generatedMasterConfig + "\n" + generatedCoffeeConfig + generatedTeaConfig = generatedMasterConfig + "\n" + generatedTeaConfig + + for _, wantHeader := range tc.wantCoffeeHeaders { + if !strings.Contains(generatedCoffeeConfig, wantHeader) { + t.Errorf("expected header %q not found in generated coffee config", wantHeader) + } + } + + for _, wantHeader := range tc.wantTeaHeaders { + if !strings.Contains(generatedTeaConfig, wantHeader) { + t.Errorf("expected header %q not found in generated tea config", wantHeader) + } + } + }) + } +} + +func TestGenerateProxySetHeadersForValidHeadersInOnlyOneMinion(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + masterAnnotations map[string]string + coffeeAnnotations map[string]string + teaAnnotations map[string]string + wantCoffeeHeaders []string + wantTeaHeaders []string + }{ + { + name: "Header in Coffee but not Tea or Master", + coffeeAnnotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-ABC: coffee", + }, + wantCoffeeHeaders: []string{ + `proxy_set_header X-Forwarded-ABC "coffee"`, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + generatedMasterConfig, err := generateProxySetHeaders(&Location{Path: ""}, tc.masterAnnotations) + if err != nil { + t.Fatal(err) + } + generatedCoffeeConfig, err := generateProxySetHeaders(&Location{Path: "coffee"}, tc.coffeeAnnotations) + if err != nil { + t.Fatal(err) + } + generatedTeaConfig, err := generateProxySetHeaders(&Location{Path: "tea"}, tc.teaAnnotations) + if err != nil { + t.Fatal(err) + } + + generatedCoffeeConfig = generatedMasterConfig + "\n" + generatedCoffeeConfig + generatedTeaConfig = generatedMasterConfig + "\n" + generatedTeaConfig + + for _, wantHeader := range tc.wantCoffeeHeaders { + if !strings.Contains(generatedCoffeeConfig, wantHeader) { + t.Errorf("expected header %q not found in generated coffee config", wantHeader) + } + } + + for _, wantHeader := range tc.wantTeaHeaders { + if !strings.Contains(generatedTeaConfig, wantHeader) { + t.Errorf("expected header %q not found in generated tea config", wantHeader) + } + } + }) + } +} diff --git a/internal/configs/version1/template_test.go b/internal/configs/version1/template_test.go index 95b39cb873..b7457b3373 100644 --- a/internal/configs/version1/template_test.go +++ b/internal/configs/version1/template_test.go @@ -829,6 +829,510 @@ func TestExecuteTemplate_ForMainForNGINXPlusWithHTTP2Off(t *testing.T) { } } +func TestExecuteTemplate_ForIngressForNGINXWithProxySetHeadersAnnotationWithDefaultValue(t *testing.T) { + t.Parallel() + + tmpl := newNGINXIngressTmpl(t) + testCases := []struct { + masterAnnotations map[string]string + coffeeAnnotations map[string]string + teaAnnotations map[string]string + wantCoffeeHeaders []string + wantTeaHeaders []string + }{ + { + masterAnnotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-ABC", + }, + wantCoffeeHeaders: []string{ + `proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;`, + }, + wantTeaHeaders: []string{ + `proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;`, + }, + }, + } + + for _, tc := range testCases { + buf := &bytes.Buffer{} + ingressCfg := createProxySetHeaderIngressConfig(tc.masterAnnotations, tc.coffeeAnnotations, tc.teaAnnotations) + + err := tmpl.Execute(buf, ingressCfg) + if err != nil { + t.Fatal(err) + } + + for _, wantHeader := range tc.wantCoffeeHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated coffee config", wantHeader) + } + } + + for _, wantHeader := range tc.wantTeaHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated tea config", wantHeader) + } + } + } +} + +func TestExecuteTemplate_ForIngressForNGINXMasterWithProxySetHeadersAnnotationWithCustomValue(t *testing.T) { + t.Parallel() + + tmpl := newNGINXIngressTmpl(t) + testCases := []struct { + masterAnnotations map[string]string + coffeeAnnotations map[string]string + teaAnnotations map[string]string + wantCoffeeHeaders []string + wantTeaHeaders []string + }{ + { + masterAnnotations: map[string]string{ + "nginx.org/proxy-set-headers": "X-Forwarded-ABC: valueABC", + }, + wantCoffeeHeaders: []string{ + `proxy_set_header X-Forwarded-ABC "valueABC";`, + }, + wantTeaHeaders: []string{ + `proxy_set_header X-Forwarded-ABC "valueABC";`, + }, + }, + } + + for _, tc := range testCases { + buf := &bytes.Buffer{} + ingressCfg := createProxySetHeaderIngressConfig(tc.masterAnnotations, tc.coffeeAnnotations, tc.teaAnnotations) + + err := tmpl.Execute(buf, ingressCfg) + if err != nil { + t.Fatal(err) + } + + for _, wantHeader := range tc.wantCoffeeHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated coffee config", wantHeader) + } + } + + for _, wantHeader := range tc.wantTeaHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated tea config", wantHeader) + } + } + } +} + +func TestExecuteTemplate_ForMergeableIngressForNGINXMasterWithoutAnnotationMinionsWithDefaultValuesWithProxySetHeadersAnnotation(t *testing.T) { + t.Parallel() + + tmpl := newNGINXIngressTmpl(t) + testCases := []struct { + masterAnnotations map[string]string + coffeeAnnotations map[string]string + teaAnnotations map[string]string + wantCoffeeHeaders []string + wantTeaHeaders []string + }{ + { + masterAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "master", + }, + coffeeAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + "nginx.org/proxy-set-headers": "X-Forwarded-Coffee", + }, + wantCoffeeHeaders: []string{ + `proxy_set_header X-Forwarded-Coffee $http_x_forwarded_coffee;`, + }, + teaAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + "nginx.org/proxy-set-headers": "X-Forwarded-Tea", + }, + wantTeaHeaders: []string{ + `proxy_set_header X-Forwarded-Tea $http_x_forwarded_tea;`, + }, + }, + } + + for _, tc := range testCases { + buf := &bytes.Buffer{} + ingressCfg := createProxySetHeaderIngressConfig(tc.masterAnnotations, tc.coffeeAnnotations, tc.teaAnnotations) + + err := tmpl.Execute(buf, ingressCfg) + if err != nil { + t.Fatal(err) + } + + for _, wantHeader := range tc.wantCoffeeHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated coffee config", wantHeader) + } + } + + for _, wantHeader := range tc.wantTeaHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated tea config", wantHeader) + } + } + } +} + +func TestExecuteTemplate_ForMergeableIngressForProxySetHeaderAnnotation(t *testing.T) { + t.Parallel() + + tmpl := newNGINXPlusIngressTmpl(t) + buf := &bytes.Buffer{} + + err := tmpl.Execute(buf, ingressCfgMasterMinionNGINXPlusMasterWithProxySetHeaderAnnotation) + t.Log(buf.String()) + if err != nil { + t.Fatal(err) + } + wantHeader := `proxy_set_header X-Forwarded-ABC "coffee";` + + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated coffee config", wantHeader) + } +} + +func TestExecuteTemplate_ForMergeableIngressForNGINXMasterWithoutAnnotationMinionsWithCustomValuesProxySetHeadersAnnotation(t *testing.T) { + t.Parallel() + + tmpl := newNGINXIngressTmpl(t) + testCases := []struct { + masterAnnotations map[string]string + coffeeAnnotations map[string]string + teaAnnotations map[string]string + wantCoffeeHeaders []string + wantTeaHeaders []string + }{ + { + masterAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "master", + }, + coffeeAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + "nginx.org/proxy-set-headers": "X-Forwarded-Minion: coffee", + }, + wantCoffeeHeaders: []string{ + `proxy_set_header X-Forwarded-Minion "coffee";`, + }, + teaAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + "nginx.org/proxy-set-headers": "X-Forwarded-Minion: tea", + }, + wantTeaHeaders: []string{ + `proxy_set_header X-Forwarded-Minion "tea";`, + }, + }, + } + + for _, tc := range testCases { + buf := &bytes.Buffer{} + ingressCfg := createProxySetHeaderIngressConfig(tc.masterAnnotations, tc.coffeeAnnotations, tc.teaAnnotations) + + err := tmpl.Execute(buf, ingressCfg) + if err != nil { + t.Fatal(err) + } + + for _, wantHeader := range tc.wantCoffeeHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated coffee config", wantHeader) + } + } + + for _, wantHeader := range tc.wantTeaHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated tea config", wantHeader) + } + } + } +} + +func TestExecuteTemplate_ForMergeableIngressForNGINXMasterWithoutAnnotationMinionsWithDifferentHeadersForProxySetHeadersAnnotation(t *testing.T) { + t.Parallel() + + tmpl := newNGINXIngressTmpl(t) + testCases := []struct { + masterAnnotations map[string]string + coffeeAnnotations map[string]string + teaAnnotations map[string]string + wantCoffeeHeaders []string + wantTeaHeaders []string + }{ + { + masterAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "master", + }, + coffeeAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + "nginx.org/proxy-set-headers": "X-Forwarded-Coffee: mocha", + }, + wantCoffeeHeaders: []string{ + `proxy_set_header X-Forwarded-Coffee "mocha";`, + }, + teaAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + "nginx.org/proxy-set-headers": "X-Forwarded-Tea: green", + }, + wantTeaHeaders: []string{ + `proxy_set_header X-Forwarded-Tea "green";`, + }, + }, + } + + for _, tc := range testCases { + buf := &bytes.Buffer{} + ingressCfg := createProxySetHeaderIngressConfig(tc.masterAnnotations, tc.coffeeAnnotations, tc.teaAnnotations) + + err := tmpl.Execute(buf, ingressCfg) + if err != nil { + t.Fatal(err) + } + + for _, wantHeader := range tc.wantCoffeeHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated coffee config", wantHeader) + } + } + + for _, wantHeader := range tc.wantTeaHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated tea config", wantHeader) + } + } + } +} + +func TestExecuteTemplate_ForMergeableIngressForNGINXMasterWithAnnotationForProxySetHeadersAnnotation(t *testing.T) { + t.Parallel() + + tmpl := newNGINXIngressTmpl(t) + testCases := []struct { + masterAnnotations map[string]string + coffeeAnnotations map[string]string + teaAnnotations map[string]string + wantCoffeeHeaders []string + wantTeaHeaders []string + }{ + { + masterAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "master", + "nginx.org/proxy-set-headers": "X-Forwarded-ABC", + }, + coffeeAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + }, + wantCoffeeHeaders: []string{ + `proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;`, + }, + teaAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + }, + wantTeaHeaders: []string{ + `proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;`, + }, + }, + } + + for _, tc := range testCases { + buf := &bytes.Buffer{} + ingressCfg := createProxySetHeaderIngressConfig(tc.masterAnnotations, tc.coffeeAnnotations, tc.teaAnnotations) + + err := tmpl.Execute(buf, ingressCfg) + if err != nil { + t.Fatal(err) + } + + for _, wantHeader := range tc.wantCoffeeHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated coffee config", wantHeader) + } + } + + for _, wantHeader := range tc.wantTeaHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated tea config", wantHeader) + } + } + } +} + +func TestExecuteTemplate_ForMergeableIngressForNGINXMasterMinionsWithDifferentHeadersForProxySetHeadersAnnotation(t *testing.T) { + t.Parallel() + + tmpl := newNGINXIngressTmpl(t) + testCases := []struct { + masterAnnotations map[string]string + coffeeAnnotations map[string]string + teaAnnotations map[string]string + wantCoffeeHeaders []string + wantTeaHeaders []string + }{ + { + masterAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "master", + "nginx.org/proxy-set-headers": "X-Forwarded-ABC", + }, + coffeeAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + "nginx.org/proxy-set-headers": "X-Forwarded-Coffee: espresso", + }, + wantCoffeeHeaders: []string{ + `proxy_set_header X-Forwarded-Coffee "espresso"`, + "proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;", + }, + teaAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + "nginx.org/proxy-set-headers": "X-Forwarded-Tea: chai", + }, + wantTeaHeaders: []string{ + `proxy_set_header X-Forwarded-Tea "chai"`, + "proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;", + }, + }, + } + + for _, tc := range testCases { + buf := &bytes.Buffer{} + ingressCfg := createProxySetHeaderIngressConfig(tc.masterAnnotations, tc.coffeeAnnotations, tc.teaAnnotations) + + err := tmpl.Execute(buf, ingressCfg) + if err != nil { + t.Fatal(err) + } + + for _, wantHeader := range tc.wantCoffeeHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated coffee config", wantHeader) + } + } + + for _, wantHeader := range tc.wantTeaHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated tea config", wantHeader) + } + } + } +} + +func TestExecuteTemplate_ForMergeableIngressForNGINXWithProxySetHeadersAnnotationForMinionOverrideMaster(t *testing.T) { + t.Parallel() + + tmpl := newNGINXIngressTmpl(t) + testCases := []struct { + masterAnnotations map[string]string + coffeeAnnotations map[string]string + teaAnnotations map[string]string + wantCoffeeHeaders []string + wantTeaHeaders []string + }{ + { + masterAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "master", + "nginx.org/proxy-set-headers": "X-Forwarded-ABC", + }, + coffeeAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + "nginx.org/proxy-set-headers": "X-Forwarded-ABC: coffee", + }, + wantCoffeeHeaders: []string{ + `proxy_set_header X-Forwarded-ABC "coffee"`, + }, + wantTeaHeaders: []string{ + "proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;", + }, + }, + } + + for _, tc := range testCases { + buf := &bytes.Buffer{} + ingressCfg := createProxySetHeaderIngressConfig(tc.masterAnnotations, tc.coffeeAnnotations, tc.teaAnnotations) + + err := tmpl.Execute(buf, ingressCfg) + if err != nil { + t.Fatal(err) + } + + for _, wantHeader := range tc.wantCoffeeHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated coffee config", wantHeader) + } + } + + for _, wantHeader := range tc.wantTeaHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated tea config", wantHeader) + } + } + } +} + +func TestExecuteTemplate_ForMergeableIngressForNGINXMasterMinionsWithMultipleDifferentHeadersForProxySetHeadersAnnotation(t *testing.T) { + t.Parallel() + + tmpl := newNGINXIngressTmpl(t) + testCases := []struct { + masterAnnotations map[string]string + coffeeAnnotations map[string]string + teaAnnotations map[string]string + wantCoffeeHeaders []string + wantTeaHeaders []string + }{ + { + masterAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "master", + "nginx.org/proxy-set-headers": "X-Forwarded-ABC,BVC,Location: master", + }, + coffeeAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + "nginx.org/proxy-set-headers": "X-Forwarded-Coffee: espresso,X-Forwarded-Minion: coffee, Location: minion", + }, + wantCoffeeHeaders: []string{ + `proxy_set_header X-Forwarded-Coffee "espresso"`, + `proxy_set_header X-Forwarded-Minion "coffee"`, + "proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;", + "proxy_set_header BVC $http_bvc;", + `proxy_set_header Location "minion"`, + }, + teaAnnotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + "nginx.org/proxy-set-headers": "X-Forwarded-Tea: chai", + }, + wantTeaHeaders: []string{ + `proxy_set_header X-Forwarded-Tea "chai"`, + "proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;", + "proxy_set_header BVC $http_bvc;", + `proxy_set_header Location "master"`, + }, + }, + } + + for _, tc := range testCases { + buf := &bytes.Buffer{} + ingressCfg := createProxySetHeaderIngressConfig(tc.masterAnnotations, tc.coffeeAnnotations, tc.teaAnnotations) + + err := tmpl.Execute(buf, ingressCfg) + if err != nil { + t.Fatal(err) + } + + for _, wantHeader := range tc.wantCoffeeHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated coffee config", wantHeader) + } + } + + for _, wantHeader := range tc.wantTeaHeaders { + if !strings.Contains(buf.String(), wantHeader) { + t.Errorf("expected header %q not found in generated tea config", wantHeader) + } + } + } +} + func TestExecuteTemplate_ForIngressForNGINXPlusWithHTTP2On(t *testing.T) { t.Parallel() @@ -2207,6 +2711,82 @@ var ( }, } + // ingressCfgMasterMinionNGINXPlusMasterWithProxySetHeaderAnnotation holds data to test the following scenario: + // + // Ingress Master - Minion + // + // - Master: without `proxy-set-headers` annotation + // - Minion 1 (cafe-ingress-coffee-minion): with `proxy-set-headers annotation + // - Minion 2 (cafe-ingress-tea-minion): with `proxy-set-headers` annotation + ingressCfgMasterMinionNGINXPlusMasterWithProxySetHeaderAnnotation = IngressNginxConfig{ + Upstreams: []Upstream{ + coffeeUpstreamNginxPlus, + teaUpstreamNGINXPlus, + }, + Servers: []Server{ + { + Name: "cafe.example.com", + ServerTokens: "on", + Locations: []Location{ + { + Path: "/coffee", + ServiceName: "coffee-svc", + Upstream: coffeeUpstreamNginxPlus, + ProxyConnectTimeout: "60s", + ProxyReadTimeout: "60s", + ProxySendTimeout: "60s", + ClientMaxBodySize: "1m", + ProxyBuffering: true, + MinionIngress: &Ingress{ + Name: "cafe-ingress-coffee-minion", + Namespace: "default", + Annotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + "nginx.org/proxy-set-headers": "X-Forwarded-ABC: coffee", + }, + }, + ProxySSLName: "coffee-svc.default.svc", + }, + { + Path: "/tea", + ServiceName: "tea-svc", + Upstream: teaUpstreamNGINXPlus, + ProxyConnectTimeout: "60s", + ProxyReadTimeout: "60s", + ProxySendTimeout: "60s", + ClientMaxBodySize: "1m", + ProxyBuffering: true, + MinionIngress: &Ingress{ + Name: "cafe-ingress-tea-minion", + Namespace: "default", + Annotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "minion", + "nginx.org/proxy-set-headers": "X-Forwarded-ABC: tea", + }, + }, + ProxySSLName: "tea-svc.default.svc", + }, + }, + SSL: true, + SSLCertificate: "/etc/nginx/secrets/default-cafe-secret", + SSLCertificateKey: "/etc/nginx/secrets/default-cafe-secret", + StatusZone: "cafe.example.com", + HSTSMaxAge: 2592000, + Ports: []int{80}, + SSLPorts: []int{443}, + SSLRedirect: true, + HealthChecks: make(map[string]HealthCheck), + }, + }, + Ingress: Ingress{ + Name: "cafe-ingress-master", + Namespace: "default", + Annotations: map[string]string{ + "nginx.org/mergeable-ingress-type": "master", + }, + }, + } + // Ingress Config example without added annotations ingressCfgHTTP2On = IngressNginxConfig{ Servers: []Server{ @@ -2458,3 +3038,34 @@ var ( Headers: headers, } ) + +func createProxySetHeaderIngressConfig(masterAnnotations map[string]string, coffeeAnnotations map[string]string, teamAnnotations map[string]string) IngressNginxConfig { + return IngressNginxConfig{ + Servers: []Server{ + { + Name: "cafe.example.com", + Locations: []Location{ + { + MinionIngress: &Ingress{ + Name: "cafe-ingress-coffee-minion", + Namespace: "default", + Annotations: coffeeAnnotations, + }, + }, + { + MinionIngress: &Ingress{ + Name: "cafe-ingress-tea-minion", + Namespace: "default", + Annotations: teamAnnotations, + }, + }, + }, + }, + }, + Ingress: Ingress{ + Name: "cafe-ingress-master", + Namespace: "default", + Annotations: masterAnnotations, + }, + } +} diff --git a/internal/k8s/validation.go b/internal/k8s/validation.go index 9db7d8117f..99ca873ccb 100644 --- a/internal/k8s/validation.go +++ b/internal/k8s/validation.go @@ -32,6 +32,7 @@ const ( proxySendTimeoutAnnotation = "nginx.org/proxy-send-timeout" proxyHideHeadersAnnotation = "nginx.org/proxy-hide-headers" proxyPassHeadersAnnotation = "nginx.org/proxy-pass-headers" // #nosec G101 + proxySetHeadersAnnotation = "nginx.org/proxy-set-headers" clientMaxBodySizeAnnotation = "nginx.org/client-max-body-size" redirectToHTTPSAnnotation = "nginx.org/redirect-to-https" sslRedirectAnnotation = "ingress.kubernetes.io/ssl-redirect" @@ -168,6 +169,9 @@ var ( proxyPassHeadersAnnotation: { validateHTTPHeadersAnnotation, }, + proxySetHeadersAnnotation: { + validateProxySetHeaderAnnotation, + }, clientMaxBodySizeAnnotation: { validateRequiredAnnotation, validateOffsetAnnotation, @@ -410,6 +414,42 @@ func validateHTTPHeadersAnnotation(context *annotationValidationContext) field.E return allErrs } +func validateProxySetHeaderAnnotation(context *annotationValidationContext) field.ErrorList { + var allErrs field.ErrorList + headers := strings.Split(context.value, commaDelimiter) + + for _, header := range headers { + header = strings.TrimSpace(header) + + parts := strings.SplitN(header, ":", 2) + if len(parts) == 1 && strings.Contains(header, " ") { + allErrs = append(allErrs, field.Invalid(context.fieldPath, header, "invalid header syntax - spaces are not allowed in header")) + } + + if len(parts) != 2 { + parts = append(parts, "") + } + + name := strings.TrimSpace(parts[0]) + for _, msg := range validation.IsHTTPHeaderName(name) { + allErrs = append(allErrs, field.Invalid(context.fieldPath, name, msg)) + } + + value := strings.TrimSpace(parts[1]) + + if name == "" { + allErrs = append(allErrs, field.Invalid(context.fieldPath, header, "empty header name: "+header)) + continue + } + + if name == "" && value == "" { + allErrs = append(allErrs, field.Invalid(context.fieldPath, header, "invalid header syntax: "+header)) + continue + } + } + return allErrs +} + func sortedAnnotationNames(annotationValidations annotationValidationConfig) []string { sortedNames := make([]string, 0) for annotationName := range annotationValidations { diff --git a/internal/k8s/validation_test.go b/internal/k8s/validation_test.go index ee1ab174ac..9ad4378968 100644 --- a/internal/k8s/validation_test.go +++ b/internal/k8s/validation_test.go @@ -1234,6 +1234,98 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { }, msg: "invalid nginx.org/proxy-pass-headers annotation, multi-value containing '$' after valid header", }, + { + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "header-1", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/proxy-set-headers annotation, single-value", + }, + { + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "header-1,header-2,header-3", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/proxy-set-headers annotation, multi-value", + }, + { + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "header-1, header-2, header-3", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/proxy-set-headers annotation, multi-value with spaces", + }, + { + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "$header1", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/proxy-set-headers: Invalid value: "$header1": a valid HTTP header must consist of alphanumeric characters or '-' (e.g. 'X-Header-Name', regex used for validation is '[-A-Za-z0-9]+')`, + }, + msg: "invalid nginx.org/proxy-set-headers annotation, single-value containing '$'", + }, + { + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "{header1", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/proxy-set-headers: Invalid value: "{header1": a valid HTTP header must consist of alphanumeric characters or '-' (e.g. 'X-Header-Name', regex used for validation is '[-A-Za-z0-9]+')`, + }, + msg: "invalid nginx.org/proxy-set-headers annotation, single-value containing '{'", + }, + { + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "$header1,header2", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/proxy-set-headers: Invalid value: "$header1": a valid HTTP header must consist of alphanumeric characters or '-' (e.g. 'X-Header-Name', regex used for validation is '[-A-Za-z0-9]+')`, + }, + msg: "invalid nginx.org/proxy-set-headers annotation, multi-value containing '$'", + }, + { + annotations: map[string]string{ + "nginx.org/proxy-set-headers": "header1,$header2", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/proxy-set-headers: Invalid value: "$header2": a valid HTTP header must consist of alphanumeric characters or '-' (e.g. 'X-Header-Name', regex used for validation is '[-A-Za-z0-9]+')`, + }, + msg: "invalid nginx.org/proxy-set-headers annotation, multi-value containing '$' after valid header", + }, { annotations: map[string]string{ "nginx.org/client-max-body-size": "16M", diff --git a/tests/data/annotations/mergeable/minion-annotations-differ.yaml b/tests/data/annotations/mergeable/minion-annotations-differ.yaml index d6b944c85d..6f9e1f32c0 100644 --- a/tests/data/annotations/mergeable/minion-annotations-differ.yaml +++ b/tests/data/annotations/mergeable/minion-annotations-differ.yaml @@ -7,6 +7,7 @@ metadata: nginx.org/mergeable-ingress-type: "master" nginx.org/proxy-send-timeout: "10s" nginx.org/max-conns: "108" + nginx.org/proxy-set-headers: "X-Forwarded-ABC" spec: rules: - host: annotations.example.com @@ -20,6 +21,7 @@ metadata: nginx.org/mergeable-ingress-type: "minion" nginx.org/proxy-send-timeout: "25s" nginx.org/max-conns: "1048" + nginx.org/proxy-set-headers: "X-Forwarded-ABC: minionA" spec: rules: - host: annotations.example.com @@ -42,6 +44,7 @@ metadata: nginx.org/mergeable-ingress-type: "minion" nginx.org/proxy-send-timeout: "33s" nginx.org/max-conns: "1024" + nginx.org/proxy-set-headers: "X-Forwarded-ABC: minionB" spec: rules: - host: annotations.example.com diff --git a/tests/data/annotations/standard/annotations-ingress.yaml b/tests/data/annotations/standard/annotations-ingress.yaml index 6d4015bcd3..c455c55886 100644 --- a/tests/data/annotations/standard/annotations-ingress.yaml +++ b/tests/data/annotations/standard/annotations-ingress.yaml @@ -3,6 +3,7 @@ kind: Ingress metadata: annotations: kubernetes.io/ingress.class: "nginx" + nginx.org/proxy-set-headers: "X-Forwarded-ABC,ABC" name: annotations-ingress spec: rules: diff --git a/tests/suite/test_annotations.py b/tests/suite/test_annotations.py index 60e6a998f4..f222998816 100644 --- a/tests/suite/test_annotations.py +++ b/tests/suite/test_annotations.py @@ -245,6 +245,7 @@ def test_nginx_config_defaults(self, kube_apis, annotations_setup, ingress_contr "nginx.org/hsts": "True", "nginx.org/hsts-behind-proxy": "True", "nginx.org/upstream-zone-size": "124k", + "nginx.org/proxy-set-headers": "X-Forwarded-ABC", }, [ "proxy_send_timeout 10s;", @@ -255,6 +256,7 @@ def test_nginx_config_defaults(self, kube_apis, annotations_setup, ingress_contr "if ($http_x_forwarded_proto = 'https')", 'set $hsts_header_val "max-age=2592000; preload";', " 124k;", + "proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;", ], ["proxy_send_timeout 60s;", "if ($https = on)", " 256k;"], ) @@ -288,7 +290,6 @@ def test_when_annotation_in_ing_only( ingress_controller_prerequisites.namespace, ) new_events = get_events(kube_apis.v1, annotations_setup.namespace) - assert_event_count_increased(annotations_setup.ingress_event_text, initial_count, new_events) for _ in expected_strings: assert _ in result_conf @@ -455,6 +456,7 @@ def test_upstream_zone_size_0( "nginx.org/proxy-send-timeout": "invalid", "nginx.org/max-conns": "-10", "nginx.org/upstream-zone-size": "-10I'm S±!@£$%^&*()invalid", + "nginx.org/proxy-set-headers": "abc!123", } ) ], @@ -493,8 +495,19 @@ class TestMergeableFlows: [ ( f"{TEST_DATA}/annotations/mergeable/minion-annotations-differ.yaml", - ["proxy_send_timeout 25s;", "proxy_send_timeout 33s;", "max_conns=1048;", "max_conns=1024;"], - ["proxy_send_timeout 10s;", "max_conns=108;"], + [ + "proxy_send_timeout 25s;", + "proxy_send_timeout 33s;", + "max_conns=1048;", + "max_conns=1024;", + 'proxy_set_header X-Forwarded-ABC "minionA";', + 'proxy_set_header X-Forwarded-ABC "minionB";', + ], + [ + "proxy_send_timeout 10s;", + "max_conns=108;", + "proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;", + ], ), ], ) @@ -520,7 +533,6 @@ def test_minion_overrides_master( ingress_controller_prerequisites.namespace, ) new_events = get_events(kube_apis.v1, annotations_setup.namespace) - assert_event_count_increased(annotations_setup.ingress_event_text, initial_count, new_events) for _ in expected_strings: assert _ in result_conf @@ -528,6 +540,50 @@ def test_minion_overrides_master( assert _ not in result_conf +@pytest.mark.ingresses +@pytest.mark.annotations +@pytest.mark.parametrize("annotations_setup", ["standard"], indirect=True) +class TestStandardFlows: + @pytest.mark.parametrize( + "yaml_file, expected_strings, unexpected_strings", + [ + ( + f"{TEST_DATA}/annotations/standard/annotations-ingress.yaml", + ["proxy_set_header X-Forwarded-ABC $http_x_forwarded_abc;", "proxy_set_header ABC $http_abc;"], + [ + 'proxy_set_header X-Forwarded-ABC "ABC";', + ], + ), + ], + ) + def test_standard_ingress( + self, + kube_apis, + annotations_setup, + ingress_controller_prerequisites, + yaml_file, + expected_strings, + unexpected_strings, + ): + initial_events = get_events(kube_apis.v1, annotations_setup.namespace) + initial_count = get_event_count(annotations_setup.ingress_event_text, initial_events) + print("Case 8: standard ingress") + replace_ingresses_from_yaml(kube_apis.networking_v1, annotations_setup.namespace, yaml_file) + wait_before_test(1) + result_conf = get_ingress_nginx_template_conf( + kube_apis.v1, + annotations_setup.namespace, + annotations_setup.ingress_name, + annotations_setup.ingress_pod_name, + ingress_controller_prerequisites.namespace, + ) + new_events = get_events(kube_apis.v1, annotations_setup.namespace) + for _ in expected_strings: + assert _ in result_conf + for _ in unexpected_strings: + assert _ not in result_conf + + @pytest.mark.ingresses @pytest.mark.annotations class TestGrpcFlows: @@ -564,7 +620,6 @@ def test_grpc_flow( ingress_controller_prerequisites.namespace, ) new_events = get_events(kube_apis.v1, annotations_grpc_setup.namespace) - assert_event_count_increased(annotations_grpc_setup.ingress_event_text, initial_count, new_events) for _ in expected_strings: assert _ in result_conf