aidanmelen / terraform-provider-snowsql

Terraform SnowSQL provider
https://registry.terraform.io/providers/aidanmelen/snowsql/latest
Other
22 stars 10 forks source link

Implement minimal update lifecycle #64

Closed christophkreutzer closed 1 year ago

christophkreutzer commented 1 year ago

As discussed in #60

@aidanmelen Would be interested in your feedback on this, thanks!

Fixes #60

aidanmelen commented 1 year ago

That tests are happy. I am going to pull this down and smoke test locally as well

aidanmelen commented 1 year ago

start with this:

resource "snowsql_exec" "user1" {
  name = "${local.name}_user1"

  create {
    statements = "CREATE USER IF NOT EXISTS ${local.name}_user1 PASSWORD='abc123' DEFAULT_ROLE = myrole DEFAULT_SECONDARY_ROLES = ('ALL') MUST_CHANGE_PASSWORD = TRUE;"
  }

  # update {
  #   statements = "ALTER USER IF EXISTS ${local.name}_user1 SET EMAIL = '${local.name}_user1@mail.com';"
  # }

  delete {
    statements = "DROP USER IF EXISTS ${local.name}_user1;"
  }
}

result on first run:

$ terraform apply --auto-approve
Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # random_pet.server will be created
  + resource "random_pet" "server" {
      + id        = (known after apply)
      + length    = 2
      + prefix    = "SIMPLE_EXAMPLE"
      + separator = "_"
    }

  # snowsql_exec.user1 will be created
  + resource "snowsql_exec" "user1" {
      + id   = (known after apply)
      + name = (known after apply)

      + create {
          + number_of_statements = -1
          + statements           = (known after apply)
        }

      + delete {
          + number_of_statements = -1
          + statements           = (known after apply)
        }
    }

Plan: 2 to add, 0 to change, 0 to destroy.
random_pet.server: Creating...
random_pet.server: Creation complete after 0s [id=SIMPLE_EXAMPLE_tight_grubworm]
snowsql_exec.user1: Creating...
snowsql_exec.user1: Creation complete after 1s [id=SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1]

Apply complete! Resources: 2 added, 0 changed, 0 destroyed.

Then check the users email:

SHOW USERS LIKE '%SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1%';
-- name,created_on,login_name,display_name,first_name,last_name,email,mins_to_unlock,days_to_expiry,comment,disabled,must_change_password,snowflake_lock,default_warehouse,default_namespace,default_role,default_secondary_roles,ext_authn_duo,ext_authn_uid,mins_to_bypass_mfa,owner,last_success_login,expires_at_time,locked_until_time,has_password,has_rsa_public_key
-- SIMPLE_EXAMPLE_TIGHT_GRUBWORM_USER1,2023-02-10T07:52:29.434-08:00,SIMPLE_EXAMPLE_TIGHT_GRUBWORM_USER1,SIMPLE_EXAMPLE_TIGHT_GRUBWORM_USER1,,,,,,,false,true,false,,,MYROLE,"[""ALL""]",false,,,ACCOUNTADMIN,,,,true,false

Now update with email:

resource "snowsql_exec" "user1" {
  name = "${local.name}_user1"

  create {
    statements = "CREATE USER IF NOT EXISTS ${local.name}_user1 PASSWORD='abc123' DEFAULT_ROLE = myrole DEFAULT_SECONDARY_ROLES = ('ALL') MUST_CHANGE_PASSWORD = TRUE;"
  }

  update {
    statements = "ALTER USER IF EXISTS ${local.name}_user1 SET EMAIL = '${local.name}_user1@mail.com';"
  }

  delete {
    statements = "DROP USER IF EXISTS ${local.name}_user1;"
  }
}

which results in the in-place update:

$ terraform apply --auto-approve
random_pet.server: Refreshing state... [id=SIMPLE_EXAMPLE_tight_grubworm]
snowsql_exec.user1: Refreshing state... [id=SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # snowsql_exec.user1 will be updated in-place
  ~ resource "snowsql_exec" "user1" {
        id   = "SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1"
        name = "SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1"

      + update {
          + number_of_statements = -1
          + statements           = "ALTER USER IF EXISTS SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1 SET EMAIL = 'SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1@mail.com';"
        }
        # (2 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

snowsql_exec.user1: Modifying... [id=SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1]
snowsql_exec.user1: Modifications complete after 1s [id=SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

Then check the users email again:

SHOW USERS LIKE '%SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1%';
-- name,created_on,login_name,display_name,first_name,last_name,email,mins_to_unlock,days_to_expiry,comment,disabled,must_change_password,snowflake_lock,default_warehouse,default_namespace,default_role,default_secondary_roles,ext_authn_duo,ext_authn_uid,mins_to_bypass_mfa,owner,last_success_login,expires_at_time,locked_until_time,has_password,has_rsa_public_key
-- SIMPLE_EXAMPLE_TIGHT_GRUBWORM_USER1,2023-02-10T07:52:29.434-08:00,SIMPLE_EXAMPLE_TIGHT_GRUBWORM_USER1,SIMPLE_EXAMPLE_TIGHT_GRUBWORM_USER1,,,SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1@mail.com,,,,false,true,false,,,MYROLE,"[""ALL""]",false,,,ACCOUNTADMIN,,,,true,false

apply idempontently without any changes:

$ terraform apply --auto-approve
random_pet.server: Refreshing state... [id=SIMPLE_EXAMPLE_tight_grubworm]
snowsql_exec.user1: Refreshing state... [id=SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Now change the name:

resource "snowsql_exec" "user1" {
  name = "${local.name}_user1_changed"

  create {
    statements = "CREATE USER IF NOT EXISTS ${local.name}_user1 PASSWORD='abc123' DEFAULT_ROLE = myrole DEFAULT_SECONDARY_ROLES = ('ALL') MUST_CHANGE_PASSWORD = TRUE;"
  }

  update {
    statements = "ALTER USER IF EXISTS ${local.name}_user1 SET EMAIL = '${local.name}_user1@mail.com';"
  }

  delete {
    statements = "DROP USER IF EXISTS ${local.name}_user1;"
  }
}

which should result in the replacement of the user resource:

terraform apply --auto-approve
random_pet.server: Refreshing state... [id=SIMPLE_EXAMPLE_tight_grubworm]
snowsql_exec.user1: Refreshing state... [id=SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # snowsql_exec.user1 must be replaced
-/+ resource "snowsql_exec" "user1" {
      ~ id   = "SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1" -> (known after apply)
      ~ name = "SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1" -> "SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1_changed" # forces replacement

        # (3 unchanged blocks hidden)
    }

Plan: 1 to add, 0 to change, 1 to destroy.

snowsql_exec.user1: Destroying... [id=SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1]
snowsql_exec.user1: Destruction complete after 1s
snowsql_exec.user1: Creating...
snowsql_exec.user1: Creation complete after 1s [id=SIMPLE_EXAMPLE_TIGHT_GRUBWORM_user1_changed]

Apply complete! Resources: 1 added, 0 changed, 1 destroyed.

@christophkreutzer interestingly, the name change caused a replacement of the resource. I am going to invalidate my early comment.

christophkreutzer commented 1 year ago

@christophkreutzer interestingly, the name change caused a replacement of the resource. I am going to invalidate my early comment.

Yes, I think ForceNew is doing it's thing here :) But thanks for explicitly testing it!

aidanmelen commented 1 year ago

@christophkreutzer I also encountered an error when adding the update block and then removing it during another apply. I think the most intuitive solution would be to do nothing, right?

christophkreutzer commented 1 year ago

Thank you very much for the swift review, additions and merging/releasing! :)

christophkreutzer commented 1 year ago

@christophkreutzer I also encountered an error when adding the update block and then removing it during another apply. I think the most intuitive solution would be to do nothing, right?

You're right - would see the same behaviour (if update is removed, just remove it from the state and do nothing). In that case, we probably need both checks in the update function: HasChange (did it actually change to the previous value) and checking the existence with GetOk. https://github.com/hashicorp/terraform-plugin-sdk/blob/v2.20.0/helper/schema/resource_diff.go#L384

Another option would be to push that check further down to the parsing, but as create and delete are required, I see no advantage in doing so.

aidanmelen commented 1 year ago

I am working on adding generic read support. I can address this bug at that time.

aidanmelen commented 1 year ago

I was testing in a dev branch with the new read functionality and found a way to fix. I will likely release early next week.

Screen Shot 2023-02-11 at 7 31 39 AM
aidanmelen commented 1 year ago

@christophkreutzer please use the v1.1.1 release for the fix.