Skip to content

Improved Helm Chart Templates #215

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

Open
sdibernardo wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Improved Helm Chart Templates #215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
453373f
Update ConfigMap to use dynamic service names
sdibernardo Aug 28, 2025
8ba619b
Update deployment.yaml for dynamic service volume mounts
sdibernardo Aug 28, 2025
3b9a9d4
Refactor ingress.yaml to remove quotes from hosts
sdibernardo Aug 28, 2025
399e30c
Add labels to ConfigMap in configmap_services.yaml
sdibernardo Aug 28, 2025
b93783e
Updated Services
sdibernardo Aug 28, 2025
13da6ce
Re-Added Label
sdibernardo Aug 28, 2025
f681266
Added review remarks
sdibernardo Aug 28, 2025
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
13 changes: 8 additions & 5 deletions beelzebub-chart/templates/configmap_services.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
{{- range $value := .Values.beelzebubServiceConfigs }}
{{- $trimmed := trimSuffix ".yaml" $value.name }}
{{- $safeName := regexReplaceAll "[^a-z0-9-]" (lower $trimmed) "-" | trimAll "-" | trunc 63 }}
---
apiVersion: v1
kind: ConfigMap
metadata:
name: "beelzebub-services"
labels:
{{- include "beelzebub-chart.labels" . | nindent 4 }}
name: beelzebub-services-cm-{{ $safeName }}
data:
service.yaml:
{{- toYaml .Values.beelsebubServiceConfigs | nindent 12 }}
{{ $value.name }}: |-
{{- toYaml $value.config | nindent 4 }}
{{- end }}
24 changes: 17 additions & 7 deletions beelzebub-chart/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,21 @@ spec:
mountPath: /configurations/beelzebub.yaml
subPath: beelzebub.yaml
readOnly: true
- name: config-services-volume
mountPath: /configurations/services/service.yaml
subPath: service.yaml
{{- range $value := .Values.beelzebubServiceConfigs }}
{{- $trimmed := trimSuffix ".yaml" $value.name }}
{{- $safeName := regexReplaceAll "[^a-z0-9-]" (lower $trimmed) "-" | trimAll "-" | trunc 63 }}
- name: config-services-volume-{{ $safeName }}
mountPath: /configurations/services/{{ $value.name }}
subPath: {{ $value.name }}
readOnly: true
{{- end }}
imagePullPolicy: {{ .Values.image.pullPolicy }}
ports:
- name: http
containerPort: {{ .Values.service.port }}
{{- range $value := .Values.beelzebubServiceConfigs }}
- name: {{ trimSuffix ".yaml" $value.name }}
containerPort: {{ trimPrefix ":" $value.config.address | default $.Values.service.port }}
protocol: TCP
{{- end }}
Comment on lines +50 to +54
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Aug 28, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Robustly parse containerPort and sanitize port names.

Handle host:port and missing address; ensure containerPort is numeric; sanitize the name.

-            {{- range $value := .Values.beelzebubServiceConfigs }}
-            - name: {{ trimSuffix ".yaml" $value.name }}
-              containerPort: {{ trimPrefix ":" $value.config.address | default $.Values.service.port }}
+            {{- range $value := .Values.beelzebubServiceConfigs }}
+            {{- $trimmed := trimSuffix ".yaml" $value.name -}}
+            {{- $safeName := (regexReplaceAll "[^a-z0-9-]" (lower $trimmed) "-" ) | trimAll "-" | trunc 63 -}}
+            {{- $raw := $value.config.address | default (printf ":%v" $.Values.service.port) -}}
+            {{- $portStr := regexFind "[0-9]+$" $raw | default (printf "%v" $.Values.service.port) -}}
+            {{- $port := $portStr | int -}}
+            - name: {{ $safeName }}
+              containerPort: {{ $port }}
               protocol: TCP
