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 verrazzano-api pod as non root user #967

Merged
merged 7 commits into from Apr 14, 2021
Merged

Run verrazzano-api pod as non root user #967

merged 7 commits into from Apr 14, 2021

Conversation

desagar
Copy link
Contributor

@desagar desagar commented Apr 13, 2021

Description

Run verrazzano-api pod as non-root user. This required changing the default prefix path to a location that the nginx user has permissions to, and changing the permissions on some of the startup shell scripts, since those are owned by root.

Note: I tried to fix fluentd to not run as root, but from what I can gather, it needs root permission to read /var/log. Found some info here and here

Partially Fixes VZ-2504

Checklist

As the author of this PR, I have:

  • Checked that I included or updated copyright and license notices in all files that I altered
  • Added or updated unit tests for any new functions I added
  • Added or updated integration tests if appropriate
  • Added or updated acceptance tests if appropriate

Code reviewer, please confirm this PR:

  • Addressed the requirement and meets the acceptance criteria
  • Does not introduce unrelated or spurious changes
  • Does not introduce any unapproved dependency
  • Makes sense and it easy to understand, and/or difficult areas of code are clearly documented so that they can be understood

Copy link
Contributor

@wmhopkins wmhopkins 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 to me, but had a couple questions.

@@ -367,16 +365,16 @@ spec:
items:
- key: startup.sh
path: startup.sh
mode: 0744
mode: 0755
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we adding execute permission for group+world?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when the config map containing these scripts is mounted at /api-config, everything is owned by root. I tried specifying a securityContext section with runAsUser id 101 (the nginx default user www-data) but still everything mounted is owned by root:root. The www-data user is not a member of root group but needs to execute the scripts mounted from the config map.

@@ -342,9 +343,6 @@ spec:
labels:
app: {{ .Values.api.name }}
spec:
securityContext:
Copy link
Contributor

Choose a reason for hiding this comment

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

If we delete the "run as user/group 0", what uid/gid does nginx run as?

Copy link
Contributor Author

@desagar desagar Apr 13, 2021

Choose a reason for hiding this comment

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

It runs by default as user id 101 which is www-data:www-data - here is their Dockerfile

Copy link
Contributor

@wmhopkins wmhopkins left a comment

Choose a reason for hiding this comment

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

lgtm

@desagar desagar merged commit bbcc3b7 into master Apr 14, 2021
@desagar desagar deleted the desagar/VZ-2504 branch April 14, 2021 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants