hashicorp / terraform-provider-kubernetes-alpha

A Terraform provider for Kubernetes that uses dynamic resource types and server-side apply. Supports all Kubernetes resources.
https://registry.terraform.io/providers/hashicorp/kubernetes-alpha/latest
Mozilla Public License 2.0
490 stars 63 forks source link

Crash when mixing integer and string values in list #231

Open dak1n1 opened 3 years ago

dak1n1 commented 3 years ago
Terraform version: v0.15.5
Provider version: v0.5.0
Kubernetes version: minikube (1.20.2)

Affected Resource(s)

kubernetes_manifest

Terraform Configuration Files

See this repo for full configuration: https://github.com/dak1n1/bug-repro-20210611

resource "kubernetes_manifest" "consul" {
  provider = kubernetes-alpha
  for_each = fileset(path.module, "consul/generated/*")
  manifest = yamldecode(file(each.key))
}

Debug Output

https://raw.githubusercontent.com/dak1n1/bug-repro-20210611/main/terraform_plan_output.txt

Panic Output

panic: lists must only contain one type of element, saw tftypes.Object["appProtocol":tftypes.String, 
"name":tftypes.String, "nodePort":tftypes.Number, "port":tftypes.Number, "protocol":tftypes.String, 
"targetPort":tftypes.Number] and tftypes.Object["appProtocol":tftypes.String, "name":tftypes.String,
"nodePort":tftypes.Number, "port":tftypes.Number, "protocol":tftypes.String, "targetPort":tftypes.String]

Steps to Reproduce

  1. Visit the reproducer repo.
  2. terraform init
  3. terraform plan

Expected Behavior

I thought I could load the yaml files into the kubernetes-alpha provider and have multiple manifests created.

Actual Behavior

crash

Important Factoids

References

Community Note

dak1n1 commented 3 years ago

If I do a simpler test, like using just two namespace yaml files, it works:

$ ls consul/generated/consul-namespace*
consul/generated/consul-namespace2.yaml  consul/generated/consul-namespace.yaml
resource "kubernetes_manifest" "consul" {
  provider = kubernetes-alpha
  for_each = fileset(path.module, "consul/generated/consul-namespace*")
  manifest = yamldecode(file(each.key))
}
$ terraform apply --auto-approve
[...]
Plan: 2 to add, 0 to change, 0 to destroy.
kubernetes_manifest.consul["consul/generated/consul-namespace2.yaml"]: Creating...
kubernetes_manifest.consul["consul/generated/consul-namespace.yaml"]: Creating...
kubernetes_manifest.consul["consul/generated/consul-namespace.yaml"]: Creation complete after 1s
kubernetes_manifest.consul["consul/generated/consul-namespace2.yaml"]: Creation complete after 1s
$ kubectl get namespace |grep consul
consul            Active   81s
consul2           Active   81s
dak1n1 commented 3 years ago

Here's the yaml file that's causing the issue: https://github.com/dak1n1/bug-repro-20210611/blob/main/consul/generated/tf-k8s-test-consul-server-svc.yaml

The others are loaded successfully.

alexsomesan commented 3 years ago

The cause of the crash here is mixing of integer and string values for targetPort attributes in the ports list of some of the resources.

In Terraform's type system list and maps must have consistent element types. However, in the Kubernetes OpenAPI spec, targetPort is defined as type IntOrString. Essentially an abomination that can either be numerical or a string. Translating this into Terraform types, lists containing attributes of type IntOrString may violate the element type invariant.

The solution is to keep all instances of targetPort in the same list to the same type: either all numerical or all strings. We should add a validation step for this.

dak1n1 commented 3 years ago

A complete solution to this is proving to be pretty tricky. The validation has been added to another branch, but testing the proposed work-around in master, I'm seeing a new issue:

I can best illustrate the behavior by giving some examples of what works and what doesn't. Here is the entire ports list used in each test, and the result of the test:

  ports:
    - name: dns-tcp
      protocol: "TCP"
      port: "8500"
      targetPort: "8600"

# result:
β”‚ Error: spec.ports[0].targetPort
β”‚ Invalid value: "8600": must contain at least one letter or number (a-z, 0-9)
 ports:
    - name: http
      port: 8500
      targetPort: 8500
    - name: dns-tcp
      protocol: "TCP"
      port: "8500"
      targetPort: dns-tcp

# result:
panic: lists must only contain one type of element, saw tftypes.Object["appProtocol":tftypes.String, "name":tftypes.String, "nodePort":tftypes.Number, "port":tftypes.Number, "protocol":tftypes.String, "targetPort":tftypes.Number] and tftypes.Object["appProtocol":tftypes.String, "name":tftypes.String, "nodePort":tftypes.Number, "port":tftypes.Number, "protocol":tftypes.String, "targetPort":tftypes.String]
    - name: http
      port: 8500
      targetPort: 8500
    - name: dns-tcp
      protocol: "TCP"
      port: 8600
      targetPort: 8600

# result:
apply successful

The error seems to be from the Kubernetes API side:

vendor/k8s.io/apimachinery/pkg/util/validation/validation.go:           errs = append(errs, "must contain at least one letter or number (a-z, 0-9)")