From 8eaff8846f8f094c5ef20c15cd1630e3e95f265e Mon Sep 17 00:00:00 2001 From: Stavros Kois <47820033+stavros-k@users.noreply.github.com> Date: Sun, 10 Mar 2024 17:53:30 +0200 Subject: [PATCH] chore(enabled): make sure the key always exist (#746) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Description** ⚒️ Fixes #728 **⚙️ Type of change** - [ ] ⚙️ Feature/App addition - [ ] 🪛 Bugfix - [ ] ⚠️ Breaking change (fix or feature that would cause existing functionality to not work as expected) - [x] 🔃 Refactor of current code **🧪 How Has This Been Tested?** **📃 Notes:** **✔️ Checklist:** - [x] ⚖️ My code follows the style guidelines of this project - [x] 👀 I have performed a self-review of my own code - [x] #️⃣ I have commented my code, particularly in hard-to-understand areas - [ ] 📄 I have made corresponding changes to the documentation - [x] ⚠️ My changes generate no new warnings - [x] 🧪 I have added tests to this description that prove my fix is effective or that my feature works - [x] ⬆️ I increased versions for any altered app according to semantic versioning **➕ App addition** If this PR is an app addition please make sure you have done the following. - [ ] 🖼️ I have added an icon in the Chart's root directory called `icon.png` --- _Please don't blindly check all the boxes. Read them and only check those that apply. Those checkboxes are there for the reviewer to see what is this all about and the status of this PR with a quick glance._ --- .../common-test/ci/statefulset-values.yaml | 1 + .../tests/addons/autoperms_test.yaml | 5 +- .../tests/container/volumeMounts_test.yaml | 2 +- .../tests/defaults/enabled_test.yaml | 78 +++++++++++++++++++ .../tests/volumeClaimTemplate/names_test.yaml | 23 +++++- library/common/Chart.yaml | 2 +- .../templates/lib/container/_volumeMounts.tpl | 9 ++- .../templates/lib/pod/_containerSpawner.tpl | 7 +- .../lib/pod/_initContainerSpawner.tpl | 18 ++--- .../common/templates/lib/util/_enabled.tpl | 13 ++-- .../templates/values/lists/_serviceList.tpl | 2 +- 11 files changed, 133 insertions(+), 27 deletions(-) create mode 100644 library/common-test/tests/defaults/enabled_test.yaml diff --git a/library/common-test/ci/statefulset-values.yaml b/library/common-test/ci/statefulset-values.yaml index 7f161ee4..5f2cdfc1 100644 --- a/library/common-test/ci/statefulset-values.yaml +++ b/library/common-test/ci/statefulset-values.yaml @@ -38,6 +38,7 @@ workload: persistence: data: + enabled: true mountPath: /data type: vct accessMode: "ReadWriteOnce" diff --git a/library/common-test/tests/addons/autoperms_test.yaml b/library/common-test/tests/addons/autoperms_test.yaml index 110fdc40..fa80ad62 100644 --- a/library/common-test/tests/addons/autoperms_test.yaml +++ b/library/common-test/tests/addons/autoperms_test.yaml @@ -87,8 +87,9 @@ tests: enabled: true chown: true mountPath: /test9 - # Should not show up, without enabled true + # Should not show up test10: + enabled: false type: hostPath hostPath: /testhost10 autoPermissions: @@ -690,7 +691,7 @@ tests: cpu: 2000m memory: 2Gi -# Failures + # Failures - it: should fail trying to set ownership on non hostPath set: workload: diff --git a/library/common-test/tests/container/volumeMounts_test.yaml b/library/common-test/tests/container/volumeMounts_test.yaml index fb2e9da7..5520f46d 100644 --- a/library/common-test/tests/container/volumeMounts_test.yaml +++ b/library/common-test/tests/container/volumeMounts_test.yaml @@ -554,7 +554,7 @@ tests: contains: path: spec.template.spec.containers[0].volumeMounts content: - name: vct-vol + name: test-release-name-common-test-vct-vol mountPath: /some/path readOnly: false - documentIndex: &otherStatefulSetDoc 1 diff --git a/library/common-test/tests/defaults/enabled_test.yaml b/library/common-test/tests/defaults/enabled_test.yaml new file mode 100644 index 00000000..986633c7 --- /dev/null +++ b/library/common-test/tests/defaults/enabled_test.yaml @@ -0,0 +1,78 @@ +suite: default test +templates: + - common.yaml +chart: + appVersion: &appVer v9.9.9 +release: + name: test-release-name + namespace: test-release-namespace +tests: + - it: should fail without enabled key in workload + set: + workload: + workload-name: + type: Deployment + podSpec: {} + asserts: + - failedTemplate: + errorMessage: Workload - Expected the key [enabled] in [workload.workload-name] to exist + + - it: should fail without enabled key in containers + set: + workload: + workload-name: + enabled: true + primary: true + type: Deployment + podSpec: + containers: + container-name: + probes: + liveness: + enabled: false + readiness: + enabled: false + startup: + enabled: false + asserts: + - failedTemplate: + errorMessage: Container - Expected the key [enabled] in [containers.container-name] to exist + + - it: should fail without enabled key in initContainers + set: + workload: + workload-name: + enabled: true + primary: true + type: Deployment + podSpec: + initContainers: + container-name: + probes: + liveness: + enabled: false + readiness: + enabled: false + startup: + enabled: false + asserts: + - failedTemplate: + errorMessage: Init Container - Expected the key [enabled] in [initContainers.container-name] to exist + + - it: should fail without enabled key in service + set: + service: + service-name: + primary: true + asserts: + - failedTemplate: + errorMessage: Service - Expected the key [enabled] in [service.service-name] to exist + + - it: should fail without enabled key in persistence + set: + persistence: + persistence-name: + primary: true + asserts: + - failedTemplate: + errorMessage: Persistence - Expected the key [enabled] in [persistence.persistence-name] to exist diff --git a/library/common-test/tests/volumeClaimTemplate/names_test.yaml b/library/common-test/tests/volumeClaimTemplate/names_test.yaml index 4029aad3..fc3b4353 100644 --- a/library/common-test/tests/volumeClaimTemplate/names_test.yaml +++ b/library/common-test/tests/volumeClaimTemplate/names_test.yaml @@ -11,15 +11,28 @@ tests: my-volume1: enabled: true type: vct + mountPath: /some/path1 my-volume2: enabled: true type: vct + mountPath: /some/path2 workload: main: enabled: true primary: true type: StatefulSet - podSpec: {} + podSpec: + containers: + main: + enabled: true + primary: true + probes: + liveness: + enabled: false + readiness: + enabled: false + startup: + enabled: false asserts: - documentIndex: &statefulSetDoc 0 isKind: @@ -31,7 +44,15 @@ tests: equal: path: spec.volumeClaimTemplates[0].metadata.name value: test-release-name-common-test-my-volume1 + - documentIndex: *statefulSetDoc + equal: + path: spec.template.spec.containers[0].volumeMounts[1].name + value: test-release-name-common-test-my-volume1 - documentIndex: *statefulSetDoc equal: path: spec.volumeClaimTemplates[1].metadata.name value: test-release-name-common-test-my-volume2 + - documentIndex: *statefulSetDoc + equal: + path: spec.template.spec.containers[0].volumeMounts[2].name + value: test-release-name-common-test-my-volume2 diff --git a/library/common/Chart.yaml b/library/common/Chart.yaml index 08ccf8dc..4977564c 100644 --- a/library/common/Chart.yaml +++ b/library/common/Chart.yaml @@ -15,7 +15,7 @@ maintainers: name: common sources: null type: library -version: 20.0.0 +version: 20.0.1 annotations: artifacthub.io/category: "integration-delivery" artifacthub.io/license: "BUSL-1.1" diff --git a/library/common/templates/lib/container/_volumeMounts.tpl b/library/common/templates/lib/container/_volumeMounts.tpl index dfe5190d..84b3cb6d 100644 --- a/library/common/templates/lib/container/_volumeMounts.tpl +++ b/library/common/templates/lib/container/_volumeMounts.tpl @@ -22,7 +22,9 @@ objectData: The object data to be used to render the container. {{- if and (eq $enabled "true") (not (and (eq $persistenceValues.type "vct") (ne $objectData.podType "StatefulSet"))) -}} {{/* Dont try to mount configmap/sercet/vct to codeserver */}} {{- if not (and (eq $objectData.shortName "codeserver") (mustHas $persistenceValues.type $codeServerIgnoredTypes)) -}} - {{- $volMount := (fromJson (include "tc.v1.common.lib.container.volumeMount.isSelected" (dict "persistenceName" $persistenceName "persistenceValues" $persistenceValues "objectData" $objectData))) -}} + {{- $volMount := (include "tc.v1.common.lib.container.volumeMount.isSelected" (dict + "rootCtx" $rootCtx "persistenceName" $persistenceName "persistenceValues" $persistenceValues "objectData" $objectData + )) | fromJson -}} {{- if $volMount -}} {{- $volMounts = mustAppend $volMounts $volMount -}} {{- end -}} @@ -69,9 +71,14 @@ objectData: The object data to be used to render the container. {{- $persistenceName := .persistenceName -}} {{- $persistenceValues := .persistenceValues -}} {{- $objectData := .objectData -}} + {{- $rootCtx := .rootCtx -}} {{/* Initialize from the default values */}} {{- $volMount := dict -}} + {{- if eq $persistenceValues.type "vct" -}} + {{- $fullname := include "tc.v1.common.lib.chart.names.fullname" $rootCtx -}} + {{- $persistenceName = printf "%s-%s" $fullname $persistenceName -}} + {{- end -}} {{- $_ := set $volMount "name" $persistenceName -}} {{- if eq $persistenceValues.type "device" -}} {{/* On devices use the hostPath as default if mountpath is not defined */}} {{- $_ := set $volMount "mountPath" ($persistenceValues.mountPath | default $persistenceValues.hostPath | default "") -}} diff --git a/library/common/templates/lib/pod/_containerSpawner.tpl b/library/common/templates/lib/pod/_containerSpawner.tpl index 66e3bb42..a1108eab 100644 --- a/library/common/templates/lib/pod/_containerSpawner.tpl +++ b/library/common/templates/lib/pod/_containerSpawner.tpl @@ -11,7 +11,12 @@ objectData: The object data to be used to render the Pod. {{- include "tc.v1.common.lib.container.primaryValidation" (dict "rootCtx" $rootCtx "objectData" $objectData) -}} {{- range $containerName, $containerValues := $objectData.podSpec.containers -}} - {{- if $containerValues.enabled -}} + {{- $enabled := (include "tc.v1.common.lib.util.enabled" (dict + "rootCtx" $rootCtx "objectData" $containerValues + "name" $containerName "caller" "Container" + "key" "containers")) -}} + + {{- if eq $enabled "true" -}} {{- $container := (mustDeepCopy $containerValues) -}} {{- $name := include "tc.v1.common.lib.chart.names.fullname" $rootCtx -}} {{- if not $container.primary -}} diff --git a/library/common/templates/lib/pod/_initContainerSpawner.tpl b/library/common/templates/lib/pod/_initContainerSpawner.tpl index b28f142f..7aa581b4 100644 --- a/library/common/templates/lib/pod/_initContainerSpawner.tpl +++ b/library/common/templates/lib/pod/_initContainerSpawner.tpl @@ -18,20 +18,12 @@ objectData: The object data to be used to render the Pod. {{- $mergedContainers := $objectData.podSpec.initContainers -}} {{- range $containerName, $containerValues := $mergedContainers -}} + {{- $enabled := (include "tc.v1.common.lib.util.enabled" (dict + "rootCtx" $rootCtx "objectData" $containerValues + "name" $containerName "caller" "Init Container" + "key" "initContainers")) -}} - {{- $enabled := $containerValues.enabled -}} - {{- if kindIs "string" $enabled -}} - {{- $enabled = tpl $enabled $rootCtx -}} - - {{/* After tpl it becomes a string, not a bool */}} - {{- if eq $enabled "true" -}} - {{- $enabled = true -}} - {{- else if eq $enabled "false" -}} - {{- $enabled = false -}} - {{- end -}} - {{- end -}} - - {{- if $enabled -}} + {{- if eq $enabled "true" -}} {{- if not ($containerValues.type) -}} {{- fail "InitContainer - Expected non-empty [type]" -}} diff --git a/library/common/templates/lib/util/_enabled.tpl b/library/common/templates/lib/util/_enabled.tpl index ce739cdc..0fb7aeb7 100644 --- a/library/common/templates/lib/util/_enabled.tpl +++ b/library/common/templates/lib/util/_enabled.tpl @@ -6,14 +6,15 @@ {{- $caller := .caller -}} {{- $enabled := false -}} - {{- if hasKey $objectData "enabled" -}} - {{- if not (kindIs "invalid" $objectData.enabled) -}} - {{- $enabled = $objectData.enabled -}} - {{- else -}} - {{- fail (printf "%s - Expected the defined key [enabled] in [%s.%s] to not be empty" $caller $key $name) -}} - {{- end -}} + {{- if not (hasKey $objectData "enabled") -}} + {{- fail (printf "%s - Expected the key [enabled] in [%s.%s] to exist" $caller $key $name) -}} {{- end -}} + {{- if (kindIs "invalid" $objectData.enabled) -}} + {{- fail (printf "%s - Expected the defined key [enabled] in [%s.%s] to not be empty" $caller $key $name) -}} + {{- end -}} + {{- $enabled = $objectData.enabled -}} + {{- if kindIs "string" $enabled -}} {{- $enabled = tpl $enabled $rootCtx -}} {{- if eq $enabled "true" -}} diff --git a/library/common/templates/values/lists/_serviceList.tpl b/library/common/templates/values/lists/_serviceList.tpl index 0bd4d073..9bcb5310 100644 --- a/library/common/templates/values/lists/_serviceList.tpl +++ b/library/common/templates/values/lists/_serviceList.tpl @@ -5,7 +5,7 @@ {{- range $svcName, $svcValues := $rootCtx.Values.service -}} {{- $enabled := (include "tc.v1.common.lib.util.enabled" (dict "rootCtx" $rootCtx "objectData" $svcValues - "name" $svcName "caller" "ServiceList" + "name" $svcName "caller" "Service" "key" "service")) -}} {{- if eq $enabled "true" -}}