Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

Crash when mixing integer and string values in list #231

Open
dak1n1 opened this issue Jun 11, 2021 · 4 comments
Open

Crash when mixing integer and string values in list #231

dak1n1 opened this issue Jun 11, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@dak1n1
Copy link
Contributor

dak1n1 commented Jun 11, 2021

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

  • GH-1234

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
@dak1n1 dak1n1 added the bug Something isn't working label Jun 11, 2021
@dak1n1
Copy link
Contributor Author

dak1n1 commented Jun 11, 2021

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

dak1n1 commented Jun 11, 2021

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

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 dak1n1 changed the title Crash when attempting to load multiple yaml files Crash when mixing integer and string values in list Jun 14, 2021
dak1n1 added a commit to dak1n1/terraform-provider-kubernetes-alpha that referenced this issue Jun 14, 2021
dak1n1 added a commit to dak1n1/terraform-provider-kubernetes-alpha that referenced this issue Jun 14, 2021
In Terraform's type system, lists and maps must have consistent element types. However, the Kubernetes API spec defines certain fields as type IntOrString, which can lead to using more than one type in a list. Since Terraform has no equivalent type to translate this to, the solution is make each element of the List or Map the same type. Validation will fail if more than one type is used.

Fixes hashicorp#231
@dak1n1
Copy link
Contributor Author

dak1n1 commented Jun 18, 2021

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)")

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants