Redis can't be disabled without manual config #584

Closed
opened 2023-12-10 11:20:10 +00:00 by Claas · 3 comments
Claas commented 2023-12-10 11:20:10 +00:00 (Migrated from gitea.com)

I want to have redis completely disabled. This is supposed to be possible using the "redis-cluster" section:

redis-cluster:
  enabled: false
  cluster: # should be useless
    nodes: 1 
    replicas: 0 

However, the session store still points to redis, which looks like this in the result manifest:

---
# Source: gitea/templates/gitea/config.yaml
apiVersion: v1
kind: Secret
metadata:
  name: gitea-inline-config
type: Opaque
stringData:
  # ....
  session: |-
    PROVIDER=redis
    PROVIDER_CONFIG=
---

My assumption is that another if-clause in the helper template would help:

  {{- if not (get .Values.gitea.config.session "PROVIDER") -}}  
    {{- $_ := set .Values.gitea.config.session "PROVIDER" "redis" -}} # always redis, unless explicitly overwritten
  {{- end -}}
  {{- if not (get .Values.gitea.config.session "PROVIDER_CONFIG") -}}
    {{- $_ := set .Values.gitea.config.session "PROVIDER_CONFIG" (include "redis.dns" .) -}}
  {{- end -}}

The workaround is to set the session type to memory manually:

  config: 
    session:
      PROVIDER: memory
I want to have redis completely disabled. This is supposed to be possible using the "redis-cluster" section: ``` redis-cluster: enabled: false cluster: # should be useless nodes: 1 replicas: 0 ``` However, the session store still points to redis, which looks like this in the result manifest: ``` --- # Source: gitea/templates/gitea/config.yaml apiVersion: v1 kind: Secret metadata: name: gitea-inline-config type: Opaque stringData: # .... session: |- PROVIDER=redis PROVIDER_CONFIG= --- ``` My assumption is that another if-clause in the helper template would help: ``` {{- if not (get .Values.gitea.config.session "PROVIDER") -}} {{- $_ := set .Values.gitea.config.session "PROVIDER" "redis" -}} # always redis, unless explicitly overwritten {{- end -}} {{- if not (get .Values.gitea.config.session "PROVIDER_CONFIG") -}} {{- $_ := set .Values.gitea.config.session "PROVIDER_CONFIG" (include "redis.dns" .) -}} {{- end -}} ``` The workaround is to set the session type to memory manually: ``` config: session: PROVIDER: memory ```
justusbunsi commented 2023-12-10 11:55:20 +00:00 (Migrated from gitea.com)

Hi. Thanks for confirming the first part of #564. The mentioned workaround should be the default in this configuration scenario. Do you fancy opening a PR for it?

Hi. Thanks for confirming the first part of #564. The mentioned workaround should be the default in this configuration scenario. Do you fancy opening a PR for it?
pat-s commented 2023-12-11 07:52:05 +00:00 (Migrated from gitea.com)

As said in other comments/issues, if you disable a default, it's on you to correctly configure a fallback as the chart won't automatically pick an alternative.

This has been discussed in quite a few issues already and there's also a section in the README covering it (although this is only covered in the changelog section 9.0.0 for now and should probably be copied/moved to a configuration section).

@justusbunsi I am not sure we should opt for an automatic fallback as otherwise people won't know/be aware what config they actually use. This assumption is partly backend by the fact that this is the third or fourth issue about this (which is not great itself) but people don't seem to read existing issues or the README section which relates to it. But we should definitely find a solution to this, either addressing it more prominently in the README or by force-overriding my opinion on this 😅 I know we are also still discussing the redis non-cluster option...

Also, I am not sure "memory" being good fallback to use for production - and I would prefer that people explicitly set it if they want to use it and the chart doesn't do it implicitly.

Given the multi-key mapping required for session and friends and the fact that we don't provide "convenience mappings" in values.yml (like gitea.config.session.use_memory: true this must be set using the PROVIDER and PROVIDER_CONFIG config mappings.

As said in other comments/issues, if you disable a default, it's on you to correctly configure a fallback as the chart won't automatically pick an alternative. This has been discussed in quite a few issues already and there's also a section in the README covering it ([although this is only covered in the changelog section 9.0.0 for now](https://gitea.com/gitea/helm-chart#user-content-upgrading) and should probably be copied/moved to a configuration section). @justusbunsi I am not sure we should opt for an automatic fallback as otherwise people won't know/be aware what config they actually use. This assumption is partly backend by the fact that this is the third or fourth issue about this (which is not great itself) but people don't seem to read existing issues or the README section which relates to it. But we should definitely find a solution to this, either addressing it more prominently in the README or by force-overriding my opinion on this 😅 I know we are also still discussing the redis non-cluster option... Also, I am not sure "memory" being good fallback to use for production - and I would prefer that people explicitly set it if they want to use it and the chart doesn't do it implicitly. Given the multi-key mapping required for `session` and friends and the fact that we don't provide "convenience mappings" in `values.yml` (like `gitea.config.session.use_memory: true` this must be set using the `PROVIDER` and `PROVIDER_CONFIG` config mappings.
pat-s commented 2023-12-11 13:20:48 +00:00 (Migrated from gitea.com)

I've rethought this topic. Given that https://docs.gitea.com/next/administration/config-cheat-sheet#session-session has memory set as the default, it makes sense to have it as the fallback option. No matter if it's a good setting or not.

I'll create a PR which conditionally checks for the present of redis-cluster and if not, configures memory as the fallback.

I've rethought this topic. Given that https://docs.gitea.com/next/administration/config-cheat-sheet#session-session has `memory` set as the default, it makes sense to have it as the fallback option. No matter if it's a good setting or not. I'll create a PR which conditionally checks for the present of redis-cluster and if not, configures memory as the fallback.
Sign in to join this conversation.
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: lunny/helm-chart#584
No description provided.