From a1d8ed59f0643010f336e4d69ff5752c0752fae4 Mon Sep 17 00:00:00 2001 From: Lilian Deloche Date: Tue, 17 Dec 2024 17:35:58 +0100 Subject: [PATCH] review 1: change behaviour on revocation --- vault/resource_pki_secret_backend_cert.go | 35 ++++++++++--------- .../resource_pki_secret_backend_cert_test.go | 23 +++++++++--- .../docs/r/pki_secret_backend_cert.html.md | 4 +-- 3 files changed, 39 insertions(+), 23 deletions(-) diff --git a/vault/resource_pki_secret_backend_cert.go b/vault/resource_pki_secret_backend_cert.go index 345deb3d3..c81083c7b 100644 --- a/vault/resource_pki_secret_backend_cert.go +++ b/vault/resource_pki_secret_backend_cert.go @@ -166,16 +166,18 @@ func pkiSecretBackendCertResource() *schema.Resource { "the expiration is less than min_seconds_remaining in the future.", }, consts.FieldRevoke: { - Type: schema.TypeBool, - Optional: true, - Default: false, - Description: "Revoke the certificate upon resource destruction.", + Type: schema.TypeBool, + Optional: true, + Default: false, + Description: "Revoke the certificate upon resource destruction.", + ConflictsWith: []string{consts.FieldRevokeWithKey}, }, consts.FieldRevokeWithKey: { - Type: schema.TypeBool, - Optional: true, - Default: false, - Description: "Revoke the certificate with private key method", + Type: schema.TypeBool, + Optional: true, + Default: false, + Description: "Revoke the certificate with private key method upon resource destruction.", + ConflictsWith: []string{consts.FieldRevoke}, }, consts.FieldIssuerRef: { Type: schema.TypeString, @@ -363,26 +365,27 @@ func pkiSecretBackendCertUpdate(ctx context.Context, d *schema.ResourceData, m i } func pkiSecretBackendCertDelete(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - if d.Get(consts.FieldRevoke).(bool) { + var revokeWithKey bool + if d.Get(consts.FieldRevokeWithKey) != nil { + revokeWithKey = d.Get(consts.FieldRevokeWithKey).(bool) + } else { + revokeWithKey = false + } + if d.Get(consts.FieldRevoke).(bool) || revokeWithKey { client, e := provider.GetClient(d, meta) if e != nil { return diag.FromErr(e) } backend := d.Get(consts.FieldBackend).(string) - - privateKey := d.Get(consts.FieldPrivateKey).(string) serialNumber := d.Get(consts.FieldSerialNumber).(string) commonName := d.Get(consts.FieldCommonName).(string) - revokeWithKey := d.Get(consts.FieldRevokeWithKey).(bool) data := map[string]interface{}{ consts.FieldSerialNumber: serialNumber, } - if revokeWithKey { - data["private_key"] = privateKey - } var path string if revokeWithKey { + data["private_key"] = d.Get(consts.FieldPrivateKey).(string) path = strings.Trim(backend, "/") + "/revoke-with-key" } else { path = strings.Trim(backend, "/") + "/revoke" @@ -391,7 +394,7 @@ func pkiSecretBackendCertDelete(_ context.Context, d *schema.ResourceData, meta log.Printf("[DEBUG] Revoking certificate %q with serial number %q on PKI secret backend %q", commonName, serialNumber, backend) _, err := client.Logical().Write(path, data) - + if err != nil { return diag.Errorf("error revoking certificate %q with serial number %q for PKI secret backend %q: %s", commonName, serialNumber, backend, err) diff --git a/vault/resource_pki_secret_backend_cert_test.go b/vault/resource_pki_secret_backend_cert_test.go index 52bcb2521..932a9ca3e 100644 --- a/vault/resource_pki_secret_backend_cert_test.go +++ b/vault/resource_pki_secret_backend_cert_test.go @@ -98,7 +98,7 @@ func TestPkiSecretBackendCert_basic(t *testing.T) { Config: testPkiSecretBackendCertConfig_basic(rootPath, intermediatePath, true, true, true), Check: resource.ComposeTestCheckFunc( append(checks, - resource.TestCheckResourceAttr(resourceName, "revoke", "true"), + resource.TestCheckResourceAttr(resourceName, "revoke_with_key", "true"), testPKICertRevocation(intermediatePath, store), testCapturePKICert(resourceName, store), )..., @@ -179,20 +179,33 @@ resource "vault_pki_secret_backend_role" "test" { `, rootPath, intermediatePath), } - if withCert { + if withCert && !revokeWithKey { fragments = append(fragments, fmt.Sprintf(` resource "vault_pki_secret_backend_cert" "test" { backend = vault_pki_secret_backend_role.test.backend name = vault_pki_secret_backend_role.test.name common_name = "cert.test.my.domain" uri_sans = ["spiffe://test.my.domain"] - user_ids = ["foo", "bar"] + user_ids = ["foo", "bar"] ttl = "720h" min_seconds_remaining = 60 revoke = %t - revoke_with_key = %t } -`, revoke, revokeWithKey)) +`, revoke)) + } + if revokeWithKey && withCert { + fragments = append(fragments, ` +resource "vault_pki_secret_backend_cert" "test" { + backend = vault_pki_secret_backend_role.test.backend + name = vault_pki_secret_backend_role.test.name + common_name = "cert.test.my.domain" + uri_sans = ["spiffe://test.my.domain"] + user_ids = ["foo", "bar"] + ttl = "720h" + min_seconds_remaining = 60 + revoke_with_key = true +} +`) } return strings.Join(fragments, "\n") diff --git a/website/docs/r/pki_secret_backend_cert.html.md b/website/docs/r/pki_secret_backend_cert.html.md index ed91bb95b..0571ccc56 100755 --- a/website/docs/r/pki_secret_backend_cert.html.md +++ b/website/docs/r/pki_secret_backend_cert.html.md @@ -67,9 +67,9 @@ The following arguments are supported: * `auto_renew` - (Optional) If set to `true`, certs will be renewed if the expiration is within `min_seconds_remaining`. Default `false` -* `revoke` - If set to `true`, the certificate will be revoked on resource destruction. +* `revoke` - If set to `true`, the certificate will be revoked on resource destruction. Needs privileged access on Vault engine. Conflicts with `revoke_with_key`. Default `false`. -* `revoke_with_key` - if set to `true`, use method `revoke-with-key` to revoke the certificate on resource destruction. Used to revoke certificate without using privileged operation. +* `revoke_with_key` - If set to `true`, use method `revoke-with-key` to revoke the certificate on resource destruction. Used to revoke certificate without using privileged operation. Conflicts with `revoke`. Default `false` ## Attributes Reference