search

projectcontour / contour-operator

Experimental repository to explore an operator for deploying Contour
Apache License 2.0
43 stars 34 forks source link

Custom SecurityContexts for envoy DaemonSet and contour Deployment #398

Closed dmorgan81 closed 1 year ago

dmorgan81 commented 3 years ago

Allow users to specify a security context for both envoy daemonsets and contour deployments. The default value is unprivileged and is equivalent to the following:

contourSecurityContext:
  runAsUser: 65534
  runAsGroup: 65534
  runAsNonRoot: true

Note that this does change the default behavior for envoy daemonsets. Currently an envoy daemonset has an empty security context, meaning it will run with the user/group specified in the envoy image, which is commonly root. Switching to a non-root user by default will prevent the envoy container from binding to a low port. Existing setups will need to be updated by adding a envoySecurityContext entry with the appropriate settings if they need to bind to a low port.

Fixes #112 Updates #378

codecov[bot] commented 3 years ago

Codecov Report

Merging #398 (62e48a2) into main (7b041eb) will decrease coverage by 20.47%. The diff coverage is 100.00%.

:exclamation: Current head 62e48a2 differs from pull request most recent head 263c7c4. Consider uploading reports for the commit 263c7c4 to get more accurate results Impacted file tree graph

@@             Coverage Diff             @@
##             main     #398       +/-   ##
===========================================
- Coverage   79.82%   59.34%   -20.48%     
===========================================
  Files          29       19       -10     
  Lines        2235     2071      -164     
===========================================
- Hits         1784     1229      -555     
- Misses        331      815      +484     
+ Partials      120       27       -93
Impacted Files Coverage Δ
internal/objects/daemonset/daemonset.go 83.33% <100.00%> (-12.39%) :arrow_down:
internal/objects/deployment/deployment.go 81.17% <100.00%> (-12.90%) :arrow_down:
internal/objects/namespace/namespace.go 15.00% <0.00%> (-50.00%) :arrow_down:
internal/objects/role/role.go 36.36% <0.00%> (-43.64%) :arrow_down:
internal/objects/configmap/configmap.go 34.14% <0.00%> (-43.07%) :arrow_down:
internal/objects/rolebinding/role_binding.go 41.07% <0.00%> (-39.29%) :arrow_down:
...objects/clusterrolebinding/cluster_role_binding.go 41.50% <0.00%> (-37.74%) :arrow_down:
internal/objects/job/job.go 53.04% <0.00%> (-28.70%) :arrow_down:
internal/objects/service/service.go 60.00% <0.00%> (-28.70%) :arrow_down:
pkg/labels/labels.go 75.00% <0.00%> (-25.00%) :arrow_down:
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7b041eb...263c7c4. Read the comment docs.

dmorgan81 commented 3 years ago

I've been trying to run the e2e tests locally but I'm getting inconsistent results, even on the main branch:

I just added a change to the e2e tests to use an empty PodSecurityContext for the contour pods since port 80 appears to be in the mix. However, I'm unable to get through the entire e2e test suite because of the above two issues.

sunjayBhatia commented 3 years ago

I've been trying to run the e2e tests locally but I'm getting inconsistent results, even on the main branch:

  • TestContourNodePortService often hangs.
  • TestGateway and TestGatewayOwnership sometimes fail. The contour pods associated with these tests are stuck in a crash loop with the only logging being contour: error: invalid Contour configuration: invalid Gateway parameters specified: controllerName required, try --help

I just added a change to the e2e tests to use an empty PodSecurityContext for the contour pods since port 80 appears to be in the mix. However, I'm unable to get through the entire e2e test suite because of the above two issues.

ah, this is probably not related to your change at all, i think this is related to this change in Contour being merged: https://github.com/projectcontour/contour/commit/9e98a7284e680a3f71c14d35195588175e7605a6

We need to fix this up on main and then you should be able to merge main and your change go green

sunjayBhatia commented 3 years ago

@dmorgan81 I think this PR might be placed on a lower priority to merge at the moment since the Operator/Contour relationship is going through a rework as part of our Gateway API support work. Apologies for that, but we will try to get to this soon!

arjunrn commented 2 years ago

@dmorgan81 The jobs created by the operator will also have to use the custom security context. For reference.

skriss commented 1 year ago

This is stale, closing out.