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

Add additionalVolumes, additionalVolumeMounts #96

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
+43 −1
Open

Add additionalVolumes, additionalVolumeMounts #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b82f091
Add additionalVolumes, additionalvolumeMounts
neilcar Jun 8, 2024
3b99ea5
Fix indent
neilcar Jun 8, 2024
d81d5f3
Really fix indent
neilcar Jun 9, 2024
3565a53
Fix values.yaml
neilcar Jun 10, 2024
0305e58
Add sample volume/volumeMount
neilcar Jun 10, 2024
bef519e
Remove extraneous
neilcar Jun 10, 2024
7b3712f
Add additionalVolumes, additionalvolumeMounts
neilcar Jun 8, 2024
9c3eb8a
Fix indent
neilcar Jun 8, 2024
6dc71ea
Really fix indent
neilcar Jun 9, 2024
ecfc89e
Fix values.yaml
neilcar Jun 10, 2024
0a91ae3
Add sample volume/volumeMount
neilcar Jun 10, 2024
8b9d917
Remove extraneous
neilcar Jun 10, 2024
c183a4f
Merge branch '8gears-main'
neilcar Dec 22, 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
16 changes: 15 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,14 @@ scaling:
redis:
enabled: false
# Other default redis values: https://github.com/bitnami/charts/blob/master/bitnami/redis/values.yaml

## specify additional volumes to mount in the core container, this can be used
## to specify additional storage of material or to inject files from ConfigMaps
## into the running container
additionalVolumes: []

## specify where the additional volumes are mounted in the core container
additionalVolumeMounts: []
```

# Typical Values Example
Expand All @@ -305,7 +313,13 @@ secret:
database:
postgresdb:
password: 'big secret'

additionalVolumeMounts:
- mountPath: /public
name: public
additionalVolumes:
- hostPath:
path: /mnt/public
name: public
```

## Setup
Expand Down
6 changes: 6 additions & 0 deletions charts/n8n/templates/deployment.webhooks.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ spec:
- name: secret-volume
mountPath: /n8n-secret
{{- end }}
{{- if .Values.additionalVolumeMounts }}
{{ toYaml .Values.additionalVolumeMounts | indent 12}}
{{- end }}
{{- with .Values.nodeSelector }}
nodeSelector:
{{- toYaml . | nindent 8 }}
Expand Down Expand Up @@ -101,4 +104,7 @@ spec:
- key: "secret.json"
path: "secret.json"
{{- end }}
{{- if .Values.additionalVolumes }}
{{ toYaml .Values.additionalVolumes | indent 8}}
{{- end }}
Comment on lines +107 to +109
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Volume name conflicts should be documented in values.yaml

The review concern is valid as there are indeed built-in volumes with names "data", "config-volume", and "secret-volume" used across deployment templates. However, since additionalVolumes is defined as an empty array in values.yaml without documentation about reserved names, this could lead to naming conflicts. Instead of adding validation (which would be complex in Helm templates), this should be addressed through documentation.

  • Add documentation in values.yaml warning users about reserved volume names:
    • data
    • config-volume
    • secret-volume
🔗 Analysis chain

Ensure volume names are unique across all volume definitions

The implementation correctly adds additional volumes at the pod spec level. However, consider adding validation to prevent volume name conflicts with built-in volumes like "data", "config-volume", and "secret-volume".

Let me check the values.yaml file to understand how additionalVolumes are defined and if there are any existing validations.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for volume name definitions across templates
echo "Checking for volume names in templates..."
rg 'name: "?(data|config-volume|secret-volume)"?' charts/n8n/templates/

Length of output: 1518

Script:

#!/bin/bash
# Check values.yaml for additionalVolumes definition and any validation
echo "Checking values.yaml for additionalVolumes..."
cat charts/n8n/values.yaml | grep -A 10 "additionalVolumes:"

Length of output: 285

{{- end }}
6 changes: 6 additions & 0 deletions charts/n8n/templates/deployment.worker.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ spec:
- name: secret-volume
mountPath: /n8n-secret
{{- end }}
{{- if .Values.additionalVolumeMounts }}
{{ toYaml .Values.additionalVolumeMounts | indent 12}}
{{- end }}
{{- with .Values.nodeSelector }}
nodeSelector:
{{- toYaml . | nindent 8 }}
Expand Down Expand Up @@ -101,4 +104,7 @@ spec:
- key: "secret.json"
path: "secret.json"
{{- end }}
{{- if .Values.additionalVolumes }}
{{ toYaml .Values.additionalVolumes | indent 8}}
{{- end }}
{{- end }}
8 changes: 8 additions & 0 deletions charts/n8n/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ spec:
{{- toYaml .Values.securityContext | nindent 12 }}
image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
imagePullPolicy: {{ .Values.image.pullPolicy }}

env:
{{- include "n8n.deploymentPodEnvironments" . | nindent 12 }}
lifecycle:
Expand Down Expand Up @@ -94,6 +95,9 @@ spec:
- name: secret-volume
mountPath: /n8n-secret
{{- end }}
{{- if .Values.additionalVolumeMounts }}
{{ toYaml .Values.additionalVolumeMounts | indent 12}}
{{- end }}
{{- with .Values.nodeSelector }}
nodeSelector:
{{- toYaml . | nindent 8 }}
Expand Down Expand Up @@ -122,3 +126,7 @@ spec:
- key: "secret.json"
path: "secret.json"
{{- end }}

{{- if .Values.additionalVolumes }}
{{ toYaml .Values.additionalVolumes | indent 8 }}
{{- end }}
8 changes: 8 additions & 0 deletions charts/n8n/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,3 +330,11 @@ redis:
enabled: true
existingClaim: ""
size: 2Gi

## specify additional volumes to mount in the core container, this can be used
## to specify additional storage of material or to inject files from ConfigMaps
## into the running container
additionalVolumes: []

## specify where the additional volumes are mounted in the core container
additionalVolumeMounts: []