From cda45ada825eaa3def5740f87165fb819870bc55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=B6hl=2C=20Lukas?= Date: Tue, 8 Aug 2023 16:18:24 +0200 Subject: [PATCH] fix: use default dialer for client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uses the default grpc dialer to allow [proxy configuration](https://github.com/grpc/grpc-go/blob/v1.57.0/Documentation/proxy.md) Signed-off-by: Höhl, Lukas --- client/client.go | 23 +++++++++++------ client/client_test.go | 31 +++++++++++++++-------- client/client_unix.go | 21 --------------- client/client_windows.go | 25 ------------------ client/connhelper/npipe/npipe.go | 8 ++++++ client/connhelper/npipe/npipe_other.go | 14 ++++++++++ client/connhelper/npipe/npipe_windows.go | 28 ++++++++++++++++++++ cmd/buildctl/main.go | 1 + util/testutil/integration/util_windows.go | 3 +++ 9 files changed, 89 insertions(+), 65 deletions(-) delete mode 100644 client/client_unix.go delete mode 100644 client/client_windows.go create mode 100644 client/connhelper/npipe/npipe.go create mode 100644 client/connhelper/npipe/npipe_other.go create mode 100644 client/connhelper/npipe/npipe_windows.go diff --git a/client/client.go b/client/client.go index 26c91c374..414e156dd 100644 --- a/client/client.go +++ b/client/client.go @@ -101,7 +101,7 @@ func New(ctx context.Context, address string, opts ...ClientOpt) (*Client, error } if tracerProvider != nil { - var propagators = propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{}) + propagators := propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{}) unary = append(unary, filterInterceptor(otelgrpc.UnaryClientInterceptor(otelgrpc.WithTracerProvider(tracerProvider), otelgrpc.WithPropagators(propagators)))) //nolint:staticcheck // TODO(thaJeztah): ignore SA1019 for deprecated options: see https://github.com/moby/buildkit/issues/4681 stream = append(stream, otelgrpc.StreamClientInterceptor(otelgrpc.WithTracerProvider(tracerProvider), otelgrpc.WithPropagators(propagators))) //nolint:staticcheck // TODO(thaJeztah): ignore SA1019 for deprecated options: see https://github.com/moby/buildkit/issues/4681 } @@ -111,11 +111,17 @@ func New(ctx context.Context, address string, opts ...ClientOpt) (*Client, error if err != nil { return nil, err } - gopts = append(gopts, grpc.WithContextDialer(dialFn)) + if dialFn != nil { + gopts = append(gopts, grpc.WithContextDialer(dialFn)) + } } if address == "" { address = appdefaults.Address } + uri, err := url.Parse(address) + if err != nil { + return nil, err + } // Setting :authority pseudo header // - HTTP/2 (RFC7540) defines :authority pseudo header includes @@ -130,12 +136,14 @@ func New(ctx context.Context, address string, opts ...ClientOpt) (*Client, error } if authority == "" { // authority as hostname from target address - uri, err := url.Parse(address) - if err != nil { - return nil, err - } authority = uri.Host } + if uri.Scheme == "tcp" { + // remove tcp scheme from address, since default dialer doesn't expect that + // name resolution is done by grpc according to the following spec: https://github.com/grpc/grpc/blob/master/doc/naming.md + address = uri.Host + } + gopts = append(gopts, grpc.WithAuthority(authority)) unary = append(unary, grpcerrors.UnaryClientInterceptor) @@ -375,8 +383,7 @@ func resolveDialer(address string) (func(context.Context, string) (net.Conn, err if ch != nil { return ch.ContextDialer, nil } - // basic dialer - return dialer, nil + return nil, nil } func filterInterceptor(intercept grpc.UnaryClientInterceptor) grpc.UnaryClientInterceptor { diff --git a/client/client_test.go b/client/client_test.go index a926ceab2..63ad1694d 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -3912,7 +3912,7 @@ func testBuildExportWithUncompressed(t *testing.T, sb integration.Sandbox) { dt, err := content.ReadBlob(ctx, img.ContentStore(), img.Target()) require.NoError(t, err) - var mfst = struct { + mfst := struct { MediaType string `json:"mediaType,omitempty"` ocispecs.Manifest }{} @@ -4181,6 +4181,7 @@ func testPullZstdImage(t *testing.T, sb integration.Sandbox) { }) } } + func testBuildPushAndValidate(t *testing.T, sb integration.Sandbox) { workers.CheckFeatureCompat(t, sb, workers.FeatureDirectPush) requiresLinux(t) @@ -4319,7 +4320,7 @@ func testBuildPushAndValidate(t *testing.T, sb integration.Sandbox) { dt, err = content.ReadBlob(ctx, img.ContentStore(), img.Target()) require.NoError(t, err) - var mfst = struct { + mfst := struct { MediaType string `json:"mediaType,omitempty"` ocispecs.Manifest }{} @@ -5397,7 +5398,8 @@ func testBasicCacheImportExport(t *testing.T, sb integration.Sandbox, cacheOptio { Type: ExporterLocal, OutputDir: destDir, - }}, + }, + }, CacheImports: cacheOptionsEntryImport, }, nil) require.NoError(t, err) @@ -5860,10 +5862,12 @@ func testMultipleRecordsWithSameLayersCacheImportExport(t *testing.T, sb integra base := llb.Image("busybox:latest") // layerA and layerB create identical layers with different LLB - layerA := base.Run(llb.Args([]string{"sh", "-c", + layerA := base.Run(llb.Args([]string{ + "sh", "-c", `echo $(( 1 + 2 )) > /result && touch -d "1970-01-01 00:00:00" /result`, })).Root() - layerB := base.Run(llb.Args([]string{"sh", "-c", + layerB := base.Run(llb.Args([]string{ + "sh", "-c", `echo $(( 2 + 1 )) > /result && touch -d "1970-01-01 00:00:00" /result`, })).Root() @@ -9757,7 +9761,8 @@ func testMountStubsTimestamp(t *testing.T, sb integration.Sandbox) { const sourceDateEpoch = int64(1234567890) // Fri Feb 13 11:31:30 PM UTC 2009 st := llb.Image("busybox:latest").Run( - llb.Args([]string{"/bin/touch", fmt.Sprintf("--date=@%d", sourceDateEpoch), + llb.Args([]string{ + "/bin/touch", fmt.Sprintf("--date=@%d", sourceDateEpoch), "/bin", "/etc", "/var", @@ -9928,8 +9933,10 @@ func (*secModeInsecure) UpdateConfigFile(in string) string { return in + "\n\ninsecure-entitlements = [\"security.insecure\"]\n" } -var securitySandbox integration.ConfigUpdater = &secModeSandbox{} -var securityInsecure integration.ConfigUpdater = &secModeInsecure{} +var ( + securitySandbox integration.ConfigUpdater = &secModeSandbox{} + securityInsecure integration.ConfigUpdater = &secModeInsecure{} +) type netModeHost struct{} @@ -9961,9 +9968,11 @@ nameservers = ["10.11.0.1"] ` } -var hostNetwork integration.ConfigUpdater = &netModeHost{} -var defaultNetwork integration.ConfigUpdater = &netModeDefault{} -var bridgeDNSNetwork integration.ConfigUpdater = &netModeBridgeDNS{} +var ( + hostNetwork integration.ConfigUpdater = &netModeHost{} + defaultNetwork integration.ConfigUpdater = &netModeDefault{} + bridgeDNSNetwork integration.ConfigUpdater = &netModeBridgeDNS{} +) func fixedWriteCloser(wc io.WriteCloser) filesync.FileOutputFunc { return func(map[string]string) (io.WriteCloser, error) { diff --git a/client/client_unix.go b/client/client_unix.go deleted file mode 100644 index dc55a4b6e..000000000 --- a/client/client_unix.go +++ /dev/null @@ -1,21 +0,0 @@ -//go:build !windows -// +build !windows - -package client - -import ( - "context" - "net" - "strings" - - "github.com/pkg/errors" -) - -func dialer(ctx context.Context, address string) (net.Conn, error) { - addrParts := strings.SplitN(address, "://", 2) - if len(addrParts) != 2 { - return nil, errors.Errorf("invalid address %s", address) - } - var d net.Dialer - return d.DialContext(ctx, addrParts[0], addrParts[1]) -} diff --git a/client/client_windows.go b/client/client_windows.go deleted file mode 100644 index a9eb87f24..000000000 --- a/client/client_windows.go +++ /dev/null @@ -1,25 +0,0 @@ -package client - -import ( - "context" - "net" - "strings" - - winio "github.com/Microsoft/go-winio" - "github.com/pkg/errors" -) - -func dialer(ctx context.Context, address string) (net.Conn, error) { - addrParts := strings.SplitN(address, "://", 2) - if len(addrParts) != 2 { - return nil, errors.Errorf("invalid address %s", address) - } - switch addrParts[0] { - case "npipe": - address = strings.Replace(addrParts[1], "/", "\\", -1) - return winio.DialPipeContext(ctx, address) - default: - var d net.Dialer - return d.DialContext(ctx, addrParts[0], addrParts[1]) - } -} diff --git a/client/connhelper/npipe/npipe.go b/client/connhelper/npipe/npipe.go new file mode 100644 index 000000000..0abe81b47 --- /dev/null +++ b/client/connhelper/npipe/npipe.go @@ -0,0 +1,8 @@ +// Package npipe provides connhelper for npipe://
+package npipe + +import "github.com/moby/buildkit/client/connhelper" + +func init() { + connhelper.Register("npipe", Helper) +} diff --git a/client/connhelper/npipe/npipe_other.go b/client/connhelper/npipe/npipe_other.go new file mode 100644 index 000000000..75936f48a --- /dev/null +++ b/client/connhelper/npipe/npipe_other.go @@ -0,0 +1,14 @@ +//go:build !windows + +package npipe + +import ( + "errors" + "net/url" + + "github.com/moby/buildkit/client/connhelper" +) + +func Helper(u *url.URL) (*connhelper.ConnectionHelper, error) { + return nil, errors.New("npipe connections are only supported on windows") +} diff --git a/client/connhelper/npipe/npipe_windows.go b/client/connhelper/npipe/npipe_windows.go new file mode 100644 index 000000000..00e832256 --- /dev/null +++ b/client/connhelper/npipe/npipe_windows.go @@ -0,0 +1,28 @@ +//go:build windows + +package npipe + +import ( + "context" + "net" + "net/url" + "strings" + + "github.com/Microsoft/go-winio" + "github.com/moby/buildkit/client/connhelper" + "github.com/pkg/errors" +) + +// Helper returns helper for connecting to a url via npipes. +func Helper(u *url.URL) (*connhelper.ConnectionHelper, error) { + addrParts := strings.SplitN(u.String(), "://", 2) + if len(addrParts) != 2 { + return nil, errors.Errorf("invalid address %s", u) + } + address := strings.Replace(addrParts[1], "/", "\\", -1) + return &connhelper.ConnectionHelper{ + ContextDialer: func(ctx context.Context, addr string) (net.Conn, error) { + return winio.DialPipeContext(ctx, address) + }, + }, nil +} diff --git a/cmd/buildctl/main.go b/cmd/buildctl/main.go index e7f158dc9..bd81cf602 100644 --- a/cmd/buildctl/main.go +++ b/cmd/buildctl/main.go @@ -7,6 +7,7 @@ import ( _ "github.com/moby/buildkit/client/connhelper/dockercontainer" _ "github.com/moby/buildkit/client/connhelper/kubepod" _ "github.com/moby/buildkit/client/connhelper/nerdctlcontainer" + _ "github.com/moby/buildkit/client/connhelper/npipe" _ "github.com/moby/buildkit/client/connhelper/podmancontainer" _ "github.com/moby/buildkit/client/connhelper/ssh" bccommon "github.com/moby/buildkit/cmd/buildctl/common" diff --git a/util/testutil/integration/util_windows.go b/util/testutil/integration/util_windows.go index eef0e37f8..42d6f3fa4 100644 --- a/util/testutil/integration/util_windows.go +++ b/util/testutil/integration/util_windows.go @@ -4,6 +4,9 @@ import ( "net" "github.com/Microsoft/go-winio" + + // include npipe connhelper for windows tests + _ "github.com/moby/buildkit/client/connhelper/npipe" ) var socketScheme = "npipe://"