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

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

Open
rickardp opened this issue Jun 29, 2023 · 14 comments
Open

Comments

@rickardp
Copy link

rickardp commented Jun 29, 2023

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

@ldhenry
Copy link
Collaborator

ldhenry commented Jul 6, 2023

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

@Gilles95
Copy link

Hello @ldhenry ,

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

@ldhenry
Copy link
Collaborator

ldhenry commented Jul 14, 2023

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

@rickardp
Copy link
Author

rickardp commented Jul 15, 2023

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
Copy link
Author

rickardp commented Jul 17, 2023

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

@ldhenry
Copy link
Collaborator

ldhenry commented Jul 17, 2023

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

@Gilles95
Copy link

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
Copy link

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

@ldhenry
Copy link
Collaborator

ldhenry commented Jul 26, 2023

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
Copy link
Collaborator

ldhenry commented Jul 26, 2023

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.

@Gilles95
Copy link

Gilles95 commented Jul 26, 2023

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.

@ldhenry
Copy link
Collaborator

ldhenry commented Jul 26, 2023

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 ldhenry closed this as completed Jul 26, 2023
@rickardp
Copy link
Author

rickardp commented Jul 26, 2023

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?

@ldhenry
Copy link
Collaborator

ldhenry commented Jul 27, 2023

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 ldhenry reopened this Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants