Merge pull request #2894 from aledbf/authbind

Use authbind to bind privileged ports

Makefile

@@ -61,7 +61,7 @@ IMAGE = $(REGISTRY)/$(IMGNAME)
 MULTI_ARCH_IMG = $(IMAGE)-$(ARCH)
 
 # Set default base image dynamically for each arch
-BASEIMAGE?=quay.io/kubernetes-ingress-controller/nginx-$(ARCH):0.55
+BASEIMAGE?=quay.io/kubernetes-ingress-controller/nginx-$(ARCH):0.57
 
 ifeq ($(ARCH),arm)
 	QEMUARCH=arm

build/go-in-docker.sh

@@ -40,7 +40,7 @@ if [ "$missing" = true ];then
   exit 1
 fi
 
-E2E_IMAGE=quay.io/kubernetes-ingress-controller/e2e:v07282018-45ba1672c
+E2E_IMAGE=quay.io/kubernetes-ingress-controller/e2e:v08042018-e2f5d90
 
 DOCKER_OPTS=${DOCKER_OPTS:-""}
 
@@ -75,6 +75,7 @@ docker run                                       \
     -v ${PWD}/bin/${ARCH}:/go/bin/linux_${ARCH}  \
     -w /go/src/${PKG}                            \
     --env-file .env                              \
-    ${E2E_IMAGE} ${FLAGS}
+    --entrypoint ${FLAGS}                        \
+    ${E2E_IMAGE}
 
 rm .env

cmd/nginx/main.go

@@ -125,7 +125,7 @@ func main() {
 
 	mc, err := metric.NewCollector(conf.ListenPorts.Status, reg)
 	if err != nil {
-		glog.Fatalf("Error creating prometheus collectos:  %v", err)
+		glog.Fatalf("Error creating prometheus collector:  %v", err)
 	}
 	mc.Start()
 

internal/ingress/controller/nginx.go

@@ -741,7 +741,10 @@ func configureDynamically(pcfg *ingress.Configuration, port int) error {
 	backends := make([]*ingress.Backend, len(pcfg.Backends))
 
 	for i, backend := range pcfg.Backends {
-		service := &apiv1.Service{Spec: backend.Service.Spec}
+		var service *apiv1.Service
+		if backend.Service != nil {
+			service = &apiv1.Service{Spec: backend.Service.Spec}
+		}
 		luaBackend := &ingress.Backend{
 			Name:            backend.Name,
 			Port:            backend.Port,

internal/ingress/controller/util.go

@@ -80,9 +80,9 @@ func nginxExecCommand(args ...string) *exec.Cmd {
 		ngx = defBinary
 	}
 
-	cmdArgs := []string{"-c", cfgPath}
+	cmdArgs := []string{"--deep", ngx, "-c", cfgPath}
 	cmdArgs = append(cmdArgs, args...)
-	return exec.Command(ngx, cmdArgs...)
+	return exec.Command("authbind", cmdArgs...)
 }
 
 func nginxTestCommand(cfg string) *exec.Cmd {
@@ -91,5 +91,5 @@ func nginxTestCommand(cfg string) *exec.Cmd {
 		ngx = defBinary
 	}
 
-	return exec.Command(ngx, "-c", cfg, "-t")
+	return exec.Command("authbind", "--deep", ngx, "-c", cfg, "-t")
 }

internal/ingress/metric/collectors/controller.go

@@ -77,7 +77,7 @@ func NewController(pod, namespace, class string) *Controller {
 			prometheus.GaugeOpts{
 				Namespace:   PrometheusNamespace,
 				Name:        "config_last_reload_successful",
-				Help:        "Whether the last configuration reload attemp was successful",
+				Help:        "Whether the last configuration reload attempt was successful",
 				ConstLabels: constLabels,
 			}),
 		configSuccessTime: prometheus.NewGauge(

internal/ingress/metric/collectors/controller_test.go

@@ -26,7 +26,7 @@ import (
 
 func TestControllerCounters(t *testing.T) {
 	const metadata = `
-		# HELP nginx_ingress_controller_config_last_reload_successful Whether the last configuration reload attemp was successful
+		# HELP nginx_ingress_controller_config_last_reload_successful Whether the last configuration reload attempt was successful
 		# TYPE nginx_ingress_controller_config_last_reload_successful gauge
 		# HELP nginx_ingress_controller_success Cumulative number of Ingress controller reload operations
 		# TYPE nginx_ingress_controller_success counter

internal/ingress/metric/collectors/socket.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"io"
 	"net"
+	"os"
 
 	"github.com/golang/glog"
 	"github.com/prometheus/client_golang/prometheus"
@@ -95,7 +96,13 @@ var (
 // NewSocketCollector creates a new SocketCollector instance using
 // the ingresss watch namespace and class used by the controller
 func NewSocketCollector(pod, namespace, class string) (*SocketCollector, error) {
-	listener, err := net.Listen("unix", "/tmp/prometheus-nginx.socket")
+	socket := "/tmp/prometheus-nginx.socket"
+	listener, err := net.Listen("unix", socket)
+	if err != nil {
+		return nil, err
+	}
+
+	err = os.Chmod(socket, 0777)
 	if err != nil {
 		return nil, err
 	}

internal/net/net.go

@@ -29,12 +29,12 @@ func IsIPV6(ip _net.IP) bool {
 
 // IsPortAvailable checks if a TCP port is available or not
 func IsPortAvailable(p int) bool {
-	ln, err := _net.Listen("tcp", fmt.Sprintf(":%v", p))
+	conn, err := _net.Dial("tcp", fmt.Sprintf(":%v", p))
 	if err != nil {
-		return false
+		return true
 	}
-	ln.Close()
-	return true
+	defer conn.Close()
+	return false
 }
 
 // IsIPv6Enabled checks if IPV6 is enabled or not

rootfs/Dockerfile

@@ -20,14 +20,13 @@ WORKDIR  /etc/nginx
 
 RUN clean-install \
   diffutils \
-  libcap2-bin \
   dumb-init
 
 COPY . /
 
 # Fix permission during the build to avoid issues at runtime
 # with volumes (custom templates)
-RUN bash -eux -c ' \
+RUN bash -eu -c ' \
   writeDirs=( \
     /etc/nginx/template \
     /etc/ingress-controller/ssl \
@@ -41,9 +40,11 @@ RUN bash -eux -c ' \
     chown -R www-data.www-data ${dir}; \
   done' \
   && chown www-data.www-data /etc/nginx/nginx.conf \
-  && chown www-data.www-data /etc/nginx/opentracing.json \
-  && chown www-data.www-data /etc/nginx
+  && chown www-data.www-data /etc/nginx/opentracing.json
 
-ENTRYPOINT ["/entrypoint.sh"]
+# Create symlinks to redirect nginx logs to stdout and stderr docker log collector
+# This only works if nginx is started with CMD or ENTRYPOINT
+RUN ln -sf /dev/stdout /var/log/nginx/access.log
+RUN ln -sf /dev/stderr /var/log/nginx/error.log
 
 CMD ["/nginx-ingress-controller"]

rootfs/entrypoint.sh

@@ -1,49 +0,0 @@
-#!/usr/bin/dumb-init /bin/bash
-
-# Copyright 2017 The Kubernetes Authors.
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# 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 -e
-
-echo 0 > /tmp/nginx.pid
-# fix directory permissions
-writeDirs=( \
-    /var/log
-    /var/log/nginx
-    /tmp
-);
-for dir in "${writeDirs[@]}";do
-    mkdir -p ${dir};
-    chown -R www-data.www-data ${dir};
-done
-
-ln -sf /dev/stdout /var/log/nginx/access.log 
-ln -sf /dev/stderr /var/log/nginx/error.log 
-chown www-data.www-data /var/log/nginx/*
-
-echo "Testing if setcap is supported..."
-if setcap cap_net_bind_service=+ep /usr/sbin/nginx; then
-    echo "setcap is supported. Setting cap_net_bind_service=+ep to allow binding port lower than 1024 as non-root"
-    setcap cap_net_bind_service=+ep    /usr/sbin/nginx
-    setcap -v cap_net_bind_service=+ep /usr/sbin/nginx
-    setcap cap_net_bind_service=+ep    /nginx-ingress-controller
-    setcap -v cap_net_bind_service=+ep /nginx-ingress-controller
-
-    echo "Droping root privileges and running as user..."
-    su-exec www-data:www-data "$@"
-else
-    echo "WARNING!!!: setcap is not supported. Running as root"
-    echo "Please check https://github.com/moby/moby/issues/1070"
-    "$@"
-fi

rootfs/etc/nginx/template/nginx.tmpl

@@ -131,6 +131,7 @@ http {
     client_body_temp_path           /tmp/client-body;
     fastcgi_temp_path               /tmp/fastcgi-temp;
     proxy_temp_path                 /tmp/proxy-temp;
+    ajp_temp_path                   /tmp/ajp-temp; 
 
     client_header_buffer_size       {{ $cfg.ClientHeaderBufferSize }};
     client_header_timeout           {{ $cfg.ClientHeaderTimeout }}s;

test/e2e/framework/framework.go

@@ -133,6 +133,13 @@ func (f *Framework) AfterEach() {
 	By("Waiting for test namespace to no longer exist")
 	err := DeleteKubeNamespace(f.KubeClientSet, f.IngressController.Namespace)
 	Expect(err).NotTo(HaveOccurred())
+
+	if CurrentGinkgoTestDescription().Failed {
+		log, err := f.NginxLogs()
+		Expect(err).ToNot(HaveOccurred())
+		By("Dumping NGINX logs after a failure running a test")
+		Logf("%v", log)
+	}
 }
 
 // IngressNginxDescribe wrapper function for ginkgo describe. Adds namespacing.

test/manifests/ingress-controller/mandatory.yaml

@@ -260,6 +260,14 @@ spec:
               valueFrom:
                 fieldRef:
                   fieldPath: metadata.namespace
+          securityContext:
+            capabilities:
+                drop:
+                - ALL
+                add:
+                - NET_BIND_SERVICE
+            # www-data -> 33
+            runAsUser: 33
           ports:
           - name: http
             containerPort: 80