Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kubernetes_ingress_v1 resource does not correctly include empty tls configuation blocks in resulting k8s ingress object #2343

Closed
andremarianiello opened this issue Nov 18, 2023 · 5 comments · Fixed by #2344
Labels

Comments

@andremarianiello
Copy link
Contributor

Terraform Version, Provider Version and Kubernetes Version

Terraform version: v1.6.2
Kubernetes provider version: v2.23.0
Kubernetes version: v1.26.9

Affected Resource(s)

kubernetes_ingress_v1. No others tested

Terraform Configuration Files

resource "kubernetes_ingress_v1" "myingress" {
  metadata {
    name      = "myingress"
  }
  spec {
    default_backend {
      service {
        name = "myservice"
        port {
          number = 80
        }
      }
    }
    tls {}
  }
}

Steps to Reproduce

  1. terraform apply
  2. kubectl get ing myingress -o yaml

Expected Behavior

Ingress should have an empty tls block

Actual Behavior

Ingress has no tls blocks

Important Factoids

This behavior is important to support because an ingress controller may treat an empty tls config different from a missing tls config, like in OpenShift. Also something to note: multiple empty tls blocks cause a panic.

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

I submitted a pull request that fixes this.

@arybolovlev
Copy link
Contributor

Hi @andremarianiello,

Could you please share a reference doc that explains the OS behavior you refer to? Could you please also elaborate on this statement "multiple empty tls blocks cause a panic."?

Thanks.

@andremarianiello
Copy link
Contributor Author

andremarianiello commented Nov 28, 2023

The behavior I described is documented here: https://docs.openshift.com/container-platform/4.14/networking/routes/route-configuration.html#creating-edge-route-with-default-certificate_route-configuration

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: frontend
  ...
spec:
  rules:
    ...
  tls:
  - {}

Use this exact syntax to specify TLS without specifying a custom certificate.

The panic I referred to looks like this:

panic: interface conversion: interface {} is nil, not map[string]interface {}

goroutine 445 [running]:
github.com/hashicorp/terraform-provider-kubernetes/kubernetes.expandIngressV1TLS({0xc0026ddf00, 0x2, 0x33d9cc6?})
        /root/terraform-provider-kubernetes/kubernetes/structure_ingress_spec_v1.go:261 +0x2cd
github.com/hashicorp/terraform-provider-kubernetes/kubernetes.expandIngressV1Spec({0xc002732380?, 0x33da3fc?, 0x0?})
        /root/terraform-provider-kubernetes/kubernetes/structure_ingress_spec_v1.go:200 +0x265
github.com/hashicorp/terraform-provider-kubernetes/kubernetes.resourceKubernetesIngressV1Create({0x380bef8?, 0xc001b22770}, 0xc0019f9780, {0x32f0240?, 0xc00134fe30})
        /root/terraform-provider-kubernetes/kubernetes/resource_kubernetes_ingress_v1.go:184 +0x1cf
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).create(0xc000f688c0, {0x380be50, 0xc002962f90}, 0xd?, {0x32f0240, 0xc00134fe30})
        /root/go-workspace/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/[email protected]/helper/schema/resource.go:707 +0x11b
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).Apply(0xc000f688c0, {0x380be50, 0xc002962f90}, 0xc001110b60, 0xc00293fa00, {0x32f0240, 0xc00134fe30})        /root/go-workspace/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/[email protected]/helper/schema/resource.go:837 +0xa69
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ApplyResourceChange(0xc0014f0168, {0x380be50?, 0xc002962f00?}, 0xc00213f4a0)
        /root/go-workspace/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/[email protected]/helper/schema/grpc_provider.go:1021 +0xdbc
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ApplyResourceChange(0xc000334a00, {0x380be50?, 0xc0029626f0?}, 0xc00293a460)
        /root/go-workspace/pkg/mod/github.com/hashicorp/[email protected]/tfprotov5/tf5server/server.go:818 +0x56a
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ApplyResourceChange_Handler({0x32f42e0?, 0xc000334a00}, {0x380be50, 0xc0029626f0}, 0xc00293a3f0, 0x0)
        /root/go-workspace/pkg/mod/github.com/hashicorp/[email protected]/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:385 +0x169
google.golang.org/grpc.(*Server).processUnaryRPC(0xc00101c3c0, {0x3815300, 0xc001cc69c0}, 0xc002957e60, 0xc002294570, 0x4bc0f80, 0x0)
        /root/go-workspace/pkg/mod/google.golang.org/[email protected]/server.go:1335 +0xde7
google.golang.org/grpc.(*Server).handleStream(0xc00101c3c0, {0x3815300, 0xc001cc69c0}, 0xc002957e60, 0x0)
        /root/go-workspace/pkg/mod/google.golang.org/[email protected]/server.go:1712 +0x9e7
google.golang.org/grpc.(*Server).serveStreams.func1.1()
        /root/go-workspace/pkg/mod/google.golang.org/[email protected]/server.go:947 +0xbb
created by google.golang.org/grpc.(*Server).serveStreams.func1 in goroutine 423
        /root/go-workspace/pkg/mod/google.golang.org/[email protected]/server.go:958 +0x145
FAIL    github.com/hashicorp/terraform-provider-kubernetes/kubernetes   5.618s
FAIL
make: *** [GNUmakefile:83: testacc] Error 1

It occurs when you use multiple TLS blocks. The first must be non-empty and a later one must be empty. This is because the expandIngressV1TLS function does not handle multiple blocks (or empty blocks) properly.

func expandIngressV1TLS(l []interface{}) []networking.IngressTLS {
  if len(l) == 0 || l[0] == nil {
    return nil
  }

  tlsList := make([]networking.IngressTLS, len(l))
  for i, t := range l {
    in := t.(map[string]interface{})
    obj := networking.IngressTLS{}

    if v, ok := in["hosts"]; ok {
      obj.Hosts = expandStringSlice(v.([]interface{}))
    }

    if v, ok := in["secret_name"].(string); ok {
      obj.SecretName = v
    }
    tlsList[i] = obj
  }

  return tlsList
}

The code checks if the first block is nil, and then, if not, assumes none of the remaining blocks are nil. If this assumption is false the code panics. This is another thing I fixed in my MR.

@andremarianiello
Copy link
Contributor Author

@arybolovlev Please let me know if you would like be to provide any more information. I appreciate you taking the time to look at this.

@arybolovlev
Copy link
Contributor

Hi @andremarianiello,

Thank you! I have left a comment in your PR. All in all, it looks great, just a few small fixes before we merge it.

TY!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants