Move NGINX configuration to other place

build/build.sh

@@ -13,7 +13,7 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-
+set -x
 GO_BUILD_CMD="go build"
 
 #if [ -n "$DEBUG" ]; then
@@ -66,4 +66,4 @@ ${GO_BUILD_CMD} \
   -X ${PKG}/version.COMMIT=${COMMIT_SHA} \
   -X ${PKG}/version.REPO=${REPO_INFO}" \
   -buildvcs=false \
-  -o "${TARGETS_DIR}/wait-shutdown" "${PKG}/cmd/waitshutdown"
+  -o "${TARGETS_DIR}/wait-shutdown" "${PKG}/cmd/waitshutdown"

cmd/dataplane/main.go

@@ -29,9 +29,7 @@ import (
 	"k8s.io/ingress-nginx/internal/ingress/controller"
 	"k8s.io/ingress-nginx/internal/ingress/metric"
 	"k8s.io/ingress-nginx/internal/nginx"
-	ingressflags "k8s.io/ingress-nginx/pkg/flags"
 	"k8s.io/ingress-nginx/pkg/metrics"
-	"k8s.io/ingress-nginx/pkg/util/file"
 	"k8s.io/ingress-nginx/pkg/util/process"
 	"k8s.io/ingress-nginx/version"
 )
@@ -41,19 +39,6 @@ func main() {
 
 	fmt.Println(version.String())
 	var err error
-	showVersion, conf, err := ingressflags.ParseFlags()
-	if showVersion {
-		os.Exit(0)
-	}
-
-	if err != nil {
-		klog.Fatal(err)
-	}
-
-	err = file.CreateRequiredDirectories()
-	if err != nil {
-		klog.Fatal(err)
-	}
 
 	reg := prometheus.NewRegistry()
 
@@ -64,13 +49,7 @@ func main() {
 	}))
 
 	mc := metric.NewDummyCollector()
-	if conf.EnableMetrics {
+	
-		// TODO: Ingress class is not a part of dataplane anymore
-		mc, err = metric.NewCollector(conf.MetricsPerHost, conf.ReportStatusClasses, reg, conf.IngressClassConfiguration.Controller, *conf.MetricsBuckets, conf.ExcludeSocketMetrics)
-		if err != nil {
-			klog.Fatalf("Error creating prometheus collector:  %v", err)
-		}
-	}
 	// Pass the ValidationWebhook status to determine if we need to start the collector
 	// for the admissionWebhook
 	// TODO: Dataplane does not contain validation webhook so the MetricCollector should not receive

docs/enhancements/20231001-split-containers.md

@@ -27,6 +27,29 @@ to start, stop and reload NGINX
   * /test - (POST) Test the configuration of a given file location
     * "config" argument is the location of temporary file that should be tested
 
+## Implementation details
+### Control Plane
+
+This container will have the following functionality:
+* Watch / Map changes on Kubernetes API Server
+* Write the configuration files on the shared directory
+* Call the Openresty endpoint to do the dynamic configuration
+* Trigger reload, when required, on the dataplane container
+
+Open ports:
+* Metrics port
+
+### Data Plane
+This container will have the following functionality
+
+* Provide Access between users and Pods
+ * (TODO): Provide TLS Passthrough
+
+Open ports:
+* HTTP/HTTPs Ports
+* Localhost only - Openresty configuration port
+* (TODO): Metrics port
+
 ### Mounting empty SA on controller container
 
 ```yaml

images/nginx-1.25/TAG

@@ -1 +1 @@
-v0.0.6
+v0.0.7

images/nginx-1.25/rootfs/build.sh

@@ -625,13 +625,13 @@ done
 
 rm -rf /etc/nginx/owasp-modsecurity-crs/.git
 rm -rf /etc/nginx/owasp-modsecurity-crs/util/regression-tests
-rm -rf /etc/nginx/fastcgi.conf.*
+rm -rf /etc/nginx/fastcgi.conf*
 rm -rf /etc/nginx/koi*
 rm -rf /etc/nginx/mime.types.default
 rm -rf /etc/nginx/nginx.conf.default
 rm -rf /etc/nginx/opentracing.json
 rm -rf /etc/nginx/scgi*
-rm -rf /etc/nginx/uwscgi*
+rm -rf /etc/nginx/uwsgi*
 rm -rf /etc/nginx/win-utf
 
 # remove .a files

internal/dataplane/nginx/nginx.go

@@ -0,0 +1,90 @@
+package nginx
+
+import (
+	"os"
+	"os/exec"
+	"syscall"
+
+	"k8s.io/klog/v2"
+)
+
+const (
+	defBinary = "/usr/bin/nginx"
+	CfgPath   = "/etc/nginx/conf/nginx.conf"
+)
+
+// NginxExecTester defines the interface to execute
+// command like reload or test configuration
+type NginxExecutor interface {
+	Reload() ([]byte, error)
+	Test(cfg string) ([]byte, error)
+	Stop() error
+	Start(chan error)
+}
+
+// NginxCommand stores context around a given nginx executable path
+type NginxCommand struct {
+	Binary string
+}
+
+// NewNginxCommand returns a new NginxCommand from which path
+// has been detected from environment variable NGINX_BINARY or default
+func NewNginxCommand() NginxCommand {
+	command := NginxCommand{
+		Binary: defBinary,
+	}
+
+	binary := os.Getenv("NGINX_BINARY")
+	if binary != "" {
+		command.Binary = binary
+	}
+
+	return command
+}
+
+// ExecCommand instanciates an exec.Cmd object to call nginx program
+func (nc NginxCommand) execCommand(args ...string) *exec.Cmd {
+	cmdArgs := []string{}
+
+	cmdArgs = append(cmdArgs, "-c", CfgPath)
+	cmdArgs = append(cmdArgs, args...)
+	//nolint:gosec // Ignore G204 error
+	executor := exec.Command(nc.Binary, cmdArgs...)
+	executor.SysProcAttr = &syscall.SysProcAttr{
+		Setpgid: true,
+		Pgid:    0,
+	}
+	return executor
+}
+
+func (nc NginxCommand) Start(errch chan error) {
+	cmd := nc.execCommand()
+	cmd.Stdout = os.Stdout
+	cmd.Stderr = os.Stderr
+	if err := cmd.Start(); err != nil {
+		klog.Fatalf("NGINX error: %v", err)
+		errch <- err
+		return
+	}
+	go func() {
+		errch <- cmd.Wait()
+	}()
+}
+
+func (nc NginxCommand) Reload() ([]byte, error) {
+	cmd := nc.execCommand("-s", "reload")
+	return cmd.CombinedOutput()
+}
+
+func (nc NginxCommand) Stop() error {
+	cmd := nc.execCommand("-s", "quit")
+	cmd.Stdout = os.Stdout
+	cmd.Stderr = os.Stderr
+	return cmd.Run()
+}
+
+// Test checks if config file is a syntax valid nginx configuration
+func (nc NginxCommand) Test(cfg string) ([]byte, error) {
+	//nolint:gosec // Ignore G204 error
+	return exec.Command(nc.Binary, "-c", cfg, "-t").CombinedOutput()
+}

internal/ingress/controller/controller_test.go

@@ -25,7 +25,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -42,6 +41,7 @@ import (
 
 	"k8s.io/ingress-nginx/pkg/apis/ingress"
 
+	nginxdataplane "k8s.io/ingress-nginx/internal/dataplane/nginx"
 	"k8s.io/ingress-nginx/internal/ingress/annotations"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/canary"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/ipallowlist"
@@ -134,8 +134,17 @@ type testNginxTestCommand struct {
 	err      error
 }
 
-func (ntc testNginxTestCommand) ExecCommand(_ ...string) *exec.Cmd {
+// TODO: Implement tests below and move to dataplane package
-	return nil
+func (ntc testNginxTestCommand) Stop() error {
+	panic("not implemented")
+}
+
+func (ntc testNginxTestCommand) Start(errch chan error) {
+	panic("not implemented")
+}
+
+func (ntc testNginxTestCommand) Reload() ([]byte, error) {
+	panic("not implemented")
 }
 
 func (ntc testNginxTestCommand) Test(cfg string) ([]byte, error) {
@@ -2543,7 +2552,7 @@ func newNGINXController(t *testing.T) *NGINXController {
 	return &NGINXController{
 		store:   storer,
 		cfg:     config,
-		command: NewNginxCommand(),
+		command: nginxdataplane.NewNginxCommand(),
 	}
 }
 
@@ -2608,7 +2617,7 @@ func newDynamicNginxController(t *testing.T, setConfigMap func(string) *corev1.C
 	return &NGINXController{
 		store:           storer,
 		cfg:             config,
-		command:         NewNginxCommand(),
+		command:         nginxdataplane.NewNginxCommand(),
 		metricCollector: metric.DummyCollector{},
 	}
 }

internal/ingress/controller/nginx.go

@@ -60,6 +60,7 @@ import (
 	"k8s.io/ingress-nginx/internal/task"
 	"k8s.io/ingress-nginx/pkg/apis/ingress"
 
+	nginxdataplane "k8s.io/ingress-nginx/internal/dataplane/nginx"
 	"k8s.io/ingress-nginx/pkg/util/file"
 	utilingress "k8s.io/ingress-nginx/pkg/util/ingress"
 
@@ -108,7 +109,7 @@ func NewNGINXController(config *Configuration, mc metric.Collector) *NGINXContro
 
 		metricCollector: mc,
 
-		command: NewNginxCommand(),
+		command: nginxdataplane.NewNginxCommand(),
 	}
 
 	if n.cfg.ValidationWebhook != "" {
@@ -259,7 +260,7 @@ type NGINXController struct {
 
 	validationWebhookServer *http.Server
 
-	command NginxExecTester
+	command nginxdataplane.NginxExecutor
 }
 
 // Start starts a new NGINX master process running in the foreground.
@@ -297,21 +298,12 @@ func (n *NGINXController) Start() {
 		})
 	}
 
-	cmd := n.command.ExecCommand()
-
-	// put NGINX in another process group to prevent it
-	// to receive signals meant for the controller
-	cmd.SysProcAttr = &syscall.SysProcAttr{
-		Setpgid: true,
-		Pgid:    0,
-	}
-
 	if n.cfg.EnableSSLPassthrough {
 		n.setupSSLProxy()
 	}
 
 	klog.InfoS("Starting NGINX process")
-	n.start(cmd)
+	n.start()
 
 	go n.syncQueue.Run(time.Second, n.stopCh)
 	// force initial sync
@@ -403,13 +395,10 @@ func (n *NGINXController) Stop() error {
 
 	// send stop signal to NGINX
 	klog.InfoS("Stopping NGINX process")
-	cmd := n.command.ExecCommand("-s", "quit")
+	if err := n.command.Stop(); err != nil {
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	err := cmd.Run()
-	if err != nil {
 		return err
 	}
+	
 
 	// wait for the NGINX process to terminate
 	timer := time.NewTicker(time.Second * 1)
@@ -424,18 +413,8 @@ func (n *NGINXController) Stop() error {
 	return nil
 }
 
-func (n *NGINXController) start(cmd *exec.Cmd) {
+func (n *NGINXController) start() {
-	cmd.Stdout = os.Stdout
+	n.command.Start(n.ngxErrCh)
-	cmd.Stderr = os.Stderr
-	if err := cmd.Start(); err != nil {
-		klog.Fatalf("NGINX error: %v", err)
-		n.ngxErrCh <- err
-		return
-	}
-
-	go func() {
-		n.ngxErrCh <- cmd.Wait()
-	}()
 }
 
 // DefaultEndpoint returns the default endpoint to be use as default server that returns 404.
@@ -692,11 +671,12 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error {
 	}
 
 	if klog.V(2).Enabled() {
-		src, err := os.ReadFile(cfgPath)
+		src, err := os.ReadFile(nginxdataplane.CfgPath)
 		if err != nil {
 			return err
 		}
 		if !bytes.Equal(src, content) {
+			// TODO: This test should run on dataplane side
 			tmpfile, err := os.CreateTemp("", "new-nginx-cfg")
 			if err != nil {
 				return err
@@ -707,7 +687,7 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error {
 				return err
 			}
 			//nolint:gosec //Ignore G204 error
-			diffOutput, err := exec.Command("diff", "-I", "'# Configuration.*'", "-u", cfgPath, tmpfile.Name()).CombinedOutput()
+			diffOutput, err := exec.Command("diff", "-I", "'# Configuration.*'", "-u", nginxdataplane.CfgPath, tmpfile.Name()).CombinedOutput()
 			if err != nil {
 				if exitError, ok := err.(*exec.ExitError); ok {
 					ws, ok := exitError.Sys().(syscall.WaitStatus)
@@ -728,12 +708,12 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error {
 		}
 	}
 
-	err = os.WriteFile(cfgPath, content, file.ReadWriteByUser)
+	err = os.WriteFile(nginxdataplane.CfgPath, content, file.ReadWriteByUser)
 	if err != nil {
 		return err
 	}
 
-	o, err := n.command.ExecCommand("-s", "reload").CombinedOutput()
+	o, err := n.command.Reload()
 	if err != nil {
 		return fmt.Errorf("%v\n%v", err, string(o))
 	}

internal/ingress/controller/util.go

@@ -19,7 +19,6 @@ package controller
 import (
 	"fmt"
 	"os"
-	"os/exec"
 	"path"
 	"strconv"
 	"strings"
@@ -97,53 +96,7 @@ func rlimitMaxNumFiles() int {
 	return int(rLimit.Max)
 }
 
-const (
-	defBinary = "/usr/bin/nginx"
-	cfgPath   = "/etc/nginx/nginx.conf"
-)
 
-// NginxExecTester defines the interface to execute
-// command like reload or test configuration
-type NginxExecTester interface {
-	ExecCommand(args ...string) *exec.Cmd
-	Test(cfg string) ([]byte, error)
-}
-
-// NginxCommand stores context around a given nginx executable path
-type NginxCommand struct {
-	Binary string
-}
-
-// NewNginxCommand returns a new NginxCommand from which path
-// has been detected from environment variable NGINX_BINARY or default
-func NewNginxCommand() NginxCommand {
-	command := NginxCommand{
-		Binary: defBinary,
-	}
-
-	binary := os.Getenv("NGINX_BINARY")
-	if binary != "" {
-		command.Binary = binary
-	}
-
-	return command
-}
-
-// ExecCommand instanciates an exec.Cmd object to call nginx program
-func (nc NginxCommand) ExecCommand(args ...string) *exec.Cmd {
-	cmdArgs := []string{}
-
-	cmdArgs = append(cmdArgs, "-c", cfgPath)
-	cmdArgs = append(cmdArgs, args...)
-	//nolint:gosec // Ignore G204 error
-	return exec.Command(nc.Binary, cmdArgs...)
-}
-
-// Test checks if config file is a syntax valid nginx configuration
-func (nc NginxCommand) Test(cfg string) ([]byte, error) {
-	//nolint:gosec // Ignore G204 error
-	return exec.Command(nc.Binary, "-c", cfg, "-t").CombinedOutput()
-}
 
 // getSysctl returns the value for the specified sysctl setting
 func getSysctl(sysctl string) (int, error) {

rootfs/Dockerfile

@@ -53,6 +53,7 @@ RUN bash -xeu -c ' \
     /etc/ingress-controller/auth \
     /etc/ingress-controller/geoip \
     /etc/ingress-controller/telemetry \
+    /etc/nginx/conf \
     /var/log \
     /var/log/nginx \
     /tmp/nginx \