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

[Experimental] Include both privileged and non-privileged binaries in Docker image #652

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nathancoleman
Copy link
Member

@nathancoleman nathancoleman commented Oct 17, 2024

Currently, the NET_BIND_SERVICE capability is added to the dataplane (and Envoy) entrypoints since we cannot know at build time whether a non-root user at runtime will attempt to bind to a privileged port. The reason this is the default behavior was to avoid a breaking change in ingress gateways, which had previously supported binding to privileged ports; however, there are still many use cases where it's preferable to avoid this capability when it isn't needed.

This change introduces a second set of entrypoints to the container that do not have the NET_BIND_SERVICE capability. In this way, we can support both use cases:

  • Ingress gateways, which have historically supported binding to privileged ports
  • All other use cases (sidecar and API, mesh, terminating gateways) which do not need to support binding to privileged ports

The container will default to the non-privileged entrypoint, and ingress gateway deployments that do want to support binding to privileged ports will need to override the entrypoint to privileged-consul-dataplane and provide a path to privileged-envoy. This way, no deployment needs the NET_BIND_SERVICE capability unless they are overriding the entrypoint in order to support binding to privileged ports.

See hashicorp/consul-k8s#4394 for an example of how this works in consul-k8s.

Warning

We still need to determine the impact – if any – of a change like this for VM and Nomad deployments of consul-dataplane

This way ingress-gateway can use the privileged entrypoint without everyone else having to add the NET_BIND_SERVICE capability when they don't need/want it
@jm96441n
Copy link
Member

this shouldn't affect nomad/VM deployments as those don't use dataplane

@@ -100,6 +100,7 @@ func init() {
IntVar(flags, &flagOpts.dataplaneConfig.Envoy.Concurrency, "envoy-concurrency", "DP_ENVOY_CONCURRENCY", "The number of worker threads that Envoy uses.")
IntVar(flags, &flagOpts.dataplaneConfig.Envoy.DrainTimeSeconds, "envoy-drain-time-seconds", "DP_ENVOY_DRAIN_TIME", "The time in seconds for which Envoy will drain connections.")
StringVar(flags, &flagOpts.dataplaneConfig.Envoy.DrainStrategy, "envoy-drain-strategy", "DP_ENVOY_DRAIN_STRATEGY", "The behaviour of Envoy during the drain sequence. Determines whether all open connections should be encouraged to drain immediately or to increase the percentage gradually as the drain time elapses.")
StringVar(flags, &flagOpts.dataplaneConfig.Envoy.ExecutablePath, "envoy-executable-path", "DP_ENVOY_EXECUTABLE_PATH", "Path to the Envoy executable to run. Defaults to the ")
Copy link
Member

Choose a reason for hiding this comment

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

~ Looks like this needs a bit more text

Copy link
Member

Choose a reason for hiding this comment

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

good catch!

@zalimeni
Copy link
Member

This seems like a great idea! Excited to see it happen. Hoping/expecting the constraints around consul-dataplane are acceptable.

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.

3 participants