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

Run IC as non-root #710

Merged
merged 1 commit into from
Oct 1, 2019
Merged

Run IC as non-root #710

merged 1 commit into from
Oct 1, 2019

Conversation

Rulox
Copy link
Contributor

@Rulox Rulox commented Sep 27, 2019

Proposed changes

Closes #529

This PR makes the changes in all the Dockerfiles (and IC code) to run IC and NGINX as non root.

Changes are:

  • Containers will use nginx as user instead of root
  • IC and NGINX will run using the nginx user
  • Changed the pid, opentracing config and unix sockets to /var/lib/nginx
  • Used setcap so we can still use ports 80 and 443 without being root
  • Removed user directive in NGINX conf (not needed anymore as we are running NGINX as nginx user)
  • Added security policies to IC Pod in all manifests (and helm manifests)

PS: I have arranged the Dockerfiles to keep consistency (order, number of layers, etc).

Thanks to @LorcanMcVeigh as he started working on this.

@Rulox
Copy link
Contributor Author

Rulox commented Sep 27, 2019

Checking CI :)

@Rulox Rulox requested a review from tellet September 27, 2019 13:58
Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

@Rulox thanks. please see my comments

build/DockerfileForAlpine Outdated Show resolved Hide resolved
build/Dockerfile Outdated Show resolved Hide resolved
deployments/daemon-set/nginx-ingress.yaml Outdated Show resolved Hide resolved
build/DockerfileForPlus Outdated Show resolved Hide resolved
Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@tellet tellet left a comment

Choose a reason for hiding this comment

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

Looks good

@Rulox Rulox merged commit 6b6ca41 into master Oct 1, 2019
@Rulox Rulox deleted the non-root-user branch October 1, 2019 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change Pull requests that introduce a change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Would you consider building unprivileged nginx-ingress controller or have you got this already?
5 participants