Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The dockerfile installs dumb-init but doesn't use it #9858

Closed
tspearconquest opened this issue Apr 13, 2023 · 8 comments
Closed

The dockerfile installs dumb-init but doesn't use it #9858

tspearconquest opened this issue Apr 13, 2023 · 8 comments
Labels
needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@tspearconquest
Copy link

What happened:

I noticed the dockerfile installs dumb-init but doesn't make use of it.

What you expected to happen:

I expected nginx to use dumb-init in the entrypoint, or not install it.

NGINX Ingress controller version (exec into the pod and run nginx-ingress-controller --version.): Look in the master branch (all versions)

Kubernetes version (use kubectl version):
Doesn't matter, but for posterity:

{
  "clientVersion": {
    "major": "1",
    "minor": "24",
    "gitVersion": "v1.24.11",
    "gitCommit": "0f75679e3346160939924550fd3591462a4afec6",
    "gitTreeState": "clean",
    "buildDate": "2023-02-22T13:39:33Z",
    "goVersion": "go1.19.6",
    "compiler": "gc",
    "platform": "darwin/amd64"
  },
  "kustomizeVersion": "v4.5.4",
  "serverVersion": {
    "major": "1",
    "minor": "24",
    "gitVersion": "v1.24.9",
    "gitCommit": "57fbbcc2804848b95cad5519f5ec9d6355430db9",
    "gitTreeState": "clean",
    "buildDate": "2023-02-08T17:22:38Z",
    "goVersion": "go1.18.9",
    "compiler": "gc",
    "platform": "linux/amd64"
  }
}

Environment:

  • Cloud provider or hardware configuration: AKS K8s 1.24 on various node sizes

  • OS (e.g. from /etc/os-release): Ubuntu 18.04

  • Kernel (e.g. uname -a): 5.4.0-1103-azure_109

  • How was the ingress-nginx-controller installed: It wasn't, I noticed this while reviewing the dockerfile code

  • Current State of the controller: N/A

  • Current state of ingress object, if applicable: N/A

  • Others:

RUN apk update \
  && apk upgrade \
  && apk add -U --no-cache \
  bash \
  openssl \
  pcre \
  zlib \
  geoip \
  curl \
  ca-certificates \
  patch \
  yajl \
  lmdb \
  libxml2 \
  libmaxminddb \
  yaml-cpp \
## Here is dumb-init being installed
  dumb-init \
  tzdata \
  && ln -s /usr/local/nginx/sbin/nginx /sbin/nginx \
  && adduser -S -D -H -u 101 -h /usr/local/nginx \
  -s /sbin/nologin -G www-data -g www-data www-data \
  && bash -eu -c ' \
  writeDirs=( \
  /var/log/nginx \
  /var/lib/nginx/body \
  /var/lib/nginx/fastcgi \
  /var/lib/nginx/proxy \
  /var/lib/nginx/scgi \
  /var/lib/nginx/uwsgi \
  /var/log/audit \
  ); \
  for dir in "${writeDirs[@]}"; do \
  mkdir -p ${dir}; \
  chown -R www-data.www-data ${dir}; \
  done'

EXPOSE 80 443

## Here I expected something like below (or not installing dumb-init above):
# ENTRYPOINT ["dumb-init", "--"]
CMD ["nginx", "-g", "daemon off;"]  

How to reproduce this issue:

Look at the dockerfile

Anything else we need to know:

Really, look at the dockerfile.

@tspearconquest tspearconquest added the kind/bug Categorizes issue or PR as related to a bug. label Apr 13, 2023
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Apr 13, 2023
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@longwuyuan
Copy link
Contributor

/remove-kind bug
@tspearconquest impact of using or not using dumb-init is not clearly explained in your issue description.

  • What is the impact on users that is not desirable
  • Have you searched the entire codebase for existence of string "dumb-init"

@k8s-ci-robot k8s-ci-robot added needs-kind Indicates a PR lacks a `kind/foo` label and requires one. and removed kind/bug Categorizes issue or PR as related to a bug. labels Apr 14, 2023
@tspearconquest
Copy link
Author

tspearconquest commented Apr 14, 2023

Hello,

The impact is that its an unnecessary package which could trigger findings in users vulnerability scans if a CVE is found in the package.

This is a similar problem to one discussed and fixed in an issue opened against the nginx upstream docker image because curl is included in the nginx image itself, so I request you to review the additional discussion which is available in that issue around this type of issue.

Have you searched the entire codebase for existence of string "dumb-init"

I just did a quick github search in this project and did not find any reference to it in the code. I am not currently a user but was evaluating the project and investigating how the container is built for a possible POC, and this caught my eye.

@tspearconquest
Copy link
Author

Making use of dumb-init would support #8034

@longwuyuan
Copy link
Contributor

@tspearconquest comment on the existence of this line

ENTRYPOINT ["/usr/bin/dumb-init", "--"]

based on the above line, it looks as if your claim is false, if/when you say/mean "dumb-init" is not being used .
If you meant to say something else, its not clear as to what problem needs to be solved.

@tspearconquest
Copy link
Author

tspearconquest commented Apr 14, 2023 via email

@longwuyuan
Copy link
Contributor

longwuyuan commented Apr 14, 2023 via email

@k8s-ci-robot
Copy link
Contributor

@longwuyuan: Closing this issue.

In response to this:

Thanks for clarifying that. Please inspect code to know more. There is a
base image and then there is a controller image.

/close

On Fri, 14 Apr, 2023, 8:15 am tspearconquest, @.***>
wrote:

Hello,

The file I'm referring to is under the images/rootfs directory, not the
rootfs directory in the repo root:
https://github.com/kubernetes/ingress-nginx/blob/4e8d0b5836096426393340273260b9000cd1e151/images/nginx/rootfs/Dockerfile

Please check it and advise

Get Outlook for Androidhttps://aka.ms/AAb9ysg


From: Long Wu Yuan @.>
Sent: Thursday, April 13, 2023 9:39:51 PM
To: kubernetes/ingress-nginx @.
>
Cc: Thomas Spear @.>; Mention @.>
Subject: Re: [kubernetes/ingress-nginx] The dockerfile installs dumb-init
but doesn't use it (Issue #9858)

CAUTION: This email originated from outside the organization. Do not click
links or open attachments unless you recognize the sender and know the
content is safe.

@tspearconquesthttps://github.com/tspearconquest comment on the
existence of this line

ENTRYPOINT ["/usr/bin/dumb-init", "--"]

based on the above line, it looks as if your claim is false, if/when you
say/mean "dumb-init" is not being used .
If you meant to say something else, its not clear as to what problem needs
to be solved.


Reply to this email directly, view it on GitHub<
https://github.com/kubernetes/ingress-nginx/issues/9858#issuecomment-1507848808>,
or unsubscribe<
https://github.com/notifications/unsubscribe-auth/ATRTFZ2CGKDTF2KB4YILJ2TXBC2HPANCNFSM6AAAAAAW5UVG2Q

.
You are receiving this because you were mentioned.Message ID: @.***>


Reply to this email directly, view it on GitHub
#9858 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABGZVWRUS2ZNIPHNEBOIW5TXBC247ANCNFSM6AAAAAAW5UVG2Q
.
You are receiving this because you commented.Message ID:
@.***>

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Archived in project
Development

No branches or pull requests

3 participants