feat: make proxy settings configurable (#186)

* Add coderd proxy settings for outbound connections
* Move utility functions into separate file
* Extract LoadChart out of TestDeployment tests to avoid repeatedly
  loading the chart for parallel tests
* Refactor Render function to accept a function closure, as with
  MustRender, to make the function signatures more consistent
* Add utility function to convert hard-coded environment variables
  into a map

Author: Jonathan Yu

README.md

@@ -25,7 +25,7 @@ View [our docs](https://coder.com/docs/setup/installation) for detailed installa
 | certs | object | Certificate that will be mounted inside Coder services. | `{"secret":{"key":"","name":""}}` |
 | certs.secret.key | string | Key pointing to a certificate in the secret. | `""` |
 | certs.secret.name | string | Name of the secret. | `""` |
-| coderd | object | Primary service responsible for all things Coder! | `{"affinity":{"podAntiAffinity":{"preferredDuringSchedulingIgnoredDuringExecution":[{"podAffinityTerm":{"labelSelector":{"matchExpressions":[{"key":"app.kubernetes.io/name","operator":"In","values":["coderd"]}]},"topologyKey":"kubernetes.io/hostname"},"weight":1}]}},"builtinProviderServiceAccount":{"annotations":{},"labels":{}},"devurlsHost":"","extraLabels":{},"image":"","oidc":{"enableRefresh":false,"redirectOptions":{}},"podSecurityContext":{"runAsGroup":1000,"runAsNonRoot":true,"runAsUser":1000,"seccompProfile":{"type":"RuntimeDefault"}},"replicas":1,"resources":{"limits":{"cpu":"250m","memory":"512Mi"},"requests":{"cpu":"250m","memory":"512Mi"}},"satellite":{"accessURL":"","enable":false,"primaryURL":""},"securityContext":{"allowPrivilegeEscalation":false,"readOnlyRootFilesystem":true,"runAsGroup":1000,"runAsNonRoot":true,"runAsUser":1000,"seccompProfile":{"type":"RuntimeDefault"}},"serviceAnnotations":{},"serviceNodePorts":{"http":null,"https":null},"serviceSpec":{"externalTrafficPolicy":"Local","loadBalancerIP":"","loadBalancerSourceRanges":[],"type":"LoadBalancer"},"superAdmin":{"passwordSecret":{"key":"password","name":""}},"tls":{"devurlsHostSecretName":"","hostSecretName":""},"trustProxyIP":false}` |
+| coderd | object | Primary service responsible for all things Coder! | `{"affinity":{"podAntiAffinity":{"preferredDuringSchedulingIgnoredDuringExecution":[{"podAffinityTerm":{"labelSelector":{"matchExpressions":[{"key":"app.kubernetes.io/name","operator":"In","values":["coderd"]}]},"topologyKey":"kubernetes.io/hostname"},"weight":1}]}},"builtinProviderServiceAccount":{"annotations":{},"labels":{}},"devurlsHost":"","extraLabels":{},"image":"","oidc":{"enableRefresh":false,"redirectOptions":{}},"podSecurityContext":{"runAsGroup":1000,"runAsNonRoot":true,"runAsUser":1000,"seccompProfile":{"type":"RuntimeDefault"}},"proxy":{"exempt":"cluster.local","http":"","https":""},"replicas":1,"resources":{"limits":{"cpu":"250m","memory":"512Mi"},"requests":{"cpu":"250m","memory":"512Mi"}},"satellite":{"accessURL":"","enable":false,"primaryURL":""},"securityContext":{"allowPrivilegeEscalation":false,"readOnlyRootFilesystem":true,"runAsGroup":1000,"runAsNonRoot":true,"runAsUser":1000,"seccompProfile":{"type":"RuntimeDefault"}},"serviceAnnotations":{},"serviceNodePorts":{"http":null,"https":null},"serviceSpec":{"externalTrafficPolicy":"Local","loadBalancerIP":"","loadBalancerSourceRanges":[],"type":"LoadBalancer"},"superAdmin":{"passwordSecret":{"key":"password","name":""}},"tls":{"devurlsHostSecretName":"","hostSecretName":""},"trustProxyIP":false}` |
 | coderd.affinity | object | Allows specifying an affinity rule for the `coderd` deployment. The default rule prefers to schedule coderd pods on different nodes, which is only applicable if coderd.replicas is greater than 1. | `{"podAntiAffinity":{"preferredDuringSchedulingIgnoredDuringExecution":[{"podAffinityTerm":{"labelSelector":{"matchExpressions":[{"key":"app.kubernetes.io/name","operator":"In","values":["coderd"]}]},"topologyKey":"kubernetes.io/hostname"},"weight":1}]}}` |
 | coderd.builtinProviderServiceAccount | object | Customize the built-in Kubernetes provider service account. | `{"annotations":{},"labels":{}}` |
 | coderd.builtinProviderServiceAccount.annotations | object | A KV mapping of annotations. See: https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/ | `{}` |
@@ -38,6 +38,10 @@ View [our docs](https://coder.com/docs/setup/installation) for detailed installa
 | coderd.podSecurityContext.runAsNonRoot | bool | Requires that containers in the pod run as an unprivileged user. If setting runAsUser to 0 (root), this will need to be set to false. | `true` |
 | coderd.podSecurityContext.runAsUser | int | Sets the user id of the pod. For security reasons, we recommend using a non-root user. | `1000` |
 | coderd.podSecurityContext.seccompProfile | object | Sets the seccomp profile for the pod. If set, the container security context setting will take precedence over this value. | `{"type":"RuntimeDefault"}` |
+| coderd.proxy | object | Whether Coder should initiate outbound connections using a proxy. | `{"exempt":"cluster.local","http":"","https":""}` |
+| coderd.proxy.exempt | string | Bypass the configured proxy rules for this comma-delimited list of hosts or prefixes. This corresponds to the no_proxy environment variable. | `"cluster.local"` |
+| coderd.proxy.http | string | Proxy to use for HTTP connections. If unset, coderd will initiate HTTP connections directly. This corresponds to the http_proxy environment variable. | `""` |
+| coderd.proxy.https | string | Proxy to use for HTTPS connections. If this is not set, coderd will use the HTTP proxy (if set), otherwise it will initiate HTTPS connections directly. This corresponds to the https_proxy environment variable. | `""` |
 | coderd.replicas | int | The number of Kubernetes Pod replicas. | `1` |
 | coderd.resources | object | Kubernetes resource specification for coderd pods. To unset a value, set it to "". To unset all values, set resources to nil. | `{"limits":{"cpu":"250m","memory":"512Mi"},"requests":{"cpu":"250m","memory":"512Mi"}}` |
 | coderd.satellite | object | Deploy a satellite to geodistribute access to workspaces for lower latency. | `{"accessURL":"","enable":false,"primaryURL":""}` |

templates/coderd.yaml

@@ -71,6 +71,12 @@ spec:
               value: {{ .Values.coderd.devurlsHost | quote }}
             - name: VERBOSE
               value: "true"
+            - name: http_proxy
+              value: {{ .Values.coderd.proxy.http | quote }}
+            - name: https_proxy
+              value: {{ .Values.coderd.proxy.https | quote }}
+            - name: no_proxy
+              value: {{ .Values.coderd.proxy.exempt | quote }}
             {{- include "coder.postgres.env" . | indent 12 }}
             {{- include "coder.workspaces.configMapEnv" . | indent 12 }}
           command:
@@ -158,13 +164,19 @@ spec:
             {{- end }}
             - name: OIDC_REDIRECT_OPTIONS
               value: {{ toJson .Values.coderd.oidc.redirectOptions | quote }}
-            {{- if ne .Values.coderd.superAdmin.passwordSecret.name "" }}
+            {{- if .Values.coderd.superAdmin.passwordSecret.name }}
             - name: SUPER_ADMIN_PASSWORD
               valueFrom:
                 secretKeyRef:
                   name: {{ .Values.coderd.superAdmin.passwordSecret.name | quote }}
                   key: {{ .Values.coderd.superAdmin.passwordSecret.key | quote }}
             {{- end }}
+            - name: http_proxy
+              value: {{ .Values.coderd.proxy.http | quote }}
+            - name: https_proxy
+              value: {{ .Values.coderd.proxy.https | quote }}
+            - name: no_proxy
+              value: {{ .Values.coderd.proxy.exempt | quote }}
             {{- include "coder.workspaces.configMapEnv" . | indent 12 }}
             {{- include "coder.postgres.env" . | indent 12 }}
           command:

tests/defaults_test.go

@@ -41,7 +41,7 @@ func TestNamespace(t *testing.T) {
 			t.Parallel()
 
 			// Render the chart with default values
-			objs, err := chart.Render(chart.OriginalValues, &opts, nil)
+			objs, err := chart.Render(nil, &opts, nil)
 			require.NoError(t, err, "chart render failed")
 
 			// Verify that all objects are using the supplied namespace

tests/deployment_test.go

@@ -9,6 +9,8 @@ import (
 func TestDeployment(t *testing.T) {
 	t.Parallel()
 
+	chart := LoadChart(t)
+
 	t.Run("Labels", func(t *testing.T) {
 		var (
 			expectedLabels = map[string]string{
@@ -21,7 +23,7 @@ func TestDeployment(t *testing.T) {
 				"foo": "bar",
 			}
 
-			objs = LoadChart(t).MustRender(t, func(cv *CoderValues) {
+			objs = chart.MustRender(t, func(cv *CoderValues) {
 				cv.Coderd.ExtraLabels = extraLabels
 			})
 			coderd = MustFindDeployment(t, objs, "coderd")

tests/ingress_test.go

@@ -106,11 +106,7 @@ func TestIngress(t *testing.T) {
 			copier.Copy(values, chart.OriginalValues)
 
 			// Run function to perform test-specific modifications of defaults
-			test.ValuesFunc(values)
-
-			// Verify the results using AssertFunc
-			objs, err := chart.Render(values, nil, nil)
-			require.NoError(t, err, "chart render failed")
+			objs := chart.MustRender(t, test.ValuesFunc)
 
 			var found bool
 			for _, obj := range objs {

tests/proxy_test.go

@@ -0,0 +1,72 @@
+package tests
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/utils/pointer"
+)
+
+func TestProxy(t *testing.T) {
+	t.Parallel()
+
+	chart := LoadChart(t)
+
+	tests := []struct {
+		Name       string
+		ValuesFunc func(v *CoderValues)
+		AssertFunc func(t testing.TB, spec *corev1.PodSpec)
+	}{
+		{
+			Name:       "default",
+			ValuesFunc: nil,
+			AssertFunc: func(t testing.TB, spec *corev1.PodSpec) {
+				require.Len(t, spec.Containers, 1, "pod spec should have 1 container")
+				vars := EnvVarsAsMap(spec.Containers[0].Env)
+				require.Empty(t, vars["https_proxy"], "https_proxy should be empty")
+				require.Empty(t, vars["http_proxy"], "http_proxy should be empty")
+				require.Equal(t, "cluster.local", vars["no_proxy"], "no_proxy did not match")
+
+				require.Len(t, spec.InitContainers, 1, "pod spec should have 1 init container")
+				vars = EnvVarsAsMap(spec.InitContainers[0].Env)
+				require.Empty(t, vars["https_proxy"], "https_proxy should be empty")
+				require.Empty(t, vars["http_proxy"], "http_proxy should be empty")
+				require.Equal(t, "cluster.local", vars["no_proxy"], "no_proxy did not match")
+			},
+		},
+		{
+			Name: "all_proxy",
+			ValuesFunc: func(v *CoderValues) {
+				v.Coderd.Proxy.HTTPS = pointer.String("http://proxy.coder.com:3128")
+				v.Coderd.Proxy.HTTP = pointer.String("https://proxy.coder.com:8888")
+				v.Coderd.Proxy.Exempt = pointer.String("coder.com,coder.app")
+			},
+			AssertFunc: func(t testing.TB, spec *corev1.PodSpec) {
+				require.Len(t, spec.Containers, 1, "pod spec should have 1 container")
+				vars := EnvVarsAsMap(spec.Containers[0].Env)
+				require.Equal(t, "http://proxy.coder.com:3128", vars["https_proxy"], "http_proxy did not match")
+				require.Equal(t, "https://proxy.coder.com:8888", vars["http_proxy"], "https_proxy did not match")
+				require.Equal(t, "coder.com,coder.app", vars["no_proxy"], "no_proxy did not match")
+
+				require.Len(t, spec.InitContainers, 1, "pod spec should have 1 init container")
+				vars = EnvVarsAsMap(spec.InitContainers[0].Env)
+				require.Equal(t, "http://proxy.coder.com:3128", vars["https_proxy"], "http_proxy did not match")
+				require.Equal(t, "https://proxy.coder.com:8888", vars["http_proxy"], "https_proxy did not match")
+				require.Equal(t, "coder.com,coder.app", vars["no_proxy"], "no_proxy did not match")
+			},
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+
+		t.Run(test.Name, func(t *testing.T) {
+			t.Parallel()
+
+			objs := chart.MustRender(t, test.ValuesFunc)
+			deployment := MustFindDeployment(t, objs, "coderd")
+			test.AssertFunc(t, &deployment.Spec.Template.Spec)
+		})
+	}
+}

tests/utils.go

@@ -0,0 +1,90 @@
+package tests
+
+import (
+	"testing"
+
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+// MustFindDeployment finds a deployment in the given slice of objects with the
+// given name, or fails the test.
+func MustFindDeployment(t testing.TB, objs []runtime.Object, name string) *appsv1.Deployment {
+	names := []string{}
+	for _, obj := range objs {
+		if deployment, ok := obj.(*appsv1.Deployment); ok {
+			if deployment.Name == name {
+				return deployment
+			}
+			names = append(names, deployment.Name)
+		}
+	}
+
+	t.Fatalf("failed to find deployment %q, found %v", name, names)
+	return nil
+}
+
+// EnvVarsAsMap converts simple key/value environment variable pairs into a
+// map, ignoring variables using a ConfigMap or Secret source. If a variable
+// is defined multiple times, the last value will be returned.
+func EnvVarsAsMap(variables []corev1.EnvVar) map[string]string {
+	values := map[string]string{}
+
+	for _, v := range variables {
+		if v.ValueFrom != nil {
+			continue
+		}
+
+		values[v.Name] = v.Value
+	}
+
+	return values
+}
+
+// AssertVolume asserts that a volume exists of the given name in the given
+// slice of volumes. If it exists, it also runs fn against the named volume.
+func AssertVolume(t testing.TB, vols []corev1.Volume, name string, fn func(t testing.TB, v corev1.Volume)) {
+	names := []string{}
+	for _, v := range vols {
+		if v.Name == name {
+			fn(t, v)
+			return
+		}
+		names = append(names, v.Name)
+	}
+
+	t.Fatalf("failed to find volume %q, found %v", name, names)
+}
+
+// AssertVolumeMount asserts that a volume mount exists of the given name in the
+// given slice of volume mounts. If it exists, it also runs fn against the named
+// volume mount.
+func AssertVolumeMount(t testing.TB, vols []corev1.VolumeMount, name string, fn func(t testing.TB, v corev1.VolumeMount)) {
+	names := []string{}
+	for _, v := range vols {
+		if v.Name == name {
+			fn(t, v)
+			return
+		}
+		names = append(names, v.Name)
+	}
+
+	t.Fatalf("failed to find volume mount %q, found %v", name, names)
+}
+
+// AssertContainer asserts that a container exists of the given name in the
+// given slice of containers. If it exists, it also runs fn against the named
+// container.
+func AssertContainer(t testing.TB, cnts []corev1.Container, name string, fn func(t testing.TB, v corev1.Container)) {
+	names := []string{}
+	for _, c := range cnts {
+		if c.Name == name {
+			fn(t, c)
+			return
+		}
+		names = append(names, c.Name)
+	}
+
+	t.Fatalf("failed to find container %q, found %v", name, names)
+}