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

Set runAsNonRoot to true on Kourier Gateway #902

Closed
wants to merge 1 commit into from
Closed

Set runAsNonRoot to true on Kourier Gateway #902

wants to merge 1 commit into from

Conversation

nak3
Copy link
Contributor

@nak3 nak3 commented Aug 23, 2022

This patch sets runAsNonRoot to false on Kourier Gateway.

As per #272, the setting was necessary as:

  • the envoy image wants to run as root.
  • the envoy needs write access to the filesystem.

For write access, I could produce the error if I changed readOnlyRootFilesystem to false.

[2022-08-22 07:08:29.879][1][critical][main] [external/envoy/source/server/server.cc:108] error initializing configuration '/tmp/config/envoy-bootstrap.yaml': cannot bind '/tmp/envoy.admin': Read-only file system
[2022-08-22 07:08:29.879][1][info][main] [external/envoy/source/server/server.cc:790] exiting

However, runAsNonRoot could be false without any isssue on OpenShift.
openshift-knative/serverless-operator#1677 verified.

Fix #274

@knative-prow
Copy link

knative-prow bot commented Aug 23, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nak3

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 23, 2022
@nak3
Copy link
Contributor Author

nak3 commented Aug 23, 2022

/hold

I think we should not merge at the release day. (Today is the release day.)

@knative-prow knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 23, 2022
@codecov
Copy link

codecov bot commented Aug 23, 2022

Codecov Report

Merging #902 (801378d) into main (f615357) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #902   +/-   ##
=======================================
  Coverage   80.77%   80.77%           
=======================================
  Files          18       18           
  Lines        1233     1233           
=======================================
  Hits          996      996           
  Misses        190      190           
  Partials       47       47           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@knative-prow
Copy link

knative-prow bot commented Aug 23, 2022

@nak3: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
integration-tests_net-kourier_main 801378d link true /test integration-tests_net-kourier_main

Your PR dashboard.

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. I understand the commands that are listed here.

@nak3 nak3 changed the title Set runAsNonRoot to false on Kourier Gateway Set runAsNonRoot to true on Kourier Gateway Aug 23, 2022
@nak3
Copy link
Contributor Author

nak3 commented Aug 23, 2022

Ah, I see. runAsNonRoot: true could work on OpenShift but does not work with upstream envoy image.

  Normal   Scheduled  4m6s                  default-scheduler  Successfully assigned kourier-system/3scale-kourier-gateway-8499649b5b-slm86 to kind-worker
  Warning  Failed     118s (x12 over 4m6s)  kubelet            Error: container has runAsNonRoot and image will run as root (pod: "3scale-kourier-gateway-8499649b5b-slm86_kourier-system(d3c4788b-5e98-4676-a04d-467d86413a1a)", container: kourier-gateway)
  Normal   Pulled     104s (x13 over 4m6s)  kubelet            Container image "docker.io/envoyproxy/envoy:v1.20-latest" already present on machine

@nak3 nak3 closed this Aug 23, 2022
@skonto
Copy link
Contributor

skonto commented Sep 6, 2022

@nak3 You could use a specific user like nobody I guess as in projectcontour/contour#4084?

@nak3
Copy link
Contributor Author

nak3 commented Sep 6, 2022

No, because of OpenShift's feature.

@psschwei psschwei mentioned this pull request Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve security constraints of the envoy image
2 participants