This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

@mneverov thanks for posting this. Your expectation seems absolutely valid.

There is shortage of resources. If you have interest to implement the annotation, please discuss in the ingress-nginx-dev channel of the Kubernetes slack. Also you can join community meetings.

But I also request you to study why client-body-timeout is a configMap option currently https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#client-body-timeout . It will take me some reading and testing to get to know this directive as I am not a developer. What is needed to be confirmed is why you can not use it from the configMap

https://nginx.org/en/docs/http/ngx_http_core_module.html#client_body_timeout

/assign

Also, just FYI, the snippets feature is targeted for deprecation so there is no future for snippets.

Also, just FYI, the snippets feature is targeted for deprecation so there is no future for snippets.

yes, thanks. That was the reason for raising this issue

Ah ok. thanks.
So in that case I am curious if you already know why you can not use the configMap to set client-body-timeout.

Ah ok. thanks. So in that case I am curious if you already know why you can not use the configMap to set client-body-timeout.

Will check, thanks

@longwuyuan thanks for your suggestion. When client-body-timeout is configured in the configmap it is propagated to nginx http (global context). I was not aware of this and it covers my use case.
OTOH, when it is specified in the server-snippet the timeout is rendered inside a server context, i.e. it is local to the created Ingress.
Please let me know if the implementation still make sense as there is a workaround (global timeout).

From user experience point of view, because of the other timeouts as annotations, it will be nice to have client-body-timeout also as annotation. But that is just user-experience.

The nginx docs is clear that the directive is accepted in all contexts, the perspective of a webserver vs a reverseproxy vs a ingress-controller comes into play. I will need to test the impact of using configMap vs server-snippet vs configuration-snippet. On top of that if there was a annotation existing, it would likely be implemented to be used in the location context.

Now that above part is theory. The practical side is there are no other demands on this directive, in the way that you are requiring, most likely because there are no open issues for timeout on both (request as well as response) streaming.

So because there is nothing broken and because there are no resources available to work on this issue, and configMap works for you, my suggestion is best not to pursue this any further now.

If a developer gets time, then they will comment and commit time so it can/will be taken up then.

Thanks you very much for the feedback as I was thinking that there is no workaround.

/assign
/close

Closing because configMap key for client-body-timeout is a existing workaround.

@longwuyuan: Closing this issue.

@longwuyuan thanks for the suggested workaround and prompt reply

@longwuyuan I'd like to resurrect the issue. The reason is as we discussed -- the timeout specified in the ConfigMap is set as global in nginx for all ingresses. It appears we have quite a few of them and setting global timeout is not an option. The current workaround is switch back to server-snippet

ok.
You can type /reopen here.
The developer guide section of docs has 2 sub-sections that may help if needed https://kubernetes.github.io/ingress-nginx/developer-guide/getting-started/

/reopen

@mneverov: Reopened this issue.

@longwuyuan I found that there is already client-body-buffere-size annotation, i.e. annotations related to client_body_* and client_header_* configurations. I can create a separate package client and add the new annotation there, OTOH I see that annotations are bundled by functionality (auth, proxy, etc), so I would like to do a refactoring: rename package clientbodybuffersize to client and create client.Config struct that contains both fields (BodyBufferSize and BodyTimeout).
It will make this PR bigger (separated commits for the refactoring and actual implementation), but cleaner and aligned with the rest of the codebase. Other optioin is I can create separate PRs. WDYT?

I am not a developer so please wait for other comments.

cc @rikatz @tao12345666333 @ElvinEfendi I think this issue and the suggested solution (PR) above, may need your comments, because its about a annotation that should have existed but is not.

@mneverov i prefer the single bigger PR (is it possible squash commits !) as that means one place to track. But as said, please wait for comments from others.

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.