davidji99 / terraform-provider-split

Terraform Split provider
https://registry.terraform.io/providers/davidji99/split/latest
Mozilla Public License 2.0
23 stars 8 forks source link

Terraform destroy doesn't delete every resource #186

Open squalliram opened 4 months ago

squalliram commented 4 months ago

Hi there,

Terraform Version

v.1.7.3

HerokuX Provider Version

Terraform v1.7.3
on darwin_arm64
provider registry.terraform.io/davidji99/split v0.12.3

Affected Resource(s)

If this issue appears to affect multiple resources, it may be an issue with Terraform's core, so please mention this.

Terraform Configuration Files

resource "split_workspace" "foobar" {
  name = "Terraform Test"
  require_title_comments = true
}

data "split_workspace" "foobar" {
  name = "Terraform Test"
}

resource "split_environment" "foobar" {
  workspace_id = data.split_workspace.foobar.id
  name = "staging-canary"
  production = false
}

resource "split_traffic_type" "foobar" {
  workspace_id = data.split_workspace.foobar.id
  name = "terraform"
}

data "split_traffic_type" "foobar" {
    workspace_id = data.split_workspace.foobar.id
    name = "terraform"
}

resource "split_split" "foobar" {
  workspace_id = data.split_workspace.foobar.id
  traffic_type_id = data.split_traffic_type.foobar.id
  name = "my_terraform_split"
  description = "my terraform split description"
}

resource "split_split_definition" "foobar" {
  workspace_id = data.split_workspace.foobar.id
  split_name = split_split.foobar.name
  environment_id = split_environment.foobar.id

  default_treatment = "treatment_123"
  treatment {
    name = "treatment_123"
    configurations = "{\"key\":\"value\"}"
    description = "my treatment 123"
  }
  treatment {
    name = "treatment_456"
    configurations = "{\"key2\":\"value2\"}"
    description = "my treatment 456"
  }

  default_rule {
    treatment = "treatment_123"
    size = 100
  }
  rule {
    bucket {
      treatment = "treatment_123"
      size = 100
    }
    condition {
      combiner = "AND"
      matcher {
        type = "EQUAL_SET"
        attribute = "test_string"
        strings = ["test_string"]
      }
    }
  }
}

resource "split_segment" "foobar" {
  workspace_id = data.split_workspace.foobar.id
  traffic_type_id = data.split_traffic_type.foobar.id
  name = "terraform_segment"
  description = "description_of_my_terraform_segment"
}

resource "split_segment_environment_association" "foobar" {
    workspace_id = data.split_workspace.foobar.id
    environment_id = split_environment.foobar.id
    segment_name = split_segment.foobar.name
}

resource "split_environment_segment_keys" "foobar" {
    environment_id = split_environment.foobar.id
    segment_name = split_segment_environment_association.foobar.segment_name
    keys = ["mytestkey1", "mytestkey2"]
}

Debug Output

run-UjbubQYXUZSZ1Dum-plan-log.txt

Expected Behavior

Destroy should have deleted all resources that were created with the apply

Actual Behavior

Destroy was unable to delete resources relating to environment and then eventually the workspace as all the traffic types were still there and also all the feature flags and segments were still there. In order for an environment to be deleted, first all flags, segments, traffic types need to be deleted. Once all environments are deleted, then the workspace can be deleted.

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. terraform init
  2. terraform plan
  3. terraform apply
  4. terraform destroy
davidji99 commented 4 months ago

From the output you shared, the error is "@timestamp":"2024-02-22T16:54:04.216807Z","diagnostic":{"severity":"error","summary":"expected length of name to be in the range (1 - 15), got, which make sense as split_environment.name has a max character limit of 15 characters. Though, I'm not sure how your sample TF code is creating an environment that is named 'production-canary'. Do you have thesplit_environment.production resource defined elsewhere?

squalliram commented 3 months ago

Hello @davidji99 , This is the error that I get when I try to run destroy.

Potential solution: Add the ability to list API keys and then delete them as resources.

Listing API keys is currently an RFE.

Something like this for deleting the key as a resource and then this would let us delete the environment. You can use this delete API key API call.

resource "split_api_key" "foobar" {
  value = "<api_key>"
}

split_environment.production: Destroying... [id=19376400-e075-11ee-bba3-e690401375a5] split_environment_segment_keys.foobar: Destroying... [id=19403da0-e075-11ee-973f-7e47be3899d9:terraform_segment] split_split_definition.foobar: Destroying... [id=199625d0-e075-11ee-bba3-e690401375a5] split_environment_segment_keys.foobar: Destruction complete after 1s split_segment_environment_association.foobar: Destroying... [id=terraform_segment] split_split_definition.foobar: Destruction complete after 1s split_split.foobar: Destroying... [id=199625d0-e075-11ee-bba3-e690401375a5] split_segment_environment_association.foobar: Destruction complete after 0s split_segment.foobar: Destroying... [id=terraform_segment] split_environment.foobar: Destroying... [id=19403da0-e075-11ee-973f-7e47be3899d9] split_split.foobar: Destruction complete after 0s split_segment.foobar: Destruction complete after 0s split_traffic_type.foobar: Destroying... [id=1937b220-e075-11ee-bd86-0e6337e3d4b0] split_traffic_type.foobar: Destruction complete after 0s ╷ │ Error: unable to delete environment 19376400-e075-11ee-bba3-e690401375a5 │ │ DELETE │ https://api.split.io/internal/api/v2/environments/ws/170f7af0-e075-11ee-8ddd-b6a67543fa27/19376400-e075-11ee-bba3-e690401375a5: │ 400 {"code":400,"message":"Environment 19376400-e075-11ee-bba3-e690401375a5 │ cannot be archived, it has apitokens associated │ [6msjcqbe9ogph2vpriar92eu9l9pri290m0o, │ ilf400bgfmouj5ssce5ks348t3ggc3o5ledh]","details":"","transactionId":"5277faac2723aa155ec920ea8536fe5f"} ╵ ╷ │ Error: unable to delete environment 19403da0-e075-11ee-973f-7e47be3899d9 │ │ DELETE │ https://api.split.io/internal/api/v2/environments/ws/170f7af0-e075-11ee-8ddd-b6a67543fa27/19403da0-e075-11ee-973f-7e47be3899d9: │ 400 {"code":400,"message":"Environment 19403da0-e075-11ee-973f-7e47be3899d9 │ cannot be archived, it has apitokens associated │ [h8o2gb5n99j1h6duifb8p547qbvdmr75ff0m, │ tbadvebapesd42r8lc4g117ecc0cubgs6d66]","details":"","transactionId":"eda7be9196ef5e801cab220b81963793"}

davidji99 commented 3 months ago

Potential solution: Add the ability to list API keys and then delete them as resources.

Listing API keys is currently an RFE.

@squalliram There is no API support for listing API keys. (I'm not sure what 'RFE' stands for).


As for your suggestion below, it would not work because this code you want to create a split_api_key resource with a predefined API key. The Split API generates the API key for you. Plus, this sort of resource, even if possible, would require you to plaintext the key into source control.

resource "split_api_key" "foobar" {
  value = "<api_key>"
}

There's not much support I can provide for existing keys since without API GET support, you can't import existing keys.

Now if you created new API keys using the split_api_key resource, you could use a depends_on to link split_environment with split_api_key so the keys are deleted first before the environment is deleted.

squalliram commented 3 months ago

Hello @davidji99 , RFE (Request for Enhancement) is something I'll take care of on the Split side. There is currently no API for listing API keys as you said. I'll create a request for our team to add support for that.

Once we have a way to fetch API keys, you should be able to get the API keys linked to an environment and then the destroy should proceed successfully. Is that fair to say? If yes, for now we don't have an action on this.

Yes, and if we created API keys using the resource you mentioned, that we could use depends_on. Could you please share an example of that setup or the resource call?

Thanks.

davidji99 commented 3 months ago

Hi @squalliram,

Once we have a way to fetch API keys, you should be able to get the API keys linked to an environment and then the destroy should proceed successfully. Is that fair to say? If yes, for now we don't have an action on this.

My apologies. The current split_api_key resource already accepts an environment_id attribute. So if you do the following:


resource "split_environment" "foobar" {
    workspace_id = data.split_workspace.default.id
    name = "staging"
    production = true
}

resource "split_api_key" "foobar" {
    workspace_id = data.split_workspace.default.id
    name = "my client side key"
    type = "client_side"
    environment_ids = [split_environment.foobar.id]
}

Terraform will delete the api key first before deleting the environment. There's no need to use depends_on unless you had some sort of adhoc setup with a null resource managing api keys.

A GET to list all API keys or a specific API key by api key ID and not the actual key itself would help in the scenario where a user wanted to terraform import an already created API key into their TF configuration.