Skip to content

Commit

Permalink
Fix: cert-auth allowed_organizational_units handling (#1496)
Browse files Browse the repository at this point in the history 
Add tests to ensure that allowed_organizational_units field is properly
handled for all lifecycle changes.
  • Loading branch information
@benashz
benashz authored Jun 9, 2022
1 parent 187d3e4 commit 70a1872
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 53 deletions.
32 changes: 6 additions & 26 deletions vault/resource_cert_auth_backend_role.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ func certAuthBackendRoleResource() *schema.Resource {
Type: schema.TypeString,
},
Optional: true,
Computed: true,
ConflictsWith: []string{"allowed_organization_units"},
},
"required_extensions": {
Expand Down Expand Up @@ -155,11 +154,8 @@ func certAuthResourceWrite(ctx context.Context, d *schema.ResourceData, meta int
data["allowed_uri_sans"] = v.(*schema.Set).List()
}

for _, k := range []string{"allowed_organizational_units", "allowed_organization_units"} {
if v, ok := d.GetOk(k); ok {
data["allowed_organizational_units"] = v.(*schema.Set).List()
break
}
if v, ok := d.GetOk("allowed_organizational_units"); ok {
data["allowed_organizational_units"] = v.(*schema.Set).List()
}

if v, ok := d.GetOk("required_extensions"); ok {
Expand Down Expand Up @@ -210,11 +206,8 @@ func certAuthResourceUpdate(ctx context.Context, d *schema.ResourceData, meta in
data["allowed_uri_sans"] = v.(*schema.Set).List()
}

for _, k := range []string{"allowed_organizational_units", "allowed_organization_units"} {
if v, ok := d.GetOk(k); ok {
data["allowed_organizational_units"] = v.(*schema.Set).List()
break
}
if d.HasChange("allowed_organizational_units") {
data["allowed_organizational_units"] = d.Get("allowed_organizational_units").(*schema.Set).List()
}

if v, ok := d.GetOk("required_extensions"); ok {
Expand Down Expand Up @@ -306,17 +299,6 @@ func certAuthResourceRead(_ context.Context, d *schema.ResourceData, meta interf
schema.HashString, []interface{}{}))
}

// Vault sometimes returns these as null instead of an empty list.
if resp.Data["allowed_organization_units"] != nil {
d.Set("allowed_organization_units",
schema.NewSet(
schema.HashString, resp.Data["allowed_organization_units"].([]interface{})))
} else {
d.Set("allowed_organization_units",
schema.NewSet(
schema.HashString, []interface{}{}))
}

// Vault sometimes returns these as null instead of an empty list.
if resp.Data["required_extensions"] != nil {
d.Set("required_extensions",
Expand All @@ -328,10 +310,8 @@ func certAuthResourceRead(_ context.Context, d *schema.ResourceData, meta interf
schema.HashString, []interface{}{}))
}

for _, k := range []string{"allowed_organizational_units", "allowed_organization_units"} {
if err := d.Set(k, resp.Data["allowed_organizational_units"]); err != nil {
return diag.FromErr(err)
}
if err := d.Set("allowed_organizational_units", resp.Data["allowed_organizational_units"]); err != nil {
return diag.FromErr(err)
}

diags := checkCIDRs(d, TokenFieldBoundCIDRs)
Expand Down
52 changes: 26 additions & 26 deletions vault/resource_cert_auth_backend_role_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/hashicorp/terraform-provider-vault/internal/provider"
"github.com/hashicorp/terraform-provider-vault/testutil"
"github.com/hashicorp/terraform-provider-vault/util"
)

const testCertificate = `
Expand Down Expand Up @@ -74,21 +75,26 @@ func TestCertAuthBackend(t *testing.T) {
acctest.RandomWithPrefix("tf-ident-2"),
}

allowedOrgUnits := []string{"foo", "baz"}

resourceName := "vault_cert_auth_backend_role.test"
resource.Test(t, resource.TestCase{
PreCheck: func() { testutil.TestAccPreCheck(t) },
Providers: testProviders,
CheckDestroy: testCertAuthBackendDestroy,
Steps: []resource.TestStep{
{
Config: testCertAuthBackendConfig_basic(backend, name, testCertificate, allowedNames),
Config: testCertAuthBackendConfig_basic(backend, name, testCertificate, allowedNames, allowedOrgUnits),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "backend", backend),
resource.TestCheckResourceAttr(resourceName, "name", name),
resource.TestCheckResourceAttr(resourceName, "token_policies.#", "2"),
resource.TestCheckResourceAttr(resourceName, "token_ttl", "300"),
resource.TestCheckResourceAttr(resourceName, "token_max_ttl", "600"),
resource.TestCheckResourceAttr(resourceName, "allowed_names.#", "2"),
resource.TestCheckResourceAttr(resourceName, "allowed_organizational_units.#", "2"),
resource.TestCheckTypeSetElemAttr(resourceName, "allowed_organizational_units.*", "foo"),
resource.TestCheckTypeSetElemAttr(resourceName, "allowed_organizational_units.*", "baz"),
testCertAuthBackendCheck_attrs(resourceName, backend, name),
),
},
Expand All @@ -101,6 +107,7 @@ func TestCertAuthBackend(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "token_ttl", "0"),
resource.TestCheckResourceAttr(resourceName, "token_max_ttl", "0"),
resource.TestCheckResourceAttr(resourceName, "allowed_names.#", "2"),
resource.TestCheckResourceAttr(resourceName, "allowed_organizational_units.#", "0"),
testCertAuthBackendCheck_attrs(resourceName, backend, name),
),
},
Expand Down Expand Up @@ -165,7 +172,6 @@ func testCertAuthBackendCheck_attrs(resourceName, backend, name string) resource
"allowed_dns_sans": "allowed_dns_sans",
"allowed_email_sans": "allowed_email_sans",
"allowed_uri_sans": "allowed_uri_sans",
"allowed_organization_units": "allowed_organizational_units",
"allowed_organizational_units": "allowed_organizational_units",
"required_extensions": "required_extensions",
"certificate": "certificate",
Expand All @@ -183,7 +189,7 @@ func testCertAuthBackendCheck_attrs(resourceName, backend, name string) resource
VaultAttr: v,
}
switch k {
case TokenFieldPolicies, "allowed_names":
case TokenFieldPolicies, "allowed_names", "allowed_organizational_units":
ta.AsSet = true
}

Expand All @@ -194,13 +200,8 @@ func testCertAuthBackendCheck_attrs(resourceName, backend, name string) resource
}
}

func testCertAuthBackendConfig_basic(backend, name, certificate string, allowedNames []string) string {
quotedNames := make([]string, len(allowedNames))
for idx, name := range allowedNames {
quotedNames[idx] = fmt.Sprintf(`"%s"`, name)
}

return fmt.Sprintf(`
func testCertAuthBackendConfig_basic(backend, name, certificate string, allowedNames, allowedOrgUnits []string) string {
config := fmt.Sprintf(`
resource "vault_auth_backend" "cert" {
path = "%s"
Expand All @@ -209,26 +210,23 @@ resource "vault_auth_backend" "cert" {
resource "vault_cert_auth_backend_role" "test" {
name = "%s"
certificate = <<__CERTIFICATE__
certificate = <<EOF
%s
__CERTIFICATE__
allowed_names = [%s]
backend = vault_auth_backend.cert.path
token_ttl = 300
token_max_ttl = 600
token_policies = ["test_policy_1", "test_policy_2"]
EOF
allowed_names = %s
backend = vault_auth_backend.cert.path
token_ttl = 300
token_max_ttl = 600
token_policies = ["test_policy_1", "test_policy_2"]
allowed_organizational_units = %s
}
`, backend, name, certificate, util.ArrayToTerraformList(allowedNames), util.ArrayToTerraformList(allowedOrgUnits))

`, backend, name, certificate, strings.Join(quotedNames, ", "))
return config
}

func testCertAuthBackendConfig_unset(backend, name, certificate string, allowedNames []string) string {
quotedNames := make([]string, len(allowedNames))
for idx, name := range allowedNames {
quotedNames[idx] = fmt.Sprintf(`"%s"`, name)
}

return fmt.Sprintf(`
config := fmt.Sprintf(`
resource "vault_auth_backend" "cert" {
path = "%s"
Expand All @@ -240,9 +238,11 @@ resource "vault_cert_auth_backend_role" "test" {
certificate = <<__CERTIFICATE__
%s
__CERTIFICATE__
allowed_names = [%s]
allowed_names = %s
backend = vault_auth_backend.cert.path
}
`, backend, name, certificate, util.ArrayToTerraformList(allowedNames),
)

`, backend, name, certificate, strings.Join(quotedNames, ", "))
return config
}
3 changes: 2 additions & 1 deletion website/docs/r/cert_auth_backend_role.html.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ The following arguments are supported:

* `allowed_uri_sans` - (Optional) Allowed URIs for authenticated client certificates

* `allowed_organization_units` - (Optional) Allowed organization units for authenticated client certificates
* `allowed_organizational_units` - (Optional) Allowed organization units for authenticated client certificates.
*In previous provider releases this field was incorrectly named `allowed_organization_units`, please update accordingly*

* `required_extensions` - (Optional) TLS extensions required on client certificates

Expand Down

0 comments on commit 70a1872

Please sign in to comment.