commit e0523a8fc806e2e00e1eb835da235e1d775810d9 Author: Travis Raines Date: Thu Oct 22 11:35:44 2020 -0700 feat(transl) disable automatic SNI handling Disable automatic SNI criteria addition by commenting out the code that adds it. This is a useful convenience feature, but would cause breakage because of https://github.com/Kong/kong/issues/6425 Once that issue is fixed, we should re-enable this functionality, but gate it with a flag, to avoid issues with older Kong versions. diff --git a/internal/ingress/controller/parser/parser_test.go b/internal/ingress/controller/parser/parser_test.go index 6e464e7..acc3d9a 100644 --- a/internal/ingress/controller/parser/parser_test.go +++ b/internal/ingress/controller/parser/parser_test.go @@ -2529,10 +2529,12 @@ func TestParserSNI(t *testing.T) { StripPath: kong.Bool(false), RegexPriority: kong.Int(0), Hosts: kong.StringSlice("example.com"), - SNIs: kong.StringSlice("example.com"), - PreserveHost: kong.Bool(true), - Paths: kong.StringSlice("/"), - Protocols: kong.StringSlice("http", "https"), + // TODO disabled pending resolution of https://github.com/Kong/kong/issues/6425 + // this relies on automatic addition of SNI criteria + //SNIs: kong.StringSlice("example.com"), + PreserveHost: kong.Bool(true), + Paths: kong.StringSlice("/"), + Protocols: kong.StringSlice("http", "https"), }, state.Services[0].Routes[0].Route) assert.Equal(kong.Route{ Name: kong.String("default.foo.10"), diff --git a/internal/ingress/controller/parser/translate.go b/internal/ingress/controller/parser/translate.go index c6ab110..1c340de 100644 --- a/internal/ingress/controller/parser/translate.go +++ b/internal/ingress/controller/parser/translate.go @@ -45,16 +45,16 @@ func fromIngressV1beta1(log logrus.FieldLogger, ingressList []*networkingv1beta1 // someone has created an Ingress whose rule hostname set is a proper superset of the Ingress's TLS hostname // set, ignoring some complications introduced by wildcards. maybe. result.SecretNameToSNIs.addFromIngressV1beta1TLS(ingressSpec.TLS, ingress.Namespace) - //var routeSNIs []*string - hasSNI := false - for i := range ingressSpec.TLS { - if len(ingressSpec.TLS[i].Hosts) > 0 { - hasSNI = true - } - // for _, hostname := range ingressSpec.TLS[i].Hosts { - // routeSNIs = append(routeSNIs, &hostname) - // } - } + // TODO currently disabled because of https://github.com/Kong/kong/issues/6425 + //hasSNI := false + //for i := range ingressSpec.TLS { + // if len(ingressSpec.TLS[i].Hosts) > 0 { + // hasSNI = true + // } + // // for _, hostname := range ingressSpec.TLS[i].Hosts { + // // routeSNIs = append(routeSNIs, &hostname) + // // } + //} for i, rule := range ingressSpec.Rules { host := rule.Host @@ -93,26 +93,21 @@ func fromIngressV1beta1(log logrus.FieldLogger, ingressList []*networkingv1beta1 if host != "" { hosts := kong.StringSlice(host) r.Hosts = hosts - // TODO maybe. this forcibly adds SNI match criteria for the TLS hostnames in an Ingress rule - // to the Kong route. That criteria arguably should exist for any Ingress rules with a hostname, - // and adding it automatically is useful for the current common (only?) use of this criteria in the - // Kong proxy, indicating when the proxy should request an mTLS client cert. It may create issues - // if users require a different SNI match criteria (unlikely) or support clients without SNI - // support (less common over time, but still a reality in regions with a large number of older - // devices with EOL OSes). A vendor-specific override can address either case, though may need to - // consider future changes to SNI matching in the Kong proxy core. Wildcard hostnames present a - // challenge, because you might reasonably want to add them (and the Ingress spec doesn't care - // about SNI, so it allows them by virtue of allowing wildcard hostnames), but the proxy doesn't - // let you configure them. - if hasSNI { - var snis []*string - for _, hostname := range hosts { - if !strings.Contains(*hostname, "*") { - snis = append(snis, hostname) - } - } - r.SNIs = snis - } + // TODO this would SNI criteria to routes with certificates automatically based on their hostnames + // It's currently disabled because of https://github.com/Kong/kong/issues/6425. Adding them as-is + // would prevent those routes from handling HTTP traffic or HTTP to HTTPS redirects at all. + // It should be safe to enable with that fixed, though older Kong versions will remain a concern. + // It's also a concern if the route needs to support SNI-incapable clients. As such, this behavior + // should probably have a flag. + //if hasSNI { + // var snis []*string + // for _, hostname := range hosts { + // if !strings.Contains(*hostname, "*") { + // snis = append(snis, hostname) + // } + // } + // r.SNIs = snis + //} } serviceName := ingress.Namespace + "." +