fix: Update Team Resource

[fishfacemcgee]

Proposed fixes for #92:

• Apply the team's name to the refreshed State during Read()
• Use the Plan's team name and not the State's during Update()

Additional related Enhancement:

• Force a resource replacement if the Team Name changes
  • Using the UI as a source of truth here (which might be mistaken, so let me know if it is), it's not intended/expected for a team's name to change once it's created. This seems supported by the original behavior of pulling the name from State during Updates. So if the expected way to rename a team is to delete and create a new one, this change is, to my knowledge, the "Terraform correct" way of expressing it.

Caveat

I don't have an effective way to test this at the moment and I don't have much functional experience with writing Terraform providers, so if something looks like a mistake, it probably is.

[alfespa17]

I think you need to remove this part

If you import the resource you will only have the team ID and the team name inside the state will be empty, so when you try to run the apply after importing the resource it will force the replace

[alfespa17]

Unless you update the import logic to include the name, similar to the following:

terraform-provider-terrakube/internal/provider/team_resource.go

Line 423 in 05331f3

resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("organization_id"), idParts[0])...)

[fishfacemcgee]

I think you need to remove this part

If you import the resource you will only have the team ID and the team name inside the state will be empty, so when you try to run the apply after importing the resource it will force the replace

If that's the case wouldn't all of the resource's attributes (minus the two being applied in the Import function) be proposed changes in a plan/apply after import? Or is there some scenario where it doesn't reconcile against the imported state in this case?

[alfespa17]

I think you need to remove this part

If you import the resource you will only have the team ID and the team name inside the state will be empty, so when you try to run the apply after importing the resource it will force the replace

If that's the case wouldn't all of the resource's attributes (minus the two being applied in the Import function) be proposed changes in a plan/apply after import? Or is there some scenario where it doesn't reconcile against the imported state in this case?

I guess you could import all the other attributes to the state but that will require more logic in the import function something similar to the read function to get all the permissions, that could be an option.

[fishfacemcgee]

I must be just misunderstanding something then. Let me explain what I'm seeing on the end-user side of things and maybe we can figure out what I'm missing. Given the following code:

resource "terrakube_team" "example" {
  name            = "[email protected]"
  organization_id = terrakube_organization.example.id

  manage_state      = true
  manage_workspace  = true
  manage_module     = true
  manage_provider   = true
  manage_vcs        = true
  manage_template   = true
  manage_job        = true
  manage_collection = true
}

import {
  to = terrakube_team.example
  id = "org_id,team_id"
}

I had this plan output

  # terrakube_team.example will be updated in-place
  # (imported from "org_id,team_id")
  ~ resource "terrakube_team" "example" {
        id                = "team_id"
        manage_collection = true
        manage_job        = true
        manage_module     = true
        manage_provider   = true
        manage_state      = true
        manage_template   = true
        manage_vcs        = true
        manage_workspace  = true
      + name              = "[email protected]"
        organization_id   = "org_id"
    }

Now, my expectation is that b842f39 and/or a4bc965 will resolve the problem of name being treated as a new/previously-unset attribute, which I would expect to create a plan output of

  # terrakube_example will be imported
  ~ resource "terrakube_team" "example" {
        id                = "team_id"
        manage_collection = true
        manage_job        = true
        manage_module     = true
        manage_provider   = true
        manage_state      = true
        manage_template   = true
        manage_vcs        = true
        manage_workspace  = true
        name              = "[email protected]"
        organization_id   = "org_id"
    }

What you appear to be suggesting is that with the inclusion of 9cc8a77, I'll instead see a plan output suggesting that the team will need to be replaced, but I don't understand how that will be the case. If the name attribute is unchanged from state, which it would be if my original fixes were correct, then TF shouldn't see a need to replace the resource. If TF wasn't reading the resource's full attributes at some point between the import and the plan generation, then shouldn't it be proposing changes to the various manage_* attributes? They default to false, so being set to true should constitute change, but their correct live state of true is being respected. Those attributes are also not manipulated in Import, which would suggest to me that Read() is being called between the import execution and the Plan generation.

[alfespa17]

It is working correctly I just tested it

user@pop-os:~/git/terraform-provider-terrakube/examples/full$ terraform apply

terrakube_team.team: Preparing import... [id=d9b58bd3-f3fc-4056-a026-1163297e80a8,5b89f40f-80d5-4267-a781-6367e04d5b5c]
terrakube_team.team: Refreshing state... [id=5b89f40f-80d5-4267-a781-6367e04d5b5c]

Terraform will perform the following actions:

  # terrakube_team.team will be imported
    resource "terrakube_team" "team" {
        id                = "5b89f40f-80d5-4267-a781-6367e04d5b5c"
        manage_collection = false
        manage_job        = false
        manage_module     = false
        manage_provider   = false
        manage_state      = false
        manage_template   = false
        manage_vcs        = false
        manage_workspace  = false
        name              = "TERRAKUBE_SUPER_ADMIN"
        organization_id   = "d9b58bd3-f3fc-4056-a026-1163297e80a8"
    }

Plan: 1 to import, 0 to add, 0 to change, 0 to destroy.
╷
│ Warning: WARNING
│ 
│   with provider["registry.terraform.io/alfespa17/terrakube"],
│   on main.tf line 9, in provider "terrakube":
│    9: provider "terrakube" {
│ 
│ In its current implementation, Terrakube creates one webhook for each instance of this resource and github has a default limitation of 20 webhooks per repository.
│ 
│ (and one more similar warning elsewhere)
╵

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

terrakube_team.team: Importing... [id=d9b58bd3-f3fc-4056-a026-1163297e80a8,5b89f40f-80d5-4267-a781-6367e04d5b5c]
terrakube_team.team: Import complete [id=d9b58bd3-f3fc-4056-a026-1163297e80a8,5b89f40f-80d5-4267-a781-6367e04d5b5c]

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

When I run the plan again it is showing

user@pop-os:~/git/terraform-provider-terrakube/examples/full$ terraform plan

terrakube_team.team: Refreshing state... [id=5b89f40f-80d5-4267-a781-6367e04d5b5c]

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.
╷
│ Warning: WARNING
│ 
│   with provider["registry.terraform.io/alfespa17/terrakube"],
│   on main.tf line 9, in provider "terrakube":
│    9: provider "terrakube" {
│ 
│ In its current implementation, Terrakube creates one webhook for each instance of this resource and github has a default limitation of 20 webhooks per repository.
│ 
│ (and one more similar warning elsewhere)