feat(urbanstack-shared-app): add urbanstack-shared-app chart#1640
feat(urbanstack-shared-app): add urbanstack-shared-app chart#1640ol-teuto wants to merge 1 commit intoteutonet:mainfrom
Conversation
ol-teuto
requested review from
a team,
cwrau,
marvinWolff and
tasches
as code owners
WalkthroughA new Helm chart named Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm
participant Kubernetes API
participant Secret
participant PVC
participant Deployment
participant Service
participant Ingress
participant NetworkPolicy
User->>Helm: Install urbanstack-shared-app chart
Helm->>Kubernetes API: Render and apply templates
Kubernetes API->>Secret: Create image pull secret (if auth provided)
Kubernetes API->>PVC: Create PersistentVolumeClaim
Kubernetes API->>Deployment: Create Deployment with env vars and probes
Kubernetes API->>Service: Create ClusterIP Service
Kubernetes API->>Ingress: Create Ingress for HTTP routing
Kubernetes API->>NetworkPolicy: Create NetworkPolicy for egress/ingress
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ol-teuto
force-pushed
the
add-urbanstack-shared-app
branch
3 times, most recently
from
7058566 to
0459b2c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (11)
charts/urbanstack-shared-app/templates/validation.yaml (1)
1-4: Rename validation stub to.tplto silence YAML lintersThe snippet renders no Kubernetes object and exists purely for template-time validation, yet its
.yamlsuffix triggers YAML-lint errors (see pipeline output).
Move it to e.g.templates/_validation.tplorvalidation.gotmpl; Helm will still evaluate it, but static analysis tools will skip it.
No behaviour change, just cleaner CI.charts/urbanstack-shared-app/templates/pvc.yaml (3)
4-4: Use a fullname helper instead of raw.Release.NameHard-coding the PVC name to the release name removes the option of
fullnameOverrideand makes the object collide with any other resource that happens to share the release.
Prefer the standard helper, e.g.{{ include "external-citytool.fullname" . }}, for consistency and flexibility.
6-6: Inline helper on a new line to avoid YAML-lint gripe- labels: - {{- include "external-citytool.labels" . | nindent 4 }} + labels: +{{- include "external-citytool.labels" . | nindent 4 }}This prevents the “expected node content, but found '-'” error some YAML linters raise.
12-12: Guardstorage.sizewith a sane default- storage: {{ .Values.storage.size | quote }} + storage: {{ .Values.storage.size | default "5Gi" | quote }}Without a default the template hard-fails on a missing value.
charts/urbanstack-shared-app/templates/ingress.yaml (1)
7-11: Move templating helpers out of the annotation line- annotations: - kubernetes.io/tls-acme: "true" - {{- with .Values.certIssuer }} - cert-manager.io/issuer: {{ . | quote }} - {{- end }} + annotations: + kubernetes.io/tls-acme: "true" +{{- with .Values.certIssuer }} + cert-manager.io/issuer: {{ . | quote }} +{{- end }}YAML tools stop complaining and the block is easier to read.
charts/urbanstack-shared-app/templates/image-pull-secret.yaml (2)
5-6: Put label helper on its own line for clean YAML- labels: {{- include "external-citytool.labels" . | nindent 4 }} + labels: +{{- include "external-citytool.labels" . | nindent 4 }}
9-9: Addusername/passwordkeys to.dockerconfigjsonOlder tooling occasionally requires the explicit fields:
- .dockerconfigjson: {{ dict "auths" (dict .Values.image.registry (dict "auth" (printf "%s:%s" .Values.registryAuth.username .Values.registryAuth.password | b64enc))) | toJson | quote }} + .dockerconfigjson: {{ dict "auths" (dict .Values.image.registry (dict "username" .Values.registryAuth.username "password" .Values.registryAuth.password "auth" (printf "%s:%s" .Values.registryAuth.username .Values.registryAuth.password | b64enc))) | toJson | quote }}charts/urbanstack-shared-app/templates/service.yaml (1)
4-6: Align with helpers and fix inline template- name: {{ .Release.Name }} - labels: - {{- include "external-citytool.labels" . | nindent 4 }} + name: {{ include "external-citytool.fullname" . }} + labels: +{{- include "external-citytool.labels" . | nindent 4 }}charts/urbanstack-shared-app/templates/networkpolicy.yaml (1)
52-60: Final newline missingFile ends without a terminating newline, which violates POSIX and some tools’ expectations.
charts/urbanstack-shared-app/templates/deployment.yaml (2)
26-33: MissingseccompProfilehardeningThe pod has a strict security context but omits an explicit
seccompProfile, leaving the runtime default ambiguous.securityContext: + seccompProfile: + type: RuntimeDefault
94-101:resourcesblock optional – default may request zero CPU/MemoryMany clusters reject pods without resource requests. Consider setting conservative defaults in
values.yamlto avoid run-time failures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.github/CODEOWNERS(1 hunks).github/release-please/config.json(1 hunks)charts/urbanstack-shared-app/.helmignore(1 hunks)charts/urbanstack-shared-app/Chart.yaml(1 hunks)charts/urbanstack-shared-app/ci/_common.sh(1 hunks)charts/urbanstack-shared-app/ci/artifacthub-values.yaml(1 hunks)charts/urbanstack-shared-app/ci/auth-values.yaml(1 hunks)charts/urbanstack-shared-app/ci/cert-issuer.yaml(1 hunks)charts/urbanstack-shared-app/templates/_helpers.tpl(1 hunks)charts/urbanstack-shared-app/templates/deployment.yaml(1 hunks)charts/urbanstack-shared-app/templates/image-pull-secret.yaml(1 hunks)charts/urbanstack-shared-app/templates/ingress.yaml(1 hunks)charts/urbanstack-shared-app/templates/networkpolicy.yaml(1 hunks)charts/urbanstack-shared-app/templates/pvc.yaml(1 hunks)charts/urbanstack-shared-app/templates/service.yaml(1 hunks)charts/urbanstack-shared-app/templates/validation.yaml(1 hunks)charts/urbanstack-shared-app/values.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- charts/urbanstack-shared-app/ci/artifacthub-values.yaml
- charts/urbanstack-shared-app/.helmignore
- charts/urbanstack-shared-app/ci/_common.sh
- charts/urbanstack-shared-app/values.yaml
- .github/release-please/config.json
- charts/urbanstack-shared-app/templates/_helpers.tpl
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/CODEOWNERS
- charts/urbanstack-shared-app/ci/auth-values.yaml
- charts/urbanstack-shared-app/ci/cert-issuer.yaml
- charts/urbanstack-shared-app/Chart.yaml
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1602
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/oauth-proxy.yaml:38-40
Timestamp: 2025-07-24T09:49:40.961Z
Learning: Official Helm charts like oauth2-proxy manage image versioning automatically through their Chart.yaml appVersion field, making manual tag pinning in consumer values unnecessary and potentially harmful. The chart version itself provides reproducibility by ensuring the correct image tag is used.
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1604
File: charts/base-cluster/templates/monitoring/metrics-server/metrics-server.yaml:20-21
Timestamp: 2025-07-24T09:41:28.072Z
Learning: The official kubernetes-sigs/metrics-server Helm chart pins the image tag to a specific version in its values.yaml by default, ensuring reproducible deployments without requiring explicit tag specification in consuming charts.
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's `valuesFrom` feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1604
File: charts/base-cluster/templates/monitoring/metrics-server/metrics-server.yaml:20-21
Timestamp: 2025-07-24T09:41:28.072Z
Learning: The kubernetes-sigs/metrics-server Helm chart uses `v{{ .Chart.AppVersion }}` as the default image tag when `image.tag` is empty, which provides pinned versioning through the chart's AppVersion rather than using floating tags like "latest".
charts/urbanstack-shared-app/templates/deployment.yaml (2)
Learnt from: cwrau
PR: #1604
File: charts/base-cluster/templates/monitoring/metrics-server/metrics-server.yaml:20-21
Timestamp: 2025-07-24T09:41:28.072Z
Learning: The official kubernetes-sigs/metrics-server Helm chart pins the image tag to a specific version in its values.yaml by default, ensuring reproducible deployments without requiring explicit tag specification in consuming charts.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
charts/urbanstack-shared-app/templates/image-pull-secret.yaml (2)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: #1602
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/oauth-proxy.yaml:38-40
Timestamp: 2025-07-24T09:49:40.961Z
Learning: Official Helm charts like oauth2-proxy manage image versioning automatically through their Chart.yaml appVersion field, making manual tag pinning in consumer values unnecessary and potentially harmful. The chart version itself provides reproducibility by ensuring the correct image tag is used.
charts/urbanstack-shared-app/templates/ingress.yaml (1)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
charts/urbanstack-shared-app/templates/pvc.yaml (1)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
charts/urbanstack-shared-app/templates/service.yaml (2)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
🪛 YAMLlint (1.37.1)
charts/urbanstack-shared-app/templates/deployment.yaml
[error] 8-8: syntax error: expected the node content, but found '-'
(syntax)
[error] 102-102: trailing spaces
(trailing-spaces)
charts/urbanstack-shared-app/templates/image-pull-secret.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/ingress.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/networkpolicy.yaml
[warning] 16-16: wrong indentation: expected 2 but found 4
(indentation)
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/pvc.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/service.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/validation.yaml
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check licenses
🔇 Additional comments (3)
charts/urbanstack-shared-app/templates/ingress.yaml (1)
25-27:secretNamecan overflow 63-char DNS label limit
{{ .Values.host }}-tlsis unbounded. Consider truncating or hashing when the host exceeds 61 chars.charts/urbanstack-shared-app/templates/service.yaml (1)
15-15: Verify thatexternal-citytool.selectorLabelsexistsIf
_helpers.tpldoes not define this helper the Service will render invalid YAML. Please double-check or add it.charts/urbanstack-shared-app/templates/deployment.yaml (1)
21-24: Image-pull secret may reference non-existent resourceThe template references
pullsecret-{{ .Release.Name }}only whenregistryAuth.usernameandpasswordare set.
If the secret template uses a different naming convention (e.g. includes the registry), the Deployment will fail.Confirm the secret template produces the same name or update either side for consistency.
| - to: # allow database access | ||
| - podSelector: | ||
| matchLabels: {{- .Values.postgres.networkPolicy.matchLabels | toYaml | nindent 14 }} | ||
| ports: | ||
| - protocol: TCP |
There was a problem hiding this comment.
🛠️ Refactor suggestion
matchLabels block is rendered on the same line – YAML breaks
toYaml already emits a newline; keep matchLabels: on its own line and drop the inline expression.
- matchLabels: {{- .Values.postgres.networkPolicy.matchLabels | toYaml | nindent 14 }}
+ matchLabels:
+ {{- .Values.postgres.networkPolicy.matchLabels | toYaml | nindent 14 }}📝 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.
| - to: # allow database access | |
| - podSelector: | |
| matchLabels: {{- .Values.postgres.networkPolicy.matchLabels | toYaml | nindent 14 }} | |
| ports: | |
| - protocol: TCP | |
| - to: # allow database access | |
| - podSelector: | |
| matchLabels: | |
| {{- .Values.postgres.networkPolicy.matchLabels | toYaml | nindent 14 }} | |
| ports: | |
| - protocol: TCP |
🤖 Prompt for AI Agents
In charts/urbanstack-shared-app/templates/networkpolicy.yaml around lines 33 to
37, the matchLabels block is rendered inline causing YAML formatting issues. To
fix this, place the matchLabels: key on its own line and then use the toYaml
function with proper indentation on the following lines, avoiding inline
rendering to ensure valid multiline YAML output.
0459b2c to
387b63b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
charts/urbanstack-shared-app/templates/ingress.yaml (1)
17-23: Key order still violates networking.k8s.io/v1 schemaPrevious feedback already flagged this.
On some clusters the manifest will be rejected becausepath/pathTypemust come beforebackend.- - backend: - service: - name: {{ .Release.Name }} - port: - name: http - pathType: Prefix - path: / + - path: / + pathType: Prefix + backend: + service: + name: {{ .Release.Name }} + port: + name: httpcharts/urbanstack-shared-app/templates/networkpolicy.yaml (2)
15-20: Indentation of list item underegress:is still off – results in invalid YAML
- to:must be indented exactly two spaces more thanegress:. With the current four-space shift the document failsyamllintand will be rejected bykubectl apply.- {{- if .Values.allowExternalEgress }} - - to: # allow external endpoints outside of the cluster + {{- if .Values.allowExternalEgress }} + - to: # allow external endpoints outside of the cluster
34-36: InlinematchLabelsbreaks YAML – keep the key and rendered map on separate lines
toYamlalready emits the newline. Rendering it inline producesmatchLabels: key: valueon one line, which is not valid.- matchLabels: {{- .Values.postgres.networkPolicy.matchLabels | toYaml | nindent 14 }} + matchLabels: + {{- .Values.postgres.networkPolicy.matchLabels | toYaml | nindent 14 }}
🧹 Nitpick comments (4)
charts/urbanstack-shared-app/templates/pvc.yaml (1)
4-6: Consider suffixing the PVC name for clarity & future-proofingUsing the bare release name makes the PVC share its name with the Deployment, Service, Ingress, etc.
While Kubernetes allows cross-kind name reuse, it becomes painful when grepping, debugging or replacing single resources later on.- name: {{ .Release.Name }} + # Keep within the 63-char DNS label limit + name: {{ printf "%s-pvc" .Release.Name | trunc 63 | trimSuffix "-" }}charts/urbanstack-shared-app/templates/image-pull-secret.yaml (1)
4-7: Minor readability nit – move label block to the next lineKeeping the
labels:key on its own line improves diff readability and mirrors the pattern used in the other templates.- labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} + labels: + {{- include "urbanstack-shared-app.labels" . | nindent 4 }}charts/urbanstack-shared-app/templates/deployment.yaml (2)
14-16: PreferRollingUpdateoverRecreatefor zero-downtime upgradesUnless the application explicitly cannot handle more than one replica, using
Recreatecauses a short outage on every deployment.RollingUpdateis the safer default and aligns with Kubernetes best practices.- strategy: - type: Recreate + strategy: + type: RollingUpdate
102-102: Trailing whitespaceThere is a single trailing space on this line – harmless but worth dropping to keep the manifest clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.github/CODEOWNERS(1 hunks).github/release-please/config.json(1 hunks)charts/urbanstack-shared-app/.helmignore(1 hunks)charts/urbanstack-shared-app/Chart.yaml(1 hunks)charts/urbanstack-shared-app/ci/_common.sh(1 hunks)charts/urbanstack-shared-app/ci/artifacthub-values.yaml(1 hunks)charts/urbanstack-shared-app/ci/auth-values.yaml(1 hunks)charts/urbanstack-shared-app/ci/cert-issuer.yaml(1 hunks)charts/urbanstack-shared-app/templates/_helpers.tpl(1 hunks)charts/urbanstack-shared-app/templates/deployment.yaml(1 hunks)charts/urbanstack-shared-app/templates/image-pull-secret.yaml(1 hunks)charts/urbanstack-shared-app/templates/ingress.yaml(1 hunks)charts/urbanstack-shared-app/templates/networkpolicy.yaml(1 hunks)charts/urbanstack-shared-app/templates/pvc.yaml(1 hunks)charts/urbanstack-shared-app/templates/service.yaml(1 hunks)charts/urbanstack-shared-app/templates/validation.yaml(1 hunks)charts/urbanstack-shared-app/values.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- .github/release-please/config.json
- charts/urbanstack-shared-app/ci/_common.sh
- charts/urbanstack-shared-app/.helmignore
- charts/urbanstack-shared-app/values.yaml
🚧 Files skipped from review as they are similar to previous changes (6)
- charts/urbanstack-shared-app/ci/cert-issuer.yaml
- charts/urbanstack-shared-app/ci/artifacthub-values.yaml
- .github/CODEOWNERS
- charts/urbanstack-shared-app/templates/_helpers.tpl
- charts/urbanstack-shared-app/Chart.yaml
- charts/urbanstack-shared-app/ci/auth-values.yaml
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1602
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/oauth-proxy.yaml:38-40
Timestamp: 2025-07-24T09:49:40.961Z
Learning: Official Helm charts like oauth2-proxy manage image versioning automatically through their Chart.yaml appVersion field, making manual tag pinning in consumer values unnecessary and potentially harmful. The chart version itself provides reproducibility by ensuring the correct image tag is used.
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1604
File: charts/base-cluster/templates/monitoring/metrics-server/metrics-server.yaml:20-21
Timestamp: 2025-07-24T09:41:28.072Z
Learning: The official kubernetes-sigs/metrics-server Helm chart pins the image tag to a specific version in its values.yaml by default, ensuring reproducible deployments without requiring explicit tag specification in consuming charts.
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's `valuesFrom` feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1604
File: charts/base-cluster/templates/monitoring/metrics-server/metrics-server.yaml:20-21
Timestamp: 2025-07-24T09:41:28.072Z
Learning: The kubernetes-sigs/metrics-server Helm chart uses `v{{ .Chart.AppVersion }}` as the default image tag when `image.tag` is empty, which provides pinned versioning through the chart's AppVersion rather than using floating tags like "latest".
charts/urbanstack-shared-app/templates/deployment.yaml (3)
Learnt from: cwrau
PR: #1604
File: charts/base-cluster/templates/monitoring/metrics-server/metrics-server.yaml:20-21
Timestamp: 2025-07-24T09:41:28.072Z
Learning: The official kubernetes-sigs/metrics-server Helm chart pins the image tag to a specific version in its values.yaml by default, ensuring reproducible deployments without requiring explicit tag specification in consuming charts.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
charts/urbanstack-shared-app/templates/image-pull-secret.yaml (1)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
charts/urbanstack-shared-app/templates/ingress.yaml (1)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
charts/urbanstack-shared-app/templates/networkpolicy.yaml (3)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:55-61
Timestamp: 2025-07-24T13:42:05.473Z
Learning: In charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml, the receiver-type parsing logic that splits keys by space and only handles exactly two tokens is intentional and matches the schema design. The schema pattern ^email($| \S+$) specifically allows "email" or "email " format, not arbitrary multi-space patterns. The current parsing implementation correctly enforces this constraint.
charts/urbanstack-shared-app/templates/pvc.yaml (1)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
charts/urbanstack-shared-app/templates/service.yaml (2)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
🪛 YAMLlint (1.37.1)
charts/urbanstack-shared-app/templates/deployment.yaml
[error] 8-8: syntax error: expected the node content, but found '-'
(syntax)
[error] 102-102: trailing spaces
(trailing-spaces)
charts/urbanstack-shared-app/templates/image-pull-secret.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/ingress.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/networkpolicy.yaml
[warning] 16-16: wrong indentation: expected 2 but found 4
(indentation)
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/pvc.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/service.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/validation.yaml
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check licenses
🔇 Additional comments (3)
charts/urbanstack-shared-app/templates/validation.yaml (1)
1-4: Validation guard is succinct and correctThe length check and fail message are spot-on and match the 52-char requirement.
charts/urbanstack-shared-app/templates/service.yaml (1)
1-15: Service template looks solidPorts, selector labels and type are all consistent with the Deployment spec (assuming the container exposes a named port
http).charts/urbanstack-shared-app/templates/deployment.yaml (1)
73-74: Overwriting the reservedHOSTNAMEenv var can break in-container discoveryKubernetes automatically sets
HOSTNAMEto the Pod’s DNS name. Redefining it may confuse libraries that rely on this variable (e.g. Java networking). Consider using a custom variable (e.g.APP_HOSTNAME) instead.
ol-teuto
force-pushed
the
add-urbanstack-shared-app
branch
from
387b63b to
3b045fd
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
charts/urbanstack-shared-app/templates/networkpolicy.yaml (2)
14-19: Indentation underegress:still breaks YAML
- to:must be indented exactly two spaces deeper thanegress:. The current four-space indent ({{- if …}}adds two) makes YAMLlint unhappy and renders an invalid manifest.egress: {{- if .Values.allowExternalEgress }} - - to: # allow external endpoints outside of the cluster + - to: # allow external endpoints outside of the cluster - ipBlock: cidr: 0.0.0.0/0 except: - 10.0.0.0/8 - 172.16.0.0/12 - 192.168.0.0/16 {{- end }}
32-37: InlinematchLabelsrenders garbled YAML – move it to its own lineRendering the mapping on the same line as
matchLabels:concatenates keys and values. EmitmatchLabels:on its own line, then indent the rendered YAML.- matchLabels: {{- .Values.postgres.networkPolicy.matchLabels | toYaml | nindent 14 }} + matchLabels: + {{- .Values.postgres.networkPolicy.matchLabels | toYaml | nindent 14 }}While touching this block, align the surrounding
ports:key two spaces back so it’s on the same level asto:.- ports: + ports:
🧹 Nitpick comments (4)
charts/urbanstack-shared-app/templates/validation.yaml (1)
1-4: Clarify comment vs conditionThe guard correctly fails at 53 chars (
gt 52), but the comment mentions a normal limit of 53.
A more explicit comment avoids the off-by-one mental gymnastic.-{{/* normal limit is 53, but we need 52 as the limit, because postgres usernames are limited to 64 chars */}} +{{/* PostgreSQL usernames are limited to 64 chars. We reserve 12 chars for suffixes, therefore the Helm + release name must be ≤ 52 chars. */}}charts/urbanstack-shared-app/templates/pvc.yaml (1)
6-11: Make storage class & access mode configurableHard-coding
ReadWriteOnceand leaving the storageClassName implicit prevents
clusters that rely on a non-default class, or that need ReadWriteMany, from
re-using this chart as-is.-spec: - accessModes: - - ReadWriteOnce +spec: + {{- if .Values.storage.accessModes }} + accessModes: {{ toYaml .Values.storage.accessModes | nindent 4 }} + {{- else }} + accessModes: + - ReadWriteOnce + {{- end }} + + {{- if .Values.storage.storageClassName }} + storageClassName: {{ .Values.storage.storageClassName | quote }} + {{- end }}Add the corresponding defaults to
values.yaml:storage: size: 8Gi # storageClassName: gp2 # accessModes: # - ReadWriteOncecharts/urbanstack-shared-app/templates/service.yaml (1)
6-13: Expose service type & ports via values for reuseLocking the chart to
ClusterIP/80 restricts legitimate scenarios (NodePort,
LoadBalancer, GRPC non-80, etc.). Parameterising these two knobs keeps the
chart flexible without breaking defaults.-spec: - type: ClusterIP +spec: + type: {{ default "ClusterIP" .Values.service.type }} ports: - - port: 80 - targetPort: http + - port: {{ default 80 .Values.service.port }} + targetPort: {{ default "http" .Values.service.targetPort }} protocol: TCP name: httpAdd to
values.yaml:service: # type: NodePort | LoadBalancer | ClusterIP # port: 80 # targetPort: httpcharts/urbanstack-shared-app/templates/deployment.yaml (1)
99-99: Remove trailing whitespaceLine 99 contains only spaces – fails
yamllinttrailing-spaces rule.- +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.github/CODEOWNERS(1 hunks).github/release-please/config.json(1 hunks)charts/urbanstack-shared-app/.helmignore(1 hunks)charts/urbanstack-shared-app/Chart.yaml(1 hunks)charts/urbanstack-shared-app/ci/_common.sh(1 hunks)charts/urbanstack-shared-app/ci/artifacthub-values.yaml(1 hunks)charts/urbanstack-shared-app/ci/auth-values.yaml(1 hunks)charts/urbanstack-shared-app/ci/cert-issuer.yaml(1 hunks)charts/urbanstack-shared-app/templates/_helpers.tpl(1 hunks)charts/urbanstack-shared-app/templates/deployment.yaml(1 hunks)charts/urbanstack-shared-app/templates/image-pull-secret.yaml(1 hunks)charts/urbanstack-shared-app/templates/ingress.yaml(1 hunks)charts/urbanstack-shared-app/templates/networkpolicy.yaml(1 hunks)charts/urbanstack-shared-app/templates/pvc.yaml(1 hunks)charts/urbanstack-shared-app/templates/service.yaml(1 hunks)charts/urbanstack-shared-app/templates/validation.yaml(1 hunks)charts/urbanstack-shared-app/values.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- .github/release-please/config.json
- charts/urbanstack-shared-app/ci/_common.sh
- charts/urbanstack-shared-app/values.yaml
- charts/urbanstack-shared-app/.helmignore
- charts/urbanstack-shared-app/templates/_helpers.tpl
🚧 Files skipped from review as they are similar to previous changes (5)
- charts/urbanstack-shared-app/ci/artifacthub-values.yaml
- charts/urbanstack-shared-app/ci/cert-issuer.yaml
- charts/urbanstack-shared-app/Chart.yaml
- .github/CODEOWNERS
- charts/urbanstack-shared-app/ci/auth-values.yaml
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1602
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/oauth-proxy.yaml:38-40
Timestamp: 2025-07-24T09:49:40.961Z
Learning: Official Helm charts like oauth2-proxy manage image versioning automatically through their Chart.yaml appVersion field, making manual tag pinning in consumer values unnecessary and potentially harmful. The chart version itself provides reproducibility by ensuring the correct image tag is used.
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1604
File: charts/base-cluster/templates/monitoring/metrics-server/metrics-server.yaml:20-21
Timestamp: 2025-07-24T09:41:28.072Z
Learning: The official kubernetes-sigs/metrics-server Helm chart pins the image tag to a specific version in its values.yaml by default, ensuring reproducible deployments without requiring explicit tag specification in consuming charts.
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's `valuesFrom` feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1604
File: charts/base-cluster/templates/monitoring/metrics-server/metrics-server.yaml:20-21
Timestamp: 2025-07-24T09:41:28.072Z
Learning: The kubernetes-sigs/metrics-server Helm chart uses `v{{ .Chart.AppVersion }}` as the default image tag when `image.tag` is empty, which provides pinned versioning through the chart's AppVersion rather than using floating tags like "latest".
charts/urbanstack-shared-app/templates/deployment.yaml (3)
Learnt from: cwrau
PR: #1604
File: charts/base-cluster/templates/monitoring/metrics-server/metrics-server.yaml:20-21
Timestamp: 2025-07-24T09:41:28.072Z
Learning: The official kubernetes-sigs/metrics-server Helm chart pins the image tag to a specific version in its values.yaml by default, ensuring reproducible deployments without requiring explicit tag specification in consuming charts.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
charts/urbanstack-shared-app/templates/image-pull-secret.yaml (2)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: #1602
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/oauth-proxy.yaml:38-40
Timestamp: 2025-07-24T09:49:40.961Z
Learning: Official Helm charts like oauth2-proxy manage image versioning automatically through their Chart.yaml appVersion field, making manual tag pinning in consumer values unnecessary and potentially harmful. The chart version itself provides reproducibility by ensuring the correct image tag is used.
charts/urbanstack-shared-app/templates/ingress.yaml (1)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
charts/urbanstack-shared-app/templates/networkpolicy.yaml (3)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:55-61
Timestamp: 2025-07-24T13:42:05.473Z
Learning: In charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml, the receiver-type parsing logic that splits keys by space and only handles exactly two tokens is intentional and matches the schema design. The schema pattern ^email($| \S+$) specifically allows "email" or "email " format, not arbitrary multi-space patterns. The current parsing implementation correctly enforces this constraint.
charts/urbanstack-shared-app/templates/pvc.yaml (1)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
charts/urbanstack-shared-app/templates/service.yaml (2)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
🪛 YAMLlint (1.37.1)
charts/urbanstack-shared-app/templates/deployment.yaml
[warning] 19-19: wrong indentation: expected 4 but found 6
(indentation)
[error] 99-99: trailing spaces
(trailing-spaces)
[error] 7-7: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/image-pull-secret.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/ingress.yaml
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/networkpolicy.yaml
[warning] 15-15: wrong indentation: expected 2 but found 4
(indentation)
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/pvc.yaml
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/service.yaml
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/validation.yaml
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check licenses
🔇 Additional comments (1)
charts/urbanstack-shared-app/templates/networkpolicy.yaml (1)
4-5: Helper prefix fixed – thanks for aligning with chart nameThe switch to
urbanstack-shared-app.*helpers removes the previous namespace-collision risk and improves operator UX. ✅
ol-teuto
force-pushed
the
add-urbanstack-shared-app
branch
from
3b045fd to
4a2fbba
Compare
ol-teuto
force-pushed
the
add-urbanstack-shared-app
branch
from
4a2fbba to
ec56d3f
Compare
| - to: # allow using nginx proxy to call other services | ||
| - podSelector: | ||
| matchLabels: | ||
| app.kubernetes.io/instance: ingress-nginx | ||
| app.kubernetes.io/name: ingress-nginx | ||
| namespaceSelector: | ||
| matchLabels: | ||
| kubernetes.io/metadata.name: ingress-nginx | ||
| ports: | ||
| - protocol: TCP | ||
| port: 80 | ||
| - protocol: TCP | ||
| port: 443 |
There was a problem hiding this comment.
Are you sure this works?
So the application (hardcoded) calls the ingress-nginx but with a different Host header / SNI so (hardcoded) nginx proxies it correctly?
There was a problem hiding this comment.
I think it should work, nginx has to handle the Host header in normal scenarios too. This is also only needed for local testing, so I could add an option to enable/disable this
| {{/* | ||
| Common labels | ||
| */}} | ||
| {{- define "urbanstack-shared-app.labels" -}} | ||
| helm.sh/chart: {{ include "urbanstack-shared-app.chart" . | quote }} | ||
| tenant: {{ .Values.tenant | quote }} | ||
| citytool: {{ .Values.citytool | quote }} | ||
| {{ include "urbanstack-shared-app.selectorLabels" . }} | ||
| app.kubernetes.io/managed-by: {{ .Release.Service | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
You should use https://artifacthub.io/packages/helm/bitnami/common#labels via our common chart
There was a problem hiding this comment.
Is it really worth it to pull in a dependency for this?
There was a problem hiding this comment.
Definitely, if the best-practices labels someday change, this chart doesn't have to be adjusted.
And with this dependency you can, and should, also use https://artifacthub.io/packages/helm/bitnami/common#names, https://artifacthub.io/packages/helm/bitnami/common#images, https://artifacthub.io/packages/helm/bitnami/common#ingress, and all the other nice templates.
Essentially, the _helpers.tpl should be empty, as all of the templates are already covered by the common chart
There was a problem hiding this comment.
I updated the MR to include the common chart where it makes sense
| {{/* | ||
| Selector labels | ||
| */}} | ||
| {{- define "urbanstack-shared-app.selectorLabels" -}} | ||
| app.kubernetes.io/name: {{ include "urbanstack-shared-app.name" . | quote }} | ||
| app.kubernetes.io/instance: {{ .Release.Name | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
Same here, use https://artifacthub.io/packages/helm/bitnami/common#labels
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (4)
charts/urbanstack-shared-app/templates/image-pull-secret.yaml (1)
8-9: Extend.dockerconfigjsonfor wider registry compatibilityAdd explicit
username&passwordkeys alongsideauth.- .dockerconfigjson: {{ dict "auths" (dict .Values.image.registry (dict "auth" (printf "%s:%s" .Values.registryAuth.username .Values.registryAuth.password | b64enc))) | toJson | quote }} + .dockerconfigjson: {{ dict "auths" (dict .Values.image.registry (dict + "username" .Values.registryAuth.username + "password" .Values.registryAuth.password + "auth" (printf "%s:%s" .Values.registryAuth.username .Values.registryAuth.password | b64enc))) | toJson | quote }}charts/urbanstack-shared-app/templates/networkpolicy.yaml (2)
34-34:matchLabelsinline rendering issue (same as previous review)The helper already returns a newline. Keeping it on the same line risks broken YAML—place it on its own line.
- matchLabels: {{- .Values.postgres.networkPolicy.matchLabels | toYaml | nindent 14 }} + matchLabels: + {{- .Values.postgres.networkPolicy.matchLabels | toYaml | nindent 14 }}
24-26:namespaceSelectorstill nested underpodSelector– move two spaces left
namespaceSelector:must be a sibling ofpodSelector:. Current 10-space indent keeps it nested and invalidates the selector block.- namespaceSelector: - matchLabels: - kubernetes.io/metadata.name: kube-system + namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: kube-systemcharts/urbanstack-shared-app/templates/deployment.yaml (1)
18-21:imagePullSecretsblock still mis-indented (previous feedback not applied)Keys under
template.specrequire 8-space indentation. Rendered YAML now has 6 spaces, breaking the document.- {{- if and .Values.registryAuth.username .Values.registryAuth.password }} - imagePullSecrets: - - name: {{ printf "pullsecret-%s" .Release.Name | quote }} - {{- end }} + {{- if and .Values.registryAuth.username .Values.registryAuth.password }} + imagePullSecrets: + - name: {{ printf "pullsecret-%s" .Release.Name | quote }} + {{- end }}
🧹 Nitpick comments (1)
charts/urbanstack-shared-app/templates/deployment.yaml (1)
99-99: Remove trailing spacesThere is dangling whitespace on the blank line; fails strict linters.
- +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.github/CODEOWNERS(1 hunks).github/release-please/config.json(1 hunks)charts/urbanstack-shared-app/.helmignore(1 hunks)charts/urbanstack-shared-app/Chart.yaml(1 hunks)charts/urbanstack-shared-app/ci/_common.sh(1 hunks)charts/urbanstack-shared-app/ci/artifacthub-values.yaml(1 hunks)charts/urbanstack-shared-app/ci/auth-values.yaml(1 hunks)charts/urbanstack-shared-app/ci/cert-issuer.yaml(1 hunks)charts/urbanstack-shared-app/templates/_helpers.tpl(1 hunks)charts/urbanstack-shared-app/templates/deployment.yaml(1 hunks)charts/urbanstack-shared-app/templates/image-pull-secret.yaml(1 hunks)charts/urbanstack-shared-app/templates/ingress.yaml(1 hunks)charts/urbanstack-shared-app/templates/networkpolicy.yaml(1 hunks)charts/urbanstack-shared-app/templates/pvc.yaml(1 hunks)charts/urbanstack-shared-app/templates/service.yaml(1 hunks)charts/urbanstack-shared-app/templates/validation.yaml(1 hunks)charts/urbanstack-shared-app/values.schema.json(1 hunks)charts/urbanstack-shared-app/values.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- .github/release-please/config.json
- charts/urbanstack-shared-app/values.schema.json
- charts/urbanstack-shared-app/.helmignore
- charts/urbanstack-shared-app/ci/_common.sh
- charts/urbanstack-shared-app/values.yaml
- charts/urbanstack-shared-app/templates/_helpers.tpl
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/CODEOWNERS
- charts/urbanstack-shared-app/ci/auth-values.yaml
- charts/urbanstack-shared-app/ci/artifacthub-values.yaml
- charts/urbanstack-shared-app/Chart.yaml
- charts/urbanstack-shared-app/ci/cert-issuer.yaml
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1602
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/oauth-proxy.yaml:38-40
Timestamp: 2025-07-24T09:49:40.961Z
Learning: Official Helm charts like oauth2-proxy manage image versioning automatically through their Chart.yaml appVersion field, making manual tag pinning in consumer values unnecessary and potentially harmful. The chart version itself provides reproducibility by ensuring the correct image tag is used.
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1604
File: charts/base-cluster/templates/monitoring/metrics-server/metrics-server.yaml:20-21
Timestamp: 2025-07-24T09:41:28.072Z
Learning: The official kubernetes-sigs/metrics-server Helm chart pins the image tag to a specific version in its values.yaml by default, ensuring reproducible deployments without requiring explicit tag specification in consuming charts.
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's `valuesFrom` feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1604
File: charts/base-cluster/templates/monitoring/metrics-server/metrics-server.yaml:20-21
Timestamp: 2025-07-24T09:41:28.072Z
Learning: The kubernetes-sigs/metrics-server Helm chart uses `v{{ .Chart.AppVersion }}` as the default image tag when `image.tag` is empty, which provides pinned versioning through the chart's AppVersion rather than using floating tags like "latest".
charts/urbanstack-shared-app/templates/deployment.yaml (3)
Learnt from: cwrau
PR: #1604
File: charts/base-cluster/templates/monitoring/metrics-server/metrics-server.yaml:20-21
Timestamp: 2025-07-24T09:41:28.072Z
Learning: The official kubernetes-sigs/metrics-server Helm chart pins the image tag to a specific version in its values.yaml by default, ensuring reproducible deployments without requiring explicit tag specification in consuming charts.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
charts/urbanstack-shared-app/templates/image-pull-secret.yaml (2)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
charts/urbanstack-shared-app/templates/ingress.yaml (2)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
charts/urbanstack-shared-app/templates/networkpolicy.yaml (3)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:55-61
Timestamp: 2025-07-24T13:42:05.473Z
Learning: In charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml, the receiver-type parsing logic that splits keys by space and only handles exactly two tokens is intentional and matches the schema design. The schema pattern ^email($| \S+$) specifically allows "email" or "email " format, not arbitrary multi-space patterns. The current parsing implementation correctly enforces this constraint.
charts/urbanstack-shared-app/templates/pvc.yaml (1)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
charts/urbanstack-shared-app/templates/service.yaml (2)
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
🪛 YAMLlint (1.37.1)
charts/urbanstack-shared-app/templates/deployment.yaml
[warning] 19-19: wrong indentation: expected 4 but found 6
(indentation)
[error] 99-99: trailing spaces
(trailing-spaces)
[error] 7-7: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/image-pull-secret.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/ingress.yaml
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/networkpolicy.yaml
[warning] 15-15: wrong indentation: expected 2 but found 4
(indentation)
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/pvc.yaml
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/service.yaml
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/validation.yaml
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
🪛 GitHub Actions: Lint Helm Charts
charts/urbanstack-shared-app/templates/deployment.yaml
[error] 1-1: Helm lint error: 'postgres' contains additional property 'matchLabels' which is not allowed; 'registryAuth' missing required fields 'username' and 'password'.
charts/urbanstack-shared-app/templates/image-pull-secret.yaml
[error] 1-1: Helm lint error: 'postgres' contains additional property 'matchLabels' which is not allowed; 'registryAuth' missing required fields 'username' and 'password'.
charts/urbanstack-shared-app/templates/ingress.yaml
[error] 1-1: Helm lint error: 'postgres' contains additional property 'matchLabels' which is not allowed; 'registryAuth' missing required fields 'username' and 'password'.
charts/urbanstack-shared-app/templates/networkpolicy.yaml
[error] 1-1: Helm lint error: 'postgres' contains additional property 'matchLabels' which is not allowed; 'registryAuth' missing required fields 'username' and 'password'.
charts/urbanstack-shared-app/templates/pvc.yaml
[error] 1-1: Helm lint error: 'postgres' contains additional property 'matchLabels' which is not allowed; 'registryAuth' missing required fields 'username' and 'password'.
charts/urbanstack-shared-app/templates/service.yaml
[error] 1-1: Helm lint error: 'postgres' contains additional property 'matchLabels' which is not allowed; 'registryAuth' missing required fields 'username' and 'password'.
charts/urbanstack-shared-app/templates/validation.yaml
[error] 1-1: Helm lint error: 'postgres' contains additional property 'matchLabels' which is not allowed; 'registryAuth' missing required fields 'username' and 'password'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check licenses
| name: {{ .Release.Name | quote }} | ||
| annotations: | ||
| contact: {{ .Values.contact | quote }} | ||
| labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} |
There was a problem hiding this comment.
Move helper output to next line for metadata.labels
Inline helper usage causes the “expected node content, but found ‘-’” syntax error.
- labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }}
+ labels:
+ {{- include "urbanstack-shared-app.labels" . | nindent 4 }}📝 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.
| labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} | |
| labels: | |
| {{- include "urbanstack-shared-app.labels" . | nindent 4 }} |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 7-7: syntax error: expected the node content, but found '-'
(syntax)
🤖 Prompt for AI Agents
In charts/urbanstack-shared-app/templates/deployment.yaml at line 7, the inline
usage of the helper for metadata.labels causes a YAML syntax error due to the
leading dash. To fix this, move the helper output to the next line and properly
indent it so that the labels are correctly parsed as a YAML map, avoiding the
syntax error.
| metadata: | ||
| name: {{ printf "pullsecret-%s" .Release.Name | quote }} | ||
| labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} | ||
| type: kubernetes.io/dockerconfigjson |
There was a problem hiding this comment.
labels needs its own block
- labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }}
+ labels:
+ {{- include "urbanstack-shared-app.labels" . | nindent 4 }}📝 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.
| metadata: | |
| name: {{ printf "pullsecret-%s" .Release.Name | quote }} | |
| labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} | |
| type: kubernetes.io/dockerconfigjson | |
| metadata: | |
| name: {{ printf "pullsecret-%s" .Release.Name | quote }} | |
| labels: | |
| {{- include "urbanstack-shared-app.labels" . | nindent 4 }} | |
| type: kubernetes.io/dockerconfigjson |
🤖 Prompt for AI Agents
In charts/urbanstack-shared-app/templates/image-pull-secret.yaml around lines 4
to 7, the labels field is incorrectly formatted inline and needs to be its own
YAML block. Change the labels line so that it starts a new block with proper
indentation, placing the included labels content on the next indented line to
ensure valid YAML structure.
| name: {{ .Release.Name | quote }} | ||
| labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} | ||
| annotations: |
There was a problem hiding this comment.
Inline labels causes malformed manifest
Same issue as in pvc.yaml – move the helper to the next indented line.
- labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }}
+ labels:
+ {{- include "urbanstack-shared-app.labels" . | nindent 4 }}📝 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.
| name: {{ .Release.Name | quote }} | |
| labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} | |
| annotations: | |
| name: {{ .Release.Name | quote }} | |
| labels: | |
| {{- include "urbanstack-shared-app.labels" . | nindent 4 }} | |
| annotations: |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
🤖 Prompt for AI Agents
In charts/urbanstack-shared-app/templates/ingress.yaml around lines 4 to 6, the
labels field is rendered inline, causing a malformed manifest. To fix this, move
the labels helper invocation to its own line and indent it properly so that the
labels are rendered as a nested map under the labels key, ensuring correct YAML
structure.
| tls: | ||
| - hosts: | ||
| - {{ .Values.host | quote }} | ||
| secretName: {{ printf "%s-tls" .Values.host | quote }} |
There was a problem hiding this comment.
Bad indentation under tls: – will be rejected by API server
List items must be indented beneath tls:. Fix:
- tls:
- - hosts:
- - {{ .Values.host | quote }}
- secretName: {{ printf "%s-tls" .Values.host | quote }}
+ tls:
+ - hosts:
+ - {{ .Values.host | quote }}
+ secretName: {{ printf "%s-tls" .Values.host | quote }}📝 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.
| tls: | |
| - hosts: | |
| - {{ .Values.host | quote }} | |
| secretName: {{ printf "%s-tls" .Values.host | quote }} | |
| tls: | |
| - hosts: | |
| - {{ .Values.host | quote }} | |
| secretName: {{ printf "%s-tls" .Values.host | quote }} |
🤖 Prompt for AI Agents
In charts/urbanstack-shared-app/templates/ingress.yaml around lines 23 to 26,
the list items under the tls key are not properly indented, which will cause
rejection by the API server. Adjust the indentation so that the dash and its
nested keys (hosts and secretName) are indented one level further under tls,
ensuring proper YAML list structure.
| name: {{ .Release.Name | quote }} | ||
| labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} | ||
| spec: |
There was a problem hiding this comment.
Fix metadata.labels – inline helper breaks YAML parsing
labels: must end the line and the helper output must start on the next line.
Current rendering produces the “expected node content, but found ‘-’” error reported by yamllint & Helm-lint.
- labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }}
+ labels:
+ {{- include "urbanstack-shared-app.labels" . | nindent 4 }}📝 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.
| name: {{ .Release.Name | quote }} | |
| labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} | |
| spec: | |
| name: {{ .Release.Name | quote }} | |
| labels: | |
| {{- include "urbanstack-shared-app.labels" . | nindent 4 }} | |
| spec: |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
🤖 Prompt for AI Agents
In charts/urbanstack-shared-app/templates/networkpolicy.yaml around lines 4 to
6, the metadata.labels field is incorrectly formatted causing YAML parsing
errors. Modify the labels line so that it ends immediately after the colon, and
place the output of the inline helper on the following indented line. This
ensures proper YAML structure and resolves the "expected node content, but found
'-'" linting errors.
| name: {{ .Release.Name | quote }} | ||
| labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} |
There was a problem hiding this comment.
labels rendered on same line – breaks YAML
The current output collapses into labels: app.kubernetes.io/name: …, which is invalid.
- labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }}
+ labels:
+ {{- include "urbanstack-shared-app.labels" . | nindent 4 }}📝 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.
| name: {{ .Release.Name | quote }} | |
| labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} | |
| name: {{ .Release.Name | quote }} | |
| labels: | |
| {{- include "urbanstack-shared-app.labels" . | nindent 4 }} |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
🤖 Prompt for AI Agents
In charts/urbanstack-shared-app/templates/pvc.yaml around lines 4 to 5, the
labels field is rendered on the same line as the key, causing invalid YAML
output. To fix this, ensure the labels block is properly indented and rendered
on a new line by removing the inline dash after labels and using correct
indentation so that the labels map appears as a nested block under the labels
key.
| name: {{ .Release.Name | quote }} | ||
| labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} | ||
| spec: |
There was a problem hiding this comment.
Invalid YAML due to inline labels
- labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }}
+ labels:
+ {{- include "urbanstack-shared-app.labels" . | nindent 4 }}📝 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.
| name: {{ .Release.Name | quote }} | |
| labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} | |
| spec: | |
| name: {{ .Release.Name | quote }} | |
| labels: | |
| {{- include "urbanstack-shared-app.labels" . | nindent 4 }} | |
| spec: |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
🤖 Prompt for AI Agents
In charts/urbanstack-shared-app/templates/service.yaml around lines 4 to 6, the
labels field is incorrectly formatted as an inline value, causing invalid YAML.
Change the labels field to be a proper YAML mapping by placing the include
statement on the next line and indenting it correctly under labels, ensuring
valid YAML structure.
| {{/* normal limit is 53, but we need 52 as the limit, because postgres usernames are limited to 64 chars */}} | ||
| {{- if gt (.Release.Name | len) 52 -}} | ||
| {{ fail "Release.Name max length is 52!" }} | ||
| {{- end -}} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Rename or silence the file – current .yaml is not valid YAML
The template contains only Helm logic, yet the .yaml suffix makes YAML-lint parse it and fail (expected '<document start>').
Either rename the file to validation.tpl / _validation.tpl or keep the name but add a pragma to disable linting:
+# yamllint disable-fileDoing so removes unnecessary CI noise without altering behaviour.
📝 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.
| {{/* normal limit is 53, but we need 52 as the limit, because postgres usernames are limited to 64 chars */}} | |
| {{- if gt (.Release.Name | len) 52 -}} | |
| {{ fail "Release.Name max length is 52!" }} | |
| {{- end -}} | |
| # yamllint disable-file | |
| {{/* normal limit is 53, but we need 52 as the limit, because postgres usernames are limited to 64 chars */}} | |
| {{- if gt (.Release.Name | len) 52 -}} | |
| {{ fail "Release.Name max length is 52!" }} | |
| {{- end -}} |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
🪛 GitHub Actions: Lint Helm Charts
[error] 1-1: Helm lint error: 'postgres' contains additional property 'matchLabels' which is not allowed; 'registryAuth' missing required fields 'username' and 'password'.
🤖 Prompt for AI Agents
In charts/urbanstack-shared-app/templates/validation.yaml lines 1 to 4, the file
is named with a .yaml extension but contains only Helm template logic, causing
YAML linting errors. To fix this, rename the file to have a .tpl or
_validation.tpl extension to indicate it is a Helm template, or keep the .yaml
extension but add a YAML linting pragma at the top to disable linting for this
file, preventing unnecessary CI noise without changing functionality.
a0e170a to
ce209cc
Compare
ol-teuto
force-pushed
the
add-urbanstack-shared-app
branch
from
ce209cc to
e280290
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (12)
charts/urbanstack-shared-app/templates/validation.yaml (1)
1-4: Keep YAML-lint quiet – rename file or add pragma
The template contains no real YAML, yet the.yamlsuffix makes YAML-lint choke (see CI output). Rename tovalidation.tpl/_validation.tplor add a pragma at the top:+# yamllint disable-file {{/* normal limit is 53, but we need 52 as the limit, because postgres usernames are limited to 64 chars */}}charts/urbanstack-shared-app/templates/pvc.yaml (1)
4-5: Inlinelabelsstill breaks linters
Same issue flagged earlier: the helper must start on its own line to keep linters happy.- labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} + labels: + {{- include "urbanstack-shared-app.labels" . | nindent 4 }}charts/urbanstack-shared-app/templates/ingress.yaml (2)
4-5: Inlinelabels→ malformed manifest- labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} + labels: + {{- include "urbanstack-shared-app.labels" . | nindent 4 }}
19-22: Indent list items undertls:
List items must be indented beneathtls:; current output is rejected by strict API servers.- tls: - - hosts: - - {{ .Values.host | quote }} - secretName: {{ printf "%s-tls" .Values.host | quote }} + tls: + - hosts: + - {{ .Values.host | quote }} + secretName: {{ printf "%s-tls" .Values.host | quote }}charts/urbanstack-shared-app/templates/image-pull-secret.yaml (2)
5-6: Inlinelabels→ YAML error- labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} + labels: + {{- include "urbanstack-shared-app.labels" . | nindent 4 }}
9-9: Registry auth JSON still omitsusername/passwordfields
Keeping onlyauthbreaks pulls on Harbor/GitLab/ECR. Full record:- .dockerconfigjson: {{ dict "auths" (dict .Values.image.registry (dict "auth" (printf "%s:%s" .Values.registryAuth.username .Values.registryAuth.password | b64enc))) | toJson | quote }} + .dockerconfigjson: {{ dict "auths" (dict .Values.image.registry (dict + "username" .Values.registryAuth.username + "password" .Values.registryAuth.password + "auth" (printf "%s:%s" .Values.registryAuth.username .Values.registryAuth.password | b64enc))) | toJson | quote }}charts/urbanstack-shared-app/templates/service.yaml (1)
4-5: Inlinelabels→ YAML error- labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} + labels: + {{- include "urbanstack-shared-app.labels" . | nindent 4 }}charts/urbanstack-shared-app/templates/networkpolicy.yaml (3)
5-5: Fix YAML syntax error in labels fieldThe inline helper usage causes a YAML parsing error. The labels key must end with a colon and the helper output should be on the next indented line.
- labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} + labels: + {{- include "urbanstack-shared-app.labels" . | nindent 4 }}
15-15: Fix indentation of egress list itemsThe list items under
egress:are indented with 4 spaces instead of the required 2 spaces. This violates YAML structure and causes linting errors.- {{- if .Values.allowExternalEgress }} - - to: # allow external endpoints outside of the cluster + {{- if .Values.allowExternalEgress }} + - to: # allow external endpoints outside of the clusterApply the same indentation fix to all other egress list items starting at lines 20, 32, and 38.
34-34: Fix inline YAML rendering for matchLabelsThe
matchLabelsfield is rendered inline, which can cause YAML formatting issues when the helper produces multi-line output.- matchLabels: {{- .Values.postgres.networkPolicy.matchLabels | toYaml | nindent 14 }} + matchLabels: + {{- .Values.postgres.networkPolicy.matchLabels | toYaml | nindent 14 }}charts/urbanstack-shared-app/templates/deployment.yaml (2)
7-7: Fix YAML syntax error in labels fieldThe inline helper usage causes a YAML parsing error identical to the NetworkPolicy template.
- labels: {{- include "urbanstack-shared-app.labels" . | nindent 4 }} + labels: + {{- include "urbanstack-shared-app.labels" . | nindent 4 }}
19-19: Fix indentation of imagePullSecrets blockThe
imagePullSecretsblock is indented 6 spaces instead of the expected 4 spaces underspec:, causing YAML linting errors.- imagePullSecrets: - - name: {{ printf "pullsecret-%s" .Release.Name | quote }} + imagePullSecrets: + - name: {{ printf "pullsecret-%s" .Release.Name | quote }}
🧹 Nitpick comments (2)
charts/urbanstack-shared-app/templates/networkpolicy.yaml (1)
38-50: Consider making ingress controller configurableThe NetworkPolicy is hardcoded to
ingress-nginx. While the maintainer mentioned this is acceptable for their specific use case, consider adding a configuration option for better chart reusability.+{{- $ingressController := .Values.ingressController | default "ingress-nginx" }} - to: # allow using nginx proxy to call other services - podSelector: matchLabels: - app.kubernetes.io/instance: ingress-nginx - app.kubernetes.io/name: ingress-nginx + app.kubernetes.io/instance: {{ $ingressController }} + app.kubernetes.io/name: {{ $ingressController }} namespaceSelector: matchLabels: - kubernetes.io/metadata.name: ingress-nginx + kubernetes.io/metadata.name: {{ $ingressController }}Apply the same pattern to the ingress section (lines 52-59).
charts/urbanstack-shared-app/templates/deployment.yaml (1)
99-100: Remove trailing whitespaceLine 99 contains trailing spaces which violate YAML formatting standards.
- +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
charts/urbanstack-shared-app/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
.github/CODEOWNERS(1 hunks).github/release-please/config.json(1 hunks)charts/urbanstack-shared-app/.helmignore(1 hunks)charts/urbanstack-shared-app/Chart.yaml(1 hunks)charts/urbanstack-shared-app/ci/_common.sh(1 hunks)charts/urbanstack-shared-app/ci/artifacthub-values.yaml(1 hunks)charts/urbanstack-shared-app/ci/auth-values.yaml(1 hunks)charts/urbanstack-shared-app/ci/cert-issuer.yaml(1 hunks)charts/urbanstack-shared-app/templates/_helpers.tpl(1 hunks)charts/urbanstack-shared-app/templates/deployment.yaml(1 hunks)charts/urbanstack-shared-app/templates/image-pull-secret.yaml(1 hunks)charts/urbanstack-shared-app/templates/ingress.yaml(1 hunks)charts/urbanstack-shared-app/templates/networkpolicy.yaml(1 hunks)charts/urbanstack-shared-app/templates/pvc.yaml(1 hunks)charts/urbanstack-shared-app/templates/service.yaml(1 hunks)charts/urbanstack-shared-app/templates/validation.yaml(1 hunks)charts/urbanstack-shared-app/values.schema.json(1 hunks)charts/urbanstack-shared-app/values.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- charts/urbanstack-shared-app/ci/_common.sh
- .github/release-please/config.json
- charts/urbanstack-shared-app/.helmignore
🚧 Files skipped from review as they are similar to previous changes (8)
- charts/urbanstack-shared-app/ci/cert-issuer.yaml
- charts/urbanstack-shared-app/ci/auth-values.yaml
- charts/urbanstack-shared-app/ci/artifacthub-values.yaml
- .github/CODEOWNERS
- charts/urbanstack-shared-app/values.yaml
- charts/urbanstack-shared-app/templates/_helpers.tpl
- charts/urbanstack-shared-app/Chart.yaml
- charts/urbanstack-shared-app/values.schema.json
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/urbanstack-shared-app/templates/deployment.yaml
[warning] 19-19: wrong indentation: expected 4 but found 6
(indentation)
[error] 99-99: trailing spaces
(trailing-spaces)
[error] 7-7: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/image-pull-secret.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/ingress.yaml
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/networkpolicy.yaml
[warning] 15-15: wrong indentation: expected 2 but found 4
(indentation)
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/pvc.yaml
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/service.yaml
[error] 5-5: syntax error: expected the node content, but found '-'
(syntax)
charts/urbanstack-shared-app/templates/validation.yaml
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
🔇 Additional comments (3)
charts/urbanstack-shared-app/templates/deployment.yaml (3)
23-30: LGTM: Strong security configurationThe security context configuration is excellent, implementing defense-in-depth with proper user/group IDs, non-root execution, and disabled host access. This follows Kubernetes security best practices.
33-42: LGTM: Container security hardeningThe container security context properly implements the principle of least privilege with read-only root filesystem, dropped capabilities, and disabled privilege escalation.
74-87: LGTM: Well-configured health probesThe health probe configuration is appropriate with a reasonable startup probe (100 seconds total) and separate liveness/readiness endpoints following best practices.
3 participants
Will be used by urbanstack to deploy shared apps. Most values will be set by the platform, only the image can be chosen by the user.
Summary by CodeRabbit
New Features
urbanstack-shared-app, providing templates for Kubernetes Deployment, Service, Ingress, NetworkPolicy, PersistentVolumeClaim, and image pull secrets.Chores