Skip to content

Conversation

NitriKx
Copy link
Contributor

@NitriKx NitriKx commented Jun 10, 2025

This PR allows to customise the requests and limits for the backend deployment

@NitriKx NitriKx force-pushed the allow-to-define-resources branch from 6b5e147 to 034de61 Compare June 10, 2025 13:15
Copy link

infrabaseai bot commented Jun 10, 2025

🛡️ Security Analysis Results

Found 4 security issues:

Severity Issue File Line Recommendation
🔴 Critical Incorrect indentation of resources block in H... charts/digger-backend/templates/backend-deployment.yaml:39 39 Adjust the indentation to match the other container-level...
🟡 Warning Conditional with on empty map may be misleading charts/digger-backend/templates/backend-deployment.yaml:36 36 Either document this behavior clearly in values.yaml (e...
🟡 Warning No Pod or container securityContext defined charts/digger-backend/templates/backend-deployment.yaml:1 1 Add a securityContext block to either the Pod spec or t...
🔵 Info Missing default resource requests/limits recomm... charts/digger-backend/values.yaml:21 21 Provide recommended default resource requests and limits ...
📋 Detailed Descriptions

🔴 Incorrect indentation of resources block in Helm template

File: charts/digger-backend/templates/backend-deployment.yaml (Line 39-42)

Description: In charts/digger-backend/templates/backend-deployment.yaml, the newly added resources: stanza is indented with nindent 10, which does not line up with the container spec’s field hierarchy. This can generate invalid YAML or place the block under the wrong parent, causing the chart to render incorrectly or Kubernetes to reject the manifest.

💡 Recommendation: Adjust the indentation to match the other container-level fields. For example, if env: is indented 8 spaces, you should use nindent 8 on the resources: key and nindent 10 (or matching the existing pattern) on its contents. Example:

{{- with .Values.digger.resources }}
  resources:
    {{- toYaml . | nindent 8 }}
{{- end }}

Run helm template locally to verify correct nesting and no YAML errors.


🟡 Conditional with on empty map may be misleading

File: charts/digger-backend/templates/backend-deployment.yaml (Line 36-42)

Description: The chart uses {{- with .Values.digger.resources }} to decide whether to emit the resources: block. In Go templating, empty maps are considered “empty” and skip the block—but developers or users might assume an empty map ({}) still renders. This subtlety can cause confusion in tests or when overriding values.

💡 Recommendation: Either document this behavior clearly in values.yaml (e.g. “set to null to disable”) or change the condition to explicitly check for a non-nil map, e.g.:

{{- if and (kindIs "map" .Values.digger.resources) (gt (len .Values.digger.resources) 0) }}
  resources:
    {{- toYaml .Values.digger.resources | nindent 8 }}
{{- end }}

🟡 No Pod or container securityContext defined

File: charts/digger-backend/templates/backend-deployment.yaml (Line 1-100)

Description: The deployment template does not define any securityContext at the Pod or container level. Without a runAsNonRoot, readOnlyRootFilesystem, or dropped capabilities policy, containers may run as root and request more privileges than necessary, violating least-privilege principles.

💡 Recommendation: Add a securityContext block to either the Pod spec or the individual container spec. For example:

spec:
securityContext:
runAsNonRoot: true
runAsUser: 1000
fsGroup: 2000
containers:
- name: digger-backend
securityContext:
allowPrivilegeEscalation: false
readOnlyRootFilesystem: true
capabilities:
drop:
- ALL

This enforces that the container runs with a non-root user and minimal privileges.


🔵 Missing default resource requests/limits recommendation

File: charts/digger-backend/values.yaml (Line 21-30)

Description: By shipping resources: {} as the default in values.yaml, consumers of the chart may forget to specify CPU/memory requests and limits, which can lead to resource contention or unexpected evictions in a multi-tenant cluster.

💡 Recommendation: Provide recommended default resource requests and limits in values.yaml (or document them in the chart’s README) to enforce sensible defaults. For example:

resources:
requests:
cpu: "100m"
memory: "140Mi"
limits:
cpu: "500m"
memory: "200Mi"

Users can still override these, but having defaults protects against runaway resource usage.

📊 Summary

  • 🔴 Critical: 1
  • 🟡 Warning: 2
  • 🔵 Info: 1

🤖 Analysis powered by Infrabase AI

@NitriKx NitriKx force-pushed the allow-to-define-resources branch from 034de61 to 5d92725 Compare June 10, 2025 14:13
@NitriKx NitriKx force-pushed the allow-to-define-resources branch from 5d92725 to d168a89 Compare June 10, 2025 14:15
@NitriKx
Copy link
Contributor Author

NitriKx commented Jun 10, 2025

I think "Incorrect indentation of resources block in Helm template" is wrong

@motatoes
Copy link
Contributor

Thanks for this! @NitriKx

@motatoes motatoes merged commit 3f9dfa5 into diggerhq:main Jun 21, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants