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

Support custom transport dialer #1520

Merged
merged 4 commits into from
Oct 14, 2021
Merged

Support custom transport dialer #1520

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1148ff1
Support custom transport dialer
tvoran Sep 28, 2021
e61ddbd
ignore TransportDialer for mapstructure ops
tvoran Oct 12, 2021
9860065
include DialContext in the interface
tvoran Oct 13, 2021
03d3cad
Move TransportDialer out of config package
tvoran Oct 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 17 additions & 0 deletions config/transport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package config

import (
"fmt"
"net"
"runtime"
"time"
)
Expand Down Expand Up @@ -36,6 +37,11 @@ var (
// TransportConfig is the configuration to tune low-level APIs for the
// interactions on the wire.
type TransportConfig struct {
// CustomDialer overrides the default net.Dial with a custom dialer. This is
// useful for instance with Vault Agent Templating to direct Consul Template
// requests through an internal cache.
CustomDialer TransportDialer `mapstructure:"-"`

// DialKeepAlive is the amount of time for keep-alives.
DialKeepAlive *time.Duration `mapstructure:"dial_keep_alive"`

Expand All @@ -61,6 +67,12 @@ type TransportConfig struct {
TLSHandshakeTimeout *time.Duration `mapstructure:"tls_handshake_timeout"`
}

// TransportDialer is an interface that allows passing a custom dialer to an
// HTTP client's transport config
type TransportDialer interface {
Dial(network, address string) (net.Conn, error)
Copy link
Member

Choose a reason for hiding this comment

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

Should the interface also cover DialContext? Both net.Dialer and bufconn.Listener should still satisfy this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I suppose it could, but DialContext isn't called anywhere in consul-template that I see.

Copy link
Member

Choose a reason for hiding this comment

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

It looks like Dial on http.Transport is deprecated, so it might be good to have this interface satisfy both in case we switch over.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! Added in 9860065.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Thanks for including DialContext. There are plans on the roadmap for updating CT to use Context.

}

// DefaultTransportConfig returns a configuration that is populated with the
// default values.
func DefaultTransportConfig() *TransportConfig {
Expand All @@ -75,6 +87,7 @@ func (c *TransportConfig) Copy() *TransportConfig {

var o TransportConfig

o.CustomDialer = c.CustomDialer
o.DialKeepAlive = c.DialKeepAlive
o.DialTimeout = c.DialTimeout
o.DisableKeepAlives = c.DisableKeepAlives
Expand Down Expand Up @@ -104,6 +117,10 @@ func (c *TransportConfig) Merge(o *TransportConfig) *TransportConfig {

r := c.Copy()

if o.CustomDialer != nil {
r.CustomDialer = o.CustomDialer
}

if o.DialKeepAlive != nil {
r.DialKeepAlive = o.DialKeepAlive
}
Expand Down
36 changes: 36 additions & 0 deletions config/transport_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package config

import (
"fmt"
"net"
"reflect"
"testing"
"time"
Expand Down Expand Up @@ -33,6 +34,17 @@ func TestTransportConfig_Copy(t *testing.T) {
TLSHandshakeTimeout: TimeDuration(30 * time.Second),
},
},
{
"same_enabled_custom_dialer",
&TransportConfig{
CustomDialer: &net.Dialer{Timeout: 10 * time.Second},
DisableKeepAlives: Bool(true),
IdleConnTimeout: TimeDuration(40 * time.Second),
MaxIdleConns: Int(150),
MaxIdleConnsPerHost: Int(15),
TLSHandshakeTimeout: TimeDuration(30 * time.Second),
},
},
}

for i, tc := range cases {
Expand Down Expand Up @@ -245,6 +257,30 @@ func TestTransportConfig_Merge(t *testing.T) {
&TransportConfig{TLSHandshakeTimeout: TimeDuration(10 * time.Second)},
&TransportConfig{TLSHandshakeTimeout: TimeDuration(10 * time.Second)},
},
{
"custom_transport_dialer",
&TransportConfig{CustomDialer: &net.Dialer{Timeout: 10 * time.Second}},
&TransportConfig{CustomDialer: &net.Dialer{Timeout: 20 * time.Second}},
&TransportConfig{CustomDialer: &net.Dialer{Timeout: 20 * time.Second}},
Copy link
Member

Choose a reason for hiding this comment

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

Does the second one or the one with the highest timeout win?

Copy link
Member Author

Choose a reason for hiding this comment

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

In this test it's the second one, i.e. the "other" takes precedence in Merge:

consul-template/config/transport.go

Lines 89 to 93 in 799d656

// Merge combines all values in this configuration with the values in the other
// configuration, with values in the other configuration taking precedence.
// Maps and slices are merged, most other values are overwritten. Complex
// structs define their own merge functionality.
func (c *TransportConfig) Merge(o *TransportConfig) *TransportConfig {

},
{
"custom_transport_dialer_empty_one",
&TransportConfig{CustomDialer: &net.Dialer{Timeout: 10 * time.Second}},
&TransportConfig{},
&TransportConfig{CustomDialer: &net.Dialer{Timeout: 10 * time.Second}},
},
{
"custom_transport_dialer_empty_two",
&TransportConfig{},
&TransportConfig{CustomDialer: &net.Dialer{Timeout: 10 * time.Second}},
&TransportConfig{CustomDialer: &net.Dialer{Timeout: 10 * time.Second}},
},
{
"custom_transport_dialer_same",
&TransportConfig{CustomDialer: &net.Dialer{Timeout: 10 * time.Second}},
&TransportConfig{CustomDialer: &net.Dialer{Timeout: 10 * time.Second}},
&TransportConfig{CustomDialer: &net.Dialer{Timeout: 10 * time.Second}},
},
}

for i, tc := range cases {
Expand Down
20 changes: 15 additions & 5 deletions dependency/client_set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
consulapi "github.com/hashicorp/consul/api"
rootcerts "github.com/hashicorp/go-rootcerts"
vaultapi "github.com/hashicorp/vault/api"

"github.com/hashicorp/consul-template/config"
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you rework this to not import hashicorp/consul-template/config. I'll need to port this to hashicat (the library replacing consul-template's library functionality) and it doesn't have a config module like this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure! Is 03d3cad what you had in mind?

Copy link
Contributor

Choose a reason for hiding this comment

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

That would totally work. Thanks!

)

// ClientSet is a collection of clients that dependencies use to communicate
Expand Down Expand Up @@ -74,6 +76,7 @@ type CreateVaultClientInput struct {
SSLCAPath string
ServerName string

TransportCustomDialer config.TransportDialer
TransportDialKeepAlive time.Duration
TransportDialTimeout time.Duration
TransportDisableKeepAlives bool
Expand Down Expand Up @@ -202,12 +205,19 @@ func (c *ClientSet) CreateVaultClient(i *CreateVaultClientInput) error {
}

// This transport will attempt to keep connections open to the Vault server.
var dialer config.TransportDialer
dialer = &net.Dialer{
Timeout: i.TransportDialTimeout,
KeepAlive: i.TransportDialKeepAlive,
}

if i.TransportCustomDialer != nil {
dialer = i.TransportCustomDialer
}

transport := &http.Transport{
Proxy: http.ProxyFromEnvironment,
Dial: (&net.Dialer{
Timeout: i.TransportDialTimeout,
KeepAlive: i.TransportDialKeepAlive,
}).Dial,
Proxy: http.ProxyFromEnvironment,
Dial: dialer.Dial,
DisableKeepAlives: i.TransportDisableKeepAlives,
MaxIdleConns: i.TransportMaxIdleConns,
IdleConnTimeout: i.TransportIdleConnTimeout,
Expand Down
1 change: 1 addition & 0 deletions manager/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1284,6 +1284,7 @@ func newClientSet(c *config.Config) (*dep.ClientSet, error) {
SSLCACert: config.StringVal(c.Vault.SSL.CaCert),
SSLCAPath: config.StringVal(c.Vault.SSL.CaPath),
ServerName: config.StringVal(c.Vault.SSL.ServerName),
TransportCustomDialer: c.Vault.Transport.CustomDialer,
TransportDialKeepAlive: config.TimeDurationVal(c.Vault.Transport.DialKeepAlive),
TransportDialTimeout: config.TimeDurationVal(c.Vault.Transport.DialTimeout),
TransportDisableKeepAlives: config.BoolVal(c.Vault.Transport.DisableKeepAlives),
Expand Down