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

Helm: Gateway Configuration accessability #12372

Open
Xalphor opened this issue Mar 27, 2024 · 1 comment
Open

Helm: Gateway Configuration accessability #12372

Xalphor opened this issue Mar 27, 2024 · 1 comment
Labels
area/helm type/feature Something new we should do

Comments

@Xalphor
Copy link

Xalphor commented Mar 27, 2024

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

I need two separate users for the gateway component of the HelmChart. One for ReadOnly access and another one for WriteOnly access.Therefore I need to change the .Values.gateway.nginxConfig.file value. But the whole configuration file is generated by a .tpl function called .loki.nginxFile. That configuration is dependant on multiple other variables from helm values, so I can't do minimal invasive changes to it. I'd have to overwrite the whole configuration somehow.

Describe the solution you'd like
I would like the configuration to be part of the helm values.

Describe alternatives you've considered

  1. Disabling the gateway component as a whole to replace it with a similar nginx with custom configuration. Abandoned this because this would be more work then just to replace .Values.gateway.nginxConfig.file
  2. Overwriting the .Values.gateway.nginxConfig.file values. This isn't applicable because managing static configuration for multiple loki instances would be to much overhead in the future. (With changes to the configuration, creating new loki instances in a multi cluster scenario.)
  3. Forking this repository to apply my needed configuration to _helpers.tpl directly. This would lead to massive maintenance overhead in the long run.

Additional context
I know this might be a classical case of the "Satisfy 95% of your userbase completely instead of 100% to a degree". I might be exactly in the 5% of users with exotic feature requests. But I wanted to voice my needs anyway.

If anyone here has an approach I didn't consider yet, I'd be happy to hear them. :)

@JStickler JStickler added type/feature Something new we should do area/helm labels Apr 1, 2024
@Xalphor
Copy link
Author

Xalphor commented Apr 3, 2024

I solved my problem. But I think this could be simplified quite a bit.

First for the solution:

The value I want to overwrite is data.nginx.conf in production/helm/loki/templates/gateway/configmap-gateway.yaml.

That value is .Values.gateway.nginxConfig.file from values.yaml rendered as a go-template.

data:
  nginx.conf: |
    {{- tpl .Values.gateway.nginxConfig.file . | indent 2 }}

Two important points that are relevant for later are:

  1. The rendered template is indented by 2 spaces.
  2. The rendered template is stripped of leading whitespace by the - character after the opening {{.

The default value of .Values.gateway.nginxConfig.file is:

gateway:
  nginxConfig:
    file: |
      {{- include "loki.nginxFile" . | indent 2 -}}

So the loki.nginxFile function from production/helm/loki/templates/gateway/_helpers-gateway.tpl which returns a multiline string containing valid Nginx configuration with additional go-templating included is rendered.


My first approach was to replace gateway.nginxConfig.file with the content of the loki.nginxFile function.

gateway:
  enabled: true
  nginxConfig:
    file: |
        worker_processes  5;  ## Default: 1
        error_log  /dev/stderr;
        pid        /tmp/nginx.pid;
        worker_rlimit_nofile 8192;
        ....

This led to errors because of two reasons.

  1. The function has a leading empty line which didn't seem important to me. Removing that line causes invalid yaml when rendering the contents of gateway.nginxConfig.file.
data:
  nginx.conf: |    worker_processes  5;  ## Default: 1
    error_log  /dev/stderr;
    pid        /tmp/nginx.pid;
    worker_rlimit_nofile 8192;
   ...
  1. The indentation done in values.yaml gets stripped like I mentioned above. So even with the blank line the rendered chart is invalid yaml.
data:
  nginx.conf: |  
  worker_processes  5;  ## Default: 1
  error_log  /dev/stderr;
  pid        /tmp/nginx.pid;
  worker_rlimit_nofile 8192;
  ...

Solution

The solution was to export the configuration into a separate value in and then indent that values with go templating in gateway.nginxConfig.file like this:

gateway:
 enabled: true
 nginxConfig:
   file: |
     {{- .Values.gateway.nginxConfig.myfile | indent 2 -}}
   myfile: |

       worker_processes  5;  ## Default: 1
       error_log  /dev/stderr;
       pid        /tmp/nginx.pid;
       worker_rlimit_nofile 8192;

Now to the simplification proposal:

In my opinion the prefix empty line could be included into the production/helm/loki/gateway/configmap-gateway.yaml template itself. Moreover the {{- stripping of indentation could be removed and the output of the _helpers-gateway.loki.nginxFile function could be indented instead. Like this:

configmap-gateway.yaml:

{{- if and .Values.gateway.enabled  }}
apiVersion: v1
kind: ConfigMap
metadata:
  name: {{ include "loki.gatewayFullname" . }}
  namespace: {{ $.Release.Namespace }}
  labels:
    {{- include "loki.gatewayLabels" . | nindent 4 }}
data:
  nginx.conf: |

    {{ tpl .Values.gateway.nginxConfig.file . | indent 2 }}
{{- end }}

_helpers.tpl:

{{/* Snippet for the nginx file used by gateway */}}
{{- define "loki.nginxFile" }}
  worker_processes  5;  ## Default: 1
  error_log  /dev/stderr;
  pid        /tmp/nginx.pid;
  worker_rlimit_nofile 8192;

  events {
    worker_connections  4096;  ## Default: 1024
  }

If that'd be done overwriting the gateway configmap with the helm values would look like this:

gateway:
  enabled: true
  nginxConfig:
    file: |
        worker_processes  5;  ## Default: 1
        error_log  /dev/stderr;
        pid        /tmp/nginx.pid;
        worker_rlimit_nofile 8192;
        ...

That'd have spared at least me from a lot of headaches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm type/feature Something new we should do
Projects
None yet
Development

No branches or pull requests

2 participants