diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index 7d6dcc3fa6..bc051e11ee 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -52,6 +52,7 @@ import ( "k8s.io/ingress-nginx/internal/ingress/controller/process" "k8s.io/ingress-nginx/internal/ingress/controller/store" ngx_template "k8s.io/ingress-nginx/internal/ingress/controller/template" + "k8s.io/ingress-nginx/internal/ingress/controller/template/crossplane" "k8s.io/ingress-nginx/internal/ingress/metric" "k8s.io/ingress-nginx/internal/ingress/status" ing_net "k8s.io/ingress-nginx/internal/net" @@ -158,7 +159,7 @@ func NewNGINXController(config *Configuration, mc metric.Collector) *NGINXContro } onTemplateChange := func() { - template, err := ngx_template.NewTemplate(nginx.TemplatePath) + template, err := crossplane.NewTemplate() if err != nil { // this error is different from the rest because it must be clear why nginx is not working klog.ErrorS(err, "Error loading new template") @@ -170,7 +171,7 @@ func NewNGINXController(config *Configuration, mc metric.Collector) *NGINXContro n.syncQueue.EnqueueTask(task.GetDummyObject("template-change")) } - ngxTpl, err := ngx_template.NewTemplate(nginx.TemplatePath) + ngxTpl, err := crossplane.NewTemplate() if err != nil { klog.Fatalf("Invalid NGINX configuration template: %v", err) } diff --git a/internal/ingress/controller/template/crossplane/crossplane.go b/internal/ingress/controller/template/crossplane/crossplane.go index 1307de2f7a..0ab34f98dc 100644 --- a/internal/ingress/controller/template/crossplane/crossplane.go +++ b/internal/ingress/controller/template/crossplane/crossplane.go @@ -18,10 +18,12 @@ package crossplane import ( "bytes" + "os" ngx_crossplane "github.com/nginxinc/nginx-go-crossplane" "k8s.io/ingress-nginx/internal/ingress/controller/config" + "k8s.io/ingress-nginx/internal/ingress/controller/template/crossplane/extramodules" ) /* @@ -41,7 +43,7 @@ type Template struct { mimeFile string } -func NewTemplate() *Template { +func NewTemplate() (*Template, error) { lua := ngx_crossplane.Lua{} return &Template{ mimeFile: "/etc/nginx/mime.types", @@ -50,7 +52,7 @@ func NewTemplate() *Template { lua.RegisterBuilder(), }, }, - } + }, nil } func (c *Template) SetMimeFile(file string) { @@ -72,5 +74,48 @@ func (c *Template) Write(conf *config.TemplateConfig) ([]byte, error) { var buf bytes.Buffer err := ngx_crossplane.Build(&buf, *c.config, &ngx_crossplane.BuildOptions{}) + if err != nil { + return nil, err + } + + lua := ngx_crossplane.Lua{} + options := ngx_crossplane.ParseOptions{ + ErrorOnUnknownDirectives: true, + StopParsingOnError: true, + IgnoreDirectives: []string{"more_clear_headers", + "more_set_headers", + "opentelemetry_config", + "opentelemetry", + "opentelemetry_propagate", + "opentelemetry_trust_incoming_spans"}, // TODO: Add more_set_headers + DirectiveSources: []ngx_crossplane.MatchFunc{ + ngx_crossplane.DefaultDirectivesMatchFunc, + ngx_crossplane.MatchLuaLatest, + extramodules.BrotliMatchFn, + }, + LexOptions: ngx_crossplane.LexOptions{ + Lexers: []ngx_crossplane.RegisterLexer{lua.RegisterLexer()}, + }, + } + + tmpFile, err := os.CreateTemp("", "") + if err != nil { + return nil, err + } + defer func() { + _ = os.Remove(tmpFile.Name()) + _ = tmpFile.Close() + }() + + _, err = tmpFile.Write(buf.Bytes()) + if err != nil { + return nil, err + } + + _, err = ngx_crossplane.Parse(tmpFile.Name(), &options) + if err != nil { + return nil, err + } + return buf.Bytes(), err } diff --git a/internal/ingress/controller/template/crossplane/crossplane_test.go b/internal/ingress/controller/template/crossplane/crossplane_test.go index 5911889329..a3c2947776 100644 --- a/internal/ingress/controller/template/crossplane/crossplane_test.go +++ b/internal/ingress/controller/template/crossplane/crossplane_test.go @@ -102,7 +102,8 @@ func TestCrossplaneTemplate(t *testing.T) { require.NoError(t, err) require.NoError(t, mimeFile.Close()) - tpl := crossplane.NewTemplate() + tpl, err := crossplane.NewTemplate() + require.NoError(t, err) t.Run("it should be able to marshall and unmarshall the default configuration", func(t *testing.T) { tplConfig := defaultConfig() @@ -190,10 +191,12 @@ func TestCrossplaneTemplate(t *testing.T) { Backend: "somebackend", ClientBodyBufferSize: "512k", Proxy: proxy.Config{ + ProxyBuffering: "on", RequestBuffering: "on", BuffersNumber: 10, BufferSize: "1024k", ProxyHTTPVersion: "1.1", + NextUpstream: "10.10.10.10", }, ExternalAuth: authreq.Config{ AuthCacheDuration: []string{"60s"}, @@ -334,7 +337,9 @@ func TestCrossplaneTemplate(t *testing.T) { tplConfig.Cfg.UpstreamKeepaliveTimeout = 200 tplConfig.Cfg.UpstreamKeepaliveRequests = 15 - tpl = crossplane.NewTemplate() + tpl, err = crossplane.NewTemplate() + require.NoError(t, err) + tpl.SetMimeFile(mimeFile.Name()) content, err := tpl.Write(tplConfig) require.NoError(t, err) diff --git a/internal/ingress/controller/template/crossplane/location.go b/internal/ingress/controller/template/crossplane/location.go index 2852b64d33..fbd13cebaf 100644 --- a/internal/ingress/controller/template/crossplane/location.go +++ b/internal/ingress/controller/template/crossplane/location.go @@ -338,6 +338,9 @@ func (c *Template) buildAllowedLocation(server *ingress.Server, location *ingres buildDirective("proxy_http_version", location.Proxy.ProxyHTTPVersion), buildDirective("proxy_cookie_domain", location.Proxy.CookieDomain), buildDirective("proxy_cookie_path", location.Proxy.CookiePath), + buildDirective("proxy_next_upstream_timeout", location.Proxy.NextUpstreamTimeout), + buildDirective("proxy_next_upstream_tries", location.Proxy.NextUpstreamTries), + buildDirective("proxy_next_upstream", buildNextUpstream(location.Proxy.NextUpstream, c.tplConfig.Cfg.RetryNonIdempotent)), ) if isValidByteSize(location.Proxy.ProxyMaxTempFileSize, true) { @@ -364,6 +367,67 @@ func (c *Template) buildAllowedLocation(server *ingress.Server, location *ingres dir = append(dir, buildDirective(proxySetHeader, k, v)) } + for k, v := range location.CustomHeaders.Headers { + dir = append(dir, buildDirective("more_set_headers", fmt.Sprintf("%s: %s", k, strings.ReplaceAll(v, `$`, `${literal_dollar}`)))) + } + + if strings.HasPrefix(location.Backend, "custom-default-backend-") { + dir = append(dir, + buildDirective("proxy_set_header", "X-Code", "503"), + buildDirective("proxy_set_header", "X-Format", "$http_accept"), + buildDirective("proxy_set_header", "X-Namespace", "$namespace"), + buildDirective("proxy_set_header", "X-Ingress-Name", "$ingress_name"), + buildDirective("proxy_set_header", "X-Service-Name", "$service_name"), + buildDirective("proxy_set_header", "X-Service-Port", "$service_port"), + buildDirective("proxy_set_header", "X-Request-ID", "$req_id"), + ) + } + + if location.Satisfy != "" { + dir = append(dir, buildDirective("satisfy", location.Satisfy)) + } + + if len(location.CustomHTTPErrors) > 0 && !location.DisableProxyInterceptErrors { + dir = append(dir, buildDirective("proxy_intercept_errors", "on")) + } + + for _, errorcode := range location.CustomHTTPErrors { + dir = append(dir, buildDirective( + "error_page", + errorcode, "=", + fmt.Sprintf("@custom_%s_%d", location.DefaultBackendUpstreamName, errorcode)), + ) + } + + switch location.BackendProtocol { + case "GRPC", "GRPCS": + dir = append(dir, + buildDirective("grpc_connect_timeout", seconds(location.Proxy.ConnectTimeout)), + buildDirective("grpc_send_timeout", seconds(location.Proxy.SendTimeout)), + buildDirective("grpc_read_timeout", seconds(location.Proxy.ReadTimeout)), + ) + case "FCGI": + dir = append(dir, buildDirective("include", "/etc/nginx/fastcgi_params")) + if location.FastCGI.Index != "" { + dir = append(dir, buildDirective("fastcgi_index", location.FastCGI.Index)) + } + for k, v := range location.FastCGI.Params { + dir = append(dir, buildDirective("fastcgi_param", k, v)) + } + } + + if location.Redirect.URL != "" { + dir = append(dir, buildDirective("return", location.Redirect.Code, location.Redirect.URL)) + } + + dir = append(dir, buildProxyPass(c.tplConfig.Backends, location)...) + + if location.Proxy.ProxyRedirectFrom == "default" || location.Proxy.ProxyRedirectFrom == "off" { + dir = append(dir, buildDirective("proxy_redirect", location.Proxy.ProxyRedirectFrom)) + } else if location.Proxy.ProxyRedirectTo != "off" { + dir = append(dir, buildDirective("proxy_redirect", location.Proxy.ProxyRedirectFrom, location.Proxy.ProxyRedirectTo)) + } + return dir } diff --git a/internal/ingress/controller/template/crossplane/server.go b/internal/ingress/controller/template/crossplane/server.go index 49ab9c74ca..de3d188594 100644 --- a/internal/ingress/controller/template/crossplane/server.go +++ b/internal/ingress/controller/template/crossplane/server.go @@ -52,23 +52,56 @@ func (c *Template) buildServerDirective(server *ingress.Server) *ngx_crossplane. }) serverBlock = append(serverBlock, matchCNBlock) } - // TODO: This part should be reserved to SSL Configurations - /* MISSING (I don't know where this if ends...) - {{ if not (empty $server.AuthTLSError) }} - # {{ $server.AuthTLSError }} - return 403; - {{ else }} - */ - serverBlock = append(serverBlock, c.buildCertificateDirectives(server)...) - // END + if server.AuthTLSError != "" { + serverBlock = append(serverBlock, buildDirective("return", 403)) + } else { - serverBlock = append(serverBlock, buildCustomErrorLocationsPerServer(server, c.tplConfig.EnableMetrics)...) + serverBlock = append(serverBlock, c.buildCertificateDirectives(server)...) + serverBlock = append(serverBlock, buildCustomErrorLocationsPerServer(server, c.tplConfig.EnableMetrics)...) + serverBlock = append(serverBlock, buildMirrorLocationDirective(server.Locations)...) - serverBlock = append(serverBlock, buildMirrorLocationDirective(server.Locations)...) + // The other locations should come here! + serverBlock = append(serverBlock, c.buildServerLocations(server, server.Locations)...) - // The other locations should come here! - serverBlock = append(serverBlock, c.buildServerLocations(server, server.Locations)...) + } + + // "/healthz" location + if server.Hostname == "_" { + dirs := ngx_crossplane.Directives{ + buildDirective("access_log", "off"), + buildDirective("return", "200"), + } + if cfg.EnableOpentelemetry { + dirs = append(dirs, buildDirective("opentelemetry", "off")) + } + healthLocation := buildBlockDirective("location", + []string{c.tplConfig.HealthzURI}, dirs) + serverBlock = append(serverBlock, healthLocation) + } + + // "/nginx_status" location + statusLocationDirs := ngx_crossplane.Directives{} + if cfg.EnableOpentelemetry { + statusLocationDirs = append(statusLocationDirs, buildDirective("opentelemetry", "off")) + } + + for _, v := range c.tplConfig.NginxStatusIpv4Whitelist { + statusLocationDirs = append(statusLocationDirs, buildDirective("allow", v)) + } + + if c.tplConfig.IsIPV6Enabled { + for _, v := range c.tplConfig.NginxStatusIpv6Whitelist { + statusLocationDirs = append(statusLocationDirs, buildDirective("allow", v)) + } + } + statusLocationDirs = append(statusLocationDirs, + buildDirective("deny", "all"), + buildDirective("access_log", "off"), + buildDirective("stub_status", "on")) + + serverBlock = append(serverBlock, buildBlockDirective("location", []string{"/nginx_status"}, statusLocationDirs)) + // End of "nginx_status" location // DO NOT MOVE! THIS IS THE END DIRECTIVE OF SERVERS serverBlock = append(serverBlock, buildCustomErrorLocation("upstream-default-backend", cfg.CustomHTTPErrors, c.tplConfig.EnableMetrics)...) diff --git a/internal/ingress/controller/template/crossplane/testdata/nginx-new.tmpl b/internal/ingress/controller/template/crossplane/testdata/nginx-new.tmpl index d70c891fc1..0a561c667d 100644 --- a/internal/ingress/controller/template/crossplane/testdata/nginx-new.tmpl +++ b/internal/ingress/controller/template/crossplane/testdata/nginx-new.tmpl @@ -252,7 +252,7 @@ http { {{ if and (ne $cfg.HTTP2MaxHeaderSize "") (ne $cfg.HTTP2MaxFieldSize "") }} # OK http2_max_field_size {{ $cfg.HTTP2MaxFieldSize }}; # OK - http2_max_header_size {{ $cfg.HTTP2MaxHeaderSize }}; # OK + http2_max_header_size {{ $cfg.HTTP2MaxHeaderSize }}; # OK {{ end }} {{ if (gt $cfg.HTTP2MaxRequests 0) }} # OK @@ -967,7 +967,7 @@ http { http2_push_preload on; # OK {{ end }} - port_in_redirect {{ if $location.UsePortInRedirects }}on{{ else }}off{{ end }}; + port_in_redirect {{ if $location.UsePortInRedirects }}on{{ else }}off{{ end }}; # OK set $balancer_ewma_score -1; # OK set $proxy_upstream_name {{ buildUpstreamName $location | quote }}; # OK @@ -1138,68 +1138,68 @@ http { proxy_cookie_path {{ $location.Proxy.CookiePath }}; # OK # In case of errors try the next upstream server before returning an error - proxy_next_upstream {{ buildNextUpstream $location.Proxy.NextUpstream $all.Cfg.RetryNonIdempotent }}; - proxy_next_upstream_timeout {{ $location.Proxy.NextUpstreamTimeout }}; - proxy_next_upstream_tries {{ $location.Proxy.NextUpstreamTries }}; + proxy_next_upstream {{ buildNextUpstream $location.Proxy.NextUpstream $all.Cfg.RetryNonIdempotent }}; # OK + proxy_next_upstream_timeout {{ $location.Proxy.NextUpstreamTimeout }}; # OK + proxy_next_upstream_tries {{ $location.Proxy.NextUpstreamTries }}; # OK {{ if or (eq $location.BackendProtocol "GRPC") (eq $location.BackendProtocol "GRPCS") }} # Grpc settings - grpc_connect_timeout {{ $location.Proxy.ConnectTimeout }}s; - grpc_send_timeout {{ $location.Proxy.SendTimeout }}s; - grpc_read_timeout {{ $location.Proxy.ReadTimeout }}s; + grpc_connect_timeout {{ $location.Proxy.ConnectTimeout }}s; # OK + grpc_send_timeout {{ $location.Proxy.SendTimeout }}s; # OK + grpc_read_timeout {{ $location.Proxy.ReadTimeout }}s; # OK {{ end }} # 1 {{ if $location.CustomHeaders }} # 2 # Custom Response Headers {{ range $k, $v := $location.CustomHeaders.Headers }} # 3 - more_set_headers {{ printf "%s: %s" $k $v | escapeLiteralDollar | quote }}; + more_set_headers {{ printf "%s: %s" $k $v | escapeLiteralDollar | quote }}; # OK {{ end }} # 2 {{ end }} # 1 {{/* if we are sending the request to a custom default backend, we add the required headers */}} {{ if (hasPrefix $location.Backend "custom-default-backend-") }} - proxy_set_header X-Code 503; - proxy_set_header X-Format $http_accept; - proxy_set_header X-Namespace $namespace; - proxy_set_header X-Ingress-Name $ingress_name; - proxy_set_header X-Service-Name $service_name; - proxy_set_header X-Service-Port $service_port; - proxy_set_header X-Request-ID $req_id; + proxy_set_header X-Code 503; # OK + proxy_set_header X-Format $http_accept; # OK + proxy_set_header X-Namespace $namespace; # OK + proxy_set_header X-Ingress-Name $ingress_name; # OK + proxy_set_header X-Service-Name $service_name; # OK + proxy_set_header X-Service-Port $service_port; # OK + proxy_set_header X-Request-ID $req_id; # OK {{ end }} # 1 {{ if $location.Satisfy }} - satisfy {{ $location.Satisfy }}; + satisfy {{ $location.Satisfy }}; # OK {{ end }} # 1 {{/* if a location-specific error override is set, add the proxy_intercept here */}} {{ if and $location.CustomHTTPErrors (not $location.DisableProxyInterceptErrors) }} # Custom error pages per ingress - proxy_intercept_errors on; + proxy_intercept_errors on; # OK {{ end }} # 1 {{ range $errCode := $location.CustomHTTPErrors }} - error_page {{ $errCode }} = @custom_{{ $location.DefaultBackendUpstreamName }}_{{ $errCode }};{{ end }} + error_page {{ $errCode }} = @custom_{{ $location.DefaultBackendUpstreamName }}_{{ $errCode }};{{ end }} # OK {{ if (eq $location.BackendProtocol "FCGI") }} - include /etc/nginx/fastcgi_params; + include /etc/nginx/fastcgi_params; OK {{ end }} # 1 {{- if $location.FastCGI.Index -}} - fastcgi_index {{ $location.FastCGI.Index | quote }}; + fastcgi_index {{ $location.FastCGI.Index | quote }}; # OK {{- end -}} # 1 {{ range $k, $v := $location.FastCGI.Params }} - fastcgi_param {{ $k }} {{ $v | quote }}; + fastcgi_param {{ $k }} {{ $v | quote }}; # OK {{ end }} # 1 {{ if not (empty $location.Redirect.URL) }} - return {{ $location.Redirect.Code }} {{ $location.Redirect.URL }}; + return {{ $location.Redirect.Code }} {{ $location.Redirect.URL }}; # OK {{ end }} # 1 - {{ buildProxyPass $server.Hostname $all.Backends $location }} + {{ buildProxyPass $server.Hostname $all.Backends $location }} # OK {{ if (or (eq $location.Proxy.ProxyRedirectFrom "default") (eq $location.Proxy.ProxyRedirectFrom "off")) }} - proxy_redirect {{ $location.Proxy.ProxyRedirectFrom }}; - {{ else if not (eq $location.Proxy.ProxyRedirectTo "off") }} - proxy_redirect {{ $location.Proxy.ProxyRedirectFrom }} {{ $location.Proxy.ProxyRedirectTo }}; + proxy_redirect {{ $location.Proxy.ProxyRedirectFrom }}; # OK + {{ else if not (eq $location.Proxy.ProxyRedirectTo "off") }} + proxy_redirect {{ $location.Proxy.ProxyRedirectFrom }} {{ $location.Proxy.ProxyRedirectTo }}; # OK {{ end }} # 1 {{ else }} # Location denied. Reason: {{ $location.Denied | quote }} # OK @@ -1231,14 +1231,14 @@ http { {{ if eq $server.Hostname "_" }} # health checks in cloud providers require the use of port {{ $all.ListenPorts.HTTP }} - location {{ $all.HealthzURI }} { + location {{ $all.HealthzURI }} { # OK {{ if $all.Cfg.EnableOpentelemetry }} - opentelemetry off; + opentelemetry off; # OK {{ end }} - access_log off; - return 200; + access_log off; # OK + return 200; # OK } # this is required to avoid error if nginx is being monitored @@ -1246,21 +1246,21 @@ http { location /nginx_status { {{ if $all.Cfg.EnableOpentelemetry }} - opentelemetry off; + opentelemetry off; # OK {{ end }} {{ range $v := $all.NginxStatusIpv4Whitelist }} - allow {{ $v }}; + allow {{ $v }}; # OK {{ end }} {{ if $all.IsIPV6Enabled -}} {{ range $v := $all.NginxStatusIpv6Whitelist }} - allow {{ $v }}; + allow {{ $v }}; # OK {{ end }} {{ end -}} - deny all; + deny all; # OK - access_log off; - stub_status on; + access_log off; # OK + stub_status on; # OK } {{ end }} diff --git a/internal/ingress/controller/template/crossplane/utils.go b/internal/ingress/controller/template/crossplane/utils.go index 1a2223e662..b3f9dd51f8 100644 --- a/internal/ingress/controller/template/crossplane/utils.go +++ b/internal/ingress/controller/template/crossplane/utils.go @@ -437,3 +437,98 @@ func buildOriginRegex(origin string) string { origin = strings.Replace(origin, "\\*", `[A-Za-z0-9\-]+`, 1) return fmt.Sprintf("(%s)", origin) } + +func buildNextUpstream(nextUpstream string, retryNonIdempotent bool) []string { + parts := strings.Split(nextUpstream, " ") + + nextUpstreamCodes := make([]string, 0, len(parts)) + for _, v := range parts { + if v != "" && v != nonIdempotent { + nextUpstreamCodes = append(nextUpstreamCodes, v) + } + + if v == nonIdempotent { + retryNonIdempotent = true + } + } + + if retryNonIdempotent { + nextUpstreamCodes = append(nextUpstreamCodes, nonIdempotent) + } + + return nextUpstreamCodes +} + +func buildProxyPass(backends []*ingress.Backend, location *ingress.Location) ngx_crossplane.Directives { + path := location.Path + proto := "http://" + proxyPass := "proxy_pass" + + switch strings.ToUpper(location.BackendProtocol) { + case autoHTTPProtocol: + proto = "$scheme://" + case httpsProtocol: + proto = "https://" + case grpcProtocol: + proto = "grpc://" + proxyPass = "grpc_pass" + case grpcsProtocol: + proto = "grpcs://" + proxyPass = "grpc_pass" + case fcgiProtocol: + proto = "" + proxyPass = "fastcgi_pass" + } + + upstreamName := "upstream_balancer" + + for _, backend := range backends { + if backend.Name == location.Backend { + if backend.SSLPassthrough { + proto = "https://" + + if location.BackendProtocol == grpcsProtocol { + proto = "grpcs://" + } + } + + break + } + } + + if location.Backend == "upstream-default-backend" { + proto = "http://" + proxyPass = "proxy_pass" + } + + // defProxyPass returns the default proxy_pass, just the name of the upstream + defProxyPass := buildDirective(proxyPass, fmt.Sprintf("%s%s", proto, upstreamName)) + + // if the path in the ingress rule is equals to the target: no special rewrite + if path == location.Rewrite.Target { + return ngx_crossplane.Directives{defProxyPass} + } + + if location.Rewrite.Target != "" { + proxySetHeader := "proxy_set_header" + dir := make(ngx_crossplane.Directives, 0) + if location.BackendProtocol == grpcProtocol || location.BackendProtocol == grpcsProtocol { + proxySetHeader = "grpc_set_header" + } + + if location.XForwardedPrefix != "" { + dir = append(dir, + buildDirective(proxySetHeader, "X-Forwarded-Prefix", location.XForwardedPrefix), + ) + } + + dir = append(dir, + buildDirective("rewrite", fmt.Sprintf("(?i)%s", path), location.Rewrite.Target, "break"), + buildDirective(proxyPass, fmt.Sprintf("%s%s", proto, upstreamName)), + ) + return dir + } + + // default proxy_pass + return ngx_crossplane.Directives{defProxyPass} +} diff --git a/test/e2e-image/e2e.sh b/test/e2e-image/e2e.sh index f8ecd5337d..f793c76819 100755 --- a/test/e2e-image/e2e.sh +++ b/test/e2e-image/e2e.sh @@ -24,7 +24,7 @@ E2E_CHECK_LEAKS=${E2E_CHECK_LEAKS:-""} reportFile="report-e2e-test-suite.xml" ginkgo_args=( - "--fail-fast" + # "--fail-fast" "--flake-attempts=2" "--junit-report=${reportFile}" "--nodes=${E2E_NODES}"