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

Tune mount when description is changed #929

Merged
merged 3 commits into from
Dec 11, 2020
Merged

Tune mount when description is changed #929

merged 3 commits into from
Dec 11, 2020

Conversation

jasonodonnell
Copy link
Contributor

@jasonodonnell jasonodonnell commented Dec 10, 2020

The vault_mount resource has a bug where if you change the description of the mount, it deletes the mount and recreates. This is due to the parameter having ForceNew: true enabled in the schema.

Instead this should be tuning the description instead of deleting the resource when updating the mount description.

Fixes #909.
Fixes #782.

@ghost ghost added the size/XS label Dec 10, 2020
Copy link
Member

@tvoran tvoran left a comment

Choose a reason for hiding this comment

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

Works for me 👍

Only thought it is it might be good to update testResourceMount_updateConfig and testResourceMount_updateCheck() to actually update the mount description to something different than the initialConfig. i.e.

diff --git a/vault/resource_mount_test.go b/vault/resource_mount_test.go
index 07a6dcde..79341efb 100644
--- a/vault/resource_mount_test.go
+++ b/vault/resource_mount_test.go
@@ -241,7 +241,7 @@ func testResourceMount_initialCheck(cfg mountConfig) resource.TestCheckFunc {
 resource "vault_mount" "test" {
        path = "remountingExample"
        type = "kv"
-       description = "Example mount for testing"
+       description = "Updated example mount for testing"
        default_lease_ttl_seconds = 7200
        max_lease_ttl_seconds = 72000
        options = {
@@ -270,7 +270,7 @@ func testResourceMount_updateCheck(s *terraform.State) error {
                return fmt.Errorf("error reading back mount: %s", err)
        }
 
-       if wanted := "Example mount for testing"; mount.Description != wanted {
+       if wanted := "Updated example mount for testing"; mount.Description != wanted {
                return fmt.Errorf("description is %v; wanted %v", mount.Description, wanted)
        }
 

@jasonodonnell
Copy link
Contributor Author

jasonodonnell commented Dec 10, 2020

This test is failing https://github.com/hashicorp/terraform-provider-vault/blob/master/vault/resource_ssh_secret_backend_ca_test.go#L29-L48 but unsure why. If I run a similar test manually I'm getting a plan:

Terraform will perform the following actions:
  # vault_mount.test will be updated in-place
  ~ resource "vault_mount" "test" {
        accessor                  = "ssh_9b0f7347"
        default_lease_ttl_seconds = 0
      ~ description               = "something" -> "something else"
        external_entropy_access   = false
        id                        = "ssh"
        local                     = false
        max_lease_ttl_seconds     = 0
        options                   = {}
        path                      = "ssh"
        seal_wrap                 = false
        type                      = "ssh"
    }
Plan: 0 to add, 1 to change, 0 to destroy.

@ghost ghost added size/S and removed size/XS labels Dec 11, 2020
@jasonodonnell
Copy link
Contributor Author

After some review, @catsby and I agreed to remove the broken test because:

  • there's a bug in our current version of the terraform SDK that we can't resolve (upgrading introduces many changes beyond the scope of this PR)
  • the test wasn't particularly valuable.

Both @catsby and I manually verified the changes are good.

@jasonodonnell jasonodonnell merged commit 5adf586 into master Dec 11, 2020
@jasonodonnell jasonodonnell deleted the description branch December 11, 2020 14:58
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
* Tune mount when description is changed

* Update test

* Remove broken test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants