Add crossplane flag and e2e test

.github/workflows/ci.yaml

@@ -320,3 +320,18 @@ jobs:
     with:
       k8s-version: ${{ matrix.k8s }}
       variation: "CHROOT"
+
+  kubernetes-crossplane:
+    name: Kubernetes Crossplane
+    needs:
+      - changes
+      - build
+    if: |
+      (needs.changes.outputs.go == 'true') || (needs.changes.outputs.baseimage == 'true') || ${{ github.event.workflow_dispatch.run_e2e == 'true' }}
+    strategy:
+      matrix:
+        k8s: [v1.30.4, v1.31.0]
+    uses: ./.github/workflows/zz-tmpl-k8s-e2e.yaml
+    with:
+      k8s-version: ${{ matrix.k8s }}
+      variation: "CROSSPLANE"

.github/workflows/zz-tmpl-k8s-e2e.yaml

@@ -44,6 +44,7 @@ jobs:
           SKIP_INGRESS_IMAGE_CREATION: true
           SKIP_E2E_IMAGE_CREATION: true
           IS_CHROOT: ${{ inputs.variation == 'CHROOT' }}
+          IS_CROSSPLANE: ${{ inputs.variation == 'CROSSPLANE' }}
         run: |
           kind get kubeconfig > $HOME/.kube/kind-config-kind
           make kind-e2e-test

.golangci.yml

@@ -25,6 +25,7 @@ linters:
     - ginkgolinter
     - gocheckcompilerdirectives
     - goconst
+    - gocritic
     - gocyclo
     - godox
     - gofmt

charts/ingress-nginx/templates/_params.tpl

@@ -60,6 +60,9 @@
 {{- if .Values.controller.enableTopologyAwareRouting }}
 - --enable-topology-aware-routing=true
 {{- end }}
+{{- if .Values.controller.templateEngine }}
+- --configuration-template-engine={{ .Values.controller.templateEngine }}
+{{- end }}
 {{- if .Values.controller.disableLeaderElection }}
 - --disable-leader-election=true
 {{- end }}

charts/ingress-nginx/values.yaml

@@ -21,6 +21,7 @@ commonLabels: {}
 
 controller:
   name: controller
+  templateEngine: "go-template"
   enableAnnotationValidations: true
   image:
     ## Keep false as default for now!

internal/ingress/controller/controller.go

@@ -63,48 +63,29 @@ const (
 type Configuration struct {
 	APIServerHost               string
 	RootCAFile                  string
-
 	KubeConfigFile              string
-
 	Client                      clientset.Interface
-
 	ResyncPeriod                time.Duration
-
 	ConfigMapName               string
 	DefaultService              string
-
 	Namespace                   string
-
 	WatchNamespaceSelector      labels.Selector
-
-	// +optional
 	TCPConfigMapName            string
-	// +optional
 	UDPConfigMapName            string
-
 	DefaultSSLCertificate       string
-
-	// +optional
 	PublishService              string
 	PublishStatusAddress        string
-
 	UpdateStatus                bool
 	UseNodeInternalIP           bool
 	ElectionID                  string
 	ElectionTTL                 time.Duration
 	UpdateStatusOnShutdown      bool
-
 	HealthCheckHost             string
 	ListenPorts                 *ngx_config.ListenPorts
-
 	DisableServiceExternalName  bool
-
 	EnableSSLPassthrough        bool
-
 	DisableLeaderElection       bool
-
 	EnableProfiling             bool
-
 	EnableMetrics               bool
 	MetricsPerHost              bool
 	MetricsPerUndefinedHost     bool
@@ -113,37 +94,26 @@ type Configuration struct {
 	MetricsMaxBuckets           uint32
 	ReportStatusClasses         bool
 	ExcludeSocketMetrics        []string
-
 	FakeCertificate             *ingress.SSLCert
-
 	SyncRateLimit               float32
-
 	DisableCatchAll             bool
-
 	IngressClassConfiguration   *ingressclass.Configuration
-
 	ValidationWebhook           string
 	ValidationWebhookCertPath   string
 	ValidationWebhookKeyPath    string
 	DisableFullValidationTest   bool
-
 	GlobalExternalAuth          *ngx_config.GlobalExternalAuth
 	MaxmindEditionFiles         *[]string
-
 	MonitorMaxBatchSize         int
-
 	PostShutdownGracePeriod     int
 	ShutdownGracePeriod         int
-
 	InternalLoggerAddress       string
 	IsChroot                    bool
 	DeepInspector               bool
-
 	DynamicConfigurationRetries int
-
 	DisableSyncEvents           bool
-
 	EnableTopologyAwareRouting  bool
+	ConfigurationTemplateEngine string
 }
 
 func getIngressPodZone(svc *apiv1.Service) string {

internal/ingress/controller/nginx.go

@@ -159,7 +159,10 @@ func NewNGINXController(config *Configuration, mc metric.Collector) *NGINXContro
 	}
 
 	onTemplateChange := func() {
-		template, err := crossplane.NewTemplate()
+		if config.ConfigurationTemplateEngine != "go-template" {
+			return
+		}
+		template, err := ngx_template.NewTemplate(nginx.TemplatePath)
 		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")
@@ -171,17 +174,27 @@ func NewNGINXController(config *Configuration, mc metric.Collector) *NGINXContro
 		n.syncQueue.EnqueueTask(task.GetDummyObject("template-change"))
 	}
 
-	ngxTpl, err := crossplane.NewTemplate()
+	var ngxTpl ngx_template.Writer
+	switch config.ConfigurationTemplateEngine {
+	case "go-template":
+		ngxTpl, err = ngx_template.NewTemplate(nginx.TemplatePath)
 		if err != nil {
 			klog.Fatalf("Invalid NGINX configuration template: %v", err)
 		}
-
-	n.t = ngxTpl
-
 		_, err = file.NewFileWatcher(nginx.TemplatePath, onTemplateChange)
 		if err != nil {
 			klog.Fatalf("Error creating file watcher for %v: %v", nginx.TemplatePath, err)
 		}
+	case "crossplane":
+		ngxTpl, err = crossplane.NewTemplate()
+		if err != nil {
+			klog.Fatalf("Invalid NGINX configuration template: %v", err)
+		}
+	default:
+		klog.Fatal("Invalid template engine, please use 'go-template' or 'crossplane'")
+	}
+
+	n.t = ngxTpl
 
 	filesToWatch := []string{}
 
@@ -653,6 +666,11 @@ func (n *NGINXController) testTemplate(cfg []byte) error {
 	if err != nil {
 		return err
 	}
+
+	if err := n.t.Validate(tmpfile.Name()); err != nil {
+		return fmt.Errorf("error during template validation: %w", err)
+	}
+
 	out, err := n.command.Test(tmpfile.Name())
 	if err != nil {
 		// this error is different from the rest because it must be clear why nginx is not working
@@ -701,7 +719,7 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error {
 
 	err = n.testTemplate(content)
 	if err != nil {
-		return fmt.Errorf("err %s content %s", err, string(content))
+		return err
 	}
 
 	if klog.V(2).Enabled() {
@@ -869,14 +887,15 @@ func (n *NGINXController) configureDynamically(pcfg *ingress.Configuration) erro
 		}
 	}
 
-	// TODO: (ricardo) - Disable in case this is crossplane, we don't support stream on this mode
-	/*streamConfigurationChanged := !reflect.DeepEqual(n.runningConfig.TCPEndpoints, pcfg.TCPEndpoints) || !reflect.DeepEqual(n.runningConfig.UDPEndpoints, pcfg.UDPEndpoints)
+	if n.cfg.ConfigurationTemplateEngine == "go-template" {
+		streamConfigurationChanged := !reflect.DeepEqual(n.runningConfig.TCPEndpoints, pcfg.TCPEndpoints) || !reflect.DeepEqual(n.runningConfig.UDPEndpoints, pcfg.UDPEndpoints)
 		if streamConfigurationChanged {
 			err := updateStreamConfiguration(pcfg.TCPEndpoints, pcfg.UDPEndpoints)
 			if err != nil {
 				return err
 			}
-	}*/
+		}
+	}
 
 	serversChanged := !reflect.DeepEqual(n.runningConfig.Servers, pcfg.Servers)
 	if serversChanged {

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

@@ -18,7 +18,6 @@ package crossplane
 
 import (
 	"bytes"
-	"os"
 
 	ngx_crossplane "github.com/nginxinc/nginx-go-crossplane"
 
@@ -31,13 +30,13 @@ Unsupported directives:
 - opentelemetry
 - modsecurity
 - any stream directive (TCP/UDP forwarding)
-- geoip2
 */
 
 // On this case we will try to use the go ngx_crossplane to write the template instead of the template renderer
 
 type Template struct {
 	options      *ngx_crossplane.BuildOptions
+	parseOptions *ngx_crossplane.ParseOptions
 	config       *ngx_crossplane.Config
 	tplConfig    *config.TemplateConfig
 	mimeFile     string
@@ -45,13 +44,36 @@ type Template struct {
 
 func NewTemplate() (*Template, error) {
 	lua := ngx_crossplane.Lua{}
-	return &Template{
-		mimeFile: "/etc/nginx/mime.types",
-		options: &ngx_crossplane.BuildOptions{
+	buildOptions := &ngx_crossplane.BuildOptions{
 		Builders: []ngx_crossplane.RegisterBuilder{
 			lua.RegisterBuilder(),
 		},
+	}
+
+	parseOptions := &ngx_crossplane.ParseOptions{
+		ParseComments:            true,
+		ErrorOnUnknownDirectives: true,
+		StopParsingOnError:       true,
+		DirectiveSources: []ngx_crossplane.MatchFunc{
+			ngx_crossplane.DefaultDirectivesMatchFunc,
+			ngx_crossplane.MatchLuaLatest,
+			ngx_crossplane.MatchHeadersMoreLatest,
+			extramodules.BrotliMatchFn,
+			extramodules.OpentelemetryMatchFn,
+			ngx_crossplane.MatchGeoip2Latest,
 		},
+		LexOptions: ngx_crossplane.LexOptions{
+			Lexers: []ngx_crossplane.RegisterLexer{lua.RegisterLexer()},
+		},
+		// Modules that needs to be ported:
+		// // https://github.com/openresty/set-misc-nginx-module?tab=readme-ov-file#set_escape_uri
+		IgnoreDirectives: []string{"set_escape_uri"},
+	}
+
+	return &Template{
+		mimeFile:     "/etc/nginx/mime.types",
+		options:      buildOptions,
+		parseOptions: parseOptions,
 	}, nil
 }
 
@@ -78,45 +100,10 @@ func (c *Template) Write(conf *config.TemplateConfig) ([]byte, error) {
 		return nil, err
 	}
 
-	lua := ngx_crossplane.Lua{}
-	options := ngx_crossplane.ParseOptions{
-		ParseComments:            true,
-		ErrorOnUnknownDirectives: true,
-		StopParsingOnError:       true,
-		DirectiveSources: []ngx_crossplane.MatchFunc{
-			ngx_crossplane.DefaultDirectivesMatchFunc,
-			ngx_crossplane.MatchLuaLatest,
-			ngx_crossplane.MatchHeadersMoreLatest,
-			extramodules.BrotliMatchFn,
-			extramodules.OpentelemetryMatchFn,
-			ngx_crossplane.MatchGeoip2Latest,
-		},
-		LexOptions: ngx_crossplane.LexOptions{
-			Lexers: []ngx_crossplane.RegisterLexer{lua.RegisterLexer()},
-		},
-		// Modules that needs to be ported:
-		// // https://github.com/openresty/set-misc-nginx-module?tab=readme-ov-file#set_escape_uri
-		IgnoreDirectives: []string{"set_escape_uri"},
-	}
-
-	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
 }
+
+func (c *Template) Validate(filename string) error {
+	_, err := ngx_crossplane.Parse(filename, c.parseOptions)
+	return err
+}

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

@@ -22,7 +22,6 @@ import (
 
 	ngx_crossplane "github.com/nginxinc/nginx-go-crossplane"
 	"github.com/stretchr/testify/require"
-	"k8s.io/ingress-nginx/internal/ingress/controller/config"
 )
 
 // THIS FILE SHOULD BE USED JUST FOR INTERNAL TESTS - Private functions
@@ -53,22 +52,6 @@ func Test_Internal_boolToStr(t *testing.T) {
 	require.Equal(t, boolToStr(false), "off")
 }
 
-func Test_Internal_buildLuaDictionaries(t *testing.T) {
-	t.Skip("Maps are not sorted, need to fix this")
-	cfg := &config.Configuration{
-		LuaSharedDicts: map[string]int{
-			"somedict":  1024,
-			"otherdict": 1025,
-		},
-	}
-	directives := buildLuaSharedDictionaries(cfg)
-	require.Len(t, directives, 2)
-	require.Equal(t, "lua_shared_dict", directives[0].Directive)
-	require.Equal(t, []string{"somedict", "1M"}, directives[0].Args)
-	require.Equal(t, "lua_shared_dict", directives[1].Directive)
-	require.Equal(t, []string{"otherdict", "1025K"}, directives[1].Args)
-}
-
 func Test_Internal_buildCorsOriginRegex(t *testing.T) {
 	tests := []struct {
 		name        string
@@ -87,7 +70,7 @@ func Test_Internal_buildCorsOriginRegex(t *testing.T) {
 			name:        "multiple hosts should be changed properly",
 			corsOrigins: []string{"*.xpto.com", "  lalala.com"},
 			want: ngx_crossplane.Directives{
-				buildBlockDirective("if", []string{"$http_origin", "~*", "([A-Za-z0-9\\-]+\\.xpto\\.com)", "|", "(lalala\\.com)"},
+				buildBlockDirective("if", []string{"$http_origin", "~*", "(([A-Za-z0-9\\-]+\\.xpto\\.com)|(lalala\\.com))$"},
 					ngx_crossplane.Directives{buildDirective("set", "$cors", "true")},
 				),
 			},

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

@@ -169,12 +169,27 @@ func TestCrossplaneTemplate(t *testing.T) {
 							Target:      "http://www.mymirror.com",
 							RequestBody: "off",
 						},
+						Proxy: proxy.Config{
+							ProxyBuffering:   "on",
+							RequestBuffering: "on",
+							NextUpstream:     "10.10.10.10",
+						},
 					},
 					{
 						DefaultBackendUpstreamName: "something",
+						Proxy: proxy.Config{
+							ProxyBuffering:   "on",
+							RequestBuffering: "on",
+							NextUpstream:     "10.10.10.10",
+						},
 						CustomHTTPErrors: []int{403, 404, 403, 409}, // Duplicated on purpose!
 					},
 					{
+						Proxy: proxy.Config{
+							ProxyBuffering:   "on",
+							RequestBuffering: "on",
+							NextUpstream:     "10.10.10.10",
+						},
 						DefaultBackendUpstreamName: "otherthing",
 						CustomHTTPErrors:           []int{403, 404, 403, 409}, // Duplicated on purpose!
 					},
@@ -255,7 +270,6 @@ func TestCrossplaneTemplate(t *testing.T) {
 
 		_, err = ngx_crossplane.Parse(tmpFile.Name(), &options)
 		require.NoError(t, err)
-		require.Equal(t, "bla", string(content))
 	})
 
 	t.Run("it should set the right logging configs", func(t *testing.T) {
@@ -352,6 +366,5 @@ func TestCrossplaneTemplate(t *testing.T) {
 
 		_, err = ngx_crossplane.Parse(tmpFile.Name(), &options)
 		require.NoError(t, err)
-		// require.Equal(t, " ", string(content))
 	})
 }

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

@@ -296,7 +296,7 @@ func (c *Template) buildAllowedLocation(server *ingress.Server, location *ingres
 	if location.CorsConfig.CorsEnabled {
 		dir = append(dir, buildCorsDirectives(location.CorsConfig)...)
 	}
-	// TODO: Implement the build Auth Location
+
 	if !isLocationInLocationList(location, c.tplConfig.Cfg.NoAuthLocations) {
 		dir = append(dir, buildAuthLocationConfig(location, locationConfig)...)
 	}

internal/ingress/controller/template/template.go

@@ -71,6 +71,10 @@ type Writer interface {
 	// NOTE: Implementors must ensure that the content of the returned slice is not modified by the implementation
 	// after the return of this function.
 	Write(conf *config.TemplateConfig) ([]byte, error)
+	// Validate is a function that can be called, containing the file name to be tested
+	// This function should be used just by specific cases like crossplane, otherwise it can return
+	// null error
+	Validate(filename string) error
 }
 
 // Template ingress template
@@ -229,6 +233,11 @@ type LuaListenPorts struct {
 	SSLProxyPort string `json:"ssl_proxy"`
 }
 
+// Validate is no-op at go-template
+func (t *Template) Validate(filename string) error {
+	return nil
+}
+
 // Write populates a buffer using a template with NGINX configuration
 // and the servers and upstreams created by Ingress rules
 func (t *Template) Write(conf *config.TemplateConfig) ([]byte, error) {

pkg/flags/flags.go

@@ -233,6 +233,8 @@ Takes the form "<host>:port". If not provided, no admission controller is starte
 		disableSyncEvents = flags.Bool("disable-sync-events", false, "Disables the creation of 'Sync' event resources")
 
 		enableTopologyAwareRouting = flags.Bool("enable-topology-aware-routing", false, "Enable topology aware routing feature, needs service object annotation service.kubernetes.io/topology-mode sets to auto.")
+
+		configurationTemplateEngine = flags.String("configuration-template-engine", "go-template", "Defines what configuration template engine should be used. Can be 'go-template' or 'crossplane'. ")
 	)
 
 	flags.StringVar(&nginx.MaxmindMirror, "maxmind-mirror", "", `Maxmind mirror url (example: http://geoip.local/databases.`)
@@ -303,6 +305,10 @@ https://blog.maxmind.com/2019/12/significant-changes-to-accessing-and-using-geol
 		return false, nil, fmt.Errorf("flags --publish-service and --publish-status-address are mutually exclusive")
 	}
 
+	if *enableSSLPassthrough && *configurationTemplateEngine != "go-template" {
+		return false, nil, fmt.Errorf("SSL Passthrough can only be enabled with 'go-template' configuration engine")
+	}
+
 	nginx.HealthPath = *defHealthzURL
 
 	if *defHealthCheckTimeout > 0 {
@@ -396,6 +402,7 @@ https://blog.maxmind.com/2019/12/significant-changes-to-accessing-and-using-geol
 		ValidationWebhookKeyPath:    *validationWebhookKey,
 		InternalLoggerAddress:       *internalLoggerAddress,
 		DisableSyncEvents:           *disableSyncEvents,
+		ConfigurationTemplateEngine: *configurationTemplateEngine,
 	}
 
 	if *apiserverHost != "" {

test/e2e/framework/exec.go

@@ -117,7 +117,12 @@ func (f *Framework) newIngressController(namespace, namespaceOverlay string) err
 		isChroot = "false"
 	}
 
-	cmd := exec.Command("./wait-for-nginx.sh", namespace, namespaceOverlay, isChroot)
+	isCrossplane, ok := os.LookupEnv("IS_CROSSPLANE")
+	if !ok {
+		isCrossplane = "false"
+	}
+
+	cmd := exec.Command("./wait-for-nginx.sh", namespace, namespaceOverlay, isChroot, isCrossplane)
 	out, err := cmd.CombinedOutput()
 	if err != nil {
 		return fmt.Errorf("unexpected error waiting for ingress controller deployment: %v.\nLogs:\n%v", err, string(out))

test/e2e/wait-for-nginx.sh

@@ -24,6 +24,12 @@ DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
 export NAMESPACE=$1
 export NAMESPACE_OVERLAY=$2
 export IS_CHROOT=$3
+export IS_CROSSPLANE=$4
+
+TPL_ENGINE="go-template"
+if [ "$IS_CROSSPLANE" == "true" ]; then
+  TPL_ENGINE="crossplane"
+fi
 
 echo "deploying NGINX Ingress controller in namespace $NAMESPACE"
 
@@ -52,12 +58,14 @@ if [[ ! -z "$NAMESPACE_OVERLAY" && -d "$DIR/namespace-overlays/$NAMESPACE_OVERLA
     echo "Namespace overlay $NAMESPACE_OVERLAY is being used for namespace $NAMESPACE"
     helm install nginx-ingress ${DIR}/charts/ingress-nginx \
         --namespace=$NAMESPACE \
-        --values "$DIR/namespace-overlays/$NAMESPACE_OVERLAY/values.yaml"
+        --values "$DIR/namespace-overlays/$NAMESPACE_OVERLAY/values.yaml" \
+        --set controller.templateEngine=${TPL_ENGINE}
 else
     cat << EOF | helm install nginx-ingress ${DIR}/charts/ingress-nginx --namespace=$NAMESPACE --values -
 # TODO: remove the need to use fullnameOverride
 fullnameOverride: nginx-ingress
 controller:
+  templateEngine: ${TPL_ENGINE}
   image:
     repository: ingress-controller/controller
     chroot: ${IS_CHROOT}