Merge pull request #132 from snyk/fix/no-proxy-protocol

fix: correctly specify NO_PROXY without protocol [HYB-593]
snyk · Aug 28, 2024 · 2e16f97 · 2e16f97
2 parents 0492126 + b5e2926
commit 2e16f97
Show file tree

Hide file tree

Showing 7 changed files with 125 additions and 4 deletions.
diff --git a/charts/snyk-broker/Chart.yaml b/charts/snyk-broker/Chart.yaml
@@ -1,5 +1,5 @@
 apiVersion: v2
 name: snyk-broker
-version: 2.7.4
+version: 2.7.5
 description: A Helm chart for Kubernetes
 type: application
diff --git a/charts/snyk-broker/templates/_helpers.tpl b/charts/snyk-broker/templates/_helpers.tpl
@@ -155,3 +155,22 @@ true
 true
 {{- end }}
 {{- end }}
+
+{{/*
+NoProxy helper
+Ensure all values are trimmed, separated by comma, and do not contain protocol or port
+Validate against RFC 1123
+*/}}
+{{- define "snyk-broker.noProxy" -}}
+{{- $proxyUrls := .Values.noProxy | nospace -}}
+{{- $proxyUrlsWithoutProtocol := mustRegexReplaceAll "http(s?)://" $proxyUrls "" -}}
+{{- $sanitisedProxyUrls := "" -}}
+{{- range $proxyUrlsWithoutProtocol | split "," -}}
+{{- if ( mustRegexMatch "^[a-zA-Z0-9.-]+$" . ) -}}
+{{- $sanitisedProxyUrls = printf "%s,%s" $sanitisedProxyUrls . -}}
+{{- else }}
+{{- fail (printf "Entry %s for .Values.noProxy is invalid. Specify hostname only (no schema or port)" . ) -}}
+{{- end }}
+{{- end }}
+{{- $sanitisedProxyUrls |  trimPrefix "," -}}
+{{- end }}
diff --git a/charts/snyk-broker/templates/broker_deployment.yaml b/charts/snyk-broker/templates/broker_deployment.yaml
@@ -450,7 +450,7 @@ spec:
          {{- if .Values.noProxy }}
          # No Proxy Settings
             - name: NO_PROXY
-              value: {{ .Values.noProxy }}
+              value: {{ include "snyk-broker.noProxy" . }}
          {{- end }}
 
          {{- if (include "snyk-broker.acceptJson" .)}}

diff --git a/charts/snyk-broker/templates/code_agent_deployment.yaml b/charts/snyk-broker/templates/code_agent_deployment.yaml
@@ -72,7 +72,7 @@ spec:
          {{- if .Values.noProxy }}
          # No Proxy Settings
             - name: NO_PROXY
-              value: {{ .Values.noProxy }}
+              value: {{ include "snyk-broker.noProxy" . }}
          {{- end }}
          {{- range .Values.env }}
          # custom env var in override.yaml

diff --git a/charts/snyk-broker/tests/broker_deployment_proxy_test.yaml b/charts/snyk-broker/tests/broker_deployment_proxy_test.yaml
@@ -0,0 +1,99 @@
+# yaml-language-server: $schema=https://raw.githubusercontent.com/helm-unittest/helm-unittest/main/schema/helm-testsuite.json
+suite: test broker proxy configuration
+chart:
+  version: 0.0.0
+templates:
+  - broker_deployment.yaml
+values:
+  - ./fixtures/default_values.yaml
+
+tests:
+  - it: sets an https proxy
+    set:
+      httpsProxy: &proxy http://my.proxy:8080
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: HTTPS_PROXY
+            value: *proxy
+  - it: sets an http proxy
+    set:
+      httpProxy: *proxy
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: HTTP_PROXY
+            value: *proxy
+  - it: sets both https and http proxy
+    set:
+      httpProxy: *proxy
+      httpsProxy: *proxy
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: HTTP_PROXY
+            value: *proxy
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: HTTPS_PROXY
+            value: *proxy
+  - it: rejects proxy without protocol
+    set:
+      httpsProxy: no.protocol.proxy:8080
+    asserts:
+      - failedTemplate: {}
+  - it: sets noproxy without protocol
+    set:
+      noProxy: my.ghe.io
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: NO_PROXY
+            value: my.ghe.io
+  - it: corrects noproxy by removing protocol
+    set:
+      noProxy: https://my.ghe.io
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: NO_PROXY
+            value: my.ghe.io
+  - it: sets noproxy with multiple domains
+    set:
+      noProxy: my.ghe.io,my.other.host.tld
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: NO_PROXY
+            value: my.ghe.io,my.other.host.tld
+  - it: corrects noproxy with multiple domains, one with protocol
+    set:
+      noProxy: my.ghe.io,https://my.private.site
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: NO_PROXY
+            value: my.ghe.io,my.private.site
+  - it: corrects noproxy with multiple domains, one with protocol, with spaces
+    set:
+      noProxy: my.ghe.io, https://my.private.site
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: NO_PROXY
+            value: my.ghe.io,my.private.site
+  - it: rejects noproxy with multiple domains, one with protocol, one with a port
+    set:
+      noProxy: my.ghe.io, https://my.private.site,notadomain:12334
+    asserts:
+      - failedTemplate:
+          errorMessage: Entry notadomain:12334 for .Values.noProxy is invalid. Specify hostname only (no schema or port)
diff --git a/charts/snyk-broker/values.schema.json b/charts/snyk-broker/values.schema.json
@@ -290,7 +290,7 @@
       "$ref": "#/$defs/urlWithSchema"
     },
     "noProxy": {
-      "$ref": "#/$defs/urlWithSchema"
+      "type": "string"
     },
     "acceptJson":{
       "type": "string"

diff --git a/charts/snyk-broker/values.yaml b/charts/snyk-broker/values.yaml
@@ -250,6 +250,9 @@ httpProxy: ""
 httpsProxy: ""
 
 # No Proxy URL - This will apply to both Snyk Broker and Snyk Code Agent
+# Do not specify protocol (http(s)://) or port
+# Separate multiple entries by a comma
+# e.g. my.first.host,my.second.host
 noProxy: ""
 
 # For custom accept.json, specify the path to the accept.json using the --set-file command when installing the chart