Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history 
Update nginx image
  • Loading branch information
@aledbf
aledbf committed Sep 25, 2018
1 parent 471049b commit 3f29e43
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 5 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.62
BASEIMAGE?=quay.io/kubernetes-ingress-controller/nginx-$(ARCH):0.63

ifeq ($(ARCH),arm)
QEMUARCH=arm
Expand Down
2 changes: 1 addition & 1 deletion build/go-in-docker.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ if [ "$missing" = true ];then
exit 1
fi

E2E_IMAGE=quay.io/kubernetes-ingress-controller/e2e:v09122018-167ed7f
E2E_IMAGE=quay.io/kubernetes-ingress-controller/e2e:v09252018-471049b

DOCKER_OPTS=${DOCKER_OPTS:-""}

Expand Down
2 changes: 1 addition & 1 deletion images/e2e/Dockerfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

FROM quay.io/kubernetes-ingress-controller/nginx-amd64:0.61
FROM quay.io/kubernetes-ingress-controller/nginx-amd64:0.63

RUN clean-install \
g++ \
Expand Down
6 changes: 4 additions & 2 deletions rootfs/Dockerfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ WORKDIR /etc/nginx

RUN clean-install \
diffutils \
valgrind \
dumb-init
libcap2-bin

COPY . /

Expand All @@ -43,6 +42,9 @@ RUN bash -eu -c ' \
&& chown www-data.www-data /etc/nginx/nginx.conf \
&& chown www-data.www-data /etc/nginx/opentracing.json

RUN setcap cap_net_bind_service=+ep /nginx-ingress-controller \
&& setcap -v cap_net_bind_service=+ep /nginx-ingress-controller

# 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
Expand Down

4 comments on commit 3f29e43

@michaelbannister
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aledbf can I ask why you had to add cap_net_bind_service for this nginx version bump?
I understand the constraints around binding to ports < 1024 but we (for example) have tended just to use command-line args to get nginx to bind to 8080, 8443 etc so that the process doesn't need that capability.

There's also a load of authbind stuff in this image which I don't fully understand but appears to be intended to solve the same problem.

@aledbf
Copy link
Member Author

@aledbf aledbf commented on 3f29e43 Nov 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelbannister because go binaries do not work with authbind and without cap_net_bind we break ssl-passthrough.

@michaelbannister
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I understand. But if I’m not using ssl-passthrough or intend to change the ssl port by setting --https-port and --ssl-passthrough-proxy-port to ports higher than 1024, I won’t need this. It’d be nicer if this could be done at runtime only if required. Not sure if that’s possible.

My use case is that I have a fairly restrictive pod security policy by default, disallowing all capabilities (I just use higher ports), so this has forced me to create a special PSP just for the ingress controller. Fine, but would be simpler if I didn’t have to. 🙂 Might put this in a new issue after discussing with my team.

@aledbf
Copy link
Member Author

@aledbf aledbf commented on 3f29e43 Nov 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’d be nicer if this could be done at runtime only if required. Not sure if that’s possible.

That's not possible running as user. We tried that in the past but that approach just introduces complexity and edge cases where it does not work.

You can fork the repo, changing just the dockerfile and use that image, you just need docker installed to build 😉

Please sign in to comment.