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.

Sign up for GitHub

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

Jump to bottom

engine: support for system proxy settings #7255

Merged
merged 8 commits into from
May 7, 2024
Merged

engine: support for system proxy settings #7255

merged 8 commits into from
May 7, 2024

Commits on May 7, 2024

  1. engine: support for system proxy settings 

    Signed-off-by: Erik Sipsma <[email protected]>
    @sipsma @jedevc
    sipsma authored and jedevc committed May 7, 2024
    Configuration menu
    Copy the full SHA
    13e453e View commit details
    Browse the repository at this point in the history

  2. networking: remove no-hosts from dnsmasq conf 

    This setting meant that containers could not resolve names that were
resolvable in the engine (even if direct IPs were still usable).

While supporting that is incredibly obscure, the need has arisen in
integ tests for proxy support, where the test setup has:
* a proxy service
* a nested engine with dep on the proxy service
* containers running in that nested engine that automatically get proxy
  settings from their engine

In this case, being able to use the service binding alias for squid,
which is set in the engine's /etc/hosts, massively simplifies TLS
testing since the proxy's certs can all reference just that alias.

From poking around, it does not seem that this setting was load-bearing
for us and was just inherited from the original `dnsname` configuration
we picked up (and in turn `dnsname` had just set that from the beginning
with no context).

Looking around online for use cases that call for this setting in
dnsmasq, it seems to generally be obscure/complex networking setups
where entries in /etc/hosts end up pointing to IPs local to a
container/machine but they want nested clients to point to a different
IP.

My imagination is limited, but I cannot currently think of any situation
in which this would apply to engine containers; the networking settings
of the engine itself should dictate the networking settings of the
containers so any aliases found in the engine's /etc/hosts can apply to
resolving names queried by containers.
* To be clear: this does *not* result in containers' /etc/hosts having
  different values from before. It just makes all names resolvable by
  the engine also resolvable by containers, which is consistent with the
  behavior of DNS names outside of aliases.

Signed-off-by: Erik Sipsma <[email protected]>
    @sipsma @jedevc
    sipsma authored and jedevc committed May 7, 2024
    Configuration menu
    Copy the full SHA
    f9bd147 View commit details
    Browse the repository at this point in the history

  3. cleanup integ tests 

    The previous commit that tweaked dnsmasq conf enables just the squid
alias to be used for the proxy endpoint, which massively simplifies the
rest of the test.

Signed-off-by: Erik Sipsma <[email protected]>
    @sipsma @jedevc
    sipsma authored and jedevc committed May 7, 2024
    Configuration menu
    Copy the full SHA
    edfb19f View commit details
    Browse the repository at this point in the history

  4. add integ tests that use basic auth for proxy 

    Signed-off-by: Erik Sipsma <[email protected]>
    @sipsma @jedevc
    sipsma authored and jedevc committed May 7, 2024
    Configuration menu
    Copy the full SHA
    877f2fa View commit details
    Browse the repository at this point in the history

  5. add integ tests for NO_PROXY 

    Signed-off-by: Erik Sipsma <[email protected]>
    @sipsma @jedevc
    sipsma authored and jedevc committed May 7, 2024
    Configuration menu
    Copy the full SHA
    b386a34 View commit details
    Browse the repository at this point in the history

  6. add support for GOPROXY setting on the Go SDK 

    This is implemented in such a way that it's a bit more generic but
hidden from external clients for now.

The overall plumbing is:
1. A hidden (via `__` prefix) API on Container for specifying env vars
   that should be inherited from the engine container, where they must
   be prefixed with `_DAGGER_ENGINE_SYSTEMENV_` (e.g.
   `_DAGGER_ENGINE_SYSTEMENV_GOPROXY`).
2. The Container implementation sets metadata that the Executor needs
   via LLB *metadata*. Importantly, this is very different from the
   previous `ftp_proxy` hack in that this is just metadata and thus
   doesn't impact LLB digests. We specifically use the `Description` LLB
   metadata, which is explicitly doc'd as never being interpreted by the
   solver and thus safe to use for this.
3. The Worker's ResolveOp method registers the solver.Vertex (which
   includes that metadata) so that it can be read by the Executor as
   needed.
4. The Executor uses that metadata to set any requested env vars from
   the system.

Once we've fleshed out the design of support for these system env vars
we can open up the API.

This plumbing of executorMetadata is also intended to be easy to re-use
for any other needs that arise in the future similar to this.

Integ tests are also included here with a pretty nice GOPROXY
implementation usable a library.

Signed-off-by: Erik Sipsma <[email protected]>
    @sipsma @jedevc
    sipsma authored and jedevc committed May 7, 2024
    Configuration menu
    Copy the full SHA
    ffae188 View commit details
    Browse the repository at this point in the history

  7. chore: tidy go.mod 

    Signed-off-by: Justin Chadwell <[email protected]>
    @jedevc
    jedevc committed May 7, 2024
    Configuration menu
    Copy the full SHA
    406fdd7 View commit details
    Browse the repository at this point in the history

  8. add clarifying comments to proxy env setting code 

    Signed-off-by: Erik Sipsma <[email protected]>
    @sipsma
    sipsma committed May 7, 2024
    Configuration menu
    Copy the full SHA
    61a2dae View commit details
    Browse the repository at this point in the history