Error from Hardcoded "client_max_body_size" in HTTPSnippet

[seanocca]

Describe the bug The bug occurs on startup of the gateway pod. It errors out the error message in the screenshot/terminal output section. I have tried to set my own value for the setting client_max_body_size and it won't let me override the value.

To Reproduce

1. Set httpSnippet to this value

httpSnippet: |
    client_max_body_size 256M

2. Start up Loki and receive error on the loki-gateway pod log

Expected behavior I would expect to be able to set this piece of config either through the httpSnippet config line or via the nginxFile. As you have abstracted the file away from the config section it makes it hard to set that value without re-importing the file into the values.yaml file.

Environment:

• Infrastructure: Kubernetes
• Deployment tool: helm 3
• Loki Version: Loki helm version 4.8.0

Screenshots, Promtail config, or terminal output If applicable, add any output to help explain your problem.

nginx: [emerg] "client_max_body_size" directive is duplicate in /etc/nginx/nginx.conf:39

Current config setup https://github.com/grafana/loki/blob/main/production/helm/loki/templates/_helpers.tpl#L515

[TheBaus]

Yeah this does seem to be a bug, I am using the latest published chart version:

NAME           CHART VERSION	APP VERSION
grafana/loki   5.6.1        	2.8.2

if you look at the helpers.tpl link you posted you'll find the hardcoded line on L571: client_max_body_size 4M;

and then below that on L597 is the user supplied config section: {{- with .Values.gateway.nginxConfig.httpSnippet }}

So when one goes to try override with your own values in the nginxConfig section:

nginxConfig:
  httpSnippet: |-
    client_max_body_size 100M;
    proxy_read_timeout   600;
    proxy_send_timeout   600;

You get the nginx emerg error and new gateway pods will not startup:

nginx: [emerg] "client_max_body_size" directive is duplicate in /etc/nginx/nginx.conf:34

Are we missing something here Grafana team? @trevorwhitney

[khanh96le]

I guess, the client_max_body_size field must 1/ have its own value, or 2/ just remove it from the _helpers.tpl file to avoid duplication Meanwhile waiting for the fix from Grafana, I made a workaround by adding a small change following the 1/ approach

In file values.yaml

gateway:
  nginxConfig:
    clientMaxBodySize: "10M"

In file _helpers.tpl

client_max_body_size {{ .Values.gateway.nginxConfig.clientMaxBodySize }};

[chkp-zivhada]

@khanh96le nice workaround. It's important to replace the hard coded values of the nginx config to variables so that we could override them from the values.yaml such as: worker_processes, worker_rlimit_nofile, worker_connections, worker_connections,client_max_body_size , etc..

[trevorwhitney]

This sounds like a bug, thanks for bringing it up. I think the best thing to due here is to move the default client_max_body_size into the default for httpSnippet. I want to be careful not to promote every nginx config to a corresponding value in values.yaml, but I think it's reasonable to move a "block" of config into the httpSnippet section.

The outcome I'd like to see is, if I don't provide any custom values, I get the defaults of what the chart is now producing. If I want to override any value is this section, I have to provide the whole section. Sound good? If so, I'd be happy to review PR. If not, happy to hear your thoughts?

Thanks!

[DanielCastronovo]

Hello, any news ?

This is a true bug ;)

[chkp-shacharsh]

Hi, This bug is somewhat annoying, any update?

Thanks :)

[leotorjeman1]

Hello,

Would greatly appreciate if you can provide any update on this issue as it effect us as well. Thank you

[chkp-pelegk]

any progress with that? this is important issue

[satyamsundaram]

Hey, @trevorwhitney can you review my PR #12924? I've implemented @khanh96le's approach as that seems reasonable, until there is a proper discussion about what should and should not be moved into the httpSnippet section.