From b2878907cc522ed7e3ddeec510ab239793b33099 Mon Sep 17 00:00:00 2001 From: MohammadHasan Akbari Date: Tue, 14 Jul 2026 09:46:00 +0400 Subject: [PATCH] remote: set grpc authority as a default option Add the ":authority" dial option to the default client options instead of appending it after the caller-provided options, so that an authority explicitly passed by the caller takes precedence over the driver default. Signed-off-by: MohammadHasan Akbari --- driver/remote/driver.go | 14 +++---- driver/remote/driver_test.go | 73 ++++++++++++++++++++++++++---------- 2 files changed, 61 insertions(+), 26 deletions(-) diff --git a/driver/remote/driver.go b/driver/remote/driver.go index 87aac15e8..62ed7646f 100644 --- a/driver/remote/driver.go +++ b/driver/remote/driver.go @@ -89,22 +89,22 @@ func (d *Driver) Rm(ctx context.Context, force, rmVolume, rmDaemon bool) error { func (d *Driver) Client(ctx context.Context, opts ...client.ClientOpt) (*client.Client, error) { d.clientOnce.Do(func() { - opts = append([]client.ClientOpt{ + defaultOpts := []client.ClientOpt{ client.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { return d.Dial(ctx) }), client.WithTracerDelegate(delegated.DefaultExporter), - }, opts...) + } // The remote driver establishes the connection itself through a custom // dialer (including TLS), so the buildkit client cannot derive the gRPC // ":authority" pseudo-header from the connection and would fall back to - // "localhost". Set it explicitly from the configured endpoint so HTTP/2 - // reverse proxies (e.g. Envoy) can route on it. Passing the endpoint - // address also keeps the gRPC dial target meaningful; the actual dial - // target is unaffected as it still goes through the dialer above. + // "localhost". Set it explicitly so HTTP/2 reverse proxies (e.g. Envoy) + // can route on it. It is added as a default option so an authority + // explicitly passed by the caller still takes precedence. if authority := d.clientAuthority(); authority != "" { - opts = append(opts, client.WithGRPCDialOption(grpc.WithAuthority(authority))) + defaultOpts = append(defaultOpts, client.WithGRPCDialOption(grpc.WithAuthority(authority))) } + opts = append(defaultOpts, opts...) c, err := client.New(ctx, d.EndpointAddr, opts...) d.client = c d.err = err diff --git a/driver/remote/driver_test.go b/driver/remote/driver_test.go index 7e54bae87..4679bf4db 100644 --- a/driver/remote/driver_test.go +++ b/driver/remote/driver_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/docker/buildx/driver" + "github.com/moby/buildkit/client" "github.com/stretchr/testify/require" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -54,17 +55,59 @@ func TestClientAuthorityValue(t *testing.T) { // TestClientAuthority verifies end-to-end that the remote driver sends the // configured endpoint address as the gRPC ":authority" pseudo-header instead -// of defaulting to "localhost" (see docker/buildx#3880). It stands up an -// in-process gRPC server on a loopback listener and asserts the authority of -// the request it receives matches the endpoint host. +// of defaulting to "localhost" (see docker/buildx#3880). func TestClientAuthority(t *testing.T) { ctx, cancel := context.WithTimeoutCause(context.Background(), 10*time.Second, context.DeadlineExceeded) defer cancel() + addr, authorityCh := startAuthorityServer(ctx, t) + + d := &Driver{ + InitConfig: driver.InitConfig{EndpointAddr: "tcp://" + addr}, + } + + c, err := d.Client(ctx) + require.NoError(t, err) + defer c.Close() + + // Any RPC will do: it fails server-side with Unimplemented, but the server + // records the ":authority" it received from the client before responding. + _, _ = c.ListWorkers(ctx) + + require.Equal(t, addr, waitAuthority(ctx, t, authorityCh)) +} + +// TestClientAuthorityCallerOverride verifies that an authority explicitly +// passed by the caller takes precedence over the driver's default one. +func TestClientAuthorityCallerOverride(t *testing.T) { + ctx, cancel := context.WithTimeoutCause(context.Background(), 10*time.Second, context.DeadlineExceeded) + defer cancel() + + addr, authorityCh := startAuthorityServer(ctx, t) + + d := &Driver{ + InitConfig: driver.InitConfig{EndpointAddr: "tcp://" + addr}, + } + + c, err := d.Client(ctx, client.WithGRPCDialOption(grpc.WithAuthority("caller.example.com"))) + require.NoError(t, err) + defer c.Close() + + _, _ = c.ListWorkers(ctx) + + require.Equal(t, "caller.example.com", waitAuthority(ctx, t, authorityCh)) +} + +// startAuthorityServer stands up an in-process gRPC server on a loopback +// listener that records the ":authority" pseudo-header of the request it +// receives. It returns the listener address and a channel delivering that +// authority. +func startAuthorityServer(ctx context.Context, t *testing.T) (string, <-chan string) { + t.Helper() + lc := net.ListenConfig{} lis, err := lc.Listen(ctx, "tcp", "127.0.0.1:0") require.NoError(t, err) - defer lis.Close() authorityCh := make(chan string, 1) srv := grpc.NewServer(grpc.UnknownServiceHandler(func(_ any, stream grpc.ServerStream) error { @@ -83,26 +126,18 @@ func TestClientAuthority(t *testing.T) { go func() { _ = srv.Serve(lis) }() - defer srv.Stop() + t.Cleanup(srv.Stop) - d := &Driver{ - InitConfig: driver.InitConfig{ - EndpointAddr: "tcp://" + lis.Addr().String(), - }, - } - - c, err := d.Client(ctx) - require.NoError(t, err) - defer c.Close() - - // Any RPC will do: it fails server-side with Unimplemented, but the server - // records the ":authority" it received from the client before responding. - _, _ = c.ListWorkers(ctx) + return lis.Addr().String(), authorityCh +} +func waitAuthority(ctx context.Context, t *testing.T, authorityCh <-chan string) string { + t.Helper() select { case authority := <-authorityCh: - require.Equal(t, lis.Addr().String(), authority) + return authority case <-ctx.Done(): t.Fatal("timed out waiting for request to reach the server") + return "" } }