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

UDPRoutes from other namespaces are not getting attached #90

Closed
jpruciak opened this issue Jun 30, 2023 · 11 comments
Closed

UDPRoutes from other namespaces are not getting attached #90

jpruciak opened this issue Jun 30, 2023 · 11 comments
Assignees

Comments

@jpruciak
Copy link

Hello,

I hit very weird problem today:

When I'm using UDPRoute in app's namespace (and allowed all namespaces on the listener) - the route seems attached in route status, but on Gateway status there are 0 attached routes. My clients also cannot connect to the backend app and are getting permission denied from the stunner.

There's a route:

apiVersion: gateway.networking.k8s.io/v1alpha2
kind: UDPRoute
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: >
      {"apiVersion":"gateway.networking.k8s.io/v1alpha2","kind":"UDPRoute","metadata":{"annotations":{},"labels":{"argocd.argoproj.io/instance":"janus-dev"},"name":"janus-dev","namespace":"dev"},"spec":{"parentRefs":[{"name":"stunner-config","namespace":"stunner"}],"rules":[{"backendRefs":[{"name":"janus-dev","namespace":"dev"}]}]}}
  creationTimestamp: '2023-06-30T01:11:58Z'
  generation: 1
  labels:
    argocd.argoproj.io/instance: janus-dev
  name: janus-dev
  namespace: dev
  resourceVersion: '71179887'
  uid: f86715d8-b32c-4459-ad64-b9b33951239b
spec:
  parentRefs:
    - group: gateway.networking.k8s.io
      kind: Gateway
      name: stunner-config
      namespace: stunner
  rules:
    - backendRefs:
        - group: ''
          kind: Service
          name: janus-dev
          namespace: dev
          weight: 1
status:
  parents:
    - conditions:
        - lastTransitionTime: '2023-06-30T01:16:09Z'
          message: parent accepts the route
          observedGeneration: 1
          reason: Accepted
          status: 'True'
          type: Accepted
        - lastTransitionTime: '2023-06-30T01:16:09Z'
          message: all backend references successfully resolved
          observedGeneration: 1
          reason: ResolvedRefs
          status: 'True'
          type: ResolvedRefs
      controllerName: stunner.l7mp.io/gateway-operator
      parentRef:
        group: gateway.networking.k8s.io
        kind: Gateway
        name: stunner-config
        namespace: stunner

But when I apply similar route to the same namespace as Gateway is - it works just fine.

apiVersion: gateway.networking.k8s.io/v1alpha2
kind: UDPRoute
metadata:
  name: janus-dev
  namespace: stunner
spec:
  parentRefs:
    - name: stunner-config
  rules:
    - backendRefs:
        - name: janus-dev
          namespace: dev

Now traffic is being passed through stunner and gateway shows attachedRoutes: 1 on the listener status.

@rg0now
Copy link
Member

rg0now commented Jun 30, 2023

Thanks for the report, this is indeed a bug. In fact, it is a combination of two things: a somewhat underdocumented STUNner limitation plus an actual bug:

  • Currently STUNner does not implement cross-namespace Gateway-UDPRoute bindings for simplicity: only UDPRoutes from the same namespace are allowed. This limitation is documented here and here. We didn't think this was an important feature for STUNner, but now that you are reporting we will make sure to fix this (hopefully in the next release). See the new issue here.
  • The bug is that for some reason the URPRoute seems to misreport the Accepted status as True, even though the Gateway rejected the route due to a cross-namespace binding attempt. This then quite understandably creates the illusion that cross-namespace bindings should work. This should be fixed ASAP, see the new issue here.

Is deploying the Gateway and the UDPRoute into the same namespace an acceptable workaround to you until we fix this? Note that, as another subtle STUNner limitation, currently the UDPRoute can refer to any Service in any namespace (see docs here): to comply with the Gateway API we would also need to implement ReferenceGrants, but this is also a low-prio item on the TODO list at this point.

@jpruciak
Copy link
Author

Is deploying the Gateway and the UDPRoute into the same namespace an acceptable workaround to you until we fix this?

This would mean I need to allow application helm chart to modify stunner namespace or add UDPRoutes to stunner-config helm chart. It can be temp workaround, but it's not good. Thank you very much for a quick response anyway, at the moment I'll use this workaround, but I'm waiting for this to be solved! :D

@rg0now
Copy link
Member

rg0now commented Jun 30, 2023

I see. We'll prioritize this feature then.

Quick question: do you want full support for ReferenceGrants (ReferenceGrant is a CRD that you place into a namespace to allow Gateways from other namespaces to accept routes from said namespace or vice versa) or is it enough if we allow UDPRoute bindings from any namespace without restriction?

Anyway, this feature is contingent on another major milestone: maganed dataplane support in the operator. Once that lands, we can easily add support for cross-namespace bindings. Until then, please bear with us. Or better yet: please keep on bugging us frequently on Discord or here so that we do not forget!...:-)

@jpruciak
Copy link
Author

This would be enough: https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1beta1.AllowedRoutes (a field on the listener).
ReferenceGrants seems to be hard to implement and too much husstle for initial support ;)

I don't use discord ;/

@rg0now
Copy link
Member

rg0now commented Jul 6, 2023

This should now fixed as of e770d05 in the gateway-operator repo, can you please test?

The below now works for me perfectly:

apiVersion: gateway.networking.k8s.io/v1alpha2
kind: Gateway
metadata:
  name: stunner-config
  namespace: stunner
spec:
  gatewayClassName: stunner-gatewayclass
  listeners:
    - name: udp-listener
      port: 3478
      protocol: UDP
      allowedRoutes:
        namespaces:
          from: All
apiVersion: gateway.networking.k8s.io/v1alpha2
kind: UDPRoute
metadata:
  name: janus-dev
  namespace: dev
spec:
  parentRefs:
    - name: stunner-config
      namespace: stunner
  rules:
    - backendRefs:
        - name: janus-dev
          namespace: dev

You can also use label selectors to choose the namespaces the gateway would accept routes from:

apiVersion: gateway.networking.k8s.io/v1alpha2
kind: Gateway
metadata:
  name: stunner-config
  namespace: stunner
spec:
  gatewayClassName: stunner-gatewayclass
  listeners:
    - name: udp-listener
      port: 3478
      protocol: UDP
      allowedRoutes:
        namespaces:
          from: Selector
          selector:
            matchLabels:
              udp-gateway: accept

Of course, this requires the target namespace to be labelled with udp-gateway=accept.

Currently this feature is only available on the dev release channel from the stunner/stunner-gateway-operator-dev chart:

helm install stunner-gateway-operator stunner/stunner-gateway-operator-dev --create-namespace --namespace=stunner-system 

We hope to put together a new stable release soon.

Please report back your findings.

@jpruciak
Copy link
Author

@FLM210 Can I please ask you to test the dev version to know if it fixes the problem?

I've no time to break my develop infra just to check if it fixes the issue.

@FLM210
Copy link

FLM210 commented Aug 24, 2023

@FLM210 Can I please ask you to test the dev version to know if it fixes the problem?

I've no time to break my develop infra just to check if it fixes the issue.

The dev version solved my problem, but there is a small issue with the dev version
l7mp/stunner-gateway-operator#39

@rg0now
Copy link
Member

rg0now commented Aug 24, 2023

The dev version solved my problem, but there is a small issue with the dev version l7mp/stunner-gateway-operator#39

This should be fixed by now.
CC: @davidkornel

@davidkornel
Copy link
Member

@FLM210 Can you check it now?
Do not forget to helm repo update.
helm install stunner-gateway-operator stunner/stunner-gateway-operator-dev --namespace=stunner --create-namespace --set stunnerGatewayOperator.dataplane.mode=managed
(Obviously, the managed dataplane flag is needed if you'd like to skip installing STUNner manually.)

@FLM210
Copy link

FLM210 commented Aug 25, 2023

@davidkornel Now the dev version can run normally

@davidkornel
Copy link
Member

Great, if you face any issues feel free to reopen, until then I'm closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants