joeferner / redis-commander

Redis management tool written in node.js
http://joeferner.github.io/redis-commander/
MIT License
3.56k stars 460 forks source link

TLS flags are ignored unless an argList check passes when specifying connections through config #554

Closed cen1 closed 6 months ago

cen1 commented 6 months ago

I am deploying redis-commander in k8s connecting to GCP memorystore with TLS enabled.

The docs recommend to use a config file instead of env for such a setup: https://github.com/joeferner/redis-commander/blob/master/docs/connections.md#connect-to-tls-secured-redis-inside-docker-container-most-flexible-config

So I specify the local.json:

{
      "connections": [
        {
          "label": "gcp",
          "host": {{ .Values.redis.host | quote }},
          "port": {{ .Values.redis.port | quote }},
          "password": {{ .Values.redis.password | quote }},
          "tls": true,
          "dbIndex": 0
        }
      ]
}

And in the env section:

- name: "REDIS_TLS"
  value: "{{ .Values.redis.tls.enabled }}"
- name: "REDIS_TLS_CA_CERT_FILE"
  value: "/redis-commander/config/ca.pem"

The startup command becomes: node ./bin/redis-commander --redis-tls --redis-tls-ca-cert-file /redis-commander/config/ca.pem

This results in CA cert error because of this condition: https://github.com/joeferner/redis-commander/blob/6f0132423d9cf22ae0fdb19b80cd7246ae97a343/bin/redis-commander.js#L463

I guess the issue here is that I wanted to use REDIS_TLS_CA_CERT_FILE which does not seem to be settable through config file and I didn't want to specify it directly as a string because passing multiline PEM into env vars and/or helm templates is just a PITA and asking for trouble. This led me into an incompatible state when using config and env at the same time. Perhaps something could be done to clarify this in the doc or add support to specify CA file path through config..

sseide commented 6 months ago

Connecting to one Redis server only can be done with env vars only with current version. There should be no need to specify an config file too. Config file is needed as soon as you want to connect to multiple servers.

Try setting:

Otoh redis ca cert is settable via config file too without the need for environment variables. But as string (one-liner) only. You cannot give a file name here unfortunately. The "tls" config parameter can be a boolean or an object - the subkeys of the object are described here: https://github.com/joeferner/redis-commander/blob/master/docs/connections.md#connection-parameters

This "tls" object is given directly to the NodeJS TLS connection object having the same parameter as described there

But thanks for the hint - maybe this part of the code can be reqworked to allow setting a custom ca cert for usage with multiple connections via config file too...

And another option might be this NodeJS env var: NODE_EXTRA_CA_CERTS https://nodejs.org/api/cli.html#cli_node_extra_ca_certs_file

cen1 commented 6 months ago

Reading the docs again, it does say that connecting to a single instance is possible through env only so it is really myself to blame for not reading more carefully.

As far as config file goes, I agree, I don't think it hurts if the option to provide a file path is also available there.

sseide commented 6 months ago

btw - updates to the documentation are always welcome. Someone starting to work with Redis Commander knows better what information are missing or unclear to get up and running compared to someone working with it for a long time...