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.

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

vault_pki_secret_backend_cert: Report when renewal is pending #1597

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

apparentlymart
Copy link
Contributor

@apparentlymart apparentlymart commented Sep 7, 2022

(This actually affects both vault_pki_secret_backend_cert and vault_pki_secret_backend_sign in the same way, because they share the same logic for refreshing and planning.)

This exports an additional boolean attribute renew_pending, which will start set to false but will transition to true during refresh if the current time is less than min_seconds_remaining seconds before the certificate's expiration time. The auto renew behavior is then triggered by this new attribute becoming true, ensuring that these two behaviors will always agree with one another about when renewal is pending.

This adds a little extra information to the plan output to explain why the provider is proposing to replace the object, and also adds a useful hook for postconditions that wish to detect (e.g. during a refresh-only plan) that a renewal is pending.

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Release note for CHANGELOG:

IMPROVEMENTS:
* Add attribute `renew_pending` to `vault_pki_secret_backend_cert` and `vault_pki_secret_backend_sign` to expose the provider's decision about whether the object is within its renewal period.

Output from acceptance testing:

# (using vault 1.11.3 on linux_amd64 with a server in dev mode)

$ TF_ACC=1 go test -v -run=TestPkiSecretBackendCert -timeout 30m ./...
?       github.com/hashicorp/terraform-provider-vault   [no test files]
?       github.com/hashicorp/terraform-provider-vault/cmd/coverage      [no test files]
?       github.com/hashicorp/terraform-provider-vault/cmd/generate      [no test files]
ok      github.com/hashicorp/terraform-provider-vault/codegen   0.079s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/generated [no test files]
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/decode    0.212s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/encode    0.471s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/alphabet    0.375s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/role        0.220s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/template    0.442s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/transformation      0.213s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/helper    [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/consts   [no test files]
ok      github.com/hashicorp/terraform-provider-vault/internal/identity/entity  0.173s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/internal/identity/group   [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/pki      [no test files]
ok      github.com/hashicorp/terraform-provider-vault/internal/provider 0.491s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/internal/semver   0.131s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/schema    [no test files]
ok      github.com/hashicorp/terraform-provider-vault/testutil  0.105s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/util      0.115s [no tests to run]
2022/09/07 09:15:30 [INFO] Using Vault token with the following policies: root
=== RUN   TestPkiSecretBackendCert_basic
--- PASS: TestPkiSecretBackendCert_basic (6.05s)
=== RUN   TestPkiSecretBackendCert_renew
--- PASS: TestPkiSecretBackendCert_renew (13.74s)
PASS
ok      github.com/hashicorp/terraform-provider-vault/vault     20.015s

$ TF_ACC=1 go test -v -run=TestPkiSecretBackendSign -timeout 30m ./...
?       github.com/hashicorp/terraform-provider-vault   [no test files]
?       github.com/hashicorp/terraform-provider-vault/cmd/coverage      [no test files]
?       github.com/hashicorp/terraform-provider-vault/cmd/generate      [no test files]
ok      github.com/hashicorp/terraform-provider-vault/codegen   0.092s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/generated [no test files]
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/decode    0.505s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/encode    0.496s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/alphabet    0.202s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/role        0.233s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/template    0.567s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/transformation      0.257s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/helper    [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/consts   [no test files]
ok      github.com/hashicorp/terraform-provider-vault/internal/identity/entity  0.153s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/internal/identity/group   [no test files]
?       github.com/hashicorp/terraform-provider-vault/internal/pki      [no test files]
ok      github.com/hashicorp/terraform-provider-vault/internal/provider 0.204s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/internal/semver   0.129s [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/schema    [no test files]
ok      github.com/hashicorp/terraform-provider-vault/testutil  0.218s [no tests to run]
ok      github.com/hashicorp/terraform-provider-vault/util      0.109s [no tests to run]
2022/09/07 09:28:16 [INFO] Using Vault token with the following policies: root
=== RUN   TestPkiSecretBackendSign_basic
--- PASS: TestPkiSecretBackendSign_basic (5.34s)
=== RUN   TestPkiSecretBackendSign_renew
--- PASS: TestPkiSecretBackendSign_renew (11.67s)
PASS
ok      github.com/hashicorp/terraform-provider-vault/vault     17.235s

@apparentlymart
Copy link
Contributor Author

It seems that the CustomizeDiff implementation for this resource type is shared with at least one other resource type, and so that other resource type is now panicking during planning. I'll dig into that now and see whether it makes sense to extend that one with a similar renew_pending attribute.

@apparentlymart apparentlymart marked this pull request as draft September 7, 2022 16:24
@apparentlymart apparentlymart force-pushed the f-pki_secret_backend_cert-renew_pending branch from da6005d to 48ff7fe Compare September 7, 2022 16:30
@apparentlymart
Copy link
Contributor Author

vault_pki_secret_backend_sign shares the same Read and CustomizeDiff functions and so I've assumed here that it is intended to have comparable functionality to vault_pki_secret_backend_cert. I've therefore updated it in the same way, so that both will now have the renew_pending attribute.

@apparentlymart apparentlymart marked this pull request as ready for review September 7, 2022 17:02
@benashz benashz added this to the 3.9.0 milestone Sep 7, 2022
Copy link
Contributor

@benashz benashz left a comment

Choose a reason for hiding this comment

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

Looks great! Only few minor nits/suggestions.

vault/resource_pki_secret_backend_cert.go Outdated Show resolved Hide resolved
vault/resource_pki_secret_backend_cert.go Outdated Show resolved Hide resolved
vault/resource_pki_secret_backend_sign.go Outdated Show resolved Hide resolved
vault/resource_pki_secret_backend_sign.go Outdated Show resolved Hide resolved
(This also includes vault_pki_secret_backend_sign, which shares the same
auto-renew logic.)

This exports an additional boolean attribute renew_pending, which should
start set to false but will transition to true during refresh if the
current time is less than "min_seconds_remaining" seconds before the
certificate's expiration time.

This adds a little extra information to the plan output to explain why
the provider is proposing to replace the object, and also adds a useful
hook for postconditions that wish to detect (e.g. during a refresh-only
plan) that a renewal is pending.
@apparentlymart apparentlymart force-pushed the f-pki_secret_backend_cert-renew_pending branch from 48ff7fe to 0f937f9 Compare September 7, 2022 21:25
Copy link
Contributor

@benashz benashz left a comment

Choose a reason for hiding this comment

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

LGTM!

@apparentlymart apparentlymart merged commit f3dd2d9 into main Sep 8, 2022
@apparentlymart apparentlymart deleted the f-pki_secret_backend_cert-renew_pending branch September 8, 2022 15:51
marcboudreau pushed a commit to marcboudreau/terraform-provider-vault that referenced this pull request Nov 6, 2022
…orp#1597)

(This also includes vault_pki_secret_backend_sign, which shares the same
auto-renew logic.)

This exports an additional boolean attribute renew_pending, which should
start set to false but will transition to true during refresh if the
current time is less than "min_seconds_remaining" seconds before the
certificate's expiration time.

This adds a little extra information to the plan output to explain why
the provider is proposing to replace the object, and also adds a useful
hook for postconditions that wish to detect (e.g. during a refresh-only
plan) that a renewal is pending.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants