From cd9a52a5c5d1c210600eeeff453519ab5c69a650 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Sun, 22 Feb 2026 06:00:39 -0700 Subject: [PATCH 1/4] Add WithDialer option and rewrite With* to auto-inject dialer Introduce a single WithDialer option at the kindling level that automatically flows to all transports (fronted, dnstt, amp, smart). Key changes: - Add exported DialContextFunc type and WithDialer option - Add Close() to Kindling interface for resource cleanup - Rewrite WithDomainFronting to accept fronted.Option params - Rewrite WithDNSTunnel to accept dnstt.Option params - Rewrite WithAMPCache to accept amp.Config + amp.Option params - Update smart dialer to use injected dialer via FuncStreamDialer - Add streamConnAdapter and closerFunc helper types - Update tests for new API signatures The With* functions now construct transport instances internally and prepend the dialer option, so callers no longer need to create transport instances themselves. Requires getlantern/fronted#67 and getlantern/dnstt#12. Co-Authored-By: Claude Opus 4.6 --- go.mod | 4 +- go.sum | 4 ++ kindling.go | 175 +++++++++++++++++++++++++++++++++++------------ kindling_test.go | 33 +++++++-- 4 files changed, 167 insertions(+), 49 deletions(-) diff --git a/go.mod b/go.mod index 8a24544..586b2a2 100644 --- a/go.mod +++ b/go.mod @@ -6,8 +6,8 @@ require ( github.com/Jigsaw-Code/outline-sdk v0.0.19 github.com/Jigsaw-Code/outline-sdk/x v0.0.2 github.com/getlantern/amp v0.0.0-20251211213807-4cbc22624b9f - github.com/getlantern/dnstt v0.0.0-20250530230749-4d64f4edcf0f - github.com/getlantern/fronted v0.0.0-20260105215156-9ae1d001d54f + github.com/getlantern/dnstt v0.0.0-20260222125454-01dcfb65c0c6 + github.com/getlantern/fronted v0.0.0-20260222125446-260488e3ed5e github.com/stretchr/testify v1.11.1 ) diff --git a/go.sum b/go.sum index 88f7028..1d247f3 100644 --- a/go.sum +++ b/go.sum @@ -33,6 +33,8 @@ github.com/getlantern/context v0.0.0-20220418194847-3d5e7a086201 h1:oEZYEpZo28Wd github.com/getlantern/context v0.0.0-20220418194847-3d5e7a086201/go.mod h1:Y9WZUHEb+mpra02CbQ/QczLUe6f0Dezxaw5DCJlJQGo= github.com/getlantern/dnstt v0.0.0-20250530230749-4d64f4edcf0f h1:CbN6CaUUrXPDpZCZ4wCq/3bdLhrBWqpnAiEen/t2UlY= github.com/getlantern/dnstt v0.0.0-20250530230749-4d64f4edcf0f/go.mod h1:LA7cwZQtgXxBJdSJDj2ZgQNo/UY3Qa7nxNxzOuMMIyw= +github.com/getlantern/dnstt v0.0.0-20260222125454-01dcfb65c0c6 h1:HhLq9wIRPEqdl1iSNWUaM+n/Jkxg/7o6O+hDezLtStA= +github.com/getlantern/dnstt v0.0.0-20260222125454-01dcfb65c0c6/go.mod h1:LA7cwZQtgXxBJdSJDj2ZgQNo/UY3Qa7nxNxzOuMMIyw= github.com/getlantern/errors v1.0.1/go.mod h1:l+xpFBrCtDLpK9qNjxs+cHU6+BAdlBaxHqikB6Lku3A= github.com/getlantern/errors v1.0.3 h1:Ne4Ycj7NI1BtSyAfVeAT/DNoxz7/S2BUc3L2Ht1YSHE= github.com/getlantern/errors v1.0.3/go.mod h1:m8C7H1qmouvsGpwQqk/6NUpIVMpfzUPn608aBZDYV04= @@ -40,6 +42,8 @@ github.com/getlantern/fdcount v0.0.0-20190912142506-f89afd7367c4 h1:JdD4XSaT6/j6 github.com/getlantern/fdcount v0.0.0-20190912142506-f89afd7367c4/go.mod h1:XZwE+iIlAgr64OFbXKFNCllBwV4wEipPx8Hlo2gZdbM= github.com/getlantern/fronted v0.0.0-20260105215156-9ae1d001d54f h1:DO5SrV7sZ8DYCXk1PxC4OGXEvtPILzSBJ3lfyeQqshM= github.com/getlantern/fronted v0.0.0-20260105215156-9ae1d001d54f/go.mod h1:1a+iv1xzGxZWj/vCHzr8Z3dF9H1sNTuMSPHUqRsgbl0= +github.com/getlantern/fronted v0.0.0-20260222125446-260488e3ed5e h1:BQCXOjoBcbKHkCfLCN+3hnO5w7CyhJqMvhtYdEofFUU= +github.com/getlantern/fronted v0.0.0-20260222125446-260488e3ed5e/go.mod h1:1a+iv1xzGxZWj/vCHzr8Z3dF9H1sNTuMSPHUqRsgbl0= github.com/getlantern/golog v0.0.0-20210606115803-bce9f9fe5a5f/go.mod h1:ZyIjgH/1wTCl+B+7yH1DqrWp6MPJqESmwmEQ89ZfhvA= github.com/getlantern/golog v0.0.0-20230503153817-8e72de7e0a65 h1:NlQedYmPI3pRAXJb+hLVVDGqfvvXGRPV8vp7XOjKAZ0= github.com/getlantern/golog v0.0.0-20230503153817-8e72de7e0a65/go.mod h1:+ZU1h+iOVqWReBpky6d5Y2WL0sF2Llxu+QcxJFs2+OU= diff --git a/kindling.go b/kindling.go index cdd588a..fd00863 100644 --- a/kindling.go +++ b/kindling.go @@ -22,6 +22,9 @@ import ( var log = slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{AddSource: true})) +// DialContextFunc is the canonical dialer type used throughout kindling and its transports. +type DialContextFunc func(ctx context.Context, network, addr string) (net.Conn, error) + // Kindling is the interface that wraps the basic Dial and DialContext methods for control // plane traffic. type Kindling interface { @@ -29,6 +32,8 @@ type Kindling interface { NewHTTPClient() *http.Client // ReplaceTransport replaces an existing transport RoundTripper generator with the provided one. ReplaceTransport(name string, rt func(ctx context.Context, addr string) (http.RoundTripper, error)) error + // Close releases resources held by transports created by kindling. + Close() error } type roundTripperGenerator func(ctx context.Context, addr string) (http.RoundTripper, error) @@ -40,8 +45,23 @@ type kindling struct { logWriter io.Writer panicListener func(string) appName string // The name of the tool using kindling, used for logging and debugging. + dialContext DialContextFunc + closers []io.Closer +} + +// closerFunc adapts a func() into an io.Closer. +type closerFunc func() error + +func (f closerFunc) Close() error { return f() } + +// streamConnAdapter wraps a net.Conn as a transport.StreamConn by adding a no-op CloseWrite/CloseRead. +type streamConnAdapter struct { + net.Conn } +func (s *streamConnAdapter) CloseWrite() error { return nil } +func (s *streamConnAdapter) CloseRead() error { return nil } + // Make sure that kindling implements the Kindling interface. var _ Kindling = &kindling{} @@ -50,6 +70,7 @@ var _ Kindling = &kindling{} const ( priorityLogWriter = iota priorityPanicListener + priorityDialer ) // Option is a functional option type that allows us to configure the Client. @@ -73,9 +94,28 @@ func NewKindling(name string, options ...Option) Kindling { for _, opt := range options { opt.apply(k) } + + // Default the dialer if none was provided. + if k.dialContext == nil { + k.dialContext = (&net.Dialer{}).DialContext + } return k } +// Close releases resources held by transports created by kindling. +func (k *kindling) Close() error { + var errs []error + for _, c := range k.closers { + if err := c.Close(); err != nil { + errs = append(errs, err) + } + } + if len(errs) > 0 { + return fmt.Errorf("errors closing kindling resources: %v", errs) + } + return nil +} + // NewHTTPClient implements the Kindling interface. func (k *kindling) NewHTTPClient() *http.Client { // Create a specialized HTTP transport that concurrently races between fronted and smart dialer. @@ -100,42 +140,73 @@ func (k *kindling) ReplaceTransport(name string, rt func(ctx context.Context, ad return fmt.Errorf("Could not find matching transport: %v", name) } -// WithDomainFronting is a functional option that sets up domain fronting for kindling using -// the provided fronted.Fronted instance from https://github.com/getlantern/fronted. -func WithDomainFronting(f fronted.Fronted) Option { - log.Info("Setting domain fronting") - if f == nil { - log.Error("Fronted instance is nil") - return &emptyOption{} - } - return WithTransport(newTransport("fronted", 0, true, func(ctx context.Context, addr string) (http.RoundTripper, error) { - return f.NewConnectedRoundTripper(ctx, addr) - })) -} - -// WithDNSTunnel is a functional option that sets up a DNS tunnel for kindling using the provided -// [dnstt.DNSTT] instance -func WithDNSTunnel(d dnstt.DNSTT) Option { - log.Info("Setting DNS tunnel") - if d == nil { - log.Error("DNSTT instance is nil") - return &emptyOption{} - } - return WithTransport(newTransport("dnstt", 0, true, func(ctx context.Context, addr string) (http.RoundTripper, error) { - return d.NewRoundTripper(ctx, addr) - })) +// WithDomainFronting is a functional option that sets up domain fronting for kindling. +// It accepts fronted.Option parameters and constructs the fronted instance internally, +// automatically injecting kindling's dialer. +func WithDomainFronting(opts ...fronted.Option) Option { + return newOption(func(k *kindling) { + log.Info("Setting domain fronting") + // Prepend our dialer so the caller's options can override if needed. + allOpts := make([]fronted.Option, 0, len(opts)+1) + allOpts = append(allOpts, fronted.WithDialer(k.dialContext)) + allOpts = append(allOpts, opts...) + f := fronted.NewFronted(allOpts...) + k.closers = append(k.closers, closerFunc(func() error { f.Close(); return nil })) + k.transports = append(k.transports, newTransport("fronted", 0, true, func(ctx context.Context, addr string) (http.RoundTripper, error) { + return f.NewConnectedRoundTripper(ctx, addr) + })) + }) } -// WithAMPCache uses the AMP cache for making requests. It adds an 'amp' round tripper from the provided amp.Client. -func WithAMPCache(c amp.Client) Option { - log.Info("Setting AMP cache") - if c == nil { - log.Error("AMP client is nil") - return &emptyOption{} - } - return WithTransport(newTransport("amp", 6000, false, func(ctx context.Context, addr string) (http.RoundTripper, error) { - return c.RoundTripper() - })) +// WithDNSTunnel is a functional option that sets up a DNS tunnel for kindling. +// It accepts dnstt.Option parameters and constructs the dnstt instance internally, +// automatically injecting kindling's dialer. +func WithDNSTunnel(opts ...dnstt.Option) Option { + return newOption(func(k *kindling) { + log.Info("Setting DNS tunnel") + // Prepend our dialer so the caller's options can override if needed. + allOpts := make([]dnstt.Option, 0, len(opts)+1) + allOpts = append(allOpts, dnstt.WithDialer(k.dialContext)) + allOpts = append(allOpts, opts...) + d, err := dnstt.NewDNSTT(allOpts...) + if err != nil { + log.Error("Failed to create DNSTT instance", "error", err) + return + } + k.closers = append(k.closers, d) + k.transports = append(k.transports, newTransport("dnstt", 0, true, func(ctx context.Context, addr string) (http.RoundTripper, error) { + return d.NewRoundTripper(ctx, addr) + })) + }) +} + +// WithAMPCache uses the AMP cache for making requests. It accepts an amp.Config and +// optional amp.Option parameters, constructs the amp client internally, and automatically +// injects kindling's dialer. +func WithAMPCache(cfg amp.Config, opts ...amp.Option) Option { + return newOption(func(k *kindling) { + log.Info("Setting AMP cache") + // Adapt DialContextFunc to amp's dialFunc (func(network, addr string) (net.Conn, error)) + ampDialer := func(network, addr string) (net.Conn, error) { + return k.dialContext(context.Background(), network, addr) + } + allOpts := make([]amp.Option, 0, len(opts)+1) + allOpts = append(allOpts, amp.WithDialer(ampDialer)) + allOpts = append(allOpts, opts...) + + ctx, cancel := context.WithCancel(context.Background()) + k.closers = append(k.closers, closerFunc(func() error { cancel(); return nil })) + + c, err := amp.NewClientWithConfig(ctx, cfg, allOpts...) + if err != nil { + cancel() + log.Error("Failed to create AMP client", "error", err) + return + } + k.transports = append(k.transports, newTransport("amp", 6000, false, func(ctx context.Context, addr string) (http.RoundTripper, error) { + return c.RoundTripper() + })) + }) } // WithProxyless is a functional option that enables proxyless mode for the Kindling such that @@ -143,7 +214,7 @@ func WithAMPCache(c amp.Client) Option { func WithProxyless(domains ...string) Option { return newOption(func(k *kindling) { slog.Info("Setting proxyless mode") - smartDialer, err := newSmartHTTPDialerFunc(k.logWriter, domains...) + smartDialer, err := newSmartHTTPDialerFunc(k.dialContext, k.logWriter, domains...) if err != nil { log.Error("Failed to create smart dialer", "error", err) return @@ -197,6 +268,16 @@ func WithTransport(transport Transport) Option { }) } +// WithDialer sets a custom dialer that will be automatically injected into all transports +// (fronted, dnstt, amp, smart) created by kindling. This runs at priorityDialer priority +// so that it is set before transport options are applied. +func WithDialer(dial DialContextFunc) Option { + return newOptionWithPriority(func(k *kindling) { + log.Info("Setting custom dialer") + k.dialContext = dial + }, priorityDialer) +} + // WithPanicListener is a functional option that sets a panic listener that should be notified // whenever any goroutine panics. We set this with a higher priority so that it is set before // any other options that may depend on it. @@ -212,8 +293,8 @@ func (k *kindling) newRaceTransport() http.RoundTripper { return newRaceTransport(k.appName, k.panicListener, k.transports...) } -func newSmartHTTPDialerFunc(logWriter io.Writer, domains ...string) (roundTripperGenerator, error) { - d, err := newSmartDialer(logWriter, domains...) +func newSmartHTTPDialerFunc(dialContext DialContextFunc, logWriter io.Writer, domains ...string) (roundTripperGenerator, error) { + d, err := newSmartDialer(dialContext, logWriter, domains...) if err != nil { return nil, fmt.Errorf("failed to create smart dialer: %v", err) } @@ -229,9 +310,12 @@ func newSmartHTTPDialerFunc(logWriter io.Writer, domains ...string) (roundTrippe } // NewSmartHTTPTransport creates a new HTTP transport that uses the Outline smart dialer to dial to the -// specified domains. -func NewSmartHTTPTransport(logWriter io.Writer, domains ...string) (*http.Transport, error) { - d, err := newSmartDialer(logWriter, domains...) +// specified domains. If dialContext is nil, a default net.Dialer is used. +func NewSmartHTTPTransport(dialContext DialContextFunc, logWriter io.Writer, domains ...string) (*http.Transport, error) { + if dialContext == nil { + dialContext = (&net.Dialer{}).DialContext + } + d, err := newSmartDialer(dialContext, logWriter, domains...) if err != nil { log.Error("Failed to create smart dialer", "error", err) return nil, fmt.Errorf("failed to create smart dialer: %v", err) @@ -259,11 +343,18 @@ func newTransportWithDialContext(dialContext func(ctx context.Context, network, //go:embed smart_dialer_config.yml var embedFS embed.FS -func newSmartDialer(logWriter io.Writer, domains ...string) (transport.StreamDialer, error) { +func newSmartDialer(dialContext DialContextFunc, logWriter io.Writer, domains ...string) (transport.StreamDialer, error) { + streamDialer := transport.FuncStreamDialer(func(ctx context.Context, addr string) (transport.StreamConn, error) { + conn, err := dialContext(ctx, "tcp", addr) + if err != nil { + return nil, err + } + return &streamConnAdapter{Conn: conn}, nil + }) finder := &smart.StrategyFinder{ TestTimeout: 5 * time.Second, LogWriter: logWriter, - StreamDialer: &transport.TCPDialer{}, + StreamDialer: streamDialer, PacketDialer: &transport.UDPDialer{}, } diff --git a/kindling_test.go b/kindling_test.go index 42d527d..480f3db 100644 --- a/kindling_test.go +++ b/kindling_test.go @@ -4,7 +4,9 @@ import ( "context" "fmt" "io" + "net" "net/http" + "sync/atomic" "testing" "github.com/getlantern/fronted" @@ -16,13 +18,13 @@ func TestNewKindling(t *testing.T) { t.Run("Success", func(t *testing.T) { t.Parallel() - f := fronted.NewFronted( - fronted.WithConfigURL("https://media.githubusercontent.com/media/getlantern/fronted/refs/heads/main/fronted.yaml.gz")) - kindling := NewKindling("kindling", - WithDomainFronting(f), + k := NewKindling("kindling", + WithDomainFronting( + fronted.WithConfigURL("https://media.githubusercontent.com/media/getlantern/fronted/refs/heads/main/fronted.yaml.gz")), WithPanicListener(func(string) {}), ) - if kindling == nil { + defer k.Close() + if k == nil { t.Errorf("NewKindling() = nil; want non-nil Kindling") } }) @@ -224,3 +226,24 @@ func TestKindling_ReplaceTransport(t *testing.T) { } }) } + +func TestWithDialer(t *testing.T) { + t.Parallel() + var called atomic.Bool + customDialer := func(ctx context.Context, network, addr string) (net.Conn, error) { + called.Store(true) + return (&net.Dialer{}).DialContext(ctx, network, addr) + } + k := NewKindling("test-app", + WithDialer(customDialer), + ) + defer k.Close() + if k == nil { + t.Fatal("NewKindling() = nil; want non-nil Kindling") + } + // Verify the dialer was stored by checking the internal struct. + ki := k.(*kindling) + if ki.dialContext == nil { + t.Fatal("dialContext should not be nil after WithDialer") + } +} From 32261d59b246bf3f96744f4109ff44affa724cdb Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Sun, 22 Feb 2026 06:46:04 -0700 Subject: [PATCH 2/4] Update dnstt dependency to latest add-with-dialer commit Co-Authored-By: Claude Opus 4.6 --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 586b2a2..70eac45 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/Jigsaw-Code/outline-sdk v0.0.19 github.com/Jigsaw-Code/outline-sdk/x v0.0.2 github.com/getlantern/amp v0.0.0-20251211213807-4cbc22624b9f - github.com/getlantern/dnstt v0.0.0-20260222125454-01dcfb65c0c6 + github.com/getlantern/dnstt v0.0.0-20260222134223-d7c46d78c1f4 github.com/getlantern/fronted v0.0.0-20260222125446-260488e3ed5e github.com/stretchr/testify v1.11.1 ) diff --git a/go.sum b/go.sum index 1d247f3..903b448 100644 --- a/go.sum +++ b/go.sum @@ -35,6 +35,8 @@ github.com/getlantern/dnstt v0.0.0-20250530230749-4d64f4edcf0f h1:CbN6CaUUrXPDpZ github.com/getlantern/dnstt v0.0.0-20250530230749-4d64f4edcf0f/go.mod h1:LA7cwZQtgXxBJdSJDj2ZgQNo/UY3Qa7nxNxzOuMMIyw= github.com/getlantern/dnstt v0.0.0-20260222125454-01dcfb65c0c6 h1:HhLq9wIRPEqdl1iSNWUaM+n/Jkxg/7o6O+hDezLtStA= github.com/getlantern/dnstt v0.0.0-20260222125454-01dcfb65c0c6/go.mod h1:LA7cwZQtgXxBJdSJDj2ZgQNo/UY3Qa7nxNxzOuMMIyw= +github.com/getlantern/dnstt v0.0.0-20260222134223-d7c46d78c1f4 h1:moJ39oky/8MpUJXo6l3GM9WBTx3hw3Rmv9/rFUsS6H0= +github.com/getlantern/dnstt v0.0.0-20260222134223-d7c46d78c1f4/go.mod h1:LA7cwZQtgXxBJdSJDj2ZgQNo/UY3Qa7nxNxzOuMMIyw= github.com/getlantern/errors v1.0.1/go.mod h1:l+xpFBrCtDLpK9qNjxs+cHU6+BAdlBaxHqikB6Lku3A= github.com/getlantern/errors v1.0.3 h1:Ne4Ycj7NI1BtSyAfVeAT/DNoxz7/S2BUc3L2Ht1YSHE= github.com/getlantern/errors v1.0.3/go.mod h1:m8C7H1qmouvsGpwQqk/6NUpIVMpfzUPn608aBZDYV04= From 891684bad596a8a872c93d592e6cf6a167a289ba Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Sun, 22 Feb 2026 07:02:56 -0700 Subject: [PATCH 3/4] Update fronted dependency and fix DialFunc type conversion Update fronted to latest commit which introduces the named DialFunc type. Add explicit type conversion for the dialer passed to fronted.WithDialer. Co-Authored-By: Claude Opus 4.6 --- go.mod | 2 +- go.sum | 2 ++ kindling.go | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 70eac45..213c54e 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/Jigsaw-Code/outline-sdk/x v0.0.2 github.com/getlantern/amp v0.0.0-20251211213807-4cbc22624b9f github.com/getlantern/dnstt v0.0.0-20260222134223-d7c46d78c1f4 - github.com/getlantern/fronted v0.0.0-20260222125446-260488e3ed5e + github.com/getlantern/fronted v0.0.0-20260222135654-665091b38978 github.com/stretchr/testify v1.11.1 ) diff --git a/go.sum b/go.sum index 903b448..e03edce 100644 --- a/go.sum +++ b/go.sum @@ -46,6 +46,8 @@ github.com/getlantern/fronted v0.0.0-20260105215156-9ae1d001d54f h1:DO5SrV7sZ8DY github.com/getlantern/fronted v0.0.0-20260105215156-9ae1d001d54f/go.mod h1:1a+iv1xzGxZWj/vCHzr8Z3dF9H1sNTuMSPHUqRsgbl0= github.com/getlantern/fronted v0.0.0-20260222125446-260488e3ed5e h1:BQCXOjoBcbKHkCfLCN+3hnO5w7CyhJqMvhtYdEofFUU= github.com/getlantern/fronted v0.0.0-20260222125446-260488e3ed5e/go.mod h1:1a+iv1xzGxZWj/vCHzr8Z3dF9H1sNTuMSPHUqRsgbl0= +github.com/getlantern/fronted v0.0.0-20260222135654-665091b38978 h1:Okzs91P/dYBWGEIkEOWdH3xvUAeCo7JYCXwx7uvEcUI= +github.com/getlantern/fronted v0.0.0-20260222135654-665091b38978/go.mod h1:1a+iv1xzGxZWj/vCHzr8Z3dF9H1sNTuMSPHUqRsgbl0= github.com/getlantern/golog v0.0.0-20210606115803-bce9f9fe5a5f/go.mod h1:ZyIjgH/1wTCl+B+7yH1DqrWp6MPJqESmwmEQ89ZfhvA= github.com/getlantern/golog v0.0.0-20230503153817-8e72de7e0a65 h1:NlQedYmPI3pRAXJb+hLVVDGqfvvXGRPV8vp7XOjKAZ0= github.com/getlantern/golog v0.0.0-20230503153817-8e72de7e0a65/go.mod h1:+ZU1h+iOVqWReBpky6d5Y2WL0sF2Llxu+QcxJFs2+OU= diff --git a/kindling.go b/kindling.go index fd00863..a3c48f7 100644 --- a/kindling.go +++ b/kindling.go @@ -148,7 +148,7 @@ func WithDomainFronting(opts ...fronted.Option) Option { log.Info("Setting domain fronting") // Prepend our dialer so the caller's options can override if needed. allOpts := make([]fronted.Option, 0, len(opts)+1) - allOpts = append(allOpts, fronted.WithDialer(k.dialContext)) + allOpts = append(allOpts, fronted.WithDialer(fronted.DialFunc(k.dialContext))) allOpts = append(allOpts, opts...) f := fronted.NewFronted(allOpts...) k.closers = append(k.closers, closerFunc(func() error { f.Close(); return nil })) From 5c2bb7f18b1292656c6b9c2419024bb2f6cdbc88 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Sun, 22 Feb 2026 07:25:31 -0700 Subject: [PATCH 4/4] Address PR review comments - streamConnAdapter: delegate CloseWrite/CloseRead to underlying conn when it supports the interface, instead of no-ops - Add mutex to protect closers slice for concurrent Close() safety - Fix WithAMPCache: only add cancel closer after successful client creation - Add comment explaining context.Background() limitation in amp adapter - Improve TestWithDialer to verify custom dialer is actually called - Add TestWithDialerDefault for default dialer behavior - Document breaking API changes in WithDomainFronting, WithDNSTunnel, WithAMPCache doc comments - Document silent failure behavior in WithDNSTunnel and WithAMPCache Co-Authored-By: Claude Opus 4.6 --- kindling.go | 44 ++++++++++++++++++++++++++++++++++++++------ kindling_test.go | 15 +++++++++++++++ 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/kindling.go b/kindling.go index a3c48f7..b78f3a8 100644 --- a/kindling.go +++ b/kindling.go @@ -46,6 +46,7 @@ type kindling struct { panicListener func(string) appName string // The name of the tool using kindling, used for logging and debugging. dialContext DialContextFunc + closersMu sync.Mutex closers []io.Closer } @@ -54,13 +55,24 @@ type closerFunc func() error func (f closerFunc) Close() error { return f() } -// streamConnAdapter wraps a net.Conn as a transport.StreamConn by adding a no-op CloseWrite/CloseRead. +// streamConnAdapter wraps a net.Conn as a transport.StreamConn by adding CloseWrite/CloseRead methods. type streamConnAdapter struct { net.Conn } -func (s *streamConnAdapter) CloseWrite() error { return nil } -func (s *streamConnAdapter) CloseRead() error { return nil } +func (s *streamConnAdapter) CloseWrite() error { + if cw, ok := s.Conn.(interface{ CloseWrite() error }); ok { + return cw.CloseWrite() + } + return nil +} + +func (s *streamConnAdapter) CloseRead() error { + if cr, ok := s.Conn.(interface{ CloseRead() error }); ok { + return cr.CloseRead() + } + return nil +} // Make sure that kindling implements the Kindling interface. var _ Kindling = &kindling{} @@ -104,12 +116,15 @@ func NewKindling(name string, options ...Option) Kindling { // Close releases resources held by transports created by kindling. func (k *kindling) Close() error { + k.closersMu.Lock() + defer k.closersMu.Unlock() var errs []error for _, c := range k.closers { if err := c.Close(); err != nil { errs = append(errs, err) } } + k.closers = nil if len(errs) > 0 { return fmt.Errorf("errors closing kindling resources: %v", errs) } @@ -143,6 +158,9 @@ func (k *kindling) ReplaceTransport(name string, rt func(ctx context.Context, ad // WithDomainFronting is a functional option that sets up domain fronting for kindling. // It accepts fronted.Option parameters and constructs the fronted instance internally, // automatically injecting kindling's dialer. +// +// Note: this is a breaking change from the previous API which accepted a fronted.Fronted instance. +// Callers must now pass fronted.Option values instead (e.g., fronted.WithConfigURL(...)). func WithDomainFronting(opts ...fronted.Option) Option { return newOption(func(k *kindling) { log.Info("Setting domain fronting") @@ -161,6 +179,12 @@ func WithDomainFronting(opts ...fronted.Option) Option { // WithDNSTunnel is a functional option that sets up a DNS tunnel for kindling. // It accepts dnstt.Option parameters and constructs the dnstt instance internally, // automatically injecting kindling's dialer. +// +// Note: this is a breaking change from the previous API which accepted a dnstt.DNSTT instance. +// Callers must now pass dnstt.Option values instead (e.g., dnstt.WithDoH(...), dnstt.WithTunnelDomain(...)). +// +// If transport creation fails, the error is logged and the transport is silently omitted. +// Kindling will still function with any remaining transports. func WithDNSTunnel(opts ...dnstt.Option) Option { return newOption(func(k *kindling) { log.Info("Setting DNS tunnel") @@ -183,10 +207,19 @@ func WithDNSTunnel(opts ...dnstt.Option) Option { // WithAMPCache uses the AMP cache for making requests. It accepts an amp.Config and // optional amp.Option parameters, constructs the amp client internally, and automatically // injects kindling's dialer. +// +// Note: this is a breaking change from the previous API which accepted an amp.Client instance. +// Callers must now pass an amp.Config and optional amp.Option values instead. +// +// If client creation fails, the error is logged and the transport is silently omitted. +// Kindling will still function with any remaining transports. func WithAMPCache(cfg amp.Config, opts ...amp.Option) Option { return newOption(func(k *kindling) { log.Info("Setting AMP cache") - // Adapt DialContextFunc to amp's dialFunc (func(network, addr string) (net.Conn, error)) + // Adapt DialContextFunc to amp's dialFunc (func(network, addr string) (net.Conn, error)). + // Note: amp.WithDialer does not accept a context, so we use context.Background() here. + // This means caller-level timeouts/cancellation won't propagate to the dial phase; + // however, the AMP client's own context (below) handles lifecycle cancellation. ampDialer := func(network, addr string) (net.Conn, error) { return k.dialContext(context.Background(), network, addr) } @@ -195,14 +228,13 @@ func WithAMPCache(cfg amp.Config, opts ...amp.Option) Option { allOpts = append(allOpts, opts...) ctx, cancel := context.WithCancel(context.Background()) - k.closers = append(k.closers, closerFunc(func() error { cancel(); return nil })) - c, err := amp.NewClientWithConfig(ctx, cfg, allOpts...) if err != nil { cancel() log.Error("Failed to create AMP client", "error", err) return } + k.closers = append(k.closers, closerFunc(func() error { cancel(); return nil })) k.transports = append(k.transports, newTransport("amp", 6000, false, func(ctx context.Context, addr string) (http.RoundTripper, error) { return c.RoundTripper() })) diff --git a/kindling_test.go b/kindling_test.go index 480f3db..0ee93aa 100644 --- a/kindling_test.go +++ b/kindling_test.go @@ -246,4 +246,19 @@ func TestWithDialer(t *testing.T) { if ki.dialContext == nil { t.Fatal("dialContext should not be nil after WithDialer") } + // Verify the custom dialer is actually the one stored by calling it and checking the flag. + _, _ = ki.dialContext(context.Background(), "tcp", "localhost:0") + if !called.Load() { + t.Fatal("custom dialer should have been called") + } +} + +func TestWithDialerDefault(t *testing.T) { + t.Parallel() + k := NewKindling("test-app") + defer k.Close() + ki := k.(*kindling) + if ki.dialContext == nil { + t.Fatal("default dialContext should be set when WithDialer is not used") + } }