VAULT-8099 LDAP secrets engine

[raymonstah]

Adds the LDAP secret engine along with the following resources:

• Config
• Static Role
• Dynamic Role
• Library Set

Adds the following data sources:

• Static Creds
• Dynamic Creds

Deprecates the existing AD secrets engine & resources / data sources.

[fairclothjm]

Max asked if there was a better way of handling this data other than HEREDOCs. I think the ldif attribute type "keys" can be custom. In that case, it might be possible to do something like kvv2 data_json

[raymonstah]

It seems like it's also possible to send a base64 encoded string as well:
https://developer.hashicorp.com/vault/api-docs/secret/ldap#creation_ldif

Alternatively, the user can create static LDIF files and reference them via the file function?
https://developer.hashicorp.com/terraform/language/functions/file

[fairclothjm]

right, I forgot about the base64 option. I am guessing using an LDIF file will be the most used scenario? In that case, I think we are good as-is.

[fairclothjm]

LGTM overall! Just wondering if we should change how we handle creation_ldif, deletion_ldif, and rollback_ldif to be more user friendly?

[bmhughes]

I'd been working on implementing this myself (closed #1860 in favour of this) and noticed a couple of things that I'd discovered myself during testing.

1. The schema property isn't being read back so any drift won't be detected, I've only looked quickly so there might be some more properties missing also. This is especially important as vault seems to be setting schema back to openldap every time the configuration is updated without schema being sent in the request body. Guessing it's a vault bug but the provider should be catching the drift anyway imo.
2. The connection_timeout property is missing from the backend resource.
3. Adding a ConflictsWith to length and password_policy would be beneficial.
4. I'd implemented this on the ttl properties so you can pass a time duration string as per the docs or seconds as a string.

StateFunc: func (v interface{}) string {
	duration, err := time.ParseDuration(v.(string))
	if err == nil {
		return fmt.Sprintf("%.0f", duration.Seconds())
	} else {
		return v.(string)
	}
}

Looks good other than that!

[fairclothjm]

@bmhughes Hi, thanks for pointing that out!

@raymonstah I think the following changes should fix the schema issue:

resource_ldap_secret_backend.go diff

diff --git a/vault/resource_ldap_secret_backend.go b/vault/resource_ldap_secret_backend.go
index 19d8c6c7..6f2876e6 100644
--- a/vault/resource_ldap_secret_backend.go
+++ b/vault/resource_ldap_secret_backend.go
@@ -159,6 +159,7 @@ func createUpdateLDAPConfigResource(ctx context.Context, d *schema.ResourceData,
 		consts.FieldLength,
 		consts.FieldPasswordPolicy,
 		consts.FieldRequestTimeout,
+		consts.FieldSchema,
 		consts.FieldUPNDomain,
 		consts.FieldURL,
 		consts.FieldUserAttr,
@@ -219,6 +220,7 @@ func readLDAPConfigResource(ctx context.Context, d *schema.ResourceData, meta in
 		consts.FieldLength,
 		consts.FieldPasswordPolicy,
 		consts.FieldRequestTimeout,
+		consts.FieldSchema,
 		consts.FieldStartTLS,
 		consts.FieldUPNDomain,
 		consts.FieldURL,
diff --git a/vault/resource_ldap_secret_backend_test.go b/vault/resource_ldap_secret_backend_test.go
index eb947a25..40d13154 100644
--- a/vault/resource_ldap_secret_backend_test.go
+++ b/vault/resource_ldap_secret_backend_test.go
@@ -22,9 +22,9 @@ To test, run the openldap service provided in the docker-compose.yaml file:
 
 Then export the following environment variables:
 
-	LDAP_BINDDN=cn=admin,dc=example,dc=org
-	LDAP_BINDPASS=adminpassword
-	LDAP_URL=ldap://localhost:1389
+	export LDAP_BINDDN=cn=admin,dc=example,dc=org
+	export LDAP_BINDPASS=adminpassword
+	export LDAP_URL=ldap://localhost:1389
 */
 func TestLDAPSecretBackend(t *testing.T) {
 	var (
@@ -37,6 +37,7 @@ func TestLDAPSecretBackend(t *testing.T) {
 		userDN                = "CN=Users,DC=corp,DC=example,DC=net"
 		updatedUserDN         = "CN=Users,DC=corp,DC=hashicorp,DC=com"
 	)
+
 	resource.Test(t, resource.TestCase{
 		ProviderFactories: providerFactories,
 		PreCheck: func() {
@@ -48,6 +49,7 @@ func TestLDAPSecretBackend(t *testing.T) {
 			{
 				Config: testLDAPSecretBackendConfig_defaults(bindDN, bindPass),
 				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, consts.FieldSchema, "openldap"),
 					resource.TestCheckResourceAttr(resourceName, consts.FieldPath, "ldap"),
 					resource.TestCheckResourceAttr(resourceName, consts.FieldDescription, description),
 					resource.TestCheckResourceAttr(resourceName, consts.FieldDefaultLeaseTTL, "0"),
@@ -60,8 +62,9 @@ func TestLDAPSecretBackend(t *testing.T) {
 				),
 			},
 			{
-				Config: testLDAPSecretBackendConfig(path, description, bindDN, bindPass, url, userDN, true),
+				Config: testLDAPSecretBackendConfig(path, description, bindDN, bindPass, url, userDN, "", true),
 				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, consts.FieldSchema, "openldap"),
 					resource.TestCheckResourceAttr(resourceName, consts.FieldPath, path),
 					resource.TestCheckResourceAttr(resourceName, consts.FieldDescription, description),
 					resource.TestCheckResourceAttr(resourceName, consts.FieldDefaultLeaseTTL, "3600"),
@@ -74,8 +77,37 @@ func TestLDAPSecretBackend(t *testing.T) {
 				),
 			},
 			{
-				Config: testLDAPSecretBackendConfig(path, updatedDescription, bindDN, bindPass, url, updatedUserDN, false),
+				Config: testLDAPSecretBackendConfig(path, updatedDescription, bindDN, bindPass, url, updatedUserDN, "", false),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, consts.FieldSchema, "openldap"),
+					resource.TestCheckResourceAttr(resourceName, consts.FieldPath, path),
+					resource.TestCheckResourceAttr(resourceName, consts.FieldDescription, updatedDescription),
+					resource.TestCheckResourceAttr(resourceName, consts.FieldDefaultLeaseTTL, "3600"),
+					resource.TestCheckResourceAttr(resourceName, consts.FieldMaxLeaseTTL, "7200"),
+					resource.TestCheckResourceAttr(resourceName, consts.FieldBindDN, bindDN),
+					resource.TestCheckResourceAttr(resourceName, consts.FieldBindPass, bindPass),
+					resource.TestCheckResourceAttr(resourceName, consts.FieldURL, url),
+					resource.TestCheckResourceAttr(resourceName, consts.FieldUserDN, updatedUserDN),
+					resource.TestCheckResourceAttr(resourceName, consts.FieldInsecureTLS, "false"),
+				),
+			},
+			testutil.GetImportTestStep(resourceName, false, nil,
+				consts.FieldBindPass, consts.FieldDescription, consts.FieldDisableRemount),
+		},
+	})
+
+	resource.Test(t, resource.TestCase{
+		ProviderFactories: providerFactories,
+		PreCheck: func() {
+			testutil.TestAccPreCheck(t)
+			SkipIfAPIVersionLT(t, testProvider.Meta(), provider.VaultVersion112)
+		}, PreventPostDestroyRefresh: true,
+		CheckDestroy: testCheckMountDestroyed(resourceType, consts.MountTypeLDAP, consts.FieldPath),
+		Steps: []resource.TestStep{
+			{
+				Config: testLDAPSecretBackendConfig(path, updatedDescription, bindDN, bindPass, url, updatedUserDN, `schema = "ad"`, false),
 				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, consts.FieldSchema, "ad"),
 					resource.TestCheckResourceAttr(resourceName, consts.FieldPath, path),
 					resource.TestCheckResourceAttr(resourceName, consts.FieldDescription, updatedDescription),
 					resource.TestCheckResourceAttr(resourceName, consts.FieldDefaultLeaseTTL, "3600"),
@@ -103,7 +135,7 @@ resource "vault_ldap_secret_backend" "test" {
 }`, bindDN, bindPass)
 }
 
-func testLDAPSecretBackendConfig(mount, description, bindDN, bindPass, url, userDN string, insecureTLS bool) string {
+func testLDAPSecretBackendConfig(mount, description, bindDN, bindPass, url, userDN, extraConfig string, insecureTLS bool) string {
 	return fmt.Sprintf(`
 resource "vault_ldap_secret_backend" "test" {
   path                      = "%s"
@@ -115,6 +147,7 @@ resource "vault_ldap_secret_backend" "test" {
   url                       = "%s"
   userdn                    = "%s"
   insecure_tls              = %v
+  %s
 }
-`, mount, description, bindDN, bindPass, url, userDN, insecureTLS)
+`, mount, description, bindDN, bindPass, url, userDN, insecureTLS, extraConfig)
 }

[raymonstah]

@bmhughes thanks for the feedback! I've updated the PR to take in your suggestions. Regarding your 4th point, we had a discussion and ultimately decided it would be easier to only accept int values. See #1821 (comment) for more info.

[bmhughes]

No worries, glad I could help!

[vinay-gopalan]

LGTM! Had a comment around missing sidebar entry, but looks good otherwise 🎉