From bdf520bf74a7ed17a62bfeaa669ed2eef7b45509 Mon Sep 17 00:00:00 2001 From: Myasnikov Daniil Date: Tue, 16 Jun 2026 14:49:10 +0500 Subject: [PATCH] fix(harbor): preserve jobservice and trivy storageClass on upgrade The cozy-harbor wrapper never passed storageClass to the jobservice jobLog PVC, so it was created without a storageClassName and the default-StorageClass admission stamped the cluster default. Because PVC storageClassName is immutable, this caused rendered-vs-live drift on upgrade. Preserve an existing PVC's class via lookup and fall back to the configured storageClass for fresh installs. The trivy StatefulSet's volumeClaimTemplate had the same latent drift -- .Values.storageClass was forwarded blindly into a VCT that cannot be patched at all -- so it gets the same lookup-preserve treatment, keyed off data--trivy-0. A live object pinned to an explicit "" (no dynamic provisioning) now round-trips through upstream's "-" sentinel instead of collapsing to an omitted storageClassName, and the spec lookup is guarded with `| default dict` so a PVC carrying no storageClassName key stays omitted. Adds a helm-unittest regression suite and wires a test target so make unit-tests stops skipping packages/apps/harbor. Fixes #2368 Assisted-By: Claude Signed-off-by: Myasnikov Daniil --- packages/apps/harbor/templates/harbor.yaml | 53 ++++++++++++++- .../tests/jobservice_storageclass_test.yaml | 64 +++++++++++++++++++ 2 files changed, 114 insertions(+), 3 deletions(-) create mode 100644 packages/apps/harbor/tests/jobservice_storageclass_test.yaml diff --git a/packages/apps/harbor/templates/harbor.yaml b/packages/apps/harbor/templates/harbor.yaml index 72bfa61a4..8cb8b38c0 100644 --- a/packages/apps/harbor/templates/harbor.yaml +++ b/packages/apps/harbor/templates/harbor.yaml @@ -23,6 +23,46 @@ {{- end }} {{- end }} +{{- /* + storageClassName on a PVC is immutable, and a StatefulSet's volumeClaimTemplate + cannot be patched at all. The upstream chart renders both the jobservice PVC and + the trivy VCT without a storageClassName unless one is passed, so on first install + the default-StorageClass admission stamps the cluster default onto the live object. + To avoid an immutable-field rejection on upgrade we keep the rendered value equal + to the live one: preserve an existing object's storageClassName, and only fall back + to the configured storageClass for a fresh install. See issue #2368. + + A live object pinned to an explicit "" (no dynamic provisioning) must round-trip + too: upstream maps the "-" sentinel back to storageClassName: "", so forward "-" + rather than letting "" collapse to an omitted field. An object carrying no + storageClassName key at all keeps the empty fallback so the field stays omitted, + matching the live object. +*/}} +{{- $jobservicePvc := lookup "v1" "PersistentVolumeClaim" .Release.Namespace (printf "%s-jobservice" .Release.Name) }} +{{- $jobserviceStorageClass := .Values.storageClass }} +{{- if $jobservicePvc }} + {{- $jobserviceSpec := $jobservicePvc.spec | default dict }} + {{- if hasKey $jobserviceSpec "storageClassName" }} + {{- $jobserviceStorageClass = $jobserviceSpec.storageClassName | default "-" }} + {{- else }} + {{- $jobserviceStorageClass = "" }} + {{- end }} +{{- end }} + +{{- /* The trivy StatefulSet's VCT is named "data", so its PVCs are + data--trivy-0..N; the class is identical across ordinals, so the + first ordinal is representative. */}} +{{- $trivyPvc := lookup "v1" "PersistentVolumeClaim" .Release.Namespace (printf "data-%s-trivy-0" .Release.Name) }} +{{- $trivyStorageClass := .Values.storageClass }} +{{- if $trivyPvc }} + {{- $trivySpec := $trivyPvc.spec | default dict }} + {{- if hasKey $trivySpec "storageClassName" }} + {{- $trivyStorageClass = $trivySpec.storageClassName | default "-" }} + {{- else }} + {{- $trivyStorageClass = "" }} + {{- end }} +{{- end }} + apiVersion: v1 kind: Secret metadata: @@ -108,13 +148,20 @@ spec: bucket: {{ .Release.Name }}-registry secure: false v4auth: true - {{- if .Values.trivy.enabled }} + {{- if or $jobserviceStorageClass .Values.trivy.enabled }} persistentVolumeClaim: + {{- with $jobserviceStorageClass }} + jobservice: + jobLog: + storageClass: {{ . | quote }} + {{- end }} + {{- if .Values.trivy.enabled }} trivy: size: {{ .Values.trivy.size }} - {{- with .Values.storageClass }} - storageClass: {{ . }} + {{- with $trivyStorageClass }} + storageClass: {{ . | quote }} {{- end }} + {{- end }} {{- end }} portal: resources: {{- include "cozy-lib.resources.defaultingSanitize" (list "t1.nano" (dict) $) | nindent 10 }} diff --git a/packages/apps/harbor/tests/jobservice_storageclass_test.yaml b/packages/apps/harbor/tests/jobservice_storageclass_test.yaml new file mode 100644 index 000000000..e1aac9933 --- /dev/null +++ b/packages/apps/harbor/tests/jobservice_storageclass_test.yaml @@ -0,0 +1,64 @@ +suite: jobservice storageClass propagation + +release: + name: harbor + namespace: tenant-test + +# Regression test for issue #2368: the jobservice PVC must receive the +# configured storageClass so it is created with an explicit storageClassName +# instead of relying on the default-StorageClass admission. Because PVC +# storageClassName is immutable, the template preserves an existing PVC's +# class when one is found; under `helm template`/unittest the cluster lookup +# returns empty, so these cases exercise the fresh-install fallback path. +# +# The rendered HelmRelease (documentIndex 1) carries the values that the +# cozy-harbor chart forwards to the upstream harbor subchart. +# helm-unittest renders every template in the chart, so the platform-injected +# globals consumed by the ingress/httproute/bucket templates must be supplied +# even though only templates/harbor.yaml is asserted on. +set: + _namespace: + host: example.org + ingress: nginx + gateway: "" + seaweedfs: seaweedfs + _cluster: + solver: http01 + +tests: + - it: forwards storageClass to jobservice jobLog on a fresh install + template: templates/harbor.yaml + documentIndex: 1 + set: + storageClass: local-3rep + asserts: + - equal: + path: spec.values.harbor.persistence.persistentVolumeClaim.jobservice.jobLog.storageClass + value: local-3rep + + - it: still forwards storageClass to trivy alongside jobservice + template: templates/harbor.yaml + documentIndex: 1 + set: + storageClass: local-3rep + asserts: + - equal: + path: spec.values.harbor.persistence.persistentVolumeClaim.trivy.storageClass + value: local-3rep + + - it: omits the jobservice block when storageClass is unset + template: templates/harbor.yaml + documentIndex: 1 + asserts: + - notExists: + path: spec.values.harbor.persistence.persistentVolumeClaim.jobservice + + - it: emits no persistentVolumeClaim block when storageClass is unset and trivy is disabled + template: templates/harbor.yaml + documentIndex: 1 + set: + trivy: + enabled: false + asserts: + - notExists: + path: spec.values.harbor.persistence.persistentVolumeClaim