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

NET-5186 Allow dataplane container to bind to privileged ports #238

Merged
merged 3 commits into from
Aug 24, 2023

Conversation

nathancoleman
Copy link
Member

@nathancoleman nathancoleman commented Aug 11, 2023

Describe the issue

Consul-dataplane is currently unable to bind to privileged ports (< 1024).

This is important for ingress-gateway use cases where customers have historically been able to bind to ports such as 443 and are encountering runtime failures when attempting to upgrade to Consul 1.15+ and the corresponding Helm chart versions. In these newer versions, consul-dataplane has taken the place of the envoyproxy/envoy containers that were used previously.

Example of failure:

[warning] envoy.config(13) delta config for type.googleapis.com/envoy.config.listener.v3.Listener rejected: Error adding/updating listener(s) http:0.0.0.0:443: cannot bind '0.0.0.0:443': Permission denied

Describe the fix

It appears that Envoy containers, which consul-dataplane has replaced for ingress-gateway use cases, run as root and then use su-exec to run as a different user. I'm thinking that we can set the NET_BIND_SERVICE capability directly on the Envoy and dataplane binaries and avoid starting up as root, but I'm depending on my own testing and reviewers here to validate this.

This PR adds a new stage to set the net_bind_service capability on the Envoy and dataplane binaries that are copied into the release image. The final images then copy their Envoy and dataplane binaries from this new stage instead of their previous source.

How to test

  • Create an ingress-gateway that binds to a privileged port, such as 443 (see example values below)
    • Before this change, the consul-dataplane container will appear healthy but spew logs containing the error below
    • After this change, the consul-dataplane container will appear healthy, and the logs will indicate successful xDS config
  • Do the same testing on OpenShift to verify no negative impact
Example `values.yaml`
global:
  name: consul
  logLevel: debug

connectInject:
  replicas: 1
  enabled: true

server:
  replicas: 3

ingressGateways:
  enabled: true
  gateways:
    - name: my-ingress
      service:
        type: LoadBalancer
        ports:
          - port: 443

@nathancoleman nathancoleman changed the base branch from main to release/1.1.x August 11, 2023 19:08
@nathancoleman nathancoleman changed the title Allow dataplane container to bind to privileged ports NET-5186 Allow dataplane container to bind to privileged ports Aug 11, 2023
@@ -22,7 +36,8 @@ RUN apk add dumb-init
# -----------------------------------
FROM gcr.io/distroless/base-debian11 AS release-default

ARG BIN_NAME
ARG BIN_NAME=consul-dataplane
Copy link
Member Author

Choose a reason for hiding this comment

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

Defaulting these for building locally like we do in consul-k8s

@nathancoleman nathancoleman changed the base branch from release/1.1.x to main August 21, 2023 17:19
@nathancoleman nathancoleman added backport/1.0 backport/1.1 Changes are backported to 1.1 labels Aug 21, 2023
@nathancoleman nathancoleman marked this pull request as ready for review August 21, 2023 17:26
@nathancoleman nathancoleman requested a review from a team as a code owner August 21, 2023 17:26
@nathancoleman nathancoleman requested review from curtbushko, DanStough, zalimeni and wilkermichael and removed request for a team and DanStough August 21, 2023 18:00
Copy link
Contributor

@curtbushko curtbushko left a comment

Choose a reason for hiding this comment

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

Thanks Nathan!

For other reviewers: we had hours upon hours of back and forth discussions on how to do this and this 'setcap' method was the best of all horrible ways to solve this problem.

We couldn't setcap at runtime since we are running a distroless image (no shell to run the setcap command...).

nathancoleman added a commit that referenced this pull request Aug 25, 2023
NET-5186 Allow dataplane container to bind to privileged ports
@nathancoleman nathancoleman added backport/1.0 backport/1.1 Changes are backported to 1.1 and removed backport/1.0 backport/1.1 Changes are backported to 1.1 labels Aug 25, 2023
nathancoleman added a commit that referenced this pull request Aug 25, 2023
NET-5186 Allow dataplane container to bind to privileged ports
hc-github-team-consul-core added a commit that referenced this pull request Aug 25, 2023
…ports into release/1.0.x (#248)

Merge pull request #238 from hashicorp/distroless-capabilities

NET-5186 Allow dataplane container to bind to privileged ports

Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com>
nathancoleman added a commit that referenced this pull request Aug 25, 2023
NET-5186 Allow dataplane container to bind to privileged ports
hc-github-team-consul-core added a commit that referenced this pull request Aug 25, 2023
…ports into release/1.2.x (#247)

Merge pull request #238 from hashicorp/distroless-capabilities

NET-5186 Allow dataplane container to bind to privileged ports

Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.1 Changes are backported to 1.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants