nginxinc / kubernetes-ingress

NGINX and NGINX Plus Ingress Controllers for Kubernetes
https://docs.nginx.com/nginx-ingress-controller
Apache License 2.0
4.65k stars 1.96k forks source link

Prevent a VirtualServerRoute from being used multiple times in a Virtual Server to avoid reload failure #6611

Open haywoodsh opened 4 days ago

haywoodsh commented 4 days ago

Is your feature request related to a problem? Please describe.

Currently, NIC does not prevent the same VSR being used twice in the same VS, allowing specs that results in generating the same location block /coffee and the same upstream twice, which causes NGINX to trigger a "duplicate upstream" error upon reload:

Consider this VSR:

apiVersion: k8s.nginx.org/v1
kind: VirtualServerRoute
metadata:
  name: coffee
  namespace: coffee
spec:
  host: cafe.example.com
  upstreams:
  - name: coffee
    service: coffee-svc
    port: 80
  subroutes:
  - path: /coffee
    action:
      pass: coffee

Currently, the following VS spec is allowed.

apiVersion: k8s.nginx.org/v1
kind: VirtualServer
metadata:
  name: cafe
  namespace: cafe
spec:
  host: cafe.example.com
  tls:
    secret: cafe-secret
  routes:
  - path: /coffee
    route: coffee/coffee
  - path: /
    route: coffee/coffee

which would result in the location block with the same path and upstreams with the same name vs_cafe_cafe_vsr_coffee_coffee_coffee generated in the NGINX config, resulting in the following reloading error

  Warning  AddedOrUpdatedWithError    11s                nginx-ingress-controller  Configuration for cafe/cafe was added or updated ; but was not applied: error when updating config from ConfigMap: nginx reload failed: command /usr/sbin/nginx -s reload -e stderr stdout: ""
stderr: "2024/10/08 08:30:19 [emerg] 42#42: duplicate upstream \"vs_cafe_cafe_vsr_coffee_coffee_coffee\" in /etc/nginx/conf.d/vs_cafe_cafe.conf:10\n"

Describe the solution you'd like Disallow a VSR being used multiple times to prevent reload failure

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context This config error in obvious in the current specification where the VSRs is explicitly referenced in the VS spec. However, it might become less apparent in the future work #5293, where VSRs are associated with a VS through different methods.

github-actions[bot] commented 4 days ago

Hi @haywoodsh 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 :slightly_smiling_face:

Cheers!