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

Using a dynamic block cause unrelated environments to be part of changes

Open rickardp opened this issue 2 years ago • 14 comments

Example terraform code

resource "launchdarkly_project" "project" {
  key  = "my-project"
  name = "My Project"

  dynamic "environments" {
    for_each = local.environments
    content {
      name  = environments.value.name
      key   = environments.key
      color = endswith(environments.key, "-live") ? "ff0000" : endswith(environments.key, "-uat") ? "0000ff" : "00ff00"
    }
  }
}

When the list of environments change, all the environments diff, e.g.

# launchdarkly_project.project will be updated in-place
      ~ resource "launchdarkly_project" "project" {
            key  = "my-project"
            name = "My Project"
            # (2 unchanged attributes hidden)
          ~ environments {
              + color                = "00ff00"
              + default_ttl          = 0
              + key                  = "(key of unchanged environment)"
              + name                 = "...."
              ....
            }
          ~ environments {
              .... // other unchanged environment
            }
          ~ environments {
              .... // added environment
            }

When trying to apply this

Error: failed to create environment "(key of uncahnged environment)" in project "us-client": 409 Conflict: {"code":"conflict","message":"Project already has an environment with key \"(key of unchanged environment)\""}

This is reasonable, because the environment exists in the LD UI. But expectation is for the unchanged environment to not be modified at all and not be part of the plan output.

This was attempted with the latest provider, 2.12.2

rickardp avatar Jun 29 '23 23:06 rickardp

Hey @rickardp,

Thanks for bringing this to our attention. I've created an internal ticket to investigate what is happening here. We'll be sure to update this issue as we learn more.

Thanks, Henry

ldhenry avatar Jul 06 '23 17:07 ldhenry

Hello @ldhenry ,

Thanks for investigating the issue, we seem to be experiencing the same behaviours. Would you have any update on it? Thanks

Gilles95 avatar Jul 12 '23 15:07 Gilles95

Hey @Gilles95 and @rickardp,

By default, the launchdarkly_project resource is used to manage a single project and all of it's underlying environments. If you wish to only manage a subset of environments in Terraform while allowing other environments to be managed via the LD UI, I would recommed using the launchdarkly_project resource in cominbation with the launchdarkly_environment resource.

Here's an example config that show how to scaffold a LaunchDarkly project with a set of environments that are defined in a loop:

locals {
  envs = {
    prod = {
      name = "Production"
      key  = "production"
    }
    staging_uat = {
      name = "Staging UAT"
      key  = "staging-uat"
    }
  }
}

resource "launchdarkly_project" "project" {
  key  = "my-project"
  name = "My Project"

  # At lest one environment needs to be specified in the LaunchDarkly project block
  environments {
    key   = "test"
    name  = "Test"
    color = "000000"
  }


  # Use the ignore_changes lifecycle meta-argument to prevent changes to
  # environments managed elsewhere from interring with the Terraform plan.
  lifecycle {
    ignore_changes = [environments]
  }
}

resource "launchdarkly_environment" "env" {
  for_each = local.envs

  project_key = "my-project"
  name        = each.value.name
  key         = each.value.key
  color       = endswith(each.value.name, "-live") ? "ff0000" : endswith(each.value.key, "-uat") ? "0000ff" : "00ff00"
}

Please note the use of the ignore_changes lifecycle meta-argument on the launchdarkly_project resource. This is used to prevent changes to environments that are managed elsewhere from interfering with the Terraform plan.

Please let me know if this solves the issue for you.

Thanks, Henry

ldhenry avatar Jul 14 '23 15:07 ldhenry

At least in my case, I am managing all of the environments in Terraform and use the nested environment block as per the recommendation in the docs. The issue is that some changes in the environments will not be possible to apply as per the above error. Changes are done in Terraform, not the UI

rickardp avatar Jul 15 '23 04:07 rickardp

Could this be related to #154? Maybe pagination is why updating the environments get the provider confused. There were more than 20 environments in the original case when I reported this issue

rickardp avatar Jul 17 '23 06:07 rickardp

That's good info @rickardp. If you have more than 20 envs then it does seem likely that #154 is related.

ldhenry avatar Jul 17 '23 11:07 ldhenry

Hello @ldhenry ,


Our scenario is when a project is created with like 2 environments, if you add a new block for an environment in the middle of the list, then the following happens.

Existing environments: 
env1 
env2

New environment block creation with the new list and order as per below:

env1
 env1b <- the new env 
env2

Then the plan highlight it will rename the current env2 to env1b and will create a brand new env2.
 We are using the provider version "2.9.4"

Thanks

Gilles95 avatar Jul 18 '23 13:07 Gilles95

I upgraded our provider to version 2.13.2, Terraform to 1.5.3 and we see the same result, when removing an environment block in the middle of the list, it renames all the environments from that block and delete the last one.

      ~ environments {
          ~ key                  = "devtfetestprd01" -> "devtfetestprd02"
          ~ name                 = "DEVTFETESTPRD01" -> "DEVTFETESTPRD02"

      - environments {
...
          - name                 = "DEVTFETESTPRD03" -> null

Gilles95 avatar Jul 21 '23 10:07 Gilles95

I upgraded our provider to version 2.13.2, Terraform to 1.5.3 and we see the same result, when removing an environment block in the middle of the list, it renames all the environments from that block and delete the last one.

      ~ environments {
          ~ key                  = "devtfetestprd01" -> "devtfetestprd02"
          ~ name                 = "DEVTFETESTPRD01" -> "DEVTFETESTPRD02"

      - environments {
...
          - name                 = "DEVTFETESTPRD03" -> null

Hey @Gilles95, I did some investigation and you are seeing this plan change because the environments block is implemented as a TypeList under the hood. We chose to use a TypeList instead of a TypeSet because whenever an attribute in a TypeSet element is changed, the Terraform plan shows that the entire set item must be destroyed and re-created. Fortunately, the plan you are seeing is purely a cosmetic artifact of using TypeList. I can confirm that running a terraform apply will not result in any changes on the LaunchDarkly side and the next time your run a terraform plan you will not see any proposed changes.

Alternatively, you should not see a diff if you add the new environment to the bottom of the list. I know this is not ideal but it

I did find a possible solution for suppressing the diff if the environments are reordered but this would not solve the case where a new environment is added to the middle of the list.

I know this is not ideal, but unfortunately we couldn't find a better solution as we are constrained by Terraform's schema limitations.

Thanks, Henry

ldhenry avatar Jul 26 '23 14:07 ldhenry

Could this be related to #154? Maybe pagination is why updating the environments get the provider confused. There were more than 20 environments in the original case when I reported this issue

Hey @rickardp, we released v2.13.3 with a fix for the 20+ environment issue (#154 ). Please give it a try and let me know if you run into any issues.

ldhenry avatar Jul 26 '23 14:07 ldhenry

Hey @Gilles95, I did some investigation and you are seeing this plan change because the environments block is implemented as a TypeList under the hood. We chose to use a TypeList instead of a TypeSet because whenever an attribute in a TypeSet element is changed, the Terraform plan shows that the entire set item must be destroyed and re-created. Fortunately, the plan you are seeing is purely a cosmetic artifact of using TypeList. I can confirm that running a terraform apply will not result in any changes on the LaunchDarkly side and the next time your run a terraform plan you will not see any proposed changes.

Alternatively, you should not see a diff if you add the new environment to the bottom of the list. I know this is not ideal but it

I did find a possible solution for suppressing the diff if the environments are reordered but this would not solve the case where a new environment is added to the middle of the list.

I know this is not ideal, but unfortunately we couldn't find a better solution as we are constrained by Terraform's schema limitations.

Thanks, Henry

Thanks for looking into it! Actually we just confirmed the apply did what was expected.

Gilles95 avatar Jul 26 '23 15:07 Gilles95

Hey @Gilles95, I did some investigation and you are seeing this plan change because the environments block is implemented as a TypeList under the hood. We chose to use a TypeList instead of a TypeSet because whenever an attribute in a TypeSet element is changed, the Terraform plan shows that the entire set item must be destroyed and re-created. Fortunately, the plan you are seeing is purely a cosmetic artifact of using TypeList. I can confirm that running a terraform apply will not result in any changes on the LaunchDarkly side and the next time your run a terraform plan you will not see any proposed changes. Alternatively, you should not see a diff if you add the new environment to the bottom of the list. I know this is not ideal but it I did find a possible solution for suppressing the diff if the environments are reordered but this would not solve the case where a new environment is added to the middle of the list. I know this is not ideal, but unfortunately we couldn't find a better solution as we are constrained by Terraform's schema limitations. Thanks, Henry

Thanks for looking into it! Actually we just confirmed the apply did what was expected.

Nice! I'm going to close this issue but feel free to re-open it if needed.

ldhenry avatar Jul 26 '23 16:07 ldhenry

Could this be related to #154? Maybe pagination is why updating the environments get the provider confused. There were more than 20 environments in the original case when I reported this issue

Hey @rickardp, we released v2.13.3 with a fix for the 20+ environment issue (#154 ). Please give it a try and let me know if you run into any issues.

Nice, will check internally if an upgrade fixed the problem.

As for the plan changes, shouldn't it be a TypeMap with the env key as key (it is immutable after all, isn't it)? I think the impact is not irrelevant, it will scare the release reviewers on our side as we use the output from terraform plan as part of our deployment pipelines. Would it be possible to fix?

rickardp avatar Jul 26 '23 20:07 rickardp

Could this be related to #154? Maybe pagination is why updating the environments get the provider confused. There were more than 20 environments in the original case when I reported this issue

Hey @rickardp, we released v2.13.3 with a fix for the 20+ environment issue (#154 ). Please give it a try and let me know if you run into any issues.

Nice, will check internally if an upgrade fixed the problem.

As for the plan changes, shouldn't it be a TypeMap with the env key as key (it is immutable after all, isn't it)? I think the impact is not irrelevant, it will scare the release reviewers on our side as we use the output from terraform plan as part of our deployment pipelines. Would it be possible to fix?

Hey @rickardp, I think a TypeMap sounds like a good change in the future. Unfortunately this would constitute a breaking change so we would have to deprecate the existing environments list while adding support for a new map. This is doable but not necessarily trivial. I've created an internal ticket to research the effort involved with making this change and I'll be sure to update this issue with any updates.

I'll also reopen this issue so others can follow along more easily.

Thanks, Henry

ldhenry avatar Jul 27 '23 10:07 ldhenry