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

support non-root containers #615

Merged
merged 5 commits into from
Dec 7, 2022

Conversation

styblope
Copy link
Contributor

@styblope styblope commented Dec 6, 2022

Ensure/enable containers to run as non-root

Many of the demo containers currently run as root by default. We should support running the containers under non-root UIDs to align with security best-practices and support running the demo app in restrictive environments (e.g. OpenShift).

There are two steps to reach the goal:

  1. Enable containers to run as non-root user.
    Make sure the containers can execute as non-root. Some of the containers that now launch under root are happy to run also as non-root. There are, however, a couple of containers (emailservice, adservice, frontend-proxy, postgres) which fail to run under a non-root user and need to be fixed.

  2. Ensure the containers run as non-root by default.
    This step actually applies non-root UID to all container deployments. This can either be done at launch-time by setting the user/UID in Compose/Helm, or it can be left up to the orchestrator (e.g. OpenShift UIDs). Some containers require to be run under a specific UID defined in /etc/passwd (e.g. envoy, postgres, apache).

The PR implements the first step by fixing the services that fail to run under non-root.

  • adservice - add read permission to opentelemetry-javaagent.jar. Can run as any non-root UID.
  • emailservice - read/write permission to Gemfile.lock + Dockerfile cleanup. Can run as any non-root UID.
  • frontendproxy - build image with built-in envoy user.
  • quoteservice - configure and run apache under built-in www-data user + Dockerfile Apache envvar substitution and cleanup.
  • ffspostgres - set postgres user in Compose, no image modification.
  • redis - set redis user in Compose, no image modification.

Services with with their default users (after the PR changes) and working non-root alternatives:

container_name default UID non-root UID
ad-service root anyuid
cart-service root anyuid
checkout-service root anyuid
currency-service root anyuid
email-service root anyuid
feature-flag-service nobody (65534) anyuid
frauddetection-service root anyuid
frontend nextjs (1001) anyuid
frontend-proxy root envoy (101) envoy
kafka appuser (1000) appuser
load-generator root anyuid
payment-service node (1000) anyuid
product-catalog-service root anyuid
quote-service root www-data (33) www-data
recommendation-service root anyuid
shipping-service root anyuid
postgres root postgres (999) postgres
jaeger root anyuid
grafana grafana (472) anyuid
otel-col N/A (scratch) N/A
prometheus nobody (65534) nobody
redis-cart root redis (999) redis

anyuid means any non-root UID.
anyuid containers were tested with user nobody uid/guid 65534.

Test commands:

Print container UIDs:

docker ps --format '{{.Names}}' | xargs -I{} -- bash -c 'echo -n "{}: " ; docker exec -t {} id' | sort

Assign nobody user to the root-by-default containers in docker-compose.yml and test-run:

cp docker-compose.yml docker-compose.yml-nobody
for i in ad-service \
         cart-service \
         checkout-service \
         currency-service \
         email-service \
         frauddetection-service \
         load-generator \
         product-catalog-service \
         recommendation-service \
         shipping-service \
         jaeger
do
  sed -i -e "s|\(container_name: $i$\)|\1\n    user: nobody|" docker-compose.yml-nobody
done
docker compose -f docker-compose.yml-nobody up -d

TODO: Helm charts mods including default non-root uid option

Merge Requirements

For new features contributions please make sure you have completed the following
essential items:

  • CHANGELOG.md updated to document new feature additions
  • Appropriate documentation updates in the docs folder

Maintainers will not merge until the above have been completed. If you're unsure
which docs need to be changed ping the
@open-telemetry/demo-approvers.

Modify containers that fail to run as non-root.
@styblope styblope requested a review from a team December 6, 2022 12:02
@puckpuck
Copy link
Contributor

puckpuck commented Dec 7, 2022

Please add a changelog entry for this.

Would you also be able to start a PR to get the Helm chart updated?

@cartersocha cartersocha added the in-progress This issue is actively being worked on label Dec 7, 2022
Signed-off-by: Pierre Tessier <pierre@pierretessier.com>
@puckpuck
Copy link
Contributor

puckpuck commented Dec 7, 2022

I merged in from main, adding the new fraud detection service. Duplicated the same pattern from the ad service to set proper permissions on the Java agent file.

Copy link
Contributor

@puckpuck puckpuck left a comment

Choose a reason for hiding this comment

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

Please set a changelog entry. This works great! 👍

@julianocosta89 julianocosta89 merged commit adad087 into open-telemetry:main Dec 7, 2022
@styblope styblope mentioned this pull request Jan 10, 2023
2 tasks
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
* support non-root containers

Modify containers that fail to run as non-root.

* support non-root containers

Signed-off-by: Pierre Tessier <pierre@pierretessier.com>

* update changelog

Signed-off-by: Pierre Tessier <pierre@pierretessier.com>
Co-authored-by: Pierre Tessier <pierre@pierretessier.com>
Co-authored-by: Juliano Costa <julianocosta89@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-progress This issue is actively being worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants