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

manifest: add support for explicit elements in `computedFields`

Open BBBmau opened this issue 1 year ago • 4 comments

Description

We only support computedFields attributes, if a user attempts to be explicit with computedFields they will get a vanished output:

│ Error: Provider produced inconsistent result after apply
│ 
│ When applying changes to kubernetes_manifest.deployment_resource_diff, provider
│ "provider[\"registry.terraform.io/hashicorp/kubernetes\"]" produced an unexpected new value:
│ .object.spec.template.spec.containers[0].resources.limits: element "cpu" has vanished.
│ 
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.

What's the value in being able to support this? Of course the work around is to just set the attribute as computedFields where the element exists, though it also would be good to give users that option of being explicit also.

A related issue where a user brings this up, advice was to just set attributes to computedFields

https://github.com/hashicorp/terraform-provider-kubernetes/issues/1530

Potential Terraform Configuration

Any tfconfig will do where you attempt to set an element in computedFields

resource "kubernetes_manifest" "deployment_resource_diff" {
computed_fields = ["spec.template.spec.containers[0].resources.limits[\"cpu\"]"]
    manifest = {
        apiVersion = "apps/v1"
        kind       = "Deployment"

        metadata = {
            name = "deployment-resource-diff"
            namespace = "default"
        }

        spec = {
    replicas = 3

    selector = {
      matchLabels = {
        test = "MyExampleApp"
      }
    }

    template = {
      metadata= {
        labels = {
          test = "MyExampleApp"
        }
      }
      

      spec = {
        containers = [{
          image = "nginx:1.21.6"
          name  = "example"

          resources = {
            limits = {
              cpu    = "0.5"
              memory = "512Mi"
            }
            requests = {
              cpu    = "250m"
              memory = "50Mi"
            }
          }
        }]
      }
    }
  }
        }
    }

References

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

BBBmau avatar Mar 01 '24 21:03 BBBmau

I am interested to work on this 😃

theloneexplorerquest avatar Mar 07 '24 11:03 theloneexplorerquest

just some progress update: I have identified code block that causing this issue. I suspect the issue is in terraform-plugin-go, will do some further investigation.

theloneexplorerquest avatar Apr 14 '24 03:04 theloneexplorerquest

@theloneexplorerquest be advised that @BBBmau is also working on an improvement in this direction. It would be good to coordinate, to not waste effort.

alexsomesan avatar Apr 17 '24 13:04 alexsomesan

@BBBmau @alexsomesan thanks for let me know, I try to get involved in OSS only on weekend (and I slack off sometimes 😅), so my pace is slow. Here is my investigation so far: The code call https://github.com/hashicorp/terraform-provider-kubernetes/blob/main/manifest/provider/apply.go#L198 leads to error out at https://github.com/hashicorp/terraform-plugin-go/blob/main/tftypes/value.go#L156 when the input path ap is spec.containers.0.resources.limits.cpu, note the last element is determined as ElementKeyString however the others are AttributeName image it appears we cannot convert the corresponding value of cpu to val.Type().(Map) for some reason. If I change https://github.com/hashicorp/terraform-plugin-go/blob/main/tftypes/value.go#L156 to val.Type().(Object), everything starts to work again. So, my guess is either we pass the wrong parameter in tftypes.WalkAttributePath or there is a bug in terraform-plugin-go.

I will work on something else for now, let me know if you need further details. :blush:

theloneexplorerquest avatar Apr 20 '24 00:04 theloneexplorerquest