[Bug]: client-max-body-size on (upstream) VirtualServer is never considered

[andrew-s]

Version

edge

What Kubernetes platforms are you running on?

Other

Steps to reproduce

Bug

When applying the client-max-body-size option on the upstream, via VirtualServer it ultimately has no effect. This is not say that it's not applied to the conf because it is, this is more to say that it doesn't work - this seems to be that the client max body size is then set on the sub location resource and not the originally matching location.

This may be either intended behaviour or a bug, but since it's nginx that's interpreting that conf, that would be a bug in nginx, however I think there would be ways to workaround this in the operator.

Reproduction and testing

I've included some examples here but, they're really to show what's going on more than to be things you can just take and run (as the upstream itself is not provided).

apiVersion: k8s.nginx.org/v1
kind: VirtualServer
metadata:
  name: vs-test
spec:
  ingressClassName: nginx  
  upstreams:
  - name: max-body-test
    service: max-body-test-service
    port: 3000
    keepalive: 32
    client-max-body-size: 15m
  routes:
  - path: ~* '^/max-body-test$'
    matches:
      - conditions:
        - variable: $request_method
          value: POST
        action:
          proxy:
            upstream: max-body-test

Once this is deployed, this becomes the following nginx conf; (this is a snippet as the actual conf is quite a bit longer but the important parts are included);

upstream vs-test-max-body-size {
    zone vs-test-max-body-size256k;
    random two least_conn;
    server 10.244.221.35:3000 max_fails=1 fail_timeout=10s max_conns=0;
    keepalive 32;
}

map $request_method $vs_test_matches_0_match_0_cond_0 {
    "POST" 1;
    default 0;
}
map $vs_test_matches_0_match_0_cond_0 $vs_test_matches_0 {
    ...
}
server {
    listen 80;
    ...
    location ~* '^/max-body-size$' {
        rewrite ^ $vs_test_matches_0 last;
    }

    location @return_0 {
        ...
    }

    location /internal_location_matches_0_match_0 {
        set $service "max-body-size-service";
        internal;

        ...
        client_max_body_size 15m;
        ...
    }
    location /internal_location_matches_0_default {
        ...
    }
}

In the above example, we can see that our client_max_body_size 15m; was added to location /internal_location_matches_0_match_0 { as that's the match that proxies to our upstream. However, the default of 1m is still used so we're unable to send anything larger still.

If we modify this slightly to change the originally matching location block;

location ~* '^/max-body-size$' {
        rewrite ^ $vs_test_matches_0 last;
    }

And add the max body size here;

location ~* '^/max-body-size$' {
        client_max_body_size 15m;
        rewrite ^ $vs_test_matches_0 last;
    }

And we copy this into nginx;

kubectl cp ./vs-test.conf nginx-ingress/nginx-ingress-controller-pod:/etc/nginx/conf.d/vs-test.conf
kubectl exec -it -n nginx-ingress nginx-ingress-controller-pod -- sh -c "nginx -s reload"

We're now able to send bodies that are larger than 1m and upto our defined 15m.

I have no doubts that sever and http snippets would also fix this but, this would then apply to all defined endpoints and upstreams within that scope which may not be what we want.

Possible Solutions

1. We can work around this potential bug by adding this configuration into the originally matching location block. It would be a case of adding this configuration option to the s.InternalRedirectLocations struct or if the option is within the loop of another struct that could be used, the template for this here;

kubernetes-ingress/internal/configs/version2/nginx.virtualserver.tmpl

Line 190 in 278f841

location {{ $l.Path }} {

2. This is reported as a nginx bug and ultimately is fixed there without any changes to this operator.

Related issues

#5859
#5317

[github-actions]

Hi @andrew-s 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!