[VC-42222] Fix proxy settings in the Helm chart (#669)

* fix: Only provide one env key in deployment.yaml
* chore: Add two tests for env key and fix failing test for helm chart
* chore: Add actual values of proxy settings from our documentation into existing test
* Run the Helm tests automatically

Signed-off-by: Peter Fiddes <peter.fiddes@jetstack.io>
Signed-off-by: Richard Wall <richard.wall@cyberark.com>
Co-authored-by: Richard Wall <richard.wall@cyberark.com>

Author: hawksight

.github/workflows/tests.yaml

@@ -32,7 +32,7 @@ jobs:
 
       - run: make -j verify
 
-  test-unit:
+  test:
     runs-on: ubuntu-latest
     timeout-minutes: 15
 
@@ -62,4 +62,4 @@ jobs:
           path: _bin/downloaded
           key: downloaded-${{ runner.os }}-${{ hashFiles('klone.yaml') }}-test-unit
 
-      - run: make -j test-unit
+      - run: make -j test-unit test-helm

deploy/charts/venafi-kubernetes-agent/templates/deployment.yaml

@@ -32,8 +32,23 @@ spec:
             {{- toYaml .Values.securityContext | nindent 12 }}
           image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
           imagePullPolicy: {{ .Values.image.pullPolicy }}
-          {{- if or .Values.http_proxy .Values.https_proxy .Values.no_proxy }}
           env:
+          - name: POD_NAMESPACE
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.namespace
+          - name: POD_NAME
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.name
+          - name: POD_UID
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.uid
+          - name: POD_NODE
+            valueFrom:
+              fieldRef:
+                fieldPath: spec.nodeName
           {{- with .Values.http_proxy }}
           - name: HTTP_PROXY
             value: {{ . }}
@@ -46,7 +61,6 @@ spec:
           - name: NO_PROXY
             value: {{ . }}
           {{- end }}
-          {{- end }}
           {{- if not (empty .Values.command) }}
           command:
           {{- range .Values.command }}
@@ -89,23 +103,6 @@ spec:
             {{- with .Values.volumeMounts }}
             {{- toYaml . | nindent 12 }}
             {{- end }}
-          env:
-          - name: POD_NAMESPACE
-            valueFrom:
-              fieldRef:
-                fieldPath: metadata.namespace
-          - name: POD_NAME
-            valueFrom:
-              fieldRef:
-                fieldPath: metadata.name
-          - name: POD_UID
-            valueFrom:
-              fieldRef:
-                fieldPath: metadata.uid
-          - name: POD_NODE
-            valueFrom:
-              fieldRef:
-                fieldPath: spec.nodeName
           {{- if .Values.metrics.enabled }}
           ports:
             - containerPort: 8081

deploy/charts/venafi-kubernetes-agent/tests/deployment_test.yaml

@@ -77,9 +77,6 @@ tests:
 
   # Check the volumes and volumeMounts works correctly
   - it: Volumes and VolumeMounts added correctly
-    set:
-      config.organisation: test_org
-      config.cluster: test_cluster
     values:
       - ./values/custom-volumes.yaml
     asserts:
@@ -102,3 +99,37 @@ tests:
             items:
               - key: cabundle
                 path: ca-certificates.crt
+
+  # Check proxy settings are additive not overriding and set to correct values.
+  # Values from our documentation: https://docs.venafi.cloud/vaas/k8s-components/c-vcp-network-requirements/#modifying-network-settings-for-kubernetes
+  - it: All environment variables present when all proxy settings are supplied
+    set:
+      http_proxy: "http://<proxy server>:<port>"
+      https_proxy: "https://<proxy server>:<port>"
+      no_proxy: "127.0.0.1,localhost,kubernetes.default.svc,kubernetes.default.svc.cluster.local"
+    template: deployment.yaml
+    asserts:
+      - isKind:
+          of: Deployment
+      - lengthEqual :
+          path: spec.template.spec.containers[0].env
+          count: 7
+      - equal:
+          path: spec.template.spec.containers[0].env[?(@.name == "NO_PROXY")].value
+          value: "127.0.0.1,localhost,kubernetes.default.svc,kubernetes.default.svc.cluster.local"
+      - equal:
+          path: spec.template.spec.containers[0].env[?(@.name == "HTTPS_PROXY")].value
+          value: "https://<proxy server>:<port>"
+      - equal:
+          path: spec.template.spec.containers[0].env[?(@.name == "HTTP_PROXY")].value
+          value: "http://<proxy server>:<port>"
+
+  # Check no proxy settings are set when no proxy settings are provided
+  - it: Only default environment variables are set when no proxy settings are provided
+    template: deployment.yaml
+    asserts:
+      - isKind:
+          of: Deployment
+      - lengthEqual :
+          path: spec.template.spec.containers[0].env
+          count: 4

make/00_mod.mk

@@ -53,3 +53,5 @@ endef
 
 golangci_lint_config := .golangci.yaml
 go_header_file := /dev/null
+
+include make/extra_tools.mk

make/02_mod.mk

@@ -52,3 +52,15 @@ shared_generate_targets += generate-crds-venconn
 ## @category Testing
 test-e2e-gke:
 	./hack/e2e/test.sh
+
+.PHONY: test-helm
+## Run `helm unittest`.
+## @category Testing
+test-helm: | $(NEEDS_HELM-UNITTEST)
+	$(HELM-UNITTEST) ./deploy/charts/venafi-kubernetes-agent/
+
+.PHONY: test-helm-snapshot
+## Update the `helm unittest` snapshots.
+## @category Testing
+test-helm-snapshot: | $(NEEDS_HELM-UNITTEST)
+	$(HELM-UNITTEST) ./deploy/charts/venafi-kubernetes-agent/ -u

make/extra_tools.mk

@@ -0,0 +1,6 @@
+ADDITIONAL_TOOLS :=
+ADDITIONAL_GO_DEPENDENCIES :=
+
+# https://pkg.go.dev/github.com/helm-unittest/helm-unittest?tab=versions
+ADDITIONAL_TOOLS += helm-unittest=v0.8.2
+ADDITIONAL_GO_DEPENDENCIES += helm-unittest=github.com/helm-unittest/helm-unittest/cmd/helm-unittest