Redis config customizations superseded by builtin environment variables

[Kaurin]

Describe your Issue

In my example I want to use TLS with the builtin redis. Config overrides get provisioned, but environmnet variables take precedence not allowing my config to use the tls:// prefix. Instead, the default tcp:// prefix is used.

Housekeeping

Clean setup, PVCs deleted

Config

nextcloud:
  defaultConfigs:
    redis.config.php: false
  configs:
    redis.config.php: |-
      <?php
      $CONFIG = array (
        'memcache.distributed' => '\OC\Memcache\Redis',
        'memcache.locking' => '\OC\Memcache\Redis',
        'redis' => array(
          'host' => "tls://nextcloud-wrapper-redis-master.default.svc.cluster.local",
          'port' => getenv('REDIS_HOST_PORT') ?: 6379,
          'password' => getenv('REDIS_HOST_PASSWORD'),
          'ssl_context' => [
              'verify_peer_name' => false
          ],
        ),
      );

redis:
  architecture: standalone
  enabled: true
  
  auth:
    enabled: true
    existingSecret: nextcloud-redis
    existingSecretPasswordKey: redisPw
  
  tls:
    enabled: true
    authClients: true
    autoGenerated: true

  metrics.enabled: true

Error

Error from /var/www/html/data/nextcloud.log (removed "Trace" and "Previous" to save space)

Notice tcp:// instead of tls://

{
  "reqId": "7zewVy1y4TPodURLtaNj",
  "level": 3,
  "time": "2024-05-08T02:16:04+00:00",
  "remoteAddr": "10.42.0.1",
  "user": "--",
  "app": "remote",
  "method": "GET",
  "url": "/status.php",
  "message": "read error on connection to tcp://nextcloud-wrapper-redis-master:6379",
  "userAgent": "kube-probe/1.29",
  "version": "29.0.0.19",
  "exception": {
    "Exception": "RedisException",
    "Message": "read error on connection to tcp://nextcloud-wrapper-redis-master:6379",
    "Code": 0,
    "Trace": [],
    "File": "/var/www/html/lib/private/Session/Internal.php",
    "Line": 213,
    "Previous": {},
    "message": "read error on connection to tcp://nextcloud-wrapper-redis-master:6379",
    "exception": {},
    "CustomMessage": "read error on connection to tcp://nextcloud-wrapper-redis-master:6379"
  }
}

Provisioned configs

Configs look good (not sure why doubled, but OK)

root@nextcloud-wrapper-6bd7885d8d-r7j5f:/var/www/html# grep -RIi redis /var/www/html/config

Output:

/var/www/html/config/redis.config.php:  'memcache.distributed' => '\OC\Memcache\Redis',
/var/www/html/config/redis.config.php:  'memcache.locking' => '\OC\Memcache\Redis',
/var/www/html/config/redis.config.php:  'redis' => array(
/var/www/html/config/redis.config.php:    'host' => "tls://nextcloud-wrapper-redis-master.default.svc.cluster.local",
/var/www/html/config/redis.config.php:    'port' => getenv('REDIS_HOST_PORT') ?: 6379,
/var/www/html/config/redis.config.php:    'password' => getenv('REDIS_HOST_PASSWORD'),
/var/www/html/config/config.php:  'memcache.distributed' => '\\OC\\Memcache\\Redis',
/var/www/html/config/config.php:  'memcache.locking' => '\\OC\\Memcache\\Redis',
/var/www/html/config/config.php:  'redis' => 
/var/www/html/config/config.php:    'host' => 'tls://nextcloud-wrapper-redis-master.default.svc.cluster.local',

Environment variables

Environment variables seem to have precedence over configs, rendering configs useless.

root@nextcloud-wrapper-6bd7885d8d-r7j5f:/var/www/html# env | grep -i redis

Output (notice tcp://)

NEXTCLOUD_WRAPPER_REDIS_MASTER_SERVICE_HOST=10.43.248.145
NEXTCLOUD_WRAPPER_REDIS_MASTER_PORT_6379_TCP_PORT=6379
REDIS_HOST=nextcloud-wrapper-redis-master
NEXTCLOUD_WRAPPER_REDIS_MASTER_PORT=tcp://10.43.248.145:6379
REDIS_HOST_PASSWORD=REDACTED_REDACTED
NEXTCLOUD_WRAPPER_REDIS_MASTER_SERVICE_PORT_TCP_REDIS=6379
NEXTCLOUD_WRAPPER_REDIS_MASTER_PORT_6379_TCP=tcp://10.43.248.145:6379
NEXTCLOUD_WRAPPER_REDIS_MASTER_SERVICE_PORT=6379
NEXTCLOUD_WRAPPER_REDIS_MASTER_PORT_6379_TCP_ADDR=10.43.248.145
NEXTCLOUD_WRAPPER_REDIS_MASTER_PORT_6379_TCP_PROTO=tcp
REDIS_HOST_PORT=6379

Probable culprit

First, the env variables are populated in the helpers file
Then, nextcloud.env is used in nextcloud containers

If I am correct in saying that nextcloud favors env variables, then this helm chart approach probably has to be reworked so these customizations can actually take place.

Workaround

I thought I could just provision redis separately and just make sure to have the REDIS_HOST_PASSWORD in extraEnv. Unfortunately, this doesn't work either (different error - Redis went away). The workaround might be possible, but I have given up on it for now and will just go without TLS for now.

Conclusion

Would be nice if helm logic would allow us to override certain aspects of the redis config.

It would be even nicer if I'm completely wrong here and missing like one line of config :)

[jessebot]

Hoi!

Looks like redis host is templated here:

helm/charts/nextcloud/templates/_helpers.tpl

Lines 215 to 231 in 1ae7421

	{{- if .Values.redis.enabled }}
	- name: REDIS_HOST
	value: {{ template "nextcloud.redis.fullname" . }}-master
	- name: REDIS_HOST_PORT
	value: {{ .Values.redis.master.service.ports.redis | quote }}
	{{- if .Values.redis.auth.enabled }}
	{{- if and .Values.redis.auth.existingSecret .Values.redis.auth.existingSecretPasswordKey }}
	- name: REDIS_HOST_PASSWORD
	valueFrom:
	secretKeyRef:
	name: {{ .Values.redis.auth.existingSecret }}
	key: {{ .Values.redis.auth.existingSecretPasswordKey }}
	{{- else }}
	- name: REDIS_HOST_PASSWORD
	value: {{ .Values.redis.auth.password }}
	{{- end }}
	{{- end }}

And the redis config is templated here if you're using default (which you're not):

helm/charts/nextcloud/templates/config.yaml

Lines 32 to 48 in 1ae7421

	{{- if index .Values.nextcloud.defaultConfigs "redis.config.php" }}
	redis.config.php: |-
	<?php
	if (getenv('REDIS_HOST')) {
	$CONFIG = array (
	'memcache.distributed' => '\OC\Memcache\Redis',
	'memcache.locking' => '\OC\Memcache\Redis',
	'redis' => array(
	'host' => getenv('REDIS_HOST'),
	'port' => getenv('REDIS_HOST_PORT') ?: 6379,
	{{- if .Values.redis.auth.enabled }}
	'password' => getenv('REDIS_HOST_PASSWORD'),
	{{- end }}
	),
	);
	}
	{{- end }}

And your custom redis config would be templated here:

helm/charts/nextcloud/templates/config.yaml

Lines 12 to 15 in 1ae7421

	{{- range $key, $value := .Values.nextcloud.configs }}
	{{ $key }}: |-
	{{- $value | nindent 4 }}
	{{- end }}

According to the above, it should allow your override, unless I'm missing something (which I could be).

Digging a bit further into this...

It looks like redis ssl support in nextcloud/server was added here 3 years ago: nextcloud/server@ed10d85

but it doesn't look like the ssl_context options were passed in to the configs above or the one in nextcloud/docker here.

The config.sample.php for redis under the ssl section in the official nextcloud/server repo shows you may need an additional section. I've included the samples below for you to reference:

click for config.sample.php redis sections

/**
 * Connection details for redis to use for memory caching in a single server configuration.
 *
 * For enhanced security it is recommended to configure Redis
 * to require a password. See http://redis.io/topics/security
 * for more information.
 *
 * We also support redis SSL/TLS encryption as of version 6.
 * See https://redis.io/topics/encryption for more information.
 */
'redis' => [
	'host' => 'localhost', // can also be a unix domain socket: '/tmp/redis.sock'
	'port' => 6379,
	'timeout' => 0.0,
	'read_timeout' => 0.0,
	'user' =>  '', // Optional: if not defined, no password will be used.
	'password' => '', // Optional: if not defined, no password will be used.
	'dbindex' => 0, // Optional: if undefined SELECT will not run and will use Redis Server's default DB Index.
	// If redis in-transit encryption is enabled, provide certificates
	// SSL context https://www.php.net/manual/en/context.ssl.php
	'ssl_context' => [
		'local_cert' => '/certs/redis.crt',
		'local_pk' => '/certs/redis.key',
		'cafile' => '/certs/ca.crt'
	]
],

/**
 * Connection details for a Redis Cluster.
 *
 * Redis Cluster support requires the php module phpredis in version 3.0.0 or
 * higher.
 *
 * Available failover modes:
 *  - \RedisCluster::FAILOVER_NONE - only send commands to master nodes (default)
 *  - \RedisCluster::FAILOVER_ERROR - failover to slaves for read commands if master is unavailable (recommended)
 *  - \RedisCluster::FAILOVER_DISTRIBUTE - randomly distribute read commands across master and slaves
 *
 * WARNING: FAILOVER_DISTRIBUTE is a not recommended setting, and we strongly
 * suggest to not use it if you use Redis for file locking. Due to the way Redis
 * is synchronized it could happen, that the read for an existing lock is
 * scheduled to a slave that is not fully synchronized with the connected master
 * which then causes a FileLocked exception.
 *
 * See https://redis.io/topics/cluster-spec for details about the Redis cluster
 *
 * Authentication works with phpredis version 4.2.1+. See
 * https://github.com/phpredis/phpredis/commit/c5994f2a42b8a348af92d3acb4edff1328ad8ce1
 */
'redis.cluster' => [
	'seeds' => [ // provide some or all of the cluster servers to bootstrap discovery, port required
		'localhost:7000',
		'localhost:7001',
	],
	'timeout' => 0.0,
	'read_timeout' => 0.0,
	'failover_mode' => \RedisCluster::FAILOVER_ERROR,
	'user' =>  '', // Optional: if not defined, no password will be used.
	'password' => '', // Optional: if not defined, no password will be used.
	// If redis in-transit encryption is enabled, provide certificates
	// SSL context https://www.php.net/manual/en/context.ssl.php
	'ssl_context' => [
		'local_cert' => '/certs/redis.crt',
		'local_pk' => '/certs/redis.key',
		'cafile' => '/certs/ca.crt'
	]
],

Perhaps you need to try passing in those ssl_context optons to your custom config? Let us know if that helps. Anyone else in the community familiar with this, please feel free to chime in.

[Kaurin]

This config:

nextcloud:
  defaultConfigs:
    redis.config.php: false
  configs:
    redis.config.php: |-
      <?php
      $CONFIG = array (
        'memcache.distributed' => '\OC\Memcache\Redis',
        'memcache.locking' => '\OC\Memcache\Redis',
        'redis' => array(
          'host' => "tls://nextcloud-wrapper-redis-master.default.svc.cluster.local",
          'port' => getenv('REDIS_HOST_PORT') ?: 6379,
          'password' => getenv('REDIS_HOST_PASSWORD'),
          'ssl_context' => [
            'verify_peer_name' => false
            'local_cert' => '/certs/redis.crt',
            'local_pk' => '/certs/redis.key',
            'cafile' => '/certs/ca.crt',
          ],
        ),
      );
  extraEnv:
    - name: PHP_MEMORY_LIMIT
      value: 4096M
    - name: PHP_UPLOAD_LIMIT
      value: 16G
  ###### Shelve for now - in case we want to try TLS again
    - name: REDIS_HOST_PASSWORD
      value: changeme!

redis:
  architecture: standalone
  enabled: true
  
  auth:
    enabled: false
    password: changeme!
  
  tls:
    enabled: true
    authClients: true
    autoGenerated: true

Produces this on the webpage itself

Internal Server Error
The server encountered an internal error and was unable to complete your request.
Please contact the server administrator if this error reappears multiple times, please include the technical details below in your report.
More details can be found in the webserver log.
<br />
<b>Parse error</b>:  syntax error, unexpected single-quoted string &quot;local_cert&quot;, expecting &quot;]&quot; in <b>/var/www/html/config/redis.config.php</b> on line <b>11</b><br />

The good news is that the override is getting interpreted by nextcloud, but I guess it doesn't know what to do with local_cert ?

[albundy83]

But due to the fact that redis php is configured also here:
https://github.com/nextcloud/docker/blob/7a4823180dc0a8c7d92a5dfb20701b04770c5706/29/apache/entrypoint.sh#L123

and as you can see tcp:// is hardcoded ... not sure it could work no ?

[Kaurin]

Ah. So fixing this would require changing the docker repo for dockerhub as well as this helm repo. Not sure where to open a feature request for this, or whether just this github issue will suffice

[albundy83]

Maybe we could add a way to override the entrypoint by mounting a ConfigMap ?
When I read this comment it's not sure we will be able to change the way.
The best proposal will be to not have this file managed by entrypoint but by a clean ConfigMap I think.

[joshtrichards]

re: the PHP session handler... Yes, that's configured in the image entrypoint. It's independent of Nextcloud's config since it's a PHP config matter, but it does pull from the same auto-config environment variables.

IIRC though there may be some weirdness there... we'll need to research. tls is supported by phpredis, but I seem to recall there being some weirdness/lack of TLS support for the session handler with standalone Redis nodes. But maybe that's changed - e.g. phpredis/phpredis#2265

If it's actually changed, their docs don't seem to reflect that:

• https://github.com/phpredis/phpredis?tab=readme-ov-file#php-session-handler

If it is possible, we can make a change in the image entrypoint to adapt to it.

@Kaurin -

          'ssl_context' => [
            'verify_peer_name' => false
            'local_cert' => '/certs/redis.crt',
            'local_pk' => '/certs/redis.key',
            'cafile' => '/certs/ca.crt',
          ],

You're missing a comma after false.