packages/system/hcloud/charts/values.yaml (1)

1-155: Duplicate values file – risk of drift

There are two almost-identical values.yaml files:

1. packages/system/hcloud/values.yaml
2. packages/system/hcloud/charts/values.yaml

Maintaining both will inevitably lead to drift and unclear source-of-truth. Prefer one of:

• Keep only the chart-local charts/values.yaml and delete the top-level copy.
• Or, symlink the second to the first during the build.

Either way, document the decision in README.md.

packages/system/hcloud/charts/.snapshots/full.daemonset.yaml (1)

10-18: Same RBAC concern as Deployment – replace cluster-admin.

Replicate the tailored ClusterRole advice here to avoid over-privileged access.

packages/system/hcloud/Makefile (1)

1-8: Missing common PHONY targets

checkmake warns that all, clean, and test phony goals are absent. Adding them improves developer UX and keeps tooling quiet.

.PHONY: all clean test

all: update

clean:
	rm -rf charts

test:
	@echo "no tests yet"

packages/system/hcloud/charts/templates/clusterrolebinding.yaml (2)

6-9: Over-privileged binding (cluster-admin)

Binding the CCM to cluster-admin grants full control over the cluster. Upstream charts typically bind to system:cloud-controller-manager or a minimal custom ClusterRole. Recommend revisiting required permissions and defaulting to least privilege.

---

1-1: YAML front-matter missing ---

Several tools expect a YAML document separator when the file starts with a Helm conditional. Add --- after the template directive to keep linters (e.g. yamllint, kube-score) happy.

{{- if .Values.rbac.create }}
+---
kind: ClusterRoleBinding

packages/system/hcloud/charts/README.md (2)

25-26: Minor style – missing comma

documentation. Perhaps → documentation. Perhaps. Tiny readability fix, no functional impact.

---

55-61: Comma after introductory phrase

“By default, the chart will deploy…” – add the comma after “By default” to follow standard technical-writing style.

packages/system/hcloud/charts/templates/daemonset.yaml (1)

58-60: hostNetwork: true – warn users

Enabling host networking is necessary for PodCIDR allocation but it also bypasses network policies and may expose node IPs. Make this trade-off explicit in the docs and consider adding a value (networking.hostNetwork) so users can override if they manage CIDRs differently.

packages/system/hcloud/charts/.snapshots/default.yaml (1)

88-88: Make image tag configurable instead of hard-coding.

Hard-coding v1.26.0 in the rendered template prevents pinning to newer patch versions via values.yaml and forces a chart bump for each image update.

-image: docker.io/hetznercloud/hcloud-cloud-controller-manager:v1.26.0 # x-releaser-pleaser-version
+image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"

packages/system/hcloud/charts/.snapshots/full.daemonset.yaml (1)

62-70: YAML lint errors – remove trailing whitespace.

Lines 64 and 69 contain stray spaces that break yamllint -d strict and some CI pipelines.

+

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: lllamnyp
PR: cozystack/cozystack#1025
File: packages/apps/kafka/charts/cozy-lib:1-1
Timestamp: 2025-06-04T06:22:17.306Z
Learning: Files in packages/apps/*/charts/cozy-lib that contain the path "../../../library/cozy-lib" are symbolic links, not regular files. Git diffs may display symlinks as regular files with their target path as content, which can be misleading in code review.

packages/system/hcloud/Makefile (1)

9-10: Extraction path still mismatched – tar leaves charts/ empty

The downloaded archive is …master.tar.gz, whose root folder is hcloud-cloud-controller-manager-master/.
tar … hcloud-cloud-controller-manager-main/chart therefore fails silently and nothing is extracted.

-	tar xzvf - --strip 2 -C charts hcloud-cloud-controller-manager-main/chart
+	tar xzvf - --strip 2 -C charts hcloud-cloud-controller-manager-master/chart

(Adjust again once upstream moves from master to main.)

packages/system/hcloud/charts/templates/clusterrolebinding.yaml (1)

5-5: Illegal : in resource name – manifest rejected by API server
Kubernetes names must be DNS-1123 sub-domains.

-  name: "system:{{ include "hcloud-cloud-controller-manager.name" . }}"
+  name: "system-{{ include "hcloud-cloud-controller-manager.name" . }}"

packages/system/hcloud/charts/templates/podmonitor.yaml (1)

1-3: Wrong API-version probe – PodMonitor never rendered

.Capabilities.APIVersions.Has expects only group/version.

-{{ if $.Capabilities.APIVersions.Has "monitoring.coreos.com/v1/PodMonitor" }}
+{{ if $.Capabilities.APIVersions.Has "monitoring.coreos.com/v1" }}

packages/system/hcloud/charts/templates/deployment.yaml (2)

68-69: Explicit command missing – inconsistent with DaemonSet

The DaemonSet template pins the entrypoint; the Deployment relies on the image default.
For determinism add the same command (or drop it from the DaemonSet).

-        - name: hcloud-cloud-controller-manager
+        - name: hcloud-cloud-controller-manager
+          command: ["/bin/hcloud-cloud-controller-manager"]

---

70-74: Empty-string values still emit invalid flags

Only nil is filtered; "" becomes --flag= which HCCM treats as invalid.

-{{- if not (eq $value nil) }}
+{{- if and (not (eq $value nil)) (ne $value "") }}

Replicate in the DaemonSet template.

packages/system/hcloud/charts/templates/daemonset.yaml (1)

65-69: Empty-string args still rendered → flags like --foo= appear.

A prior review already flagged this; the check still only guards against nil, not "". Use hasKey + and (ne (trim $value) "") (or similar) so empty strings are skipped too.

packages/system/hcloud/charts/.snapshots/default.yaml (2)

10-18: cluster-admin role remains – RBAC is still over-permissive.

Same concern as earlier: binding full cluster-admin to the CCM needlessly increases blast radius. Replace with a narrowed ClusterRole containing only the verbs/resources the controller actually uses.

---

62-96: Missing securityContext – identical hardening recommendation as before.

The Deployment snapshot still lacks runAsNonRoot, allowPrivilegeEscalation: false, etc. Apply the same securityContext block suggested in previous reviews.

packages/system/hcloud/charts/.snapshots/full.daemonset.yaml (3)

105-113: Secret name mismatch (hcloud-token vs hcloud) persists.

Volume token-volume references hcloud-token but the env vars read from secret hcloud; this will 404 the mount and crash-loop. Align both to the same secret name.

---

24-33: cluster-admin binding still present in DaemonSet snapshot.

Issue identical to Deployment snapshot – please scope RBAC down.

---

71-100: DaemonSet container lacks securityContext.

Add the same non-root, no-priv-esc block to the DaemonSet variant for consistency and compliance.

packages/system/hcloud/Makefile (2)

6-8: update is not resilient to failures

If curl or tar fail, the Make run continues, leaving the package half-updated.
Consider adding set -euo pipefail or using Make’s .ONESHELL: plus || exit 1 guards.

---

1-3: Missing standard phony targets

checkmake correctly complains about missing all, clean, test. Even stubs improve DX and CI tooling.

.PHONY: all clean test
all: update
clean:
	@rm -rf charts
test:
	@echo "No tests yet"

packages/system/hcloud/charts/README.md (1)

25-27: Minor punctuation & style nits

A few missing commas hamper readability; consider:

-If you're unfamiliar with Helm it would behoove you to peep around the documentation.
+If you're unfamiliar with Helm, it would behoove you to explore the documentation.

-By default the chart will deploy a single replica as a Deployment.
+By default, the chart deploys a single replica as a Deployment.

Also applies to: 55-61

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: lllamnyp
PR: cozystack/cozystack#1025
File: packages/apps/kafka/charts/cozy-lib:1-1
Timestamp: 2025-06-04T06:22:17.306Z
Learning: Files in packages/apps/*/charts/cozy-lib that contain the path "../../../library/cozy-lib" are symbolic links, not regular files. Git diffs may display symlinks as regular files with their target path as content, which can be misleading in code review.

packages/system/hcloud/Chart.yaml (1)

1-2: Second Chart.yaml still present – pick one authoritative chart

A Chart.yaml also exists under packages/system/hcloud/charts/. Two sibling charts in the same package tree confuse helm package, ct, and chart-releaser – they’ll treat them as two distinct charts. Please delete or relocate one of them (e.g. upstream/ vs overlay/) so there is exactly one canonical descriptor.

packages/system/hcloud/charts/templates/clusterrolebinding.yaml (1)

5-5: Colon in metadata.name is illegal for K8s resources

DNS-1123 subdomain names must not contain :. The API server will reject the manifest.

-  name: "system:{{ include "hcloud-cloud-controller-manager.name" . }}"
+  name: "system-{{ include "hcloud-cloud-controller-manager.name" . }}"

packages/system/hcloud/charts/templates/podmonitor.yaml (2)

1-3: API-version probe string still incorrect – PodMonitor never rendered

.Capabilities.APIVersions.Has expects "group/version", not "group/version/Kind".

-{{ if $.Capabilities.APIVersions.Has "monitoring.coreos.com/v1/PodMonitor" }}
+{{ if $.Capabilities.APIVersions.Has "monitoring.coreos.com/v1" }}

---

18-21: Hard-coded selector risks duplicates / invalid YAML

Users can already inject selector via .Values.monitoring.podMonitor.spec.
If they do, this block will duplicate the key; if they don’t and the helper returns no labels, matchLabels: becomes empty – invalid YAML. Wrap it with a guard or remove and document override flow.

packages/system/hcloud/charts/templates/deployment.yaml (2)

68-73: Still missing an explicit command – inconsistent with DaemonSet
Previous feedback not addressed. Add the binary as entrypoint to avoid future image-entrypoint drift.

        - name: hcloud-cloud-controller-manager
+          command:
+            - "/bin/hcloud-cloud-controller-manager"
           args:

---

70-74: Empty-string values produce invalid flags (--foo=)
Same guard used in DaemonSet is still missing here.

-            {{- if not (eq $value nil) }}
+            {{- if and (not (eq $value nil)) (ne $value "") }}

packages/system/hcloud/charts/templates/daemonset.yaml (3)

29-30: dnsPolicy / hostNetwork mismatch
Same critical issue as Deployment – use ClusterFirstWithHostNet when host networking is on.

-      dnsPolicy: Default
+      dnsPolicy: {{ ternary "ClusterFirstWithHostNet" "Default" $.Values.networking.enabled }}

Also applies to: 58-60

---

65-69: Guard against empty strings when rendering flags

-            {{- if not (eq $value nil) }}
+            {{- if and (not (eq $value nil)) (ne $value "") }}

---

91-97: Harden container with securityContext

          image: {{ $.Values.image.repository }}:{{ tpl $.Values.image.tag . }} # x-releaser-pleaser-version
+          securityContext:
+            runAsNonRoot: true
+            runAsUser: 65534
+            allowPrivilegeEscalation: false
+            capabilities:
+              drop: ["ALL"]

packages/system/hcloud/charts/.snapshots/default.yaml (2)

62-70: Add securityContext to silence CKV_K8S_20/23

        - name: hcloud-cloud-controller-manager
+          securityContext:
+            runAsNonRoot: true
+            runAsUser: 65534
+            allowPrivilegeEscalation: false
+            capabilities:
+              drop: ["ALL"]

---

14-18: cluster-admin binding is dangerously over-permissive
Bind to a narrowed ClusterRole mirroring upstream HCCM rules.

-  kind: ClusterRole
-  name: cluster-admin
+  kind: ClusterRole
+  name: hcloud-cloud-controller-manager

packages/system/hcloud/charts/.snapshots/full.daemonset.yaml (3)

44-47: Same DNS / hostNetwork clash as templates – adjust dnsPolicy
Refer to earlier diff in templates.

---

97-105: Missing securityContext hardening

          image: docker.io/hetznercloud/hcloud-cloud-controller-manager:v1.26.0 # x-releaser-pleaser-version
+          securityContext:
+            runAsNonRoot: true
+            runAsUser: 65534
+            allowPrivilegeEscalation: false
+            capabilities:
+              drop: ["ALL"]

---

111-113: Secret name mismatch – env uses hcloud, volume uses hcloud-token

-            secretName: hcloud-token
+            secretName: hcloud

packages/system/hcloud/Makefile (1)

6-10: Add standard phony targets & checksum guard

Static-analysis warns that the customary all, clean, and test phony targets are missing.
Consider:

.PHONY: all update clean test

all: update

clean:
	rm -rf charts

test:
	helm lint charts

Also guard the download with curl --fail --location --silent plus a SHA256 check to avoid supply-chain surprises.

packages/system/hcloud/charts/README.md (2)

25-26: Punctuation nit – add comma after introductory clause

-If you're unfamiliar with Helm it would behoove you to peep around the documentation.
+If you're unfamiliar with Helm, it would behoove you to explore the documentation.

---

55-58: Minor readability tweaks

-By default the chart will deploy a single replica as a Deployment.
+By default, the chart deploys a single replica as a Deployment.

-If you want to change the replica count you can adjust the value `replicaCount` inside the helm values.
+To change the replica count, adjust the `replicaCount` value in `values.yaml`.

packages/system/hcloud/charts/.snapshots/full.daemonset.yaml (1)

65-70: Trailing spaces + empty nodeSelector block
YAML-lint complains; drop the blanks or remove empty map.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: lllamnyp
PR: cozystack/cozystack#1025
File: packages/apps/kafka/charts/cozy-lib:1-1
Timestamp: 2025-06-04T06:22:17.306Z
Learning: Files in packages/apps/*/charts/cozy-lib that contain the path "../../../library/cozy-lib" are symbolic links, not regular files. Git diffs may display symlinks as regular files with their target path as content, which can be misleading in code review.

Learnt from: lllamnyp
PR: cozystack/cozystack#1025
File: packages/apps/kafka/charts/cozy-lib:1-1
Timestamp: 2025-06-04T06:22:17.306Z
Learning: Files in packages/apps/*/charts/cozy-lib that contain the path "../../../library/cozy-lib" are symbolic links, not regular files. Git diffs may display symlinks as regular files with their target path as content, which can be misleading in code review.

packages/system/hcloud/Makefile (1)

6-11: helm repo add is not idempotent – use --force-update or tolerate existing repo

Running make update twice fails because helm repo add hcloud … errors when the repo already exists.
Use --force-update (Helm ≥ 3.11) or guard the command.

-	helm repo add hcloud https://charts.hetzner.cloud
+	helm repo add hcloud https://charts.hetzner.cloud --force-update || true

packages/system/hcloud/Makefile (1)

1-3: Declare phony targets to silence checkmake and avoid name clashes

update (and usually all/clean/test) should be declared as phony to prevent GNU make from looking for files with the same name and to satisfy the checkmake warnings.

+.PHONY: update

packages/system/hcloud/charts/hcloud-cloud-controller-manager/README.md (1)

25-26: Minor punctuation tweaks for readability

A couple of commas are missing after introductory phrases.

-If you're unfamiliar with Helm it would behoove you to peep around the documentation.
+If you're unfamiliar with Helm, it would behoove you to peep around the documentation.

packages/system/hcloud/charts/hcloud-cloud-controller-manager/.snapshots/default.yaml (2)

42-62: Missing taint for uninitialised nodes is fine, but tolerations lack effect: NoSchedule / value consistency

You tolerate node.cloudprovider.kubernetes.io/uninitialized=true:NoSchedule, which is correct, but the subsequent tolerations include "CriticalAddonsOnly" which is deprecated and the old master-node taint. Consider removing "CriticalAddonsOnly" and using only the control-plane taint going forward (K8s ≥1.24).

---

88-88: Image tag is hard-pinned; lacks digest pinning

Pinning to v1.26.0 is good, but add a digest (@sha256:…) to guarantee immutability and avoid surprise re-tags.

packages/system/hcloud/charts/hcloud-cloud-controller-manager/templates/daemonset.yaml (1)

65-69: Template risks emitting --foo= for nil values

if not (eq $value nil) avoids nil, but empty strings still produce --arg= which many binaries treat as set but empty, not unset. Guard for empty strings as well:

{{- if and $value (ne $value "") }}

packages/system/hcloud/charts/hcloud-cloud-controller-manager/.snapshots/full.daemonset.yaml (1)

62-68: Trailing spaces & YAML style

Trailing spaces on lines 64 & 69 make YAML-lint fail. Please strip whitespace to keep the snapshot clean.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: lllamnyp
PR: cozystack/cozystack#1025
File: packages/apps/kafka/charts/cozy-lib:1-1
Timestamp: 2025-06-04T06:22:17.306Z
Learning: Files in packages/apps/*/charts/cozy-lib that contain the path "../../../library/cozy-lib" are symbolic links, not regular files. Git diffs may display symlinks as regular files with their target path as content, which can be misleading in code review.