sdk/ldaputil: add connection_timeout configurable (hashicorp#20144) (h…

…ashicorp#20148)

* sdk/ldaputil: add connection_timeout configurable

* changelog

* Update doc

* Fix test

* Change default to 30s

Co-authored-by: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com>

builtin/credential/ldap/backend_test.go

@@ -829,6 +829,7 @@ func testAccStepConfigUrl(t *testing.T, cfg *ldaputil.ConfigEntry) logicaltest.T
 			"case_sensitive_names": true,
 			"token_policies":       "abc,xyz",
 			"request_timeout":      cfg.RequestTimeout,
+			"connection_timeout":   cfg.ConnectionTimeout,
 			"username_as_alias":    cfg.UsernameAsAlias,
 		},
 	}
@@ -851,6 +852,7 @@ func testAccStepConfigUrlWithAuthBind(t *testing.T, cfg *ldaputil.ConfigEntry) l
 			"case_sensitive_names": true,
 			"token_policies":       "abc,xyz",
 			"request_timeout":      cfg.RequestTimeout,
+			"connection_timeout":   cfg.ConnectionTimeout,
 		},
 	}
 }
@@ -871,6 +873,7 @@ func testAccStepConfigUrlWithDiscover(t *testing.T, cfg *ldaputil.ConfigEntry) l
 			"case_sensitive_names": true,
 			"token_policies":       "abc,xyz",
 			"request_timeout":      cfg.RequestTimeout,
+			"connection_timeout":   cfg.ConnectionTimeout,
 		},
 	}
 }
@@ -888,6 +891,7 @@ func testAccStepConfigUrlNoGroupDN(t *testing.T, cfg *ldaputil.ConfigEntry) logi
 			"discoverdn":           true,
 			"case_sensitive_names": true,
 			"request_timeout":      cfg.RequestTimeout,
+			"connection_timeout":   cfg.ConnectionTimeout,
 		},
 	}
 }
@@ -908,6 +912,7 @@ func testAccStepConfigUrlWarningCheck(t *testing.T, cfg *ldaputil.ConfigEntry, o
 			"case_sensitive_names": true,
 			"token_policies":       "abc,xyz",
 			"request_timeout":      cfg.RequestTimeout,
+			"connection_timeout":   cfg.ConnectionTimeout,
 		},
 		Check: func(response *logical.Response) error {
 			if len(response.Warnings) == 0 {
@@ -1189,6 +1194,7 @@ func TestLdapAuthBackend_ConfigUpgrade(t *testing.T) {
 			"token_period":           "5m",
 			"token_explicit_max_ttl": "24h",
 			"request_timeout":        cfg.RequestTimeout,
+			"connection_timeout":     cfg.ConnectionTimeout,
 		},
 		Storage:    storage,
 		Connection: &logical.Connection{},
@@ -1230,6 +1236,7 @@ func TestLdapAuthBackend_ConfigUpgrade(t *testing.T) {
 			CaseSensitiveNames:       falseBool,
 			UsePre111GroupCNBehavior: new(bool),
 			RequestTimeout:           cfg.RequestTimeout,
+			ConnectionTimeout:        cfg.ConnectionTimeout,
 			UsernameAsAlias:          false,
 		},
 	}

changelog/20144.txt

@@ -0,0 +1,4 @@
+```release-note:improvement
+sdk/ldaputil: added `connection_timeout` to tune connection timeout duration 
+for all LDAP plugins.
+```

sdk/helper/ldaputil/client.go

@@ -28,6 +28,12 @@ func (c *Client) DialLDAP(cfg *ConfigEntry) (Connection, error) {
 	var retErr *multierror.Error
 	var conn Connection
 	urls := strings.Split(cfg.Url, ",")
+
+	// Default timeout in the pacakge is 60 seconds, which we default to on our
+	// end. This is useful if you want to take advantage of the URL list to increase
+	// availability of LDAP.
+	ldap.DefaultTimeout = time.Duration(cfg.ConnectionTimeout) * time.Second
+
 	for _, uut := range urls {
 		u, err := url.Parse(uut)
 		if err != nil {

sdk/helper/ldaputil/config.go

@@ -226,6 +226,12 @@ Default: ({{.UserAttr}}={{.Username}})`,
 			Description: "Timeout, in seconds, for the connection when making requests against the server before returning back an error.",
 			Default:     "90s",
 		},
+
+		"connection_timeout": {
+			Type:        framework.TypeDurationSecond,
+			Description: "Timeout, in seconds, when attempting to connect to the LDAP server before trying the next URL in the configuration.",
+			Default:     "30s",
+		},
 	}
 }
 
@@ -392,6 +398,10 @@ func NewConfigEntry(existing *ConfigEntry, d *framework.FieldData) (*ConfigEntry
 		cfg.RequestTimeout = d.Get("request_timeout").(int)
 	}
 
+	if _, ok := d.Raw["connection_timeout"]; ok || !hadExisting {
+		cfg.ConnectionTimeout = d.Get("connection_timeout").(int)
+	}
+
 	return cfg, nil
 }
 
@@ -418,6 +428,7 @@ type ConfigEntry struct {
 	UseTokenGroups           bool   `json:"use_token_groups"`
 	UsePre111GroupCNBehavior *bool  `json:"use_pre111_group_cn_behavior"`
 	RequestTimeout           int    `json:"request_timeout"`
+	ConnectionTimeout        int    `json:"connection_timeout"`
 
 	// These json tags deviate from snake case because there was a past issue
 	// where the tag was being ignored, causing it to be jsonified as "CaseSensitiveNames", etc.
@@ -455,6 +466,7 @@ func (c *ConfigEntry) PasswordlessMap() map[string]interface{} {
 		"use_token_groups":       c.UseTokenGroups,
 		"anonymous_group_search": c.AnonymousGroupSearch,
 		"request_timeout":        c.RequestTimeout,
+		"connection_timeout":     c.ConnectionTimeout,
 		"username_as_alias":      c.UsernameAsAlias,
 	}
 	if c.CaseSensitiveNames != nil {

sdk/helper/ldaputil/config_test.go

@@ -71,15 +71,16 @@ func testConfig(t *testing.T) *ConfigEntry {
 	t.Helper()
 
 	return &ConfigEntry{
-		Url:            "ldap://138.91.247.105",
-		UserDN:         "example,com",
-		BindDN:         "kitty",
-		BindPassword:   "cats",
-		TLSMaxVersion:  "tls12",
-		TLSMinVersion:  "tls12",
-		RequestTimeout: 30,
-		ClientTLSCert:  "",
-		ClientTLSKey:   "",
+		Url:               "ldap://138.91.247.105",
+		UserDN:            "example,com",
+		BindDN:            "kitty",
+		BindPassword:      "cats",
+		TLSMaxVersion:     "tls12",
+		TLSMinVersion:     "tls12",
+		RequestTimeout:    30,
+		ConnectionTimeout: 15,
+		ClientTLSCert:     "",
+		ClientTLSKey:      "",
 	}
 }
 
@@ -138,6 +139,7 @@ var jsonConfig = []byte(`{
 	"tls_max_version": "tls12",
 	"tls_min_version": "tls12",
 	"request_timeout": 30,
+	"connection_timeout": 15,
 	"ClientTLSCert":  "",
 	"ClientTLSKey":   ""
 }`)
@@ -168,6 +170,7 @@ var jsonConfigDefault = []byte(`
   "use_pre111_group_cn_behavior": null,
   "username_as_alias": false,
   "request_timeout": 90,
+  "connection_timeout": 30,
   "CaseSensitiveNames": false,
   "ClientTLSCert": "",
   "ClientTLSKey": ""

website/content/api-docs/auth/ldap.mdx

@@ -35,6 +35,9 @@ This endpoint configures the LDAP auth method.
   names will be normalized to lower case. Case will still be preserved when
   sending the username to the LDAP server at login time; this is only for
   matching local user/group definitions.
+- `connection_timeout` `(integer: 30 or string: "30s")` - Timeout, in seconds, 
+  when attempting to connect to the LDAP server before trying the next URL in 
+  the configuration.
 - `request_timeout` `(integer: 90 or string: "90s")` - Timeout, in seconds, for
   the connection when making requests against the server before returning back
   an error.

website/content/api-docs/secret/ad.mdx

@@ -47,6 +47,7 @@ text that fulfills those requirements. `{{PASSWORD}}` must appear exactly once a
 ### Connection parameters
 
 - `url` (string, optional) - The LDAP server to connect to. Examples: `ldaps://ldap.myorg.com`, `ldaps://ldap.myorg.com:636`. This can also be a comma-delineated list of URLs, e.g. `ldaps://ldap.myorg.com,ldaps://ldap.myorg.com:636`, in which case the servers will be tried in-order if there are errors during the connection process. Default is `ldap://127.0.0.1`.
+- `connection_timeout` `(integer: 30 or string: "30s")` - Timeout, in seconds, when attempting to connect to the LDAP server before trying the next URL in the configuration.
 - `request_timeout` `(integer: 90 or string: "90s")` - Timeout, in seconds, for the connection when making requests against the server before returning back an error.
 - `starttls` (bool, optional) - If true, issues a `StartTLS` command after establishing an unencrypted connection.
 - `insecure_tls` - (bool, optional) - If true, skips LDAP server SSL certificate verification - insecure, use with caution!

website/content/api-docs/secret/ldap.mdx

@@ -53,6 +53,9 @@ to search and change entry passwords in LDAP.
   string for authentication. The constructed UPN will appear as `[binddn]@[upndomain]`. For
   example, if `upndomain=example.com` and `binddn=admin`, the UPN string `admin@example.com`
   will be used to log in to Active Directory.
+- `connection_timeout` `(integer: 30 or string: "30s")` - Timeout, in seconds, 
+  when attempting to connect to the LDAP server before trying the next URL in 
+  the configuration.
 - `request_timeout` `(integer: 90, string: "90s" <optional>)` - Timeout, in seconds, for the connection when
   making requests against the server before returning back an error.
 - `starttls` `(bool: <optional>)` - If true, issues a `StartTLS` command after establishing an unencrypted connection.