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.

Sign up for GitHub

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

feat(argo-cd): add authentication for builtin Redis #2616

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+157 −2
Open

feat(argo-cd): add authentication for builtin Redis #2616

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e9ddaff
feat(argo-cd): add authentication for builtin Redis
afrimberger Mar 30, 2024
27622f5
feat(argo-cd): add authentication for builtin Redis
afrimberger Apr 1, 2024
80f6f3d
Merge branch 'main' into feature/redis-auth
afrimberger Apr 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion charts/argo-cd/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ appVersion: v2.10.5
kubeVersion: ">=1.23.0-0"
description: A Helm chart for Argo CD, a declarative, GitOps continuous delivery tool for Kubernetes.
name: argo-cd
version: 6.7.6
version: 6.8.0
home: https://github.com/argoproj/argo-helm
icon: https://argo-cd.readthedocs.io/en/stable/assets/logo.png
sources:
Expand All @@ -26,5 +26,7 @@ annotations:
fingerprint: 2B8F22F57260EFA67BE1C5824B11F800CD9D2252
url: https://argoproj.github.io/argo-helm/pgp_keys.asc
artifacthub.io/changes: |
- kind: added
description: added authentication for builtin Redis
- kind: fixed
description: added missing crd change for 2.10.5
5 changes: 5 additions & 0 deletions charts/argo-cd/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1154,6 +1154,10 @@ NAME: my-release
| Key | Type | Default | Description |
|-----|------|---------|-------------|
| redis.affinity | object | `{}` (defaults to global.affinity preset) | Assign custom [affinity] rules to the deployment |
| redis.auth.configAnnotations | object | `{}` | Annotations to be added to Redis config secret |
| redis.auth.enabled | bool | `false` | enable authentication for Redis. Passwords are auto-generated and stored in argocd-redis |
| redis.auth.secretAnnotations | object | `{}` | Annotations to be added to Redis secret |
| redis.auth.username | string | `"argocd"` | username for connecting to Redis |
| redis.containerPorts.metrics | int | `9121` | Metrics container port |
| redis.containerPorts.redis | int | `6379` | Redis container port |
| redis.containerSecurityContext | object | See [values.yaml] | Redis container-level security context |
Expand Down Expand Up @@ -1183,6 +1187,7 @@ NAME: my-release
| redis.exporter.readinessProbe.timeoutSeconds | int | `15` | Number of seconds after which the [probe] times out |
| redis.exporter.resources | object | `{}` | Resource limits and requests for redis-exporter sidecar |
| redis.extraArgs | list | `[]` | Additional command line arguments to pass to redis-server |
| redis.extraConfig | string | `""` | Redis extra configuration settings (https://redis.io/docs/management/config-file/) |
| redis.extraContainers | list | `[]` | Additional containers to be added to the redis pod |
| redis.image.imagePullPolicy | string | `""` (defaults to global.image.imagePullPolicy) | Redis image pull policy |
| redis.image.repository | string | `"public.ecr.aws/docker/library/redis"` | Redis repository |
Expand Down
25 changes: 25 additions & 0 deletions charts/argo-cd/templates/redis/configmap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{{- $redisHa := index .Values "redis-ha" -}}
{{- if and .Values.redis.enabled (or .Values.redis.auth.enabled .Values.redis.extraConfig) (not $redisHa.enabled) -}}
---
apiVersion: v1
kind: ConfigMap
metadata:
name: "{{ include "argo-cd.redis.fullname" . }}-config"
namespace: {{ .Release.Namespace | quote }}
labels:
{{- include "argo-cd.labels" (dict "context" . "component" .Values.redis.name "name" .Values.redis.name) | nindent 4 }}
{{- with .Values.redis.auth.secretAnnotations }}
annotations:
{{- range $key, $value := . }}
{{ $key }}: {{ $value | quote }}
{{- end }}
{{- end }}
data:
{{- if or .Values.redis.auth.enabled .Values.redis.extraConfig }}
redis.conf: |
{{- if .Values.redis.auth.enabled }}
aclfile /etc/redis/users.acl
{{- end }}
{{- .Values.redis.extraConfig | nindent 4 }}
{{- end }}
{{- end }}
22 changes: 21 additions & 1 deletion charts/argo-cd/templates/redis/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ spec:
{{- with (mergeOverwrite (deepCopy .Values.global.podLabels) .Values.redis.podLabels) }}
{{- toYaml . | nindent 8 }}
{{- end }}
{{- with (mergeOverwrite (deepCopy .Values.global.podAnnotations) .Values.redis.podAnnotations) }}
annotations:
checksum/redis-config: {{ include (print $.Template.BasePath "/redis/secret.yaml") . | sha256sum }}
{{- with (mergeOverwrite (deepCopy .Values.global.podAnnotations) .Values.redis.podAnnotations) }}
{{- range $key, $value := . }}
{{ $key }}: {{ $value | quote }}
{{- end }}
Expand Down Expand Up @@ -60,6 +61,9 @@ spec:
{{- with .Values.redis.extraArgs }}
{{- toYaml . | nindent 8 }}
{{- end }}
{{- if or .Values.redis.auth.enabled .Values.redis.extraConfig }}
- /etc/redis/redis.conf
{{- end }}
- --save
- ""
- --appendonly
Expand Down Expand Up @@ -111,6 +115,11 @@ spec:
volumeMounts:
- mountPath: /health
name: health
{{- if or .Values.redis.auth.enabled .Values.redis.extraConfig }}
- mountPath: /etc/redis
name: config
readOnly: true
{{- end }}
{{- with .Values.redis.volumeMounts }}
{{- toYaml . | nindent 10 }}
{{- end }}
Expand Down Expand Up @@ -194,6 +203,17 @@ spec:
configMap:
name: {{ include "argo-cd.redis.fullname" . }}-health-configmap
defaultMode: 493
{{- if or .Values.redis.auth.enabled .Values.redis.extraConfig }}
- name: config
projected:
sources:
- configMap:
name: "{{ include "argo-cd.redis.fullname" . }}-config"
{{- if .Values.redis.auth.enabled }}
- secret:
name: "{{ include "argo-cd.redis.fullname" . }}-users"
{{- end }}
{{- end }}
{{- with .Values.redis.volumes }}
{{- toYaml . | nindent 8}}
{{- end }}
Expand Down
60 changes: 60 additions & 0 deletions charts/argo-cd/templates/redis/secret.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# lookup existing secret
{{- $secretName := include "argo-cd.redis.fullname" . -}}
{{- $secretObj := (lookup "v1" "Secret" .Release.Namespace $secretName) | default dict }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using lookup inside the Argo ecosystem is IMHO a bad idea. Argo CD uses helm only as a template engine (argocd only executes helm template ...).
And helm template does not support looking up values for security reasons.

Copy link
Author

@afrimberger afrimberger Mar 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mkilchhofer, Thanks for bringing up this topic! If we're talking about ArgoCD in general, I fully agree. However, in this particular case the Helm Chart is meant for bootstrapping ArgoCD. Basically, this can be achieved in two ways:

  1. Install directly via Helm -> Lookup works
  2. ArgoCD manages ArgoCD -> ignoreDifferences for the password fields needs to be specified

I would really love to see a more secure default installation of ArgoCD. Especially, because not all clusters support NetworkPolicies out of the box (e.g. Flannel doesn't).

I suggest to add an example for ignoreDifferences and some explanation to the README to address the questions arising by using lookup. WDYT?

Copy link
Member

@mkilchhofer mkilchhofer Apr 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean changing the default installation without being in-line with with upstrean kustomize manifests is not what we want.
Upstream is our reference for the default installation as mentioned in the charts README.

I appreciate a more secure default installation but it has to be promoted first as a PR in the upstream repo (https://github.com/argoproj/argo-cd).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the Chart's README:

Hence, it makes sense to try to achieve a similar output of rendered .yaml resources when calling helm template using the default settings in values.yaml

According to this, the default should be redis.auth.enabled=false (as it is in the current implementation). Additionally, a user can easily enable a more secure installation by setting redis.auth.enabled to true.
Sounds reasonable to me.

{{- $secretData := (get $secretObj "data") | default dict }}
# generate random password if secret doesn't exist
{{- $defaultUserPassword := (get $secretData "redis-password-default") | default (randAlphaNum 48 | b64enc) }}
{{- $adminUserPassword := (get $secretData "redis-password-admin") | default (randAlphaNum 48 | b64enc) }}
{{- $argoUserPassword := (get $secretData "redis-password") | default (randAlphaNum 48 | b64enc) }}
{{- if .Values.redis.auth.enabled }}
apiVersion: v1
kind: Secret
metadata:
name: {{ $secretName }}
namespace: {{ .Release.Namespace | quote }}
labels:
{{- include "argo-cd.labels" (dict "context" . "component" .Values.redis.name "name" .Values.redis.name) | nindent 4 }}
{{- with .Values.redis.auth.secretAnnotations }}
annotations:
{{- range $key, $value := . }}
{{ $key }}: {{ $value | quote }}
{{- end }}
{{- end }}
type: Opaque
immutable: true
data:
redis-username-default: {{ "default" | b64enc }}
redis-password-default: {{ $defaultUserPassword | quote }}

redis-username-admin: {{ "admin" | b64enc }}
redis-password-admin: {{ $adminUserPassword | quote }}

{{- with .Values.redis.auth.username }}
redis-username: {{ . | b64enc }}
{{- end }}
redis-password: {{ $argoUserPassword | quote }}
{{- end }}

{{- $redisHa := index .Values "redis-ha" -}}
{{- if and .Values.redis.enabled .Values.redis.auth.enabled (not $redisHa.enabled) }}
---
apiVersion: v1
kind: Secret
metadata:
name: "{{ include "argo-cd.redis.fullname" . }}-users"
namespace: {{ .Release.Namespace | quote }}
labels:
{{- include "argo-cd.labels" (dict "context" . "component" .Values.redis.name "name" .Values.redis.name) | nindent 4 }}
{{- with .Values.redis.auth.secretAnnotations }}
annotations:
{{- range $key, $value := . }}
{{ $key }}: {{ $value | quote }}
{{- end }}
{{- end }}
stringData:
users.acl: |
user default on +@all -@admin -@dangerous ~* &* >{{ $defaultUserPassword | b64dec }}
user admin on +@all -@admin -@dangerous ~* &* >{{ $adminUserPassword | b64dec }}
user {{ .Values.redis.auth.username }} on +@all ~* &* >{{ $argoUserPassword | b64dec }}

{{- end }}
14 changes: 14 additions & 0 deletions charts/argo-cd/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1163,6 +1163,20 @@ redis:
# -- Redis name
name: redis

# -- Redis extra configuration settings (https://redis.io/docs/management/config-file/)
extraConfig: ""

## Redis authentication
auth:
# -- enable authentication for Redis. Passwords are auto-generated and stored in argocd-redis
enabled: false
# -- username for connecting to Redis
username: argocd
# -- Annotations to be added to Redis secret
secretAnnotations: {}
# -- Annotations to be added to Redis config secret
configAnnotations: {}

## Redis Pod Disruption Budget
## Ref: https://kubernetes.io/docs/tasks/run-application/configure-pdb/
pdb:
Expand Down
Loading