Add crossplane test and more directives (#11670)

* Add roundtrip test

* Add more directives and fix lint

internal/ingress/controller/template/crossplane/config.go

@@ -20,7 +20,7 @@ import (
 	ngx_crossplane "github.com/nginxinc/nginx-go-crossplane"
 )
 
-func (c *crossplaneTemplate) buildConfig() {
+func (c *Template) buildConfig() {
 	// Write basic directives
 	config := &ngx_crossplane.Config{
 		Parsed: ngx_crossplane.Directives{
@@ -34,5 +34,13 @@ func (c *crossplaneTemplate) buildConfig() {
 	if c.tplConfig.Cfg.WorkerCPUAffinity != "" {
 		config.Parsed = append(config.Parsed, buildDirective("worker_cpu_affinity", c.tplConfig.Cfg.WorkerCPUAffinity))
 	}
+
+	if c.tplConfig.Cfg.EnableBrotli {
+		config.Parsed = append(config.Parsed,
+			buildDirective("load_module", "/etc/nginx/modules/ngx_http_brotli_filter_module.so"),
+			buildDirective("load_module", "/etc/nginx/modules/ngx_http_brotli_static_module.so"),
+		)
+	}
+
 	c.config = config
 }

internal/ingress/controller/template/crossplane/crossplane.go

@@ -34,15 +34,17 @@ Unsupported directives:
 
 // On this case we will try to use the go ngx_crossplane to write the template instead of the template renderer
 
-type crossplaneTemplate struct {
+type Template struct {
 	options   *ngx_crossplane.BuildOptions
 	config    *ngx_crossplane.Config
 	tplConfig *config.TemplateConfig
+	mimeFile  string
 }
 
-func NewCrossplaneTemplate() *crossplaneTemplate {
+func NewTemplate() *Template {
 	lua := ngx_crossplane.Lua{}
-	return &crossplaneTemplate{
+	return &Template{
+		mimeFile: "/etc/nginx/mime.types",
 		options: &ngx_crossplane.BuildOptions{
 			Builders: []ngx_crossplane.RegisterBuilder{
 				lua.RegisterBuilder(),
@@ -51,7 +53,11 @@ func NewCrossplaneTemplate() *crossplaneTemplate {
 	}
 }
 
-func (c *crossplaneTemplate) Write(conf *config.TemplateConfig) ([]byte, error) {
+func (c *Template) SetMimeFile(file string) {
+	c.mimeFile = file
+}
+
+func (c *Template) Write(conf *config.TemplateConfig) ([]byte, error) {
 	c.tplConfig = conf
 
 	// build root directives

internal/ingress/controller/template/crossplane/crossplane_internal_test.go

@@ -48,7 +48,7 @@ func Test_Internal_buildEvents(t *testing.T) {
 			},
 		}
 
-		cplane := NewCrossplaneTemplate()
+		cplane := NewTemplate()
 		cplane.config = &c
 		cplane.tplConfig = tplConfig
 		cplane.buildEvents()
@@ -81,7 +81,7 @@ func Test_Internal_buildEvents(t *testing.T) {
 			},
 		}
 
-		cplane := NewCrossplaneTemplate()
+		cplane := NewTemplate()
 		cplane.config = &c
 		cplane.tplConfig = tplConfig
 		cplane.buildEvents()

internal/ingress/controller/template/crossplane/crossplane_internal_utils_test.go

@@ -19,6 +19,7 @@ package crossplane
 import (
 	"testing"
 
+	ngx_crossplane "github.com/nginxinc/nginx-go-crossplane"
 	"github.com/stretchr/testify/require"
 	"k8s.io/ingress-nginx/internal/ingress/controller/config"
 )
@@ -36,6 +37,16 @@ func Test_Internal_buildDirectives(t *testing.T) {
 	})
 }
 
+func Test_Internal_buildMapDirectives(t *testing.T) {
+	t.Run("should be able to run build a map directive with empty block", func(t *testing.T) {
+		directive := buildMapDirective("somedirective", "bla", ngx_crossplane.Directives{buildDirective("something", "otherstuff")})
+		require.Equal(t, directive.Directive, "map")
+		require.Equal(t, directive.Args, []string{"somedirective", "bla"})
+		require.Equal(t, directive.Block[0].Directive, "something")
+		require.Equal(t, directive.Block[0].Args, []string{"otherstuff"})
+	})
+}
+
 func Test_Internal_boolToStr(t *testing.T) {
 	require.Equal(t, boolToStr(true), "on")
 	require.Equal(t, boolToStr(false), "off")

internal/ingress/controller/template/crossplane/crossplane_test.go

@@ -16,13 +16,72 @@ limitations under the License.
 
 package crossplane_test
 
-import "testing"
+import (
+	"os"
+	"testing"
 
-// TestCrossplaneTemplate should be a roundtrip test.
+	ngx_crossplane "github.com/nginxinc/nginx-go-crossplane"
+	"github.com/stretchr/testify/require"
+
+	"k8s.io/ingress-nginx/internal/ingress/controller/config"
+	"k8s.io/ingress-nginx/internal/ingress/controller/template/crossplane"
+	"k8s.io/ingress-nginx/pkg/apis/ingress"
+)
+
+const mockMimeTypes = `
+types {
+    text/html                                        html htm shtml;
+    text/css                                         css;
+    text/xml                                         xml;
+}
+`
+
+// TestTemplate should be a roundtrip test.
 // We should initialize the scenarios based on the template configuration
 // Then Parse and write a crossplane configuration, and roundtrip/parse back to check
 // if the directives matches
 // we should ignore line numbers and comments
 func TestCrossplaneTemplate(t *testing.T) {
-	// implement
+	lua := ngx_crossplane.Lua{}
+	options := ngx_crossplane.ParseOptions{
+		ErrorOnUnknownDirectives: true,
+		StopParsingOnError:       true,
+		IgnoreDirectives:         []string{"more_clear_headers"},
+		DirectiveSources:         []ngx_crossplane.MatchFunc{ngx_crossplane.DefaultDirectivesMatchFunc, ngx_crossplane.LuaDirectivesMatchFn},
+		LexOptions: ngx_crossplane.LexOptions{
+			Lexers: []ngx_crossplane.RegisterLexer{lua.RegisterLexer()},
+		},
+	}
+	defaultCertificate := &ingress.SSLCert{
+		PemFileName: "bla.crt",
+		PemCertKey:  "bla.key",
+	}
+
+	mimeFile, err := os.CreateTemp("", "")
+	require.NoError(t, err)
+	_, err = mimeFile.WriteString(mockMimeTypes)
+	require.NoError(t, err)
+	require.NoError(t, mimeFile.Close())
+
+	t.Run("it should be able to marshall and unmarshall the current configuration", func(t *testing.T) {
+		tplConfig := &config.TemplateConfig{
+			Cfg: config.NewDefault(),
+		}
+		tplConfig.Cfg.DefaultSSLCertificate = defaultCertificate
+		tplConfig.Cfg.EnableBrotli = true
+
+		tpl := crossplane.NewTemplate()
+		tpl.SetMimeFile(mimeFile.Name())
+		content, err := tpl.Write(tplConfig)
+		require.NoError(t, err)
+
+		tmpFile, err := os.CreateTemp("", "")
+		require.NoError(t, err)
+		_, err = tmpFile.Write(content)
+		require.NoError(t, err)
+		require.NoError(t, tmpFile.Close())
+
+		_, err = ngx_crossplane.Parse(tmpFile.Name(), &options)
+		require.NoError(t, err)
+	})
 }

internal/ingress/controller/template/crossplane/events.go

@@ -20,7 +20,7 @@ import (
 	ngx_crossplane "github.com/nginxinc/nginx-go-crossplane"
 )
 
-func (c *crossplaneTemplate) buildEvents() {
+func (c *Template) buildEvents() {
 	events := &ngx_crossplane.Directive{
 		Directive: "events",
 		Block: ngx_crossplane.Directives{

internal/ingress/controller/template/crossplane/http.go

@@ -24,11 +24,11 @@ import (
 	ngx_crossplane "github.com/nginxinc/nginx-go-crossplane"
 )
 
-func (c *crossplaneTemplate) initHTTPDirectives() ngx_crossplane.Directives {
+func (c *Template) initHTTPDirectives() ngx_crossplane.Directives {
 	cfg := c.tplConfig.Cfg
 	httpBlock := ngx_crossplane.Directives{
 		buildDirective("lua_package_path", "/etc/nginx/lua/?.lua;;"),
-		buildDirective("include", "/etc/nginx/mime.types"),
+		buildDirective("include", c.mimeFile),
 		buildDirective("default_type", cfg.DefaultType),
 		buildDirective("real_ip_recursive", "on"),
 		buildDirective("aio", "threads"),
@@ -46,7 +46,7 @@ func (c *crossplaneTemplate) initHTTPDirectives() ngx_crossplane.Directives {
 		buildDirective("proxy_temp_path", "/tmp/nginx/proxy-temp"),
 		buildDirective("client_header_buffer_size", cfg.ClientHeaderBufferSize),
 		buildDirective("client_header_timeout", seconds(cfg.ClientHeaderTimeout)),
-		buildDirective("large_client_header_buffers", cfg.LargeClientHeaderBuffers),
+		buildDirective("large_client_header_buffers", strings.Split(cfg.LargeClientHeaderBuffers, " ")),
 		buildDirective("client_body_buffer_size", cfg.ClientBodyBufferSize),
 		buildDirective("client_body_timeout", seconds(cfg.ClientBodyTimeout)),
 		buildDirective("types_hash_max_size", "2048"),
@@ -64,6 +64,7 @@ func (c *crossplaneTemplate) initHTTPDirectives() ngx_crossplane.Directives {
 		buildDirective("uninitialized_variable_warn", "off"),
 		buildDirective("server_name_in_redirect", "off"),
 		buildDirective("port_in_redirect", "off"),
+		buildDirective("http2_max_concurrent_streams", cfg.HTTP2MaxConcurrentStreams),
 		buildDirective("ssl_protocols", strings.Split(cfg.SSLProtocols, " ")),
 		buildDirective("ssl_early_data", cfg.SSLEarlyData),
 		buildDirective("ssl_session_tickets", cfg.SSLSessionTickets),
@@ -80,7 +81,7 @@ func (c *crossplaneTemplate) initHTTPDirectives() ngx_crossplane.Directives {
 	return httpBlock
 }
 
-func (c *crossplaneTemplate) buildHTTP() {
+func (c *Template) buildHTTP() {
 	cfg := c.tplConfig.Cfg
 	httpBlock := c.initHTTPDirectives()
 	httpBlock = append(httpBlock, buildLuaSharedDictionaries(&c.tplConfig.Cfg)...)
@@ -121,7 +122,7 @@ func (c *crossplaneTemplate) buildHTTP() {
 		httpBlock = append(httpBlock, buildDirective("gzip_vary", "on"))
 
 		if cfg.GzipDisable != "" {
-			httpBlock = append(httpBlock, buildDirective("gzip_disable", strings.Split(cfg.GzipDisable, "")))
+			httpBlock = append(httpBlock, buildDirective("gzip_disable", strings.Split(cfg.GzipDisable, " ")))
 		}
 	}
 
@@ -146,18 +147,12 @@ func (c *crossplaneTemplate) buildHTTP() {
 
 	httpBlock = append(httpBlock, buildDirective("log_format", "upstreaminfo", escape, cfg.LogFormatUpstream))
 
-	// buildMap directive
+	loggableMap := make(ngx_crossplane.Directives, 0)
-	mapLogDirective := &ngx_crossplane.Directive{
-		Directive: "map",
-		Args:      []string{"$request_uri", "$loggable"},
-		Block:     make(ngx_crossplane.Directives, 0),
-	}
 	for k := range cfg.SkipAccessLogURLs {
-		mapLogDirective.Block = append(mapLogDirective.Block, buildDirective(cfg.SkipAccessLogURLs[k], "0"))
+		loggableMap = append(loggableMap, buildDirective(cfg.SkipAccessLogURLs[k], "0"))
 	}
-	mapLogDirective.Block = append(mapLogDirective.Block, buildDirective("default", "1"))
+	loggableMap = append(loggableMap, buildDirective("default", "1"))
-	httpBlock = append(httpBlock, mapLogDirective)
+	httpBlock = append(httpBlock, buildMapDirective("$request_uri", "$loggable", loggableMap))
-	// end of build mapLog
 
 	if cfg.DisableAccessLog || cfg.DisableHTTPAccessLog {
 		httpBlock = append(httpBlock, buildDirective("access_log", "off"))
@@ -180,6 +175,79 @@ func (c *crossplaneTemplate) buildHTTP() {
 		httpBlock = append(httpBlock, buildDirective("error_log", cfg.ErrorLogPath, cfg.ErrorLogLevel))
 	}
 
+	if cfg.SSLSessionCache {
+		httpBlock = append(httpBlock,
+			buildDirective("ssl_session_cache", fmt.Sprintf("shared:SSL:%s", cfg.SSLSessionCacheSize)),
+			buildDirective("ssl_session_timeout", cfg.SSLSessionTimeout),
+		)
+	}
+
+	if cfg.SSLSessionTicketKey != "" {
+		httpBlock = append(httpBlock, buildDirective("ssl_session_ticket_key", "/etc/ingress-controller/tickets.key"))
+	}
+
+	if cfg.SSLCiphers != "" {
+		httpBlock = append(httpBlock,
+			buildDirective("ssl_ciphers", cfg.SSLCiphers),
+			buildDirective("ssl_prefer_server_ciphers", "on"),
+		)
+	}
+
+	if cfg.SSLDHParam != "" {
+		httpBlock = append(httpBlock, buildDirective("ssl_dhparam", cfg.SSLDHParam))
+	}
+
+	if len(cfg.CustomHTTPErrors) > 0 && !cfg.DisableProxyInterceptErrors {
+		httpBlock = append(httpBlock, buildDirective("proxy_intercept_errors", "on"))
+	}
+
+	httpUpgradeMap := ngx_crossplane.Directives{buildDirective("default", "upgrade")}
+	if cfg.UpstreamKeepaliveConnections < 1 {
+		httpUpgradeMap = append(httpUpgradeMap, buildDirective("", "close"))
+	}
+	httpBlock = append(httpBlock, buildMapDirective("$http_upgrade", "$connection_upgrade", httpUpgradeMap))
+
+	reqIDMap := ngx_crossplane.Directives{buildDirective("default", "$http_x_request_id")}
+	if cfg.GenerateRequestID {
+		reqIDMap = append(reqIDMap, buildDirective("", "$request_id"))
+	}
+	httpBlock = append(httpBlock, buildMapDirective("$http_x_request_id", "$req_id", reqIDMap))
+
+	if cfg.UseForwardedHeaders && cfg.ComputeFullForwardedFor {
+		forwardForMap := make(ngx_crossplane.Directives, 0)
+		if cfg.UseProxyProtocol {
+			forwardForMap = append(forwardForMap,
+				buildDirective("default", "$http_x_forwarded_for, $proxy_protocol_addr"),
+				buildDirective("", "$http_x_forwarded_for, $proxy_protocol_addr"),
+			)
+		} else {
+			forwardForMap = append(forwardForMap,
+				buildDirective("default", "$http_x_forwarded_for, $realip_remote_addr"),
+				buildDirective("", "$realip_remote_addr"),
+			)
+		}
+		httpBlock = append(httpBlock, buildMapDirective("$http_x_forwarded_for", "$full_x_forwarded_for", forwardForMap))
+	}
+
+	if cfg.AllowBackendServerHeader {
+		httpBlock = append(httpBlock, buildDirective("proxy_pass_header", "Server"))
+	}
+
+	if cfg.EnableBrotli {
+		httpBlock = append(httpBlock,
+			buildDirective("brotli", "on"),
+			buildDirective("brotli_comp_level", cfg.BrotliLevel),
+			buildDirective("brotli_min_length", cfg.BrotliMinLength),
+			buildDirective("brotli_types", cfg.BrotliTypes),
+		)
+	}
+
+	if len(cfg.HideHeaders) > 0 {
+		for k := range cfg.HideHeaders {
+			httpBlock = append(httpBlock, buildDirective("proxy_hide_header", cfg.HideHeaders[k]))
+		}
+	}
+
 	c.config.Parsed = append(c.config.Parsed, &ngx_crossplane.Directive{
 		Directive: "http",
 		Block:     httpBlock,

internal/ingress/controller/template/crossplane/testdata/README.md

@@ -0,0 +1,8 @@
+This directory contains the following files:
+
+* nginx.tmpl - Should be used to track migrated directives. We will test later to see
+if the ending result of it is the same as the one parsed by go-crossplane
+* various nginx.conf - Will be used to see if the template parser reacts as 
+expected, creating files that matches and can be parsed by go-crossplane
+
+TODO: move files to embed.FS 

internal/ingress/controller/template/crossplane/utils.go

@@ -85,6 +85,15 @@ func buildResolversInternal(res []net.IP, disableIpv6 bool) []string {
 	return r
 }
 
+// buildMapDirective is used to build a map directive
+func buildMapDirective(name, variable string, block ngx_crossplane.Directives) *ngx_crossplane.Directive {
+	return &ngx_crossplane.Directive{
+		Directive: "map",
+		Args:      []string{name, variable},
+		Block:     block,
+	}
+}
+
 func boolToStr(b bool) string {
 	if b {
 		return "on"