IBM-Cloud / terraform-provider-ibm

https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs
Mozilla Public License 2.0
341 stars 669 forks source link

Bad formatting on KMS instance policy output #5163

Closed MatthewLemmond closed 6 months ago

MatthewLemmond commented 8 months ago

When using the output from the ibm_kms_instance_policies resource, some values have their types (like tolist()) or strange null /* object */ values as the output, here is a sample from one of our runs:

key_protect_instance_policies = {
  "dual_auth_delete" = tolist([])
  "id" = "crn:v1:bluemix:public:kms:us-south:a/abac0df06b644a9cabc6e44f55b3880e:4d20584d-7917-4f56-8767-8bf1cc7f4609::"
  "instance_id" = "4d20584d-7917-4f56-8767-8bf1cc7f4609"
  "key_create_import_access" = tolist([])
  "metrics" = tolist([])
  "rotation" = tolist([])
  "timeouts" = null /* object */
}

We are able to filter this using a local, looping over the output, and only including non-null values, so the issue seems to be occurring from within the provider based on the underlying types the null values have causing it to return something other than null for null values.

Community Note

Terraform CLI and Terraform IBM Provider Version

Terraform v1.5.7 on darwin_amd64 provider registry.terraform.io/ibm-cloud/ibm v1.62.0

Affected Resource(s)

Terraform Configuration Files

Please include all Terraform configurations required to reproduce the bug. Bug reports without a functional reproduction may be closed without investigation.

# Copy-paste your Terraform configurations here - for large Terraform configs,
# please share a link to the ZIP file.

resource "ibm_resource_group" "resource_group" {
  name = "error-example-rg"
}

resource "ibm_resource_instance" "key_protect_instance" {
  name = "error-example-kp"
  resource_group_id = ibm_resource_group.resource_group.id
  service = "kms"
  plan = "tiered-pricing"
  parameters = {
    allowed_network : "public-and-private"
  }
}

resource "ibm_kms_instance_policies" "key_protect_instance_policies" {
  instance_id = ibm_resource_instance.key_protect_instance.guid
  rotation {
    enabled        = true
    interval_month = 1
  }
  dual_auth_delete {
    enabled = false
  }
  metrics {
    enabled = true
  }
  key_create_import_access {
    enabled             = true
    create_root_key     = true
    create_standard_key = true
    import_root_key     = true
    import_standard_key = true
    enforce_token       = false
  }
}

output "kms_instance_policies" {
  value = ibm_kms_instance_policies.key_protect_instance_policies
}

Debug Output

Panic Output

Expected Behavior

providers sends back null for objects/lists that do not have a value

Actual Behavior

provider sends tolist([]) and null /* object */ for these objects and lists that do not have values

Steps to Reproduce

  1. terraform apply

Important Factoids

References

Example of output and link to PR with workaround: https://github.com/terraform-ibm-modules/terraform-ibm-kms-all-inclusive/issues/417

william8siew commented 6 months ago

@MatthewLemmond just to be clear, I think we can resolve the issue with the underlying resource drift that was mentioned here https://github.com/terraform-ibm-modules/terraform-ibm-kms-all-inclusive/issues/448#issuecomment-2072957406

but as for formatting, I believe tolist([]) is just the native formatting in terraform for ListType when absent The timeouts is also a native thing we can't control as part of adding timeouts(this is present in our other APIs supported by terrafom) "timeouts" = null /* object */

Even when the list is populated, it still would use this tolist() formatting

kms_instance_policies = {
  "dual_auth_delete" = tolist([])
  "id" = "crn:v1:bluemix:public:kms:us-south:a/eba0f7b1166e441ab74ac94e564c72ec:dd38247f-9acd-4586-8516-8136950b7dda::"
  "instance_id" = "dd38247f-9acd-4586-8516-8136950b7dda"
  "key_create_import_access" = tolist([])
  "metrics" = tolist([])
  "rotation" = tolist([
    {
      "created_by" = "IBMid-6620028GBG"
      "creation_date" = "2024-03-22 14:33:35 +0000 UTC"
      "enabled" = true
      "interval_month" = 3
      "last_updated" = "2024-04-24 22:06:36 +0000 UTC"
      "updated_by" = "IBMid-6620028GBG"
    },
  ])
  "timeouts" = null /* object */
}

I would recommend, if you need to extract certain fields from the output and format them, you can do it this way:

output "kms_instance_policies_rotation" {
  value = flatten(ibm_kms_instance_policies.instance_policy.rotation)[0]
}

Result:

kms_instance_policies_rotation = {
  "created_by" = "IBMid-6620028GBG"
  "creation_date" = "2024-03-22 14:33:35 +0000 UTC"
  "enabled" = true
  "interval_month" = 3
  "last_updated" = "2024-04-24 22:33:13 +0000 UTC"
  "updated_by" = "IBMid-6620028GBG"
}
william8siew commented 6 months ago

If you have seen other teams returning a null in output for List types, I would appreciate if you could tell me which ones so I can see how they are doing it

william8siew commented 6 months ago

https://github.com/IBM-Cloud/terraform-provider-ibm/pull/5308 Drift issue fixed here in this PR

bhakta-ibm commented 6 months ago

@MatthewLemmond - Please validate this fix so eventually https://github.com/terraform-ibm-modules/terraform-ibm-kms-all-inclusive/issues/448 can be closed.

MatthewLemmond commented 6 months ago

@william8siew after some testing with other resources I also see their output is similar to the instance policy output with respect to the null / object / or tolist([]) style, as this is the case and the drift issue has been fixed we can close this issue and we will keep our cleanup of the output in place on our end. Thanks for your help, closing this out now

MatthewLemmond commented 6 months ago

Adding comment to note that I tested the 1.56.0-beta0 release that contains the fix and saw no drift in the output so issue with idempotency is resolved and we can fix the issues in our key protect modules once the beta version is fully released