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

Unroot docker examples and fix stdout permissions in container #11523

Merged
merged 18 commits into from
Jun 28, 2020

Conversation

phlax
Copy link
Member

@phlax phlax commented Jun 9, 2020

Commit Message: Update examples to use non-well known ports and not the root user
Additional Description:
I have updated 80 -> 8000 for any ports I could see that were exposed by an envoy container.

I left one example (load-reporting-service) using port 80 and instead added the ENVOY_UID env var

Some docs (eg docs/root/start/sandboxes) that have port 80 coded in relation to these examples, have been updated

Also adds fix for envoy permissions on /dev/stdout and /dev/stderr

Risk Level: Low
Testing: Manual testing
Docs Changes:

  • Sandbox examples updated to reflect port changes
  • Note added about uid/gid of container user

Release Notes: N/A
Fix #11506
Fix #11551

Copy link
Member

@junr03 junr03 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, do you mind updating any stale docs? Thanks!

/wait

@phlax
Copy link
Member Author

phlax commented Jun 13, 2020

@junr03 i have updated the relevant docs - not sure if there are others - but these seem to be the ones that directly referenced the port 80 settings.

I have again committed separately - but will happily squash

@phlax
Copy link
Member Author

phlax commented Jun 13, 2020

also, im wondering if i should add a doc about the ENVOY_UID setting. Adding to the existing load_reporting_service example seems a bit arbitrary

@junr03
Copy link
Member

junr03 commented Jun 17, 2020

also, im wondering if i should add a doc about the ENVOY_UID setting.

Yes, please.

/wait

@phlax
Copy link
Member Author

phlax commented Jun 18, 2020

documentation added

@lu4t
Copy link

lu4t commented Jun 22, 2020

Adding a comment and link to this issue:

#11679

as @dio requested....

@phlax
Copy link
Member Author

phlax commented Jun 22, 2020

@lu4t @dio - i updated the docs to use py3-pip - but then i saw #11680 so i can remove this commit in favour of that pr if required

commit removed

@junr03
Copy link
Member

junr03 commented Jun 26, 2020

@lu4t @dio does this look good to both of you?

@dio
Copy link
Member

dio commented Jun 27, 2020

Looks good, just a question: @phlax, do you want to tackle the access issue to /dev/stdout here as well (since it is related to privileged access too)? Thank you!

phlax added 10 commits June 27, 2020 07:34
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
Copy link
Member Author

phlax commented Jun 27, 2020

@dio i added a fix for permissions on /dev/stdout.

I also added it for /dev/stderr on the basis that a config may well use that pipe too.

I have tested the built image with the fault-injection example and this seems to work ok.

I wasn't entirely clear where the envoy-dev image is built - but im assuming its based on the recipe in ci/Dockerfile-envoy

@phlax phlax changed the title Unroot docker examples Unroot docker examples and fix stdout permissions in container Jun 27, 2020
@dio
Copy link
Member

dio commented Jun 27, 2020

I wasn't entirely clear where the envoy-dev image is built - but im assuming its based on the recipe in ci/Dockerfile-envoy

Yes, you're right.

Copy link
Member

@dio dio left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM. Just an ask to add a comment on the decision of making the envoy user owns stdout and stderr (probably since it is a common use case when we have access logging) and a tiny nit to make sure the doc is rendered as intended.

ci/docker-entrypoint.sh Show resolved Hide resolved
Comment on lines 151 to 161
By default the Docker image will run as the `envoy` user created at build time.

The `uid` and `gid` of this user can be set at runtime using the `ENVOY_UID` and `ENVOY_GID`
environment variables. This can be done, for example, on the Docker command line:

$ docker run -d --name envoy -e ENVOY_UID=777 -e ENVOY_GID=777 -p 9901:9901 -p 10000:10000 envoy:v1

This can be useful if you wish to restrict or provide access to `unix` sockets inside the container, or
for controlling access to an `envoy` socket from outside of the container.

If you wish to run the container as the `root` user you can set `ENVOY_UID` to `0`.
Copy link
Member

@dio dio Jun 27, 2020

Choose a reason for hiding this comment

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

Nit. To use double backticks instead of the single one, e.g. for uid and gid, and other words that need to be rendered inside <span class="pre"></span> tag. For example:

``docker``

gives:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
Copy link
Member

@dio dio left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Nice!

@mattklein123 mattklein123 merged commit 504d78b into envoyproxy:master Jun 28, 2020
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.

Getting permission denied on latest dev image for access logs examples/front-proxy tutorial is broken
5 participants