diff --git a/Makefile b/Makefile index 8ff0f0ca0..60e1e4b7f 100644 --- a/Makefile +++ b/Makefile @@ -80,6 +80,19 @@ image: clean-image ## Build image for a particular arch. --build-arg BUILD_ID="$(BUILD_ID)" \ -t $(REGISTRY)/controller:$(TAG) rootfs +.PHONY: image-dataplane +image-dataplane: clean-dataplane-image ## Build image for a particular arch. + echo "Building docker image dataplane ($(ARCH))..." + docker build \ + ${PLATFORM_FLAG} ${PLATFORM} \ + --no-cache \ + --build-arg BASE_IMAGE="$(BASE_IMAGE)" \ + --build-arg VERSION="$(TAG)" \ + --build-arg TARGETARCH="$(ARCH)" \ + --build-arg COMMIT_SHA="$(COMMIT_SHA)" \ + --build-arg BUILD_ID="$(BUILD_ID)" \ + -t $(REGISTRY)/dataplane:$(TAG) -f rootfs/Dockerfile.dataplane rootfs + .PHONY: gosec gosec: docker run --rm -it -w /source/ -v "$(pwd)"/:/source securego/gosec:2.11.0 -exclude=G109,G601,G104,G204,G304,G306,G307 -tests=false -exclude-dir=test -exclude-dir=images/ -exclude-dir=docs/ /source/... @@ -101,12 +114,15 @@ clean-image: ## Removes local image echo "removing old image $(REGISTRY)/controller:$(TAG)" @docker rmi -f $(REGISTRY)/controller:$(TAG) || true - .PHONY: clean-chroot-image clean-chroot-image: ## Removes local image echo "removing old image $(REGISTRY)/controller-chroot:$(TAG)" @docker rmi -f $(REGISTRY)/controller-chroot:$(TAG) || true +.PHONY: clean-dataplane-image +clean-dataplane-image: ## Removes local image + echo "removing old image $(REGISTRY)/dataplane:$(TAG)" + @docker rmi -f $(REGISTRY)/dataplane:$(TAG) || true .PHONY: build build: ## Build ingress controller, debug tool and pre-stop hook. diff --git a/build/build.sh b/build/build.sh index f3504eada..d44c7e402 100755 --- a/build/build.sh +++ b/build/build.sh @@ -48,6 +48,16 @@ ${GO_BUILD_CMD} \ -buildvcs=false \ -o "${TARGETS_DIR}/nginx-ingress-controller" "${PKG}/cmd/nginx" +echo "Building ${PKG}/cmd/dataplane" + +${GO_BUILD_CMD} \ + -trimpath -ldflags="-buildid= -w -s \ + -X ${PKG}/version.RELEASE=${TAG} \ + -X ${PKG}/version.COMMIT=${COMMIT_SHA} \ + -X ${PKG}/version.REPO=${REPO_INFO}" \ + -buildvcs=false \ + -o "${TARGETS_DIR}/nginx-ingress-dataplane" "${PKG}/cmd/dataplane" + echo "Building ${PKG}/cmd/dbg" ${GO_BUILD_CMD} \ diff --git a/cmd/dataplane/main.go b/cmd/dataplane/main.go index f11fb02ec..58698089f 100644 --- a/cmd/dataplane/main.go +++ b/cmd/dataplane/main.go @@ -61,8 +61,11 @@ func main() { signal.Notify(signalChan, syscall.SIGTERM) // TODO: Turn delay configurable n := dataplanenginx.NewNGINXExecutor(mux, 10, errCh, stopCh) - //go executor.Start() - go metrics.StartHTTPServer("127.0.0.1", 12345, mux) + err := n.Start() + if err != nil { + klog.Fatalf("error starting nginx process: %s", err) + } + go metrics.StartHTTPServer("0.0.0.0", 12345, mux) // TODO: deal with OS signals select { diff --git a/cmd/dataplane/pkg/nginx/nginx.go b/cmd/dataplane/pkg/nginx/nginx.go index a33830100..a292e272c 100644 --- a/cmd/dataplane/pkg/nginx/nginx.go +++ b/cmd/dataplane/pkg/nginx/nginx.go @@ -3,6 +3,7 @@ package nginx import ( "fmt" "net/http" + "path/filepath" nginxdataplane "k8s.io/ingress-nginx/internal/dataplane/nginx" "k8s.io/klog/v2" @@ -26,8 +27,12 @@ func NewNGINXExecutor(mux *http.ServeMux, stopdelay int, errch chan error, stopc return n } -func (n *nginxExecutor) Start() { - n.cmd.Start(n.errch) +func (n *nginxExecutor) Start() error { + err := n.cmd.Start(n.errch) + if err != nil { + return err + } + return nil } func (n *nginxExecutor) Stop() error { @@ -81,7 +86,10 @@ func (n *nginxExecutor) handleTest(w http.ResponseWriter, r *http.Request) { return } - o, err := n.cmd.Test(testFile) + // Avoid transversal path. We will get the final file from /etc/ingress-controller/tempconf + tmpFileFinal := filepath.Base(testFile) + + o, err := n.cmd.Test(tmpFileFinal) if err != nil { w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error() + "\n")) diff --git a/internal/dataplane/nginx/nginx.go b/internal/dataplane/nginx/nginx.go index 5feb3be5f..320e90687 100644 --- a/internal/dataplane/nginx/nginx.go +++ b/internal/dataplane/nginx/nginx.go @@ -3,6 +3,7 @@ package nginx import ( "os" "os/exec" + "path/filepath" "syscall" "k8s.io/klog/v2" @@ -11,6 +12,7 @@ import ( const ( defBinary = "/usr/bin/nginx" CfgPath = "/etc/nginx/conf/nginx.conf" + TempDir = "/etc/ingress-controller/tempconf" ) // NginxExecTester defines the interface to execute @@ -19,7 +21,7 @@ type NginxExecutor interface { Reload() ([]byte, error) Test(cfg string) ([]byte, error) Stop() error - Start(chan error) + Start(chan error) error } // NginxCommand stores context around a given nginx executable path @@ -57,18 +59,18 @@ func (nc NginxCommand) execCommand(args ...string) *exec.Cmd { return executor } -func (nc NginxCommand) Start(errch chan error) { +func (nc NginxCommand) Start(errch chan error) 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 + klog.ErrorS(err, "NGINX error") + return err } go func() { errch <- cmd.Wait() }() + return nil } func (nc NginxCommand) Reload() ([]byte, error) { @@ -86,5 +88,6 @@ func (nc NginxCommand) Stop() error { // Test checks if config file is a syntax valid nginx configuration func (nc NginxCommand) Test(cfg string) ([]byte, error) { //nolint:gosec // Ignore G204 error + cfg = filepath.Join(TempDir, cfg) return exec.Command(nc.Binary, "-c", cfg, "-t").CombinedOutput() } \ No newline at end of file diff --git a/internal/dataplane/nginx/remote.go b/internal/dataplane/nginx/remote.go new file mode 100644 index 000000000..beba5fdf0 --- /dev/null +++ b/internal/dataplane/nginx/remote.go @@ -0,0 +1,103 @@ +package nginx + +import ( + "fmt" + "io" + "net/http" + "net/url" + "strings" +) + +// NginxCommand stores context around a given nginx executable path +type NginxRemote struct { + host string +} + +// NewNginxCommand returns a new NginxCommand from which path +// has been detected from environment variable NGINX_BINARY or default +func NewNginxRemote(host string) NginxExecutor { + return NginxRemote{ + host: host, + } +} + +func (nc NginxRemote) Start(errch chan error) error { + getStart, err := url.JoinPath(nc.host, "start") // TODO: Turn this path a constant on dataplane + if err != nil { + return err + } + resp, err := http.Get(getStart) + if err != nil { + return err + } + body, err := io.ReadAll(resp.Body) + if err != nil { + return err + } + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("error executing start: %s", string(body)) + } + + // TODO: Add a ping/watcher to backend and populate error channel + return nil +} + +func (nc NginxRemote) Reload() ([]byte, error) { + getReload, err := url.JoinPath(nc.host, "reload") // TODO: Turn this path a constant on dataplane + if err != nil { + return nil, err + } + resp, err := http.Get(getReload) + if err != nil { + return nil, err + } + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err + } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("error executing reload: %s", string(body)) + } + return body, nil +} + +func (nc NginxRemote) Stop() error { + getStop, err := url.JoinPath(nc.host, "stop") // TODO: Turn this path a constant on dataplane + if err != nil { + return err + } + resp, err := http.Get(getStop) + if err != nil { + return err + } + body, err := io.ReadAll(resp.Body) + if err != nil { + return err + } + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("error executing stop: %s", string(body)) + } + return nil +} + +// Test checks if config file is a syntax valid nginx configuration +func (nc NginxRemote) Test(cfg string) ([]byte, error) { + form := url.Values{} + form.Add("testfile", cfg) + getStop, err := url.JoinPath(nc.host, "test") // TODO: Turn this path a constant on dataplane + if err != nil { + return nil, err + } + resp, err := http.Post(getStop, "application/x-www-form-urlencoded", strings.NewReader(form.Encode())) + if err != nil { + return nil, err + } + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err + } + if resp.StatusCode != http.StatusOK { + return body, fmt.Errorf("error executing stop: %s", string(body)) + } + return body, nil +} \ No newline at end of file diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index 320ac1b49..1f1c0a6d3 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -109,7 +109,9 @@ func NewNGINXController(config *Configuration, mc metric.Collector) *NGINXContro metricCollector: mc, - command: nginxdataplane.NewNginxCommand(), + command: nginxdataplane.NewNginxRemote("http://127.0.0.1:12345"), + //command: nginxdataplane.NewNginxCommand(), + } if n.cfg.ValidationWebhook != "" { @@ -298,9 +300,12 @@ func (n *NGINXController) Start() { }) } + // TODO: SSLPassthrough is not yet supported on dataplane mode + /* if n.cfg.EnableSSLPassthrough { n.setupSSLProxy() } + */ klog.InfoS("Starting NGINX process") n.start() @@ -331,6 +336,7 @@ func (n *NGINXController) Start() { for { select { + // TODO: create a separate error channel for the remote executor case err := <-n.ngxErrCh: if n.isShuttingDown { return @@ -414,7 +420,11 @@ func (n *NGINXController) Stop() error { } func (n *NGINXController) start() { - n.command.Start(n.ngxErrCh) + // TODO: do a better retry of start before failing + if err := n.command.Start(n.ngxErrCh); err != nil { + n.stopCh <- struct{}{} + klog.Fatalf("error starting NGINX: %s", err) + } } // DefaultEndpoint returns the default endpoint to be use as default server that returns 404. @@ -618,7 +628,7 @@ func (n *NGINXController) testTemplate(cfg []byte) error { if len(cfg) == 0 { return fmt.Errorf("invalid NGINX configuration (empty)") } - tmpDir := os.TempDir() + "/nginx" + tmpDir := nginxdataplane.TempDir tmpfile, err := os.CreateTemp(tmpDir, tempNginxPattern) if err != nil { return err @@ -628,7 +638,8 @@ func (n *NGINXController) testTemplate(cfg []byte) error { if err != nil { return err } - out, err := n.command.Test(tmpfile.Name()) + tmpfileName := filepath.Base(tmpfile.Name()) + out, err := n.command.Test(tmpfileName) if err != nil { // this error is different from the rest because it must be clear why nginx is not working oe := fmt.Sprintf(` @@ -1021,7 +1032,7 @@ func createOpentelemetryCfg(cfg *ngx_config.Configuration) error { func cleanTempNginxCfg() error { var files []string - err := filepath.Walk(os.TempDir(), func(path string, info os.FileInfo, err error) error { + err := filepath.Walk(nginxdataplane.TempDir, func(path string, info os.FileInfo, err error) error { if err != nil { return err } diff --git a/rootfs/Dockerfile b/rootfs/Dockerfile index 0fe3a55f9..16c998282 100644 --- a/rootfs/Dockerfile +++ b/rootfs/Dockerfile @@ -12,9 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -ARG BASE_IMAGE +#ARG BASE_IMAGE -FROM ${BASE_IMAGE} +FROM alpine:3.19 ARG TARGETARCH ARG VERSION @@ -31,13 +31,13 @@ LABEL org.opencontainers.image.revision="${COMMIT_SHA}" LABEL build_id="${BUILD_ID}" -WORKDIR /etc/nginx - RUN apk update \ && apk upgrade \ && apk add --no-cache \ - diffutils \ - && rm -rf /var/cache/apk/* + diffutils bash \ + && rm -rf /var/cache/apk/* \ + && adduser -S -D -H -u 101 -h /usr/local/nginx \ + -s /sbin/nologin -G www-data -g www-data www-data COPY --chown=www-data:www-data etc /etc @@ -53,6 +53,7 @@ RUN bash -xeu -c ' \ /etc/ingress-controller/auth \ /etc/ingress-controller/geoip \ /etc/ingress-controller/telemetry \ + /etc/ingress-controller/tempconf \ /etc/nginx/conf \ /var/log \ /var/log/nginx \ @@ -71,18 +72,11 @@ RUN bash -xeu -c ' \ RUN apk add --no-cache libcap \ && setcap cap_net_bind_service=+ep /nginx-ingress-controller \ && setcap -v cap_net_bind_service=+ep /nginx-ingress-controller \ - && setcap cap_net_bind_service=+ep /usr/local/nginx/sbin/nginx \ - && setcap -v cap_net_bind_service=+ep /usr/local/nginx/sbin/nginx \ - && setcap cap_net_bind_service=+ep /usr/bin/dumb-init \ - && setcap -v cap_net_bind_service=+ep /usr/bin/dumb-init \ - && apk del libcap \ - && ln -sf /usr/local/nginx/sbin/nginx /usr/bin/nginx - + && apk del libcap USER www-data # Create symlinks to redirect nginx logs to stdout and stderr docker log collector RUN ln -sf /dev/stdout /var/log/nginx/access.log \ && ln -sf /dev/stderr /var/log/nginx/error.log -ENTRYPOINT ["/usr/bin/dumb-init", "--"] -CMD ["/nginx-ingress-controller"] +ENTRYPOINT ["/nginx-ingress-controller"] diff --git a/cmd/dataplane/Dockerfile b/rootfs/Dockerfile.dataplane similarity index 94% rename from cmd/dataplane/Dockerfile rename to rootfs/Dockerfile.dataplane index c6b8576fa..a8ca42043 100644 --- a/cmd/dataplane/Dockerfile +++ b/rootfs/Dockerfile.dataplane @@ -54,6 +54,7 @@ RUN bash -xeu -c ' \ /etc/ingress-controller/ssl \ /etc/ingress-controller/auth \ /etc/ingress-controller/geoip \ + /etc/ingress-controller/tempconf \ /etc/ingress-controller/telemetry \ /etc/nginx/conf \ /var/log \ @@ -71,8 +72,8 @@ RUN bash -xeu -c ' \ RUN apk add --no-cache libcap \ - && setcap cap_net_bind_service=+ep /nginx-ingress-controller \ - && setcap -v cap_net_bind_service=+ep /nginx-ingress-controller \ + && setcap cap_net_bind_service=+ep /nginx-ingress-dataplane \ + && setcap -v cap_net_bind_service=+ep /nginx-ingress-dataplane \ && setcap cap_net_bind_service=+ep /usr/local/nginx/sbin/nginx \ && setcap -v cap_net_bind_service=+ep /usr/local/nginx/sbin/nginx \ && setcap cap_net_bind_service=+ep /usr/bin/dumb-init \