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

Relax protocol matching requirement between Gateway API listener and Route #4011

Closed
wondersd opened this issue Oct 31, 2023 · 3 comments · Fixed by #4213
Closed

Relax protocol matching requirement between Gateway API listener and Route #4011

wondersd opened this issue Oct 31, 2023 · 3 comments · Fixed by #4213
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@wondersd
Copy link

What would you like to be added:

remove or allow to disable this check

https://github.com/kubernetes-sigs/external-dns/blob/master/source/gateway.go#L318-L321

// Confirm that the Listener and Route protocols match.
if !gwProtocolMatches(rt.Protocol(), lis.Protocol) {
  continue
}

Or to extend gwProtocolMatches to include considering TLS and TCP the same:

https://github.com/kubernetes-sigs/external-dns/blob/master/source/gateway.go#L476-L486

// gwProtocolMatches returns whether a and b are the same protocol,
// where HTTP and HTTPS are considered the same.
func gwProtocolMatches(a, b v1beta1.ProtocolType) bool {
	if a == v1beta1.HTTPSProtocolType {
		a = v1beta1.HTTPProtocolType
	}
	if b == v1beta1.HTTPSProtocolType {
		b = v1beta1.HTTPProtocolType
	}
	return a == b
}

Why is this needed:

When using a Gateway as a terminating proxy for TCP connections as per https://gateway-api.sigs.k8s.io/guides/tls/#clientserver-and-tls

TLS | Terminate | TCPRoute

The protocols will be mixed between the listener and route (TLS and TCP). This check seems to prevent what is otherwise considered valid usages from having DNS records generated.

{"level":"debug","msg":"No endpoints could be generated from TCPRoute ...","time":"..."}
{"level":"debug","msg":"Gateway ... section \"...\" does not match TCPRoute ... hostnames [\"...\" \"\"]","time":"..."}

Given

apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
  name: ...
  namespace: ...
spec:
  gatewayClassName: ...
  listeners:
  - allowedRoutes:
      kinds:
      - group: gateway.networking.k8s.io
        kind: TCPRoute
      namespaces:
        from: Same
    hostname: ...
    name: ...
    port: ...
    protocol: TLS
    tls:
      certificateRefs:
      - group: ""
        kind: Secret
        name: ...
      mode: Terminate
---
apiVersion: gateway.networking.k8s.io/v1alpha2
kind: TCPRoute
metadata:
  labels:
    external-dns.alpha.kubernetes.io/hostname: ...
  name: ...
  namespace: ...
spec:
  parentRefs:
  - group: gateway.networking.k8s.io
    kind: Gateway
    name: ...
    sectionName: ...
  rules:
  - backendRefs:
    - group: ""
      kind: Service
      name: ...
      port: ...
      weight: 1
@wondersd wondersd added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 31, 2023
@matiza5
Copy link

matiza5 commented Nov 13, 2023

Faced the same problem. TLS protocol and TCPRoute combination should be supported, as it is supported in GatewayAPI.

@zs-ko
Copy link
Contributor

zs-ko commented Jan 26, 2024

Faced the same problem. TLS termination with TCPRoute

@zs-ko
Copy link
Contributor

zs-ko commented Jan 27, 2024

fixed the issue and created a pr. Should also add more test to TCPRoute to reflect HTTPRoute testing.
Here is a image that can be used until it's merged
https://github.com/users/zs-ko/packages/container/package/external-dns

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants