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

Unable to use a regex with multiple subroute of VirtualServerRoute #5258

Closed
lucaspouzac opened this issue Mar 15, 2024 · 8 comments
Closed

Comments

@lucaspouzac
Copy link

I would like to delegate a complete route defined in the VirtualServer to the subroute level of the VirtualServerRoute and use regex on subroute.

apiVersion: k8s.nginx.org/v1
kind: VirtualServer
metadata:
  name: test-foo-virtualserver
spec:
  host: foo.example.com
  routes:
    - path: /foo
      route: test-foo
---
apiVersion: k8s.nginx.org/v1
kind: VirtualServerRoute
metadata:
  name: test-foo
spec:
  host: foo.example.com
  subroutes:
    - path: /foo/bar/
      action:
        pass: bar
    - path: ~ ^/foo/(bar1|bar2)
      action:
        pass: other
  upstreams:
    - name: bar
      service: bar
    - name: other
      service: other

When I create these VS/VSR, a warning is throwed.

$kubectl get vs,vsr
NAME                                                 STATE     HOST              IP         PORTS      AGE
virtualserver.k8s.nginx.org/test-foo-virtualserver   Warning   foo.example.com   10.x.y.z   [80,443]   7m36s

NAME                                        STATE     HOST              IP         PORTS      AGE
virtualserverroute.k8s.nginx.org/test-foo   Warning   foo.example.com   10.x.y.z   [80,443]   7m36s

$ kubectl describe virtualserver.k8s.nginx.org/test-foo-virtualserver
....
Events:
  Type     Reason                     Age   From                      Message
  ----     ------                     ----  ----                      -------
  Warning  AddedOrUpdatedWithWarning  5m9s  nginx-ingress-controller  Configuration for default/test-foo-virtualserver was added or updated with warning(s): VirtualServerRoute default/test-foo doesn't exist or invalid

$ kubectl describe virtualserverroute.k8s.nginx.org/test-foo
....
Events:
  Type     Reason                Age    From                      Message
  ----     ------                ----   ----                      -------
  Warning  NoVirtualServerFound  6m40s  nginx-ingress-controller  VirtualServer is invalid or doesn't exist
  Warning  NoVirtualServerFound  6m40s  nginx-ingress-controller  VirtualServer is invalid or doesn't exist
  Warning  Ignored               6m40s  nginx-ingress-controller  VirtualServer default/test-foo-virtualserver ignores VirtualServerRoute

Copy link

Hi @lucaspouzac thanks for reporting!

Be sure to check out the docs and the Contributing Guidelines while you wait for a human to take a look at this 🙂

Cheers!

@lucaspouzac
Copy link
Author

As this issue : #797, maybe a regression on latest version 3.4.3 ?

@haywoodsh
Copy link
Contributor

Hi @lucaspouzac this is the expected behavior, since we did not intend to allow paths in VS and the VSR referenced to be in different types (prefix, regex, exact paths). The reason you were able to do this in the previous releases was due to a bug, which if you have a prefix subroute path in the virtual server and a regex subroute path in VSR, the validation is skipped, which means not only ~ ^/foo/(bar1|bar2) but also random strings like ~ ^/abc are accepted. #4744 was included in 3.4.3 specifically to address this.

For your specific example, you might want to separate the subroute path in your VS into one prefix path and one regex subroute path, such as the following.

apiVersion: k8s.nginx.org/v1
kind: VirtualServer
metadata:
  name: test-foo-virtualserver
spec:
  host: foo.example.com
  routes:
    - path: /foo
      route: test-foo
    - path: ~ ^/foo/(bar1|bar2)
     route: test-foo-regex
---
apiVersion: k8s.nginx.org/v1
kind: VirtualServerRoute
metadata:
  name: test-foo
spec:
  host: foo.example.com
  subroutes:
    - path: /foo/bar/
      action:
        pass: bar
  upstreams:
    - name: bar
      service: bar
---
apiVersion: k8s.nginx.org/v1
kind: VirtualServerRoute
metadata:
  name: test-foo-regex
spec:
  host: foo.example.com
  subroutes:
    - path: ~ ^/foo/(bar1|bar2)
      action:
        pass: other
  upstreams:
    - name: other
      service: other

Please let me know if this works for you or if you have any feedbacks.

@lucaspouzac
Copy link
Author

Hi @haywoodsh, this is what I did to get around the problem. It works well technically. But different teams manage the VirtualServer (ops team) and the VirtualServerRoute (apps teams). This adds a strong relationship between ops teams and apps teams.

Teams apps cannot be autonomous when delegating a complete application if there is a need to use regex.

I don't understand the blocking to allow this operation if no regex is used on the VirtualServer side.

Is there just a need to validate that the virtualServer path matches the different virtualServerroute paths?

@lucaspouzac
Copy link
Author

lucaspouzac commented Mar 19, 2024

In idea, the algorithm could be this?

package main

import (
	"fmt"
	"strings"
)

func main() {
	testPath([]string{"/foo/bar/", "~ ^/foo/(bar1|bar2)"}, "/foo")
	testPath([]string{"/foo/bar/", "~ ^/foo/(bar1|bar2)"}, "/bar")
	testPath([]string{"/foo/bar/", "~ ^/bar/(bar1|bar2)"}, "/foo")
}

func testPath(virtualServerRoutePaths []string, virtualServerPath string) {
	match := true
	for _, virtualServerRoutePath := range virtualServerRoutePaths {
		match = match && strings.HasPrefix(strings.TrimLeft(virtualServerRoutePath, "~^ "), virtualServerPath)
	}
	fmt.Println(virtualServerRoutePaths, " matches with ", virtualServerPath, " ? ", match)
}
[/foo/bar/ ~ ^/foo/(bar1|bar2)]  matches with  /foo  ?  true
[/foo/bar/ ~ ^/foo/(bar1|bar2)]  matches with  /bar  ?  false
[/foo/bar/ ~ ^/bar/(bar1|bar2)]  matches with  /foo  ?  false

Maybe missing cases ?

@jjngx
Copy link
Contributor

jjngx commented Mar 26, 2024

Hi @lucaspouzac, as @haywoodsh mentioned before, this is expected behaviour:

"we did not intend to allow paths in VS and the VSR referenced to be in different types (prefix, regex, exact paths). The reason you were able to do this in the previous releases was due to a bug"

@jjngx jjngx closed this as completed Mar 26, 2024
@rmacian
Copy link

rmacian commented May 30, 2024

Hi @lucaspouzac this is the expected behavior, since we did not intend to allow paths in VS and the VSR referenced to be in different types (prefix, regex, exact paths). The reason you were able to do this in the previous releases was due to a bug, which if you have a prefix subroute path in the virtual server and a regex subroute path in VSR, the validation is skipped, which means not only ~ ^/foo/(bar1|bar2) but also random strings like ~ ^/abc are accepted. #4744 was included in 3.4.3 specifically to address this.

For your specific example, you might want to separate the subroute path in your VS into one prefix path and one regex subroute path, such as the following.

apiVersion: k8s.nginx.org/v1
kind: VirtualServer
metadata:
  name: test-foo-virtualserver
spec:
  host: foo.example.com
  routes:
    - path: /foo
      route: test-foo
    - path: ~ ^/foo/(bar1|bar2)
     route: test-foo-regex
---
apiVersion: k8s.nginx.org/v1
kind: VirtualServerRoute
metadata:
  name: test-foo
spec:
  host: foo.example.com
  subroutes:
    - path: /foo/bar/
      action:
        pass: bar
  upstreams:
    - name: bar
      service: bar
---
apiVersion: k8s.nginx.org/v1
kind: VirtualServerRoute
metadata:
  name: test-foo-regex
spec:
  host: foo.example.com
  subroutes:
    - path: ~ ^/foo/(bar1|bar2)
      action:
        pass: other
  upstreams:
    - name: other
      service: other

Please let me know if this works for you or if you have any feedbacks.

same problem here. I was having virvualserversroutes like

apiVersion: k8s.nginx.org/v1
kind: VirtualServerRoute
metadata:
  name: nginx-ingress-system-logs
  namespace: {{ kubernetes_system_namespace }}
spec:
  host: "{{ env }}.domain.com"
  upstreams:
  - name: kibana
    service: eck-kibana-kb-http
    port: {{ k8s_config.system.kibana.exposed_port_number }}
  subroutes:
  - path: ~* ^/logs/?(.*)

but the VirtualServer was in exact form

 - path: /logs
    route: {{ kubernetes_system_namespace }}/nginx-ingress-system-logs

using the same regex on both vsr and vs solved my problem

@sass1997
Copy link

This is creating a lot of useless overhead by splitting each regex path into a seperate virtualserver. As far as I understood @rmacian the fix was more on the validation side. This means the previous sturcutre 1 prefix virtualserver and then many diffrent on the virtualserver route side. For me the question now is should I do the overhead by touching all virtualserver and create additonal virtualserver route or I move everything into my virtualserverroute and I don't have any issues anymore. What would recommend me. Is there any advantage from now to use both resources?

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

No branches or pull requests

5 participants