Fix #2809 : replace TLS dockerd connection with unix socket (#2833)

Co-authored-by: Bassem Dghaidi <[email protected]>
actions · Sep 22, 2023 · 16666e1 · 16666e1
1 parent 2ae3982
commit 16666e1
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 54 deletions.
diff --git a/charts/gha-runner-scale-set/templates/_helpers.tpl b/charts/gha-runner-scale-set/templates/_helpers.tpl
@@ -93,19 +93,26 @@ volumeMounts:
 
 {{- define "gha-runner-scale-set.dind-container" -}}
 image: docker:dind
+args:
+  - dockerd
+  - --host=unix:///run/docker/docker.sock
+  - --group=$(DOCKER_GROUP_GID)
+env:
+  - name: DOCKER_GROUP_GID
+    value: "123"
 securityContext:
   privileged: true
 volumeMounts:
   - name: work
     mountPath: /home/runner/_work
-  - name: dind-cert
-    mountPath: /certs/client
+  - name: dind-sock
+    mountPath: /run/docker
   - name: dind-externals
     mountPath: /home/runner/externals
 {{- end }}
 
 {{- define "gha-runner-scale-set.dind-volume" -}}
-- name: dind-cert
+- name: dind-sock
   emptyDir: {}
 - name: dind-externals
   emptyDir: {}
@@ -185,8 +192,6 @@ volumeMounts:
       {{- end }}
     {{- end }}
     {{- $setDockerHost := 1 }}
-    {{- $setDockerTlsVerify := 1 }}
-    {{- $setDockerCertPath := 1 }}
     {{- $setRunnerWaitDocker := 1 }}
     {{- $setNodeExtraCaCerts := 0 }}
     {{- $setRunnerUpdateCaCerts := 0 }}
@@ -200,12 +205,6 @@ env:
         {{- if eq $env.name "DOCKER_HOST" }}
           {{- $setDockerHost = 0 }}
         {{- end }}
-        {{- if eq $env.name "DOCKER_TLS_VERIFY" }}
-          {{- $setDockerTlsVerify = 0 }}
-        {{- end }}
-        {{- if eq $env.name "DOCKER_CERT_PATH" }}
-          {{- $setDockerCertPath = 0 }}
-        {{- end }}
         {{- if eq $env.name "RUNNER_WAIT_FOR_DOCKER_IN_SECONDS" }}
           {{- $setRunnerWaitDocker = 0 }}
         {{- end }}
@@ -220,15 +219,7 @@ env:
     {{- end }}
     {{- if $setDockerHost }}
   - name: DOCKER_HOST
-    value: tcp://localhost:2376
-    {{- end }}
-    {{- if $setDockerTlsVerify }}
-  - name: DOCKER_TLS_VERIFY
-    value: "1"
-    {{- end }}
-    {{- if $setDockerCertPath }}
-  - name: DOCKER_CERT_PATH
-    value: /certs/client
+    value: unix:///run/docker/docker.sock
     {{- end }}
     {{- if $setRunnerWaitDocker }}
   - name: RUNNER_WAIT_FOR_DOCKER_IN_SECONDS
@@ -254,7 +245,7 @@ volumeMounts:
         {{- if eq $volMount.name "work" }}
           {{- $mountWork = 0 }}
         {{- end }}
-        {{- if eq $volMount.name "dind-cert" }}
+        {{- if eq $volMount.name "dind-sock" }}
           {{- $mountDindCert = 0 }}
         {{- end }}
         {{- if eq $volMount.name "github-server-tls-cert" }}
@@ -268,8 +259,8 @@ volumeMounts:
     mountPath: /home/runner/_work
     {{- end }}
     {{- if $mountDindCert }}
-  - name: dind-cert
-    mountPath: /certs/client
+  - name: dind-sock
+    mountPath: /run/docker
     readOnly: true
     {{- end }}
     {{- if $mountGitHubServerTLS }}

diff --git a/charts/gha-runner-scale-set/tests/template_test.go b/charts/gha-runner-scale-set/tests/template_test.go
@@ -767,7 +767,7 @@ func TestTemplateRenderedAutoScalingRunnerSet_DinD_ExtraVolumes(t *testing.T) {
 	helm.UnmarshalK8SYaml(t, output, &ars)
 
 	assert.Len(t, ars.Spec.Template.Spec.Volumes, 5, "Volumes should be 5")
-	assert.Equal(t, "dind-cert", ars.Spec.Template.Spec.Volumes[0].Name, "Volume name should be dind-cert")
+	assert.Equal(t, "dind-sock", ars.Spec.Template.Spec.Volumes[0].Name, "Volume name should be dind-sock")
 	assert.Equal(t, "dind-externals", ars.Spec.Template.Spec.Volumes[1].Name, "Volume name should be dind-externals")
 	assert.Equal(t, "work", ars.Spec.Template.Spec.Volumes[2].Name, "Volume name should be work")
 	assert.Equal(t, "/data", ars.Spec.Template.Spec.Volumes[2].HostPath.Path, "Volume host path should be /data")
@@ -863,40 +863,36 @@ func TestTemplateRenderedAutoScalingRunnerSet_EnableDinD(t *testing.T) {
 	assert.Len(t, ars.Spec.Template.Spec.Containers, 2, "Template.Spec should have 2 container")
 	assert.Equal(t, "runner", ars.Spec.Template.Spec.Containers[0].Name)
 	assert.Equal(t, "ghcr.io/actions/actions-runner:latest", ars.Spec.Template.Spec.Containers[0].Image)
-	assert.Len(t, ars.Spec.Template.Spec.Containers[0].Env, 4, "The runner container should have 4 env vars, DOCKER_HOST, DOCKER_TLS_VERIFY, DOCKER_CERT_PATH and RUNNER_WAIT_FOR_DOCKER_IN_SECONDS")
+	assert.Len(t, ars.Spec.Template.Spec.Containers[0].Env, 2, "The runner container should have 2 env vars, DOCKER_HOST and RUNNER_WAIT_FOR_DOCKER_IN_SECONDS")
 	assert.Equal(t, "DOCKER_HOST", ars.Spec.Template.Spec.Containers[0].Env[0].Name)
-	assert.Equal(t, "tcp://localhost:2376", ars.Spec.Template.Spec.Containers[0].Env[0].Value)
-	assert.Equal(t, "DOCKER_TLS_VERIFY", ars.Spec.Template.Spec.Containers[0].Env[1].Name)
-	assert.Equal(t, "1", ars.Spec.Template.Spec.Containers[0].Env[1].Value)
-	assert.Equal(t, "DOCKER_CERT_PATH", ars.Spec.Template.Spec.Containers[0].Env[2].Name)
-	assert.Equal(t, "/certs/client", ars.Spec.Template.Spec.Containers[0].Env[2].Value)
-	assert.Equal(t, "RUNNER_WAIT_FOR_DOCKER_IN_SECONDS", ars.Spec.Template.Spec.Containers[0].Env[3].Name)
-	assert.Equal(t, "120", ars.Spec.Template.Spec.Containers[0].Env[3].Value)
-
-	assert.Len(t, ars.Spec.Template.Spec.Containers[0].VolumeMounts, 2, "The runner container should have 2 volume mounts, dind-cert and work")
+	assert.Equal(t, "unix:///run/docker/docker.sock", ars.Spec.Template.Spec.Containers[0].Env[0].Value)
+	assert.Equal(t, "RUNNER_WAIT_FOR_DOCKER_IN_SECONDS", ars.Spec.Template.Spec.Containers[0].Env[1].Name)
+	assert.Equal(t, "120", ars.Spec.Template.Spec.Containers[0].Env[1].Value)
+
+	assert.Len(t, ars.Spec.Template.Spec.Containers[0].VolumeMounts, 2, "The runner container should have 2 volume mounts, dind-sock and work")
 	assert.Equal(t, "work", ars.Spec.Template.Spec.Containers[0].VolumeMounts[0].Name)
 	assert.Equal(t, "/home/runner/_work", ars.Spec.Template.Spec.Containers[0].VolumeMounts[0].MountPath)
 	assert.False(t, ars.Spec.Template.Spec.Containers[0].VolumeMounts[0].ReadOnly)
 
-	assert.Equal(t, "dind-cert", ars.Spec.Template.Spec.Containers[0].VolumeMounts[1].Name)
-	assert.Equal(t, "/certs/client", ars.Spec.Template.Spec.Containers[0].VolumeMounts[1].MountPath)
+	assert.Equal(t, "dind-sock", ars.Spec.Template.Spec.Containers[0].VolumeMounts[1].Name)
+	assert.Equal(t, "/run/docker", ars.Spec.Template.Spec.Containers[0].VolumeMounts[1].MountPath)
 	assert.True(t, ars.Spec.Template.Spec.Containers[0].VolumeMounts[1].ReadOnly)
 
 	assert.Equal(t, "dind", ars.Spec.Template.Spec.Containers[1].Name)
 	assert.Equal(t, "docker:dind", ars.Spec.Template.Spec.Containers[1].Image)
 	assert.True(t, *ars.Spec.Template.Spec.Containers[1].SecurityContext.Privileged)
-	assert.Len(t, ars.Spec.Template.Spec.Containers[1].VolumeMounts, 3, "The dind container should have 3 volume mounts, dind-cert, work and externals")
+	assert.Len(t, ars.Spec.Template.Spec.Containers[1].VolumeMounts, 3, "The dind container should have 3 volume mounts, dind-sock, work and externals")
 	assert.Equal(t, "work", ars.Spec.Template.Spec.Containers[1].VolumeMounts[0].Name)
 	assert.Equal(t, "/home/runner/_work", ars.Spec.Template.Spec.Containers[1].VolumeMounts[0].MountPath)
 
-	assert.Equal(t, "dind-cert", ars.Spec.Template.Spec.Containers[1].VolumeMounts[1].Name)
-	assert.Equal(t, "/certs/client", ars.Spec.Template.Spec.Containers[1].VolumeMounts[1].MountPath)
+	assert.Equal(t, "dind-sock", ars.Spec.Template.Spec.Containers[1].VolumeMounts[1].Name)
+	assert.Equal(t, "/run/docker", ars.Spec.Template.Spec.Containers[1].VolumeMounts[1].MountPath)
 
 	assert.Equal(t, "dind-externals", ars.Spec.Template.Spec.Containers[1].VolumeMounts[2].Name)
 	assert.Equal(t, "/home/runner/externals", ars.Spec.Template.Spec.Containers[1].VolumeMounts[2].MountPath)
 
 	assert.Len(t, ars.Spec.Template.Spec.Volumes, 3, "Volumes should be 3")
-	assert.Equal(t, "dind-cert", ars.Spec.Template.Spec.Volumes[0].Name, "Volume name should be dind-cert")
+	assert.Equal(t, "dind-sock", ars.Spec.Template.Spec.Volumes[0].Name, "Volume name should be dind-sock")
 	assert.Equal(t, "dind-externals", ars.Spec.Template.Spec.Volumes[1].Name, "Volume name should be dind-externals")
 	assert.Equal(t, "work", ars.Spec.Template.Spec.Volumes[2].Name, "Volume name should be work")
 	assert.NotNil(t, ars.Spec.Template.Spec.Volumes[2].EmptyDir, "Volume work should be an emptyDir")
@@ -1833,10 +1829,6 @@ func TestTemplateRenderedAutoScalingRunnerSet_DinDMergePodSpec(t *testing.T) {
 	assert.Equal(t, "tcp://localhost:9999", ars.Spec.Template.Spec.Containers[0].Env[0].Value, "DOCKER_HOST should be set to `tcp://localhost:9999`")
 	assert.Equal(t, "MY_NODE_NAME", ars.Spec.Template.Spec.Containers[0].Env[1].Name, "MY_NODE_NAME should be set")
 	assert.Equal(t, "spec.nodeName", ars.Spec.Template.Spec.Containers[0].Env[1].ValueFrom.FieldRef.FieldPath, "MY_NODE_NAME should be set to `spec.nodeName`")
-	assert.Equal(t, "DOCKER_TLS_VERIFY", ars.Spec.Template.Spec.Containers[0].Env[2].Name, "DOCKER_TLS_VERIFY should be set")
-	assert.Equal(t, "1", ars.Spec.Template.Spec.Containers[0].Env[2].Value, "DOCKER_TLS_VERIFY should be set to `1`")
-	assert.Equal(t, "DOCKER_CERT_PATH", ars.Spec.Template.Spec.Containers[0].Env[3].Name, "DOCKER_CERT_PATH should be set")
-	assert.Equal(t, "/certs/client", ars.Spec.Template.Spec.Containers[0].Env[3].Value, "DOCKER_CERT_PATH should be set to `/certs/client`")
 	assert.Equal(t, "work", ars.Spec.Template.Spec.Containers[0].VolumeMounts[0].Name, "VolumeMount name should be work")
 	assert.Equal(t, "/work", ars.Spec.Template.Spec.Containers[0].VolumeMounts[0].MountPath, "VolumeMount mountPath should be /work")
 	assert.Equal(t, "others", ars.Spec.Template.Spec.Containers[0].VolumeMounts[1].Name, "VolumeMount name should be others")

diff --git a/charts/gha-runner-scale-set/values.yaml b/charts/gha-runner-scale-set/values.yaml
@@ -122,32 +122,35 @@ template:
   ##       command: ["/home/runner/run.sh"]
   ##       env:
   ##         - name: DOCKER_HOST
-  ##           value: tcp://localhost:2376
-  ##         - name: DOCKER_TLS_VERIFY
-  ##           value: "1"
-  ##         - name: DOCKER_CERT_PATH
-  ##           value: /certs/client
+  ##           value: unix:///run/docker/docker.sock
   ##       volumeMounts:
   ##         - name: work
   ##           mountPath: /home/runner/_work
-  ##         - name: dind-cert
-  ##           mountPath: /certs/client
+  ##         - name: dind-sock
+  ##           mountPath: /run/docker
   ##           readOnly: true
   ##     - name: dind
   ##       image: docker:dind
+  ##       args:
+  ##         - dockerd
+  ##         - --host=unix:///run/docker/docker.sock
+  ##         - --group=$(DOCKER_GROUP_GID)
+  ##       env:
+  ##         - name: DOCKER_GROUP_GID
+  ##           value: "123"
   ##       securityContext:
   ##         privileged: true
   ##       volumeMounts:
   ##         - name: work
   ##           mountPath: /home/runner/_work
-  ##         - name: dind-cert
-  ##           mountPath: /certs/client
+  ##         - name: dind-sock
+  ##           mountPath: /run/docker
   ##         - name: dind-externals
   ##           mountPath: /home/runner/externals
   ##     volumes:
   ##     - name: work
   ##       emptyDir: {}
-  ##     - name: dind-cert
+  ##     - name: dind-sock
   ##       emptyDir: {}
   ##     - name: dind-externals
   ##       emptyDir: {}