Skip to content

resource/ssh_secret_backend_ca: detect misconfigured resource and remove from state - #856

Merged
catsby merged 2 commits into
masterfrom
b-backend-ca-exists
Sep 1, 2020
Merged

resource/ssh_secret_backend_ca: detect misconfigured resource and remove from state#856
catsby merged 2 commits into
masterfrom
b-backend-ca-exists

Conversation

@catsby

@catsby catsby commented Aug 28, 2020

Copy link
Copy Markdown
Contributor

The vault_ssh_secret_backend_ca resource depends on a SSH secret backend mounted at path. If generate_signing_key is used (which is the default) this resource generates the signing key pair internally at creation time.

If the SSH secret engine gets replaced (meaning recreated, at the same path), the generated keys are lost but the backend_ca resource does not detect this because the path has not changed. In follow up state refreshes, the resource throws an error from Vault that the keys are not present:

URL: GET https://vault.foo.com/v1/ssh-devel/config/ca
Code: 400. Errors:

* keys haven't been configured yet

(See #846)

Unfortunately due to how Terraform works, because the path value does not change, the vault_ssh_secret_backend_ca does not get marked for update when the SSH resource is changed. Vault requires paths to be unique, but does not offer unique IDs for relationships, or at least the provider doesn't utilize them.

This PR does not completely fix this problem, however it improves error handling in the vault_ssh_secret_backend_ca resource such that if the "keys haven't been configured yet" error message is received, it removes the resource from state so that a future terraform plan can recreate it.

Improves, but does not really "fix", #846

@calvn calvn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good and seems reasonable as a stop-gap against breaking the statefile. The true fix is probably the one we discussed offline, where we properly handle a description update as a tune operation and not a mount/resource re-creation.

@catsby
catsby merged commit 3250cfa into master Sep 1, 2020
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
…ove from state (hashicorp#856)

* gracefully handle non-configured CA

* Add test that expects a non-empty plan and does not error when a mount is edited
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