-            {{- end }}  
+            {{- end }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{{- range $value := .Values.beelzebubServiceConfigs }}
- name: {{ trimSuffix ".yaml" $value.name }}
containerPort: {{ trimPrefix ":" $value.config.address | default $.Values.service.port }}
protocol: TCP
{{- end }}
{{- range $value := .Values.beelzebubServiceConfigs }}
{{- $trimmed := trimSuffix ".yaml" $value.name -}}
{{- $safeName := (regexReplaceAll "[^a-z0-9-]" (lower $trimmed) "-" ) | trimAll "-" | trunc 63 -}}
{{- $raw := $value.config.address | default (printf ":%v" $.Values.service.port) -}}
{{- $portStr := regexFind "[0-9]+$" $raw | default (printf "%v" $.Values.service.port) -}}
{{- $port := $portStr | int -}}
- name: {{ $safeName }}
containerPort: {{ $port }}
protocol: TCP
{{- end }}
🤖 Prompt for AI Agents 
In beelzebub-chart/templates/deployment.yaml around lines 48-52, the port/name
templating must robustly handle host:port formats, missing addresses, ensure
containerPort is numeric, and sanitize the name: extract the port by splitting
on ":" and taking the last segment (so host:port -> port), fallback to
$.Values.service.port when address is empty, coerce the port to an integer (use
toInt or int conversion) and provide a safe default if conversion fails, and
sanitize the name by trimming suffix ".yaml" then lowercasing and replacing any
disallowed characters with "-" (use regexReplaceAll("[^a-z0-9-]", "-") and trim
leading/trailing "-") so the resulting name and containerPort are valid.

# livenessProbe:
# httpGet:
# path: /
Expand All @@ -60,9 +66,13 @@ spec:
- name: config-core-volume
configMap:
name: beelzebub-configuration
- name: config-services-volume
{{- range $value := .Values.beelzebubServiceConfigs }}
{{- $trimmed := trimSuffix ".yaml" $value.name }}
{{- $safeName := regexReplaceAll "[^a-z0-9-]" (lower $trimmed) "-" | trimAll "-" | trunc 63 }}
- name: config-services-volume-{{ $safeName }}
configMap:
name: beelzebub-services
name: beelzebub-services-cm-{{ $safeName }}
{{- end }}
{{- with .Values.nodeSelector }}
nodeSelector:
{{- toYaml . | nindent 8 }}
Expand Down
8 changes: 5 additions & 3 deletions beelzebub-chart/templates/service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ metadata:
spec:
type: {{ .Values.service.type }}
ports:
- port: {{ .Values.service.port }}
targetPort: http
{{- range $value := .Values.beelzebubServiceConfigs }}
- port: {{ trimPrefix ":" $value.config.address }}
targetPort: {{ trimPrefix ":" $value.config.address }}
protocol: TCP
name: http
name: {{ trimSuffix ".yaml" $value.name }}
{{- end }}
Comment on lines +10 to +15
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Aug 28, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Address parsing is brittle; trimPrefix ":" fails for hosts like 0.0.0.0:8080.

Use a regex to extract the trailing port. Current rendering will break if address includes a host.

Included in the diff above via regexFind "[0-9]{2,5}$".

🧰 Tools
🪛 YAMLlint (1.37.1)

[warning] 11-11: wrong indentation: expected 2 but found 4

(indentation)

🤖 Prompt for AI Agents 
In beelzebub-chart/templates/service.yaml around lines 10 to 15, the current use
of trimPrefix ":" to get the port is brittle for addresses that include a host
(e.g. 0.0.0.0:8080); update the template to extract the trailing port with a
regex (e.g. use regexFind "[0-9]{2,5}$" on $value.config.address), use the regex
result for both port and targetPort, and add a safe fallback (default port or
skip the entry) if the regex returns empty so rendering won't fail.

⚠️ Potential issue

Port name may violate IANA_SVC_NAME (length ≤15, lowercase, a-z start); sanitize and ensure uniqueness.

{{ trimSuffix ".yaml" $value.name }} can exceed 15 chars, include invalid chars, or start with a digit. Also, duplicates across entries will be rejected. Sanitize and cap to 15, and suffix with the loop index if needed.

Apply:

-  ports:
-    {{- range $value := .Values.beelzebubServiceConfigs }}
-    - port: {{ trimPrefix ":" $value.config.address }}
-      targetPort: {{ trimPrefix ":" $value.config.address }}
-      protocol: TCP
-      name: {{ trimSuffix ".yaml" $value.name }}
-    {{- end }}
+  ports:
+    {{- range $i, $value := .Values.beelzebubServiceConfigs }}
+    {{- $trimmed := trimSuffix ".yaml" $value.name -}}
+    {{- $base := (regexReplaceAll "[^a-z0-9-]" (lower $trimmed) "-" ) | trimAll "-" -}}
+    {{- $portName := trunc 15 $base -}}
+    - port: {{ (regexFind "[0-9]{2,5}$" (toString $value.config.address)) | default (toString .Values.service.port) }}
+      targetPort: {{ (regexFind "[0-9]{2,5}$" (toString $value.config.address)) | default (toString .Values.service.port) }}
+      protocol: TCP
+      name: {{ $portName }}-{{ $i }}
+    {{- end }}

Note: If you want to strictly enforce starting with a letter, prefix with p- when it doesn’t. I can provide a helper to do this cleanly.

🧰 Tools
🪛 YAMLlint (1.37.1)

[warning] 11-11: wrong indentation: expected 2 but found 4

(indentation)

selector:
{{- include "beelzebub-chart.selectorLabels" . | nindent 4 }}
4 changes: 3 additions & 1 deletion beelzebub-chart/templates/tests/test-connection.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ metadata:
"helm.sh/hook": test
spec:
containers:
{{- range $value := .Values.beelzebubServiceConfigs }}
- name: wget
image: busybox
command: ['wget']
args: ['{{ include "beelzebub-chart.fullname" . }}:{{ .Values.service.port }}']
args: ['{{ include "beelzebub-chart.fullname" . }}:{{ trimPrefix ":" $value.config.address }}']
{{- end }}
Comment on lines +11 to +16
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Aug 28, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Duplicate container names and wrong template scope inside range.

  • All containers use name: wget; container names must be unique.
  • include "…fullname" . passes the loop item (not the root) due to range; use $ for the root.
  • Address parsing should tolerate host:port (same concern as Service).

Apply:

-  containers:
-    {{- range $value := .Values.beelzebubServiceConfigs }}
-    - name: wget
+  containers:
+    {{- range $i, $value := .Values.beelzebubServiceConfigs }}
+    {{- $trimmed := trimSuffix ".yaml" $value.name -}}
+    {{- $safeName := (regexReplaceAll "[^a-z0-9-]" (lower $trimmed) "-" ) | trimAll "-" | trunc 63 -}}
+    {{- $port := (regexFind "[0-9]{2,5}$" (toString $value.config.address)) | default (toString .Values.service.port) -}}
+    - name: wget-{{ $safeName }}-{{ $i }}
       image: busybox
       command: ['wget']
-      args: ['{{ include "beelzebub-chart.fullname" . }}:{{ trimPrefix ":" $value.config.address }}']
+      args: ['{{ include "beelzebub-chart.fullname" $ }}:{{ $port }}']
     {{- end }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{{- range $value := .Values.beelzebubServiceConfigs }}
- name: wget
image: busybox
command: ['wget']
args: ['{{ include "beelzebub-chart.fullname" . }}:{{ .Values.service.port }}']
args: ['{{ include "beelzebub-chart.fullname" . }}:{{ trimPrefix ":" $value.config.address }}']
{{- end }}
containers:
{{- range $i, $value := .Values.beelzebubServiceConfigs }}
{{- $trimmed := trimSuffix ".yaml" $value.name -}}
{{- $safeName := (regexReplaceAll "[^a-z0-9-]" (lower $trimmed) "-" ) | trimAll "-" | trunc 63 -}}
{{- $port := (regexFind "[0-9]{2,5}$" (toString $value.config.address)) | default (toString .Values.service.port) -}}
- name: wget-{{ $safeName }}-{{ $i }}
image: busybox
command: ['wget']
args: ['{{ include "beelzebub-chart.fullname" $ }}:{{ $port }}']
{{- end }}
🧰 Tools
🪛 YAMLlint (1.37.1)

[warning] 12-12: wrong indentation: expected 2 but found 4

(indentation)

🤖 Prompt for AI Agents 
In beelzebub-chart/templates/tests/test-connection.yaml around lines 11-16, fix
duplicate container names and wrong template scope: make each container name
unique (e.g., append the loop index or a service identifier from the loop item),
stop using the loop-scope dot when calling the fullname template (use the root
context $ so: include "beelzebub-chart.fullname" $), and keep address parsing
tolerant of host:port by preserving the host:port portion while only trimming a
leading ":" if present (apply trimPrefix to the loop item’s address but do not
lose the host:port). Ensure the name generation and include use the correct
variables ($ and the loop index or item field) consistently.

restartPolicy: Never