terraform-provider-gitlab icon indicating copy to clipboard operation
terraform-provider-gitlab copied to clipboard

gitlab_user resource causes destroy/add of imported resource, because of the `skip_confirmation` attribute

Open olhado opened this issue 2 years ago • 4 comments

GitLab Provider version

3.14.0

GitLab version

Gitlab EE 4.5

Terraform version

Terraform v1.0.11

Relevant Terraform Configuration

resource "gitlab_user" "example_user" {
  name             = "Example User"
  username         = "exampleuser"
  email            = "[email protected]"
  is_admin         = true
  projects_limit   = 0
  can_create_group = false
  is_external      = false
  note             = "Ipsum Lorem."
}

Relevant log output

# gitlab_user.example_user must be replaced
-/+ resource "gitlab_user" "example_user" {
      ~ id                = "4" -> (known after apply)
        name              = "Example User"
      ~ namespace_id      = 0 -> (known after apply)
      + skip_confirmation = true # forces replacement
        # (8 unchanged attributes hidden)
    }

Description

I imported the user defined above (details differ due to sensitive information in actual user) using terraform import gitlab.example_user 4. I then run terraform plan and get the log output, which wants to recreate the user due to the skip_confirmation field. Setting this field to true doesn't change the behavior. I assume it is related to #491 ?

The skip_confirmation field shouldn't force a recreate.

(As an aside, it appears the import is incorrect. The namespace_id in the plan output says 0, and I confirmed the state file had the same data. However, the namespace_id for the existing user was actually a different number.)

olhado avatar May 25 '22 21:05 olhado

Thanks for the report @olhado!

I've just dug into the reasoning in #491 why it was marked as ForceNew and understand the conflict here. Since this field can't be updated, we cannot just mark it Optional. I also doubt that the skip_reconfirmation in the PUT endpoint helps.

There also doesn't seem to be a reliable way to read it from the API. The only chance we have is to check if the user was confirmed (using the confirmed_at field), but we wouldn't know if the skip_confirmation was true / false in the config (which might actually not matter).

Maybe we just want to suppress diffs for skip_confirmation ? Or we just advise users to use lifecycle { ignore_changes = ["skip_confirmation"] } ?

Any ideas @armsnyder @PatrickRice-KSC ?


What version of GitLab are you running? I assume 14.5 and not 4.5, right?

(As an aside, it appears the import is incorrect. The namespace_id in the plan output says 0, and I confirmed the state file had the same data. However, the namespace_id for the existing user was actually a different number.)

I've only recently implemented in GitLab itself that it returns the namespace_id for a user. Thus, I assume that's the reason you get the "zero value" for the namespace.

timofurrer avatar May 29 '22 15:05 timofurrer

Maybe we just want to suppress diffs for skip_confirmation

I think this is probably the right way to handle this, as long as it doesn't mess up the create (which it shouldn't). Asking users to use a lifecycle block as the norm doesn't quite sit right with me, as it means the lifecycle block doesn't really convey any useful information regarding the developer's intent in the configuration anymore. This should be an easy PR to test, I'll check this out here shortly.

RicePatrick avatar May 29 '22 17:05 RicePatrick

Yep, easy peasy fix with a DiffSuppressFunc. I added a new test for this case too, to ensure that the fix worked properly. The new test fails without the DiffSuppress.

RicePatrick avatar May 29 '22 17:05 RicePatrick

@timofurrer @PatrickRice-KSC Thank you all! I will to add an ignore on my side for the skip_confirmation attribute for now, and keep an eye on #1106 .

olhado avatar May 31 '22 18:05 olhado