From c56930427167eaacfa5385ea5b58dfa6bdb021e0 Mon Sep 17 00:00:00 2001 From: Francesco Renzi Date: Thu, 9 Mar 2023 17:23:32 +0000 Subject: [PATCH] Add support for self-signed CA certificates (#2268) Co-authored-by: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Co-authored-by: Nikola Jokic Co-authored-by: Tingluo Huang --- .../v1alpha1/autoscalinglistener_types.go | 3 + .../v1alpha1/autoscalingrunnerset_types.go | 40 +- .../v1alpha1/tls_config_test.go | 105 +++++ .../v1alpha1/zz_generated.deepcopy.go | 34 +- ...tions.github.com_autoscalinglisteners.yaml | 22 ++ ...ions.github.com_autoscalingrunnersets.yaml | 20 +- .../actions.github.com_ephemeralrunners.yaml | 20 +- ...ctions.github.com_ephemeralrunnersets.yaml | 20 +- .../templates/manager_role.yaml | 9 +- .../tests/template_test.go | 2 +- .../templates/_helpers.tpl | 154 +++++++- .../templates/autoscalingrunnerset.yaml | 18 +- .../tests/template_test.go | 359 ++++++++++++++++++ charts/gha-runner-scale-set/values.yaml | 25 +- cmd/githubrunnerscalesetlistener/main.go | 18 +- cmd/githubrunnerscalesetlistener/main_test.go | 52 +++ ...tions.github.com_autoscalinglisteners.yaml | 22 ++ ...ions.github.com_autoscalingrunnersets.yaml | 20 +- .../actions.github.com_ephemeralrunners.yaml | 20 +- ...ctions.github.com_ephemeralrunnersets.yaml | 20 +- .../autoscalinglistener_controller.go | 50 +++ .../autoscalinglistener_controller_test.go | 164 ++++++++ .../autoscalingrunnerset_controller.go | 53 ++- .../autoscalingrunnerset_controller_test.go | 241 ++++++++++++ .../ephemeralrunner_controller.go | 48 ++- .../ephemeralrunner_controller_test.go | 100 +++++ .../ephemeralrunnerset_controller.go | 49 ++- .../ephemeralrunnerset_controller_test.go | 149 ++++++++ .../actions.github.com/resourcebuilder.go | 1 + controllers/actions.github.com/suite_test.go | 8 +- github/actions/actions_server_test.go | 6 - github/actions/client.go | 6 + github/actions/client_tls_test.go | 8 +- github/actions/github_api_request_test.go | 21 +- github/actions/identifier_test.go | 47 +++ github/actions/multi_client.go | 19 +- 36 files changed, 1860 insertions(+), 93 deletions(-) create mode 100644 apis/actions.github.com/v1alpha1/tls_config_test.go diff --git a/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go b/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go index e4e5c383..82458657 100644 --- a/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go @@ -57,6 +57,9 @@ type AutoscalingListenerSpec struct { // +optional Proxy *ProxyConfig `json:"proxy,omitempty"` + + // +optional + GitHubServerTLS *GitHubServerTLSConfig `json:"githubServerTLS,omitempty"` } // AutoscalingListenerStatus defines the observed state of AutoscalingListener diff --git a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go index 8f2bf102..adc9a94e 100644 --- a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go @@ -17,6 +17,7 @@ limitations under the License. package v1alpha1 import ( + "crypto/x509" "fmt" "net/http" "net/url" @@ -80,7 +81,44 @@ type AutoscalingRunnerSetSpec struct { type GitHubServerTLSConfig struct { // Required - RootCAsConfigMapRef string `json:"certConfigMapRef,omitempty"` + CertificateFrom *TLSCertificateSource `json:"certificateFrom,omitempty"` +} + +func (c *GitHubServerTLSConfig) ToCertPool(keyFetcher func(name, key string) ([]byte, error)) (*x509.CertPool, error) { + if c.CertificateFrom == nil { + return nil, fmt.Errorf("certificateFrom not specified") + } + + if c.CertificateFrom.ConfigMapKeyRef == nil { + return nil, fmt.Errorf("configMapKeyRef not specified") + } + + cert, err := keyFetcher(c.CertificateFrom.ConfigMapKeyRef.Name, c.CertificateFrom.ConfigMapKeyRef.Key) + if err != nil { + return nil, fmt.Errorf( + "failed to fetch key %q in configmap %q: %w", + c.CertificateFrom.ConfigMapKeyRef.Key, + c.CertificateFrom.ConfigMapKeyRef.Name, + err, + ) + } + + systemPool, err := x509.SystemCertPool() + if err != nil { + return nil, fmt.Errorf("failed to get system cert pool: %w", err) + } + + pool := systemPool.Clone() + if !pool.AppendCertsFromPEM(cert) { + return nil, fmt.Errorf("failed to parse certificate") + } + + return pool, nil +} + +type TLSCertificateSource struct { + // Required + ConfigMapKeyRef *corev1.ConfigMapKeySelector `json:"configMapKeyRef,omitempty"` } type ProxyConfig struct { diff --git a/apis/actions.github.com/v1alpha1/tls_config_test.go b/apis/actions.github.com/v1alpha1/tls_config_test.go new file mode 100644 index 00000000..c3a74bf7 --- /dev/null +++ b/apis/actions.github.com/v1alpha1/tls_config_test.go @@ -0,0 +1,105 @@ +package v1alpha1_test + +import ( + "crypto/tls" + "crypto/x509" + "net/http" + "os" + "path/filepath" + "testing" + + "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" + "github.com/actions/actions-runner-controller/github/actions/testserver" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" +) + +func TestGitHubServerTLSConfig_ToCertPool(t *testing.T) { + t.Run("returns an error if CertificateFrom not specified", func(t *testing.T) { + c := &v1alpha1.GitHubServerTLSConfig{ + CertificateFrom: nil, + } + + pool, err := c.ToCertPool(nil) + assert.Nil(t, pool) + + require.Error(t, err) + assert.Equal(t, err.Error(), "certificateFrom not specified") + }) + + t.Run("returns an error if CertificateFrom.ConfigMapKeyRef not specified", func(t *testing.T) { + c := &v1alpha1.GitHubServerTLSConfig{ + CertificateFrom: &v1alpha1.TLSCertificateSource{}, + } + + pool, err := c.ToCertPool(nil) + assert.Nil(t, pool) + + require.Error(t, err) + assert.Equal(t, err.Error(), "configMapKeyRef not specified") + }) + + t.Run("returns a valid cert pool with correct configuration", func(t *testing.T) { + c := &v1alpha1.GitHubServerTLSConfig{ + CertificateFrom: &v1alpha1.TLSCertificateSource{ + ConfigMapKeyRef: &v1.ConfigMapKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "name", + }, + Key: "key", + }, + }, + } + + certsFolder := filepath.Join( + "../../../", + "github", + "actions", + "testdata", + ) + + fetcher := func(name, key string) ([]byte, error) { + cert, err := os.ReadFile(filepath.Join(certsFolder, "rootCA.crt")) + require.NoError(t, err) + + pool := x509.NewCertPool() + ok := pool.AppendCertsFromPEM(cert) + assert.True(t, ok) + + return cert, nil + } + + pool, err := c.ToCertPool(fetcher) + require.NoError(t, err) + require.NotNil(t, pool) + + // can be used to communicate with a server + serverSuccessfullyCalled := false + server := testserver.NewUnstarted(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + serverSuccessfullyCalled = true + w.WriteHeader(http.StatusOK) + })) + + cert, err := tls.LoadX509KeyPair( + filepath.Join(certsFolder, "server.crt"), + filepath.Join(certsFolder, "server.key"), + ) + require.NoError(t, err) + + server.TLS = &tls.Config{Certificates: []tls.Certificate{cert}} + server.StartTLS() + + client := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + RootCAs: pool, + }, + }, + } + + _, err = client.Get(server.URL) + assert.NoError(t, err) + assert.True(t, serverSuccessfullyCalled) + }) +} diff --git a/apis/actions.github.com/v1alpha1/zz_generated.deepcopy.go b/apis/actions.github.com/v1alpha1/zz_generated.deepcopy.go index 324707b2..3246592b 100644 --- a/apis/actions.github.com/v1alpha1/zz_generated.deepcopy.go +++ b/apis/actions.github.com/v1alpha1/zz_generated.deepcopy.go @@ -98,6 +98,11 @@ func (in *AutoscalingListenerSpec) DeepCopyInto(out *AutoscalingListenerSpec) { *out = new(ProxyConfig) (*in).DeepCopyInto(*out) } + if in.GitHubServerTLS != nil { + in, out := &in.GitHubServerTLS, &out.GitHubServerTLS + *out = new(GitHubServerTLSConfig) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AutoscalingListenerSpec. @@ -195,7 +200,7 @@ func (in *AutoscalingRunnerSetSpec) DeepCopyInto(out *AutoscalingRunnerSetSpec) if in.GitHubServerTLS != nil { in, out := &in.GitHubServerTLS, &out.GitHubServerTLS *out = new(GitHubServerTLSConfig) - **out = **in + (*in).DeepCopyInto(*out) } in.Template.DeepCopyInto(&out.Template) if in.MaxRunners != nil { @@ -395,7 +400,7 @@ func (in *EphemeralRunnerSpec) DeepCopyInto(out *EphemeralRunnerSpec) { if in.GitHubServerTLS != nil { in, out := &in.GitHubServerTLS, &out.GitHubServerTLS *out = new(GitHubServerTLSConfig) - **out = **in + (*in).DeepCopyInto(*out) } in.PodTemplateSpec.DeepCopyInto(&out.PodTemplateSpec) } @@ -435,6 +440,11 @@ func (in *EphemeralRunnerStatus) DeepCopy() *EphemeralRunnerStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GitHubServerTLSConfig) DeepCopyInto(out *GitHubServerTLSConfig) { *out = *in + if in.CertificateFrom != nil { + in, out := &in.CertificateFrom, &out.CertificateFrom + *out = new(TLSCertificateSource) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitHubServerTLSConfig. @@ -491,3 +501,23 @@ func (in *ProxyServerConfig) DeepCopy() *ProxyServerConfig { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TLSCertificateSource) DeepCopyInto(out *TLSCertificateSource) { + *out = *in + if in.ConfigMapKeyRef != nil { + in, out := &in.ConfigMapKeyRef, &out.ConfigMapKeyRef + *out = new(v1.ConfigMapKeySelector) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TLSCertificateSource. +func (in *TLSCertificateSource) DeepCopy() *TLSCertificateSource { + if in == nil { + return nil + } + out := new(TLSCertificateSource) + in.DeepCopyInto(out) + return out +} diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml index f0f3f8fb..6df9c051 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml @@ -55,6 +55,28 @@ spec: githubConfigUrl: description: Required type: string + githubServerTLS: + properties: + certificateFrom: + description: Required + properties: + configMapKeyRef: + description: Required + properties: + key: + description: The key to select. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + optional: + description: Specify whether the ConfigMap or its key must be defined + type: boolean + required: + - key + type: object + type: object + type: object image: description: Required type: string diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml index 12c4b5b8..6c4c82cb 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml @@ -51,9 +51,25 @@ spec: type: string githubServerTLS: properties: - certConfigMapRef: + certificateFrom: description: Required - type: string + properties: + configMapKeyRef: + description: Required + properties: + key: + description: The key to select. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + optional: + description: Specify whether the ConfigMap or its key must be defined + type: boolean + required: + - key + type: object + type: object type: object maxRunners: minimum: 0 diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml index 41cdc81b..b0ce1e4b 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml @@ -64,9 +64,25 @@ spec: type: string githubServerTLS: properties: - certConfigMapRef: + certificateFrom: description: Required - type: string + properties: + configMapKeyRef: + description: Required + properties: + key: + description: The key to select. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + optional: + description: Specify whether the ConfigMap or its key must be defined + type: boolean + required: + - key + type: object + type: object type: object metadata: description: 'Standard object''s metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata' diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml index 072cd265..86a3f40c 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml @@ -46,9 +46,25 @@ spec: type: string githubServerTLS: properties: - certConfigMapRef: + certificateFrom: description: Required - type: string + properties: + configMapKeyRef: + description: Required + properties: + key: + description: The key to select. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + optional: + description: Specify whether the ConfigMap or its key must be defined + type: boolean + required: + - key + type: object + type: object type: object metadata: description: 'Standard object''s metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata' diff --git a/charts/gha-runner-scale-set-controller/templates/manager_role.yaml b/charts/gha-runner-scale-set-controller/templates/manager_role.yaml index f51b47c4..e9457cfa 100644 --- a/charts/gha-runner-scale-set-controller/templates/manager_role.yaml +++ b/charts/gha-runner-scale-set-controller/templates/manager_role.yaml @@ -146,6 +146,13 @@ rules: - get - list - watch +- apiGroups: + - "" + resources: + - configmaps + verbs: + - list + - watch - apiGroups: - rbac.authorization.k8s.io resources: @@ -167,4 +174,4 @@ rules: - get - update - list - - watch \ No newline at end of file + - watch diff --git a/charts/gha-runner-scale-set-controller/tests/template_test.go b/charts/gha-runner-scale-set-controller/tests/template_test.go index 6f9c47d6..00ab04b5 100644 --- a/charts/gha-runner-scale-set-controller/tests/template_test.go +++ b/charts/gha-runner-scale-set-controller/tests/template_test.go @@ -169,7 +169,7 @@ func TestTemplate_CreateManagerRole(t *testing.T) { assert.Empty(t, managerRole.Namespace, "ClusterRole should not have a namespace") assert.Equal(t, "test-arc-gha-runner-scale-set-controller-manager-role", managerRole.Name) - assert.Equal(t, 17, len(managerRole.Rules)) + assert.Equal(t, 18, len(managerRole.Rules)) } func TestTemplate_ManagerRoleBinding(t *testing.T) { diff --git a/charts/gha-runner-scale-set/templates/_helpers.tpl b/charts/gha-runner-scale-set/templates/_helpers.tpl index 944bf979..433d84eb 100644 --- a/charts/gha-runner-scale-set/templates/_helpers.tpl +++ b/charts/gha-runner-scale-set/templates/_helpers.tpl @@ -111,6 +111,15 @@ volumeMounts: emptyDir: {} {{- end }} +{{- define "gha-runner-scale-set.tls-volume" -}} +- name: github-server-tls-cert + configMap: + name: {{ .certificateFrom.configMapKeyRef.name }} + items: + - key: {{ .certificateFrom.configMapKeyRef.key }} + path: {{ .certificateFrom.configMapKeyRef.key }} +{{- end }} + {{- define "gha-runner-scale-set.dind-work-volume" -}} {{- $createWorkVolume := 1 }} {{- range $i, $volume := .Values.template.spec.volumes }} @@ -155,12 +164,7 @@ volumeMounts: {{- define "gha-runner-scale-set.non-work-volumes" -}} {{- range $i, $volume := .Values.template.spec.volumes }} {{- if ne $volume.name "work" }} -- name: {{ $volume.name }} - {{- range $key, $val := $volume }} - {{- if ne $key "name" }} - {{ $key }}: {{ $val }} - {{- end }} - {{- end }} +- {{ $volume | toYaml | nindent 2 }} {{- end }} {{- end }} {{- end }} @@ -179,6 +183,7 @@ volumeMounts: {{- end }} {{- define "gha-runner-scale-set.dind-runner-container" -}} +{{- $tlsConfig := (default (dict) .Values.githubServerTLS) }} {{- range $i, $container := .Values.template.spec.containers -}} {{- if eq $container.name "runner" -}} {{- range $key, $val := $container }} @@ -190,6 +195,12 @@ volumeMounts: {{- $setDockerTlsVerify := 1 }} {{- $setDockerCertPath := 1 }} {{- $setRunnerWaitDocker := 1 }} + {{- $setNodeExtraCaCerts := 0 }} + {{- $setRunnerUpdateCaCerts := 0 }} + {{- if $tlsConfig.runnerMountPath }} + {{- $setNodeExtraCaCerts = 1 }} + {{- $setRunnerUpdateCaCerts = 1 }} + {{- end }} env: {{- with $container.env }} {{- range $i, $env := . }} @@ -205,6 +216,12 @@ env: {{- if eq $env.name "RUNNER_WAIT_FOR_DOCKER_IN_SECONDS" }} {{- $setRunnerWaitDocker = 0 -}} {{- end }} + {{- if eq $env.name "NODE_EXTRA_CA_CERTS" }} + {{- $setNodeExtraCaCerts = 0 -}} + {{- end }} + {{- if eq $env.name "RUNNER_UPDATE_CA_CERTS" }} + {{- $setRunnerUpdateCaCerts = 0 -}} + {{- end }} - name: {{ $env.name }} {{- range $envKey, $envVal := $env }} {{- if ne $envKey "name" }} @@ -229,8 +246,20 @@ env: - name: RUNNER_WAIT_FOR_DOCKER_IN_SECONDS value: "120" {{- end }} + {{- if $setNodeExtraCaCerts }} + - name: NODE_EXTRA_CA_CERTS + value: {{ clean (print $tlsConfig.runnerMountPath "/" $tlsConfig.certificateFrom.configMapKeyRef.key) }} + {{- end }} + {{- if $setRunnerUpdateCaCerts }} + - name: RUNNER_UPDATE_CA_CERTS + value: "1" + {{- end }} {{- $mountWork := 1 }} {{- $mountDindCert := 1 }} + {{- $mountGitHubServerTLS := 0 }} + {{- if $tlsConfig.runnerMountPath }} + {{- $mountGitHubServerTLS = 1 }} + {{- end }} volumeMounts: {{- with $container.volumeMounts }} {{- range $i, $volMount := . }} @@ -240,6 +269,9 @@ volumeMounts: {{- if eq $volMount.name "dind-cert" }} {{- $mountDindCert = 0 -}} {{- end }} + {{- if eq $volMount.name "github-server-tls-cert" }} + {{- $mountGitHubServerTLS = 0 -}} + {{- end }} - name: {{ $volMount.name }} {{- range $mountKey, $mountVal := $volMount }} {{- if ne $mountKey "name" }} @@ -257,11 +289,17 @@ volumeMounts: mountPath: /certs/client readOnly: true {{- end }} + {{- if $mountGitHubServerTLS }} + - name: github-server-tls-cert + mountPath: {{ clean (print $tlsConfig.runnerMountPath "/" $tlsConfig.certificateFrom.configMapKeyRef.key) }} + subPath: {{ $tlsConfig.certificateFrom.configMapKeyRef.key }} + {{- end }} {{- end }} {{- end }} {{- end }} {{- define "gha-runner-scale-set.kubernetes-mode-runner-container" -}} +{{- $tlsConfig := (default (dict) .Values.githubServerTLS) }} {{- range $i, $container := .Values.template.spec.containers -}} {{- if eq $container.name "runner" -}} {{- range $key, $val := $container }} @@ -272,6 +310,12 @@ volumeMounts: {{- $setContainerHooks := 1 }} {{- $setPodName := 1 }} {{- $setRequireJobContainer := 1 }} + {{- $setNodeExtraCaCerts := 0 }} + {{- $setRunnerUpdateCaCerts := 0 }} + {{- if $tlsConfig.runnerMountPath }} + {{- $setNodeExtraCaCerts = 1 }} + {{- $setRunnerUpdateCaCerts = 1 }} + {{- end }} env: {{- with $container.env }} {{- range $i, $env := . }} @@ -284,6 +328,12 @@ env: {{- if eq $env.name "ACTIONS_RUNNER_REQUIRE_JOB_CONTAINER" }} {{- $setRequireJobContainer = 0 -}} {{- end }} + {{- if eq $env.name "NODE_EXTRA_CA_CERTS" }} + {{- $setNodeExtraCaCerts = 0 -}} + {{- end }} + {{- if eq $env.name "RUNNER_UPDATE_CA_CERTS" }} + {{- $setRunnerUpdateCaCerts = 0 -}} + {{- end }} - name: {{ $env.name }} {{- range $envKey, $envVal := $env }} {{- if ne $envKey "name" }} @@ -306,13 +356,28 @@ env: - name: ACTIONS_RUNNER_REQUIRE_JOB_CONTAINER value: "true" {{- end }} + {{- if $setNodeExtraCaCerts }} + - name: NODE_EXTRA_CA_CERTS + value: {{ clean (print $tlsConfig.runnerMountPath "/" $tlsConfig.certificateFrom.configMapKeyRef.key) }} + {{- end }} + {{- if $setRunnerUpdateCaCerts }} + - name: RUNNER_UPDATE_CA_CERTS + value: "1" + {{- end }} {{- $mountWork := 1 }} + {{- $mountGitHubServerTLS := 0 }} + {{- if $tlsConfig.runnerMountPath }} + {{- $mountGitHubServerTLS = 1 }} + {{- end }} volumeMounts: {{- with $container.volumeMounts }} {{- range $i, $volMount := . }} {{- if eq $volMount.name "work" }} {{- $mountWork = 0 -}} {{- end }} + {{- if eq $volMount.name "github-server-tls-cert" }} + {{- $mountGitHubServerTLS = 0 -}} + {{- end }} - name: {{ $volMount.name }} {{- range $mountKey, $mountVal := $volMount }} {{- if ne $mountKey "name" }} @@ -325,6 +390,81 @@ volumeMounts: - name: work mountPath: /actions-runner/_work {{- end }} + {{- if $mountGitHubServerTLS }} + - name: github-server-tls-cert + mountPath: {{ clean (print $tlsConfig.runnerMountPath "/" $tlsConfig.certificateFrom.configMapKeyRef.key) }} + subPath: {{ $tlsConfig.certificateFrom.configMapKeyRef.key }} + {{- end }} {{- end }} {{- end }} -{{- end }} \ No newline at end of file +{{- end }} + +{{- define "gha-runner-scale-set.default-mode-runner-containers" -}} +{{- $tlsConfig := (default (dict) .Values.githubServerTLS) }} +{{- range $i, $container := .Values.template.spec.containers -}} +{{- if ne $container.name "runner" -}} +- {{ $container | toYaml | nindent 2 }} +{{- else }} +- name: {{ $container.name }} + {{- range $key, $val := $container }} + {{- if and (ne $key "env") (ne $key "volumeMounts") (ne $key "name") }} + {{ $key }}: {{ $val }} + {{- end }} + {{- end }} + {{- $setNodeExtraCaCerts := 0 }} + {{- $setRunnerUpdateCaCerts := 0 }} + {{- if $tlsConfig.runnerMountPath }} + {{- $setNodeExtraCaCerts = 1 }} + {{- $setRunnerUpdateCaCerts = 1 }} + {{- end }} + env: + {{- with $container.env }} + {{- range $i, $env := . }} + {{- if eq $env.name "NODE_EXTRA_CA_CERTS" }} + {{- $setNodeExtraCaCerts = 0 -}} + {{- end }} + {{- if eq $env.name "RUNNER_UPDATE_CA_CERTS" }} + {{- $setRunnerUpdateCaCerts = 0 -}} + {{- end }} + - name: {{ $env.name }} + {{- range $envKey, $envVal := $env }} + {{- if ne $envKey "name" }} + {{ $envKey }}: {{ $envVal | toYaml | nindent 10 }} + {{- end }} + {{- end }} + {{- end }} + {{- end }} + {{- if $setNodeExtraCaCerts }} + - name: NODE_EXTRA_CA_CERTS + value: {{ clean (print $tlsConfig.runnerMountPath "/" $tlsConfig.certificateFrom.configMapKeyRef.key) }} + {{- end }} + {{- if $setRunnerUpdateCaCerts }} + - name: RUNNER_UPDATE_CA_CERTS + value: "1" + {{- end }} + {{- $mountGitHubServerTLS := 0 }} + {{- if $tlsConfig.runnerMountPath }} + {{- $mountGitHubServerTLS = 1 }} + {{- end }} + volumeMounts: + {{- with $container.volumeMounts }} + {{- range $i, $volMount := . }} + {{- if eq $volMount.name "github-server-tls-cert" }} + {{- $mountGitHubServerTLS = 0 -}} + {{- end }} + - name: {{ $volMount.name }} + {{- range $mountKey, $mountVal := $volMount }} + {{- if ne $mountKey "name" }} + {{ $mountKey }}: {{ $mountVal | toYaml | nindent 10 }} + {{- end }} + {{- end }} + {{- end }} + {{- end }} + {{- if $mountGitHubServerTLS }} + - name: github-server-tls-cert + mountPath: {{ clean (print $tlsConfig.runnerMountPath "/" $tlsConfig.certificateFrom.configMapKeyRef.key) }} + subPath: {{ $tlsConfig.certificateFrom.configMapKeyRef.key }} + {{- end }} +{{- end }} +{{- end }} +{{- end }} diff --git a/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml b/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml index e7f66156..6ec90340 100644 --- a/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml +++ b/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml @@ -21,6 +21,16 @@ spec: runnerScaleSetName: {{ . }} {{- end }} + {{- if .Values.githubServerTLS }} + githubServerTLS: + {{- with .Values.githubServerTLS.certificateFrom }} + certificateFrom: + configMapKeyRef: + name: {{ .configMapKeyRef.name }} + key: {{ .configMapKeyRef.key }} + {{- end }} + {{- end }} + {{- if .Values.proxy }} proxy: {{- if .Values.proxy.http }} @@ -103,10 +113,14 @@ spec: {{- include "gha-runner-scale-set.kubernetes-mode-runner-container" . | nindent 8 }} {{- include "gha-runner-scale-set.non-runner-containers" . | nindent 6 }} {{- else }} - {{ .Values.template.spec.containers | toYaml | nindent 6 }} + {{- include "gha-runner-scale-set.default-mode-runner-containers" . | nindent 6 }} {{- end }} - {{- if or .Values.template.spec.volumes (eq .Values.containerMode.type "dind") (eq .Values.containerMode.type "kubernetes") }} + {{- $tlsConfig := (default (dict) .Values.githubServerTLS) }} + {{- if or .Values.template.spec.volumes (eq .Values.containerMode.type "dind") (eq .Values.containerMode.type "kubernetes") $tlsConfig.runnerMountPath }} volumes: + {{- if $tlsConfig.runnerMountPath }} + {{- include "gha-runner-scale-set.tls-volume" $tlsConfig | nindent 6 }} + {{- end }} {{- if eq .Values.containerMode.type "dind" }} {{- include "gha-runner-scale-set.dind-volume" . | nindent 6 }} {{- include "gha-runner-scale-set.dind-work-volume" . | nindent 6 }} diff --git a/charts/gha-runner-scale-set/tests/template_test.go b/charts/gha-runner-scale-set/tests/template_test.go index 01b522c6..3493deb6 100644 --- a/charts/gha-runner-scale-set/tests/template_test.go +++ b/charts/gha-runner-scale-set/tests/template_test.go @@ -828,6 +828,365 @@ func TestTemplateRenderedWithProxy(t *testing.T) { assert.Contains(t, ars.Spec.Proxy.NoProxy, "example.org") } +func TestTemplateRenderedWithTLS(t *testing.T) { + t.Parallel() + + namespaceName := "test-" + strings.ToLower(random.UniqueId()) + + render := func(t *testing.T, options *helm.Options) v1alpha1.AutoscalingRunnerSet { + // Path to the helm chart we will test + helmChartPath, err := filepath.Abs("../../gha-runner-scale-set") + require.NoError(t, err) + + releaseName := "test-runners" + + output := helm.RenderTemplate( + t, + options, + helmChartPath, + releaseName, + []string{"templates/autoscalingrunnerset.yaml"}, + ) + + var ars v1alpha1.AutoscalingRunnerSet + helm.UnmarshalK8SYaml(t, output, &ars) + + return ars + } + + t.Run("providing githubServerTLS.runnerMountPath", func(t *testing.T) { + t.Run("mode: default", func(t *testing.T) { + options := &helm.Options{ + SetValues: map[string]string{ + "githubConfigUrl": "https://github.com/actions", + "githubConfigSecret": "pre-defined-secrets", + "githubServerTLS.certificateFrom.configMapKeyRef.name": "certs-configmap", + "githubServerTLS.certificateFrom.configMapKeyRef.key": "cert.pem", + "githubServerTLS.runnerMountPath": "/runner/mount/path", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + } + + ars := render(t, options) + + require.NotNil(t, ars.Spec.GitHubServerTLS) + expected := &v1alpha1.GitHubServerTLSConfig{ + CertificateFrom: &v1alpha1.TLSCertificateSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "certs-configmap", + }, + Key: "cert.pem", + }, + }, + } + assert.Equal(t, expected, ars.Spec.GitHubServerTLS) + + var volume *corev1.Volume + for _, v := range ars.Spec.Template.Spec.Volumes { + if v.Name == "github-server-tls-cert" { + volume = &v + break + } + } + require.NotNil(t, volume) + assert.Equal(t, "certs-configmap", volume.ConfigMap.LocalObjectReference.Name) + assert.Equal(t, "cert.pem", volume.ConfigMap.Items[0].Key) + assert.Equal(t, "cert.pem", volume.ConfigMap.Items[0].Path) + + assert.Contains(t, ars.Spec.Template.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{ + Name: "github-server-tls-cert", + MountPath: "/runner/mount/path/cert.pem", + SubPath: "cert.pem", + }) + + assert.Contains(t, ars.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "NODE_EXTRA_CA_CERTS", + Value: "/runner/mount/path/cert.pem", + }) + + assert.Contains(t, ars.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "RUNNER_UPDATE_CA_CERTS", + Value: "1", + }) + }) + + t.Run("mode: dind", func(t *testing.T) { + options := &helm.Options{ + SetValues: map[string]string{ + "githubConfigUrl": "https://github.com/actions", + "githubConfigSecret": "pre-defined-secrets", + "githubServerTLS.certificateFrom.configMapKeyRef.name": "certs-configmap", + "githubServerTLS.certificateFrom.configMapKeyRef.key": "cert.pem", + "githubServerTLS.runnerMountPath": "/runner/mount/path/", + "containerMode.type": "dind", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + } + + ars := render(t, options) + + require.NotNil(t, ars.Spec.GitHubServerTLS) + expected := &v1alpha1.GitHubServerTLSConfig{ + CertificateFrom: &v1alpha1.TLSCertificateSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "certs-configmap", + }, + Key: "cert.pem", + }, + }, + } + assert.Equal(t, expected, ars.Spec.GitHubServerTLS) + + var volume *corev1.Volume + for _, v := range ars.Spec.Template.Spec.Volumes { + if v.Name == "github-server-tls-cert" { + volume = &v + break + } + } + require.NotNil(t, volume) + assert.Equal(t, "certs-configmap", volume.ConfigMap.LocalObjectReference.Name) + assert.Equal(t, "cert.pem", volume.ConfigMap.Items[0].Key) + assert.Equal(t, "cert.pem", volume.ConfigMap.Items[0].Path) + + assert.Contains(t, ars.Spec.Template.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{ + Name: "github-server-tls-cert", + MountPath: "/runner/mount/path/cert.pem", + SubPath: "cert.pem", + }) + + assert.Contains(t, ars.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "NODE_EXTRA_CA_CERTS", + Value: "/runner/mount/path/cert.pem", + }) + + assert.Contains(t, ars.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "RUNNER_UPDATE_CA_CERTS", + Value: "1", + }) + }) + + t.Run("mode: kubernetes", func(t *testing.T) { + options := &helm.Options{ + SetValues: map[string]string{ + "githubConfigUrl": "https://github.com/actions", + "githubConfigSecret": "pre-defined-secrets", + "githubServerTLS.certificateFrom.configMapKeyRef.name": "certs-configmap", + "githubServerTLS.certificateFrom.configMapKeyRef.key": "cert.pem", + "githubServerTLS.runnerMountPath": "/runner/mount/path", + "containerMode.type": "kubernetes", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + } + + ars := render(t, options) + + require.NotNil(t, ars.Spec.GitHubServerTLS) + expected := &v1alpha1.GitHubServerTLSConfig{ + CertificateFrom: &v1alpha1.TLSCertificateSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "certs-configmap", + }, + Key: "cert.pem", + }, + }, + } + assert.Equal(t, expected, ars.Spec.GitHubServerTLS) + + var volume *corev1.Volume + for _, v := range ars.Spec.Template.Spec.Volumes { + if v.Name == "github-server-tls-cert" { + volume = &v + break + } + } + require.NotNil(t, volume) + assert.Equal(t, "certs-configmap", volume.ConfigMap.LocalObjectReference.Name) + assert.Equal(t, "cert.pem", volume.ConfigMap.Items[0].Key) + assert.Equal(t, "cert.pem", volume.ConfigMap.Items[0].Path) + + assert.Contains(t, ars.Spec.Template.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{ + Name: "github-server-tls-cert", + MountPath: "/runner/mount/path/cert.pem", + SubPath: "cert.pem", + }) + + assert.Contains(t, ars.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "NODE_EXTRA_CA_CERTS", + Value: "/runner/mount/path/cert.pem", + }) + + assert.Contains(t, ars.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "RUNNER_UPDATE_CA_CERTS", + Value: "1", + }) + }) + }) + + t.Run("without providing githubServerTLS.runnerMountPath", func(t *testing.T) { + t.Run("mode: default", func(t *testing.T) { + options := &helm.Options{ + SetValues: map[string]string{ + "githubConfigUrl": "https://github.com/actions", + "githubConfigSecret": "pre-defined-secrets", + "githubServerTLS.certificateFrom.configMapKeyRef.name": "certs-configmap", + "githubServerTLS.certificateFrom.configMapKeyRef.key": "cert.pem", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + } + + ars := render(t, options) + + require.NotNil(t, ars.Spec.GitHubServerTLS) + expected := &v1alpha1.GitHubServerTLSConfig{ + CertificateFrom: &v1alpha1.TLSCertificateSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "certs-configmap", + }, + Key: "cert.pem", + }, + }, + } + assert.Equal(t, expected, ars.Spec.GitHubServerTLS) + + var volume *corev1.Volume + for _, v := range ars.Spec.Template.Spec.Volumes { + if v.Name == "github-server-tls-cert" { + volume = &v + break + } + } + assert.Nil(t, volume) + + assert.NotContains(t, ars.Spec.Template.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{ + Name: "github-server-tls-cert", + MountPath: "/runner/mount/path/cert.pem", + SubPath: "cert.pem", + }) + + assert.NotContains(t, ars.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "NODE_EXTRA_CA_CERTS", + Value: "/runner/mount/path/cert.pem", + }) + + assert.NotContains(t, ars.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "RUNNER_UPDATE_CA_CERTS", + Value: "1", + }) + }) + + t.Run("mode: dind", func(t *testing.T) { + options := &helm.Options{ + SetValues: map[string]string{ + "githubConfigUrl": "https://github.com/actions", + "githubConfigSecret": "pre-defined-secrets", + "githubServerTLS.certificateFrom.configMapKeyRef.name": "certs-configmap", + "githubServerTLS.certificateFrom.configMapKeyRef.key": "cert.pem", + "containerMode.type": "dind", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + } + + ars := render(t, options) + + require.NotNil(t, ars.Spec.GitHubServerTLS) + expected := &v1alpha1.GitHubServerTLSConfig{ + CertificateFrom: &v1alpha1.TLSCertificateSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "certs-configmap", + }, + Key: "cert.pem", + }, + }, + } + assert.Equal(t, expected, ars.Spec.GitHubServerTLS) + + var volume *corev1.Volume + for _, v := range ars.Spec.Template.Spec.Volumes { + if v.Name == "github-server-tls-cert" { + volume = &v + break + } + } + assert.Nil(t, volume) + + assert.NotContains(t, ars.Spec.Template.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{ + Name: "github-server-tls-cert", + MountPath: "/runner/mount/path/cert.pem", + SubPath: "cert.pem", + }) + + assert.NotContains(t, ars.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "NODE_EXTRA_CA_CERTS", + Value: "/runner/mount/path/cert.pem", + }) + + assert.NotContains(t, ars.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "RUNNER_UPDATE_CA_CERTS", + Value: "1", + }) + }) + + t.Run("mode: kubernetes", func(t *testing.T) { + options := &helm.Options{ + SetValues: map[string]string{ + "githubConfigUrl": "https://github.com/actions", + "githubConfigSecret": "pre-defined-secrets", + "githubServerTLS.certificateFrom.configMapKeyRef.name": "certs-configmap", + "githubServerTLS.certificateFrom.configMapKeyRef.key": "cert.pem", + "containerMode.type": "kubernetes", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + } + + ars := render(t, options) + + require.NotNil(t, ars.Spec.GitHubServerTLS) + expected := &v1alpha1.GitHubServerTLSConfig{ + CertificateFrom: &v1alpha1.TLSCertificateSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "certs-configmap", + }, + Key: "cert.pem", + }, + }, + } + assert.Equal(t, expected, ars.Spec.GitHubServerTLS) + + var volume *corev1.Volume + for _, v := range ars.Spec.Template.Spec.Volumes { + if v.Name == "github-server-tls-cert" { + volume = &v + break + } + } + assert.Nil(t, volume) + + assert.NotContains(t, ars.Spec.Template.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{ + Name: "github-server-tls-cert", + MountPath: "/runner/mount/path/cert.pem", + SubPath: "cert.pem", + }) + + assert.NotContains(t, ars.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "NODE_EXTRA_CA_CERTS", + Value: "/runner/mount/path/cert.pem", + }) + + assert.NotContains(t, ars.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "RUNNER_UPDATE_CA_CERTS", + Value: "1", + }) + }) + }) +} + func TestTemplateNamingConstraints(t *testing.T) { t.Parallel() diff --git a/charts/gha-runner-scale-set/values.yaml b/charts/gha-runner-scale-set/values.yaml index 8845daa8..94ea1d7d 100644 --- a/charts/gha-runner-scale-set/values.yaml +++ b/charts/gha-runner-scale-set/values.yaml @@ -4,7 +4,7 @@ githubConfigUrl: "" ## githubConfigSecret is the k8s secrets to use when auth with GitHub API. ## You can choose to use GitHub App or a PAT token -githubConfigSecret: +githubConfigSecret: ### GitHub Apps Configuration ## NOTE: IDs MUST be strings, use quotes #github_app_id: "" @@ -47,6 +47,27 @@ githubConfigSecret: ## name of the runner scale set to create. Defaults to the helm release name # runnerScaleSetName: "" +## A self-signed CA certificate for communication with the GitHub server can be +## provided using a config map key selector. If `runnerMountPath` is set, for +## each runner pod ARC will: +## - create a `github-server-tls-cert` volume containing the certificate +## specified in `certificateFrom` +## - mount that volume on path `runnerMountPath`/{certificate name} +## - set NODE_EXTRA_CA_CERTS environment variable to that same path +## - set RUNNER_UPDATE_CA_CERTS environment variable to "1" (as of version +## 2.303.0 this will instruct the runner to reload certificates on the host) +## +## If any of the above had already been set by the user in the runner pod +## template, ARC will observe those and not overwrite them. +## Example configuration: +# +# githubServerTLS: +# certificateFrom: +# configMapKeyRef: +# name: config-map-name +# key: ca.pem +# runnerMountPath: /usr/local/share/ca-certificates/ + ## template is the PodSpec for each runner Pod template: spec: @@ -139,4 +160,4 @@ containerMode: storageClassName: "dynamic-blob-storage" resources: requests: - storage: 1Gi \ No newline at end of file + storage: 1Gi diff --git a/cmd/githubrunnerscalesetlistener/main.go b/cmd/githubrunnerscalesetlistener/main.go index 3813617a..64abf6cf 100644 --- a/cmd/githubrunnerscalesetlistener/main.go +++ b/cmd/githubrunnerscalesetlistener/main.go @@ -18,6 +18,7 @@ package main import ( "context" + "crypto/x509" "fmt" "net/http" "net/url" @@ -44,6 +45,7 @@ type RunnerScaleSetListenerConfig struct { MaxRunners int `split_words:"true"` MinRunners int `split_words:"true"` RunnerScaleSetId int `split_words:"true"` + ServerRootCA string `split_words:"true"` } func main() { @@ -90,8 +92,8 @@ func run(rc RunnerScaleSetListenerConfig, logger logr.Logger) error { actionsServiceClient, err := newActionsClientFromConfig( rc, creds, - actions.WithUserAgent(fmt.Sprintf("actions-runner-controller/%s", build.Version)), actions.WithLogger(logger), + actions.WithUserAgent(fmt.Sprintf("actions-runner-controller/%s", build.Version)), ) if err != nil { return fmt.Errorf("failed to create an Actions Service client: %w", err) @@ -160,6 +162,20 @@ func validateConfig(config *RunnerScaleSetListenerConfig) error { } func newActionsClientFromConfig(config RunnerScaleSetListenerConfig, creds *actions.ActionsAuth, options ...actions.ClientOption) (*actions.Client, error) { + if config.ServerRootCA != "" { + systemPool, err := x509.SystemCertPool() + if err != nil { + return nil, fmt.Errorf("failed to load system cert pool: %w", err) + } + pool := systemPool.Clone() + ok := pool.AppendCertsFromPEM([]byte(config.ServerRootCA)) + if !ok { + return nil, fmt.Errorf("failed to parse root certificate") + } + + options = append(options, actions.WithRootCAs(pool)) + } + proxyFunc := httpproxy.FromEnvironment().ProxyFunc() options = append(options, actions.WithProxy(func(req *http.Request) (*url.URL, error) { return proxyFunc(req.URL) diff --git a/cmd/githubrunnerscalesetlistener/main_test.go b/cmd/githubrunnerscalesetlistener/main_test.go index 619bfeb6..e4c1df03 100644 --- a/cmd/githubrunnerscalesetlistener/main_test.go +++ b/cmd/githubrunnerscalesetlistener/main_test.go @@ -1,16 +1,20 @@ package main import ( + "context" + "crypto/tls" "fmt" "net/http" "net/http/httptest" "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/actions/actions-runner-controller/github/actions" + "github.com/actions/actions-runner-controller/github/actions/testserver" ) func TestConfigValidationMinMax(t *testing.T) { @@ -97,6 +101,54 @@ func TestConfigValidationConfigUrl(t *testing.T) { assert.ErrorContains(t, err, "GitHubConfigUrl is not provided", "Expected error about missing ConfigureUrl") } +func TestCustomerServerRootCA(t *testing.T) { + ctx := context.Background() + certsFolder := filepath.Join( + "../../", + "github", + "actions", + "testdata", + ) + certPath := filepath.Join(certsFolder, "server.crt") + keyPath := filepath.Join(certsFolder, "server.key") + + serverCalledSuccessfully := false + + server := testserver.NewUnstarted(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + serverCalledSuccessfully = true + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"count": 0}`)) + })) + cert, err := tls.LoadX509KeyPair(certPath, keyPath) + require.NoError(t, err) + + server.TLS = &tls.Config{Certificates: []tls.Certificate{cert}} + server.StartTLS() + + var certsString string + rootCA, err := os.ReadFile(filepath.Join(certsFolder, "rootCA.crt")) + require.NoError(t, err) + certsString = string(rootCA) + + intermediate, err := os.ReadFile(filepath.Join(certsFolder, "intermediate.pem")) + require.NoError(t, err) + certsString = certsString + string(intermediate) + + config := RunnerScaleSetListenerConfig{ + ConfigureUrl: server.ConfigURLForOrg("myorg"), + ServerRootCA: certsString, + } + creds := &actions.ActionsAuth{ + Token: "token", + } + + client, err := newActionsClientFromConfig(config, creds) + require.NoError(t, err) + _, err = client.GetRunnerScaleSet(ctx, "test") + require.NoError(t, err) + assert.True(t, serverCalledSuccessfully) +} + func TestProxySettings(t *testing.T) { t.Run("http", func(t *testing.T) { wentThroughProxy := false diff --git a/config/crd/bases/actions.github.com_autoscalinglisteners.yaml b/config/crd/bases/actions.github.com_autoscalinglisteners.yaml index f0f3f8fb..6df9c051 100644 --- a/config/crd/bases/actions.github.com_autoscalinglisteners.yaml +++ b/config/crd/bases/actions.github.com_autoscalinglisteners.yaml @@ -55,6 +55,28 @@ spec: githubConfigUrl: description: Required type: string + githubServerTLS: + properties: + certificateFrom: + description: Required + properties: + configMapKeyRef: + description: Required + properties: + key: + description: The key to select. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + optional: + description: Specify whether the ConfigMap or its key must be defined + type: boolean + required: + - key + type: object + type: object + type: object image: description: Required type: string diff --git a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml index 12c4b5b8..6c4c82cb 100644 --- a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml +++ b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml @@ -51,9 +51,25 @@ spec: type: string githubServerTLS: properties: - certConfigMapRef: + certificateFrom: description: Required - type: string + properties: + configMapKeyRef: + description: Required + properties: + key: + description: The key to select. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + optional: + description: Specify whether the ConfigMap or its key must be defined + type: boolean + required: + - key + type: object + type: object type: object maxRunners: minimum: 0 diff --git a/config/crd/bases/actions.github.com_ephemeralrunners.yaml b/config/crd/bases/actions.github.com_ephemeralrunners.yaml index 41cdc81b..b0ce1e4b 100644 --- a/config/crd/bases/actions.github.com_ephemeralrunners.yaml +++ b/config/crd/bases/actions.github.com_ephemeralrunners.yaml @@ -64,9 +64,25 @@ spec: type: string githubServerTLS: properties: - certConfigMapRef: + certificateFrom: description: Required - type: string + properties: + configMapKeyRef: + description: Required + properties: + key: + description: The key to select. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + optional: + description: Specify whether the ConfigMap or its key must be defined + type: boolean + required: + - key + type: object + type: object type: object metadata: description: 'Standard object''s metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata' diff --git a/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml b/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml index 072cd265..86a3f40c 100644 --- a/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml +++ b/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml @@ -46,9 +46,25 @@ spec: type: string githubServerTLS: properties: - certConfigMapRef: + certificateFrom: description: Required - type: string + properties: + configMapKeyRef: + description: Required + properties: + key: + description: The key to select. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + optional: + description: Specify whether the ConfigMap or its key must be defined + type: boolean + required: + - key + type: object + type: object type: object metadata: description: 'Standard object''s metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata' diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index 5ad3f0c3..e82fc8da 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller.go +++ b/controllers/actions.github.com/autoscalinglistener_controller.go @@ -423,6 +423,15 @@ func (r *AutoscalingListenerReconciler) createListenerPod(ctx context.Context, a } } + if autoscalingListener.Spec.GitHubServerTLS != nil { + env, err := r.certificateEnvVarForListener(ctx, autoscalingRunnerSet, autoscalingListener) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to create certificate env var for listener: %v", err) + } + + envs = append(envs, env) + } + newPod := r.resourceBuilder.newScaleSetListenerPod(autoscalingListener, serviceAccount, secret, envs...) if err := ctrl.SetControllerReference(autoscalingListener, newPod, r.Scheme); err != nil { @@ -439,6 +448,47 @@ func (r *AutoscalingListenerReconciler) createListenerPod(ctx context.Context, a return ctrl.Result{}, nil } +func (r *AutoscalingListenerReconciler) certificateEnvVarForListener(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, autoscalingListener *v1alpha1.AutoscalingListener) (corev1.EnvVar, error) { + if autoscalingListener.Spec.GitHubServerTLS.CertificateFrom == nil { + return corev1.EnvVar{}, fmt.Errorf("githubServerTLS.certificateFrom is not specified") + } + + if autoscalingListener.Spec.GitHubServerTLS.CertificateFrom.ConfigMapKeyRef == nil { + return corev1.EnvVar{}, fmt.Errorf("githubServerTLS.certificateFrom.configMapKeyRef is not specified") + } + + var configmap corev1.ConfigMap + err := r.Get( + ctx, + types.NamespacedName{ + Namespace: autoscalingRunnerSet.Namespace, + Name: autoscalingListener.Spec.GitHubServerTLS.CertificateFrom.ConfigMapKeyRef.Name, + }, + &configmap, + ) + if err != nil { + return corev1.EnvVar{}, fmt.Errorf( + "failed to get configmap %s: %w", + autoscalingListener.Spec.GitHubServerTLS.CertificateFrom.ConfigMapKeyRef.Name, + err, + ) + } + + certificate, ok := configmap.Data[autoscalingListener.Spec.GitHubServerTLS.CertificateFrom.ConfigMapKeyRef.Key] + if !ok { + return corev1.EnvVar{}, fmt.Errorf( + "key %s is not found in configmap %s", + autoscalingListener.Spec.GitHubServerTLS.CertificateFrom.ConfigMapKeyRef.Key, + autoscalingListener.Spec.GitHubServerTLS.CertificateFrom.ConfigMapKeyRef.Name, + ) + } + + return corev1.EnvVar{ + Name: "GITHUB_SERVER_ROOT_CA", + Value: certificate, + }, nil +} + func (r *AutoscalingListenerReconciler) createSecretsForListener(ctx context.Context, autoscalingListener *v1alpha1.AutoscalingListener, secret *corev1.Secret, logger logr.Logger) (ctrl.Result, error) { newListenerSecret := r.resourceBuilder.newScaleSetListenerSecretMirror(autoscalingListener, secret) diff --git a/controllers/actions.github.com/autoscalinglistener_controller_test.go b/controllers/actions.github.com/autoscalinglistener_controller_test.go index 487d9eb6..d4932797 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller_test.go +++ b/controllers/actions.github.com/autoscalinglistener_controller_test.go @@ -3,6 +3,8 @@ package actionsgithubcom import ( "context" "fmt" + "os" + "path/filepath" "time" corev1 "k8s.io/api/core/v1" @@ -554,3 +556,165 @@ var _ = Describe("Test AutoScalingListener controller with proxy", func() { autoscalingListenerTestInterval).Should(Succeed(), "failed to delete secret with proxy details") }) }) + +var _ = Describe("Test GitHub Server TLS configuration", func() { + var ctx context.Context + var mgr ctrl.Manager + var autoscalingNS *corev1.Namespace + var autoscalingRunnerSet *actionsv1alpha1.AutoscalingRunnerSet + var configSecret *corev1.Secret + var autoscalingListener *actionsv1alpha1.AutoscalingListener + var rootCAConfigMap *corev1.ConfigMap + + BeforeEach(func() { + ctx = context.Background() + autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient) + configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) + + cert, err := os.ReadFile(filepath.Join( + "../../", + "github", + "actions", + "testdata", + "rootCA.crt", + )) + Expect(err).NotTo(HaveOccurred(), "failed to read root CA cert") + rootCAConfigMap = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "root-ca-configmap", + Namespace: autoscalingNS.Name, + }, + Data: map[string]string{ + "rootCA.crt": string(cert), + }, + } + err = k8sClient.Create(ctx, rootCAConfigMap) + Expect(err).NotTo(HaveOccurred(), "failed to create configmap with root CAs") + + controller := &AutoscalingListenerReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + } + err = controller.SetupWithManager(mgr) + Expect(err).NotTo(HaveOccurred(), "failed to setup controller") + + min := 1 + max := 10 + autoscalingRunnerSet = &actionsv1alpha1.AutoscalingRunnerSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-asrs", + Namespace: autoscalingNS.Name, + }, + Spec: actionsv1alpha1.AutoscalingRunnerSetSpec{ + GitHubConfigUrl: "https://github.com/owner/repo", + GitHubConfigSecret: configSecret.Name, + GitHubServerTLS: &actionsv1alpha1.GitHubServerTLSConfig{ + CertificateFrom: &actionsv1alpha1.TLSCertificateSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: rootCAConfigMap.Name, + }, + Key: "rootCA.crt", + }, + }, + }, + MaxRunners: &max, + MinRunners: &min, + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "runner", + Image: "ghcr.io/actions/runner", + }, + }, + }, + }, + }, + } + + err = k8sClient.Create(ctx, autoscalingRunnerSet) + Expect(err).NotTo(HaveOccurred(), "failed to create AutoScalingRunnerSet") + + autoscalingListener = &actionsv1alpha1.AutoscalingListener{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-asl", + Namespace: autoscalingNS.Name, + }, + Spec: actionsv1alpha1.AutoscalingListenerSpec{ + GitHubConfigUrl: "https://github.com/owner/repo", + GitHubConfigSecret: configSecret.Name, + GitHubServerTLS: &actionsv1alpha1.GitHubServerTLSConfig{ + CertificateFrom: &actionsv1alpha1.TLSCertificateSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: rootCAConfigMap.Name, + }, + Key: "rootCA.crt", + }, + }, + }, + RunnerScaleSetId: 1, + AutoscalingRunnerSetNamespace: autoscalingRunnerSet.Namespace, + AutoscalingRunnerSetName: autoscalingRunnerSet.Name, + EphemeralRunnerSetName: "test-ers", + MaxRunners: 10, + MinRunners: 1, + Image: "ghcr.io/owner/repo", + }, + } + + err = k8sClient.Create(ctx, autoscalingListener) + Expect(err).NotTo(HaveOccurred(), "failed to create AutoScalingListener") + + startManagers(GinkgoT(), mgr) + }) + + Context("When creating a new AutoScalingListener", func() { + It("It should set the certificates as an environment variable on the pod", func() { + pod := new(corev1.Pod) + Eventually( + func(g Gomega) { + err := k8sClient.Get( + ctx, + client.ObjectKey{ + Name: autoscalingListener.Name, + Namespace: autoscalingListener.Namespace, + }, + pod, + ) + + g.Expect(err).NotTo(HaveOccurred(), "failed to get pod") + g.Expect(pod.Spec.Containers).NotTo(BeEmpty(), "pod should have containers") + g.Expect(pod.Spec.Containers[0].Env).NotTo(BeEmpty(), "pod should have env variables") + + var env *corev1.EnvVar + for _, e := range pod.Spec.Containers[0].Env { + if e.Name == "GITHUB_SERVER_ROOT_CA" { + env = &e + break + } + } + g.Expect(env).NotTo(BeNil(), "pod should have an env variable named GITHUB_SERVER_ROOT_CA_PATH") + + cert, err := os.ReadFile(filepath.Join( + "../../", + "github", + "actions", + "testdata", + "rootCA.crt", + )) + g.Expect(err).NotTo(HaveOccurred(), "failed to read rootCA.crt") + + g.Expect(env.Value).To( + BeEquivalentTo(string(cert)), + "GITHUB_SERVER_ROOT_CA should be the rootCA.crt", + ) + }). + WithTimeout(autoscalingRunnerSetTestTimeout). + WithPolling(autoscalingListenerTestInterval). + Should(Succeed(), "failed to create pod with volume and env variable") + }) + }) +}) diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go index 1c77acd9..c7e95201 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -541,7 +541,23 @@ func (r *AutoscalingRunnerSetReconciler) actionsClientFor(ctx context.Context, a return nil, fmt.Errorf("failed to find GitHub config secret: %w", err) } - var opts []actions.ClientOption + opts, err := r.actionsClientOptionsFor(ctx, autoscalingRunnerSet) + if err != nil { + return nil, fmt.Errorf("failed to get actions client options: %w", err) + } + + return r.ActionsClient.GetClientFromSecret( + ctx, + autoscalingRunnerSet.Spec.GitHubConfigUrl, + autoscalingRunnerSet.Namespace, + configSecret.Data, + opts..., + ) +} + +func (r *AutoscalingRunnerSetReconciler) actionsClientOptionsFor(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet) ([]actions.ClientOption, error) { + var options []actions.ClientOption + if autoscalingRunnerSet.Spec.Proxy != nil { proxyFunc, err := autoscalingRunnerSet.Spec.Proxy.ProxyFunc(func(s string) (*corev1.Secret, error) { var secret corev1.Secret @@ -556,16 +572,35 @@ func (r *AutoscalingRunnerSetReconciler) actionsClientFor(ctx context.Context, a return nil, fmt.Errorf("failed to get proxy func: %w", err) } - opts = append(opts, actions.WithProxy(proxyFunc)) + options = append(options, actions.WithProxy(proxyFunc)) } - return r.ActionsClient.GetClientFromSecret( - ctx, - autoscalingRunnerSet.Spec.GitHubConfigUrl, - autoscalingRunnerSet.Namespace, - configSecret.Data, - opts..., - ) + tlsConfig := autoscalingRunnerSet.Spec.GitHubServerTLS + if tlsConfig != nil { + pool, err := tlsConfig.ToCertPool(func(name, key string) ([]byte, error) { + var configmap corev1.ConfigMap + err := r.Get( + ctx, + types.NamespacedName{ + Namespace: autoscalingRunnerSet.Namespace, + Name: name, + }, + &configmap, + ) + if err != nil { + return nil, fmt.Errorf("failed to get configmap %s: %w", name, err) + } + + return []byte(configmap.Data[key]), nil + }) + if err != nil { + return nil, fmt.Errorf("failed to get tls config: %w", err) + } + + options = append(options, actions.WithRootCAs(pool)) + } + + return options, nil } // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go index e999fc7e..aa2ae57d 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go @@ -2,10 +2,13 @@ package actionsgithubcom import ( "context" + "crypto/tls" "encoding/base64" "fmt" "net/http" "net/http/httptest" + "os" + "path/filepath" "strings" "time" @@ -787,4 +790,242 @@ var _ = Describe("Test Client optional configuration", func() { ).Should(BeTrue(), "server was not called") }) }) + + Context("When specifying a configmap for root CAs", func() { + var ctx context.Context + var mgr ctrl.Manager + var autoscalingNS *corev1.Namespace + var configSecret *corev1.Secret + var rootCAConfigMap *corev1.ConfigMap + var controller *AutoscalingRunnerSetReconciler + + BeforeEach(func() { + ctx = context.Background() + autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient) + configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) + + cert, err := os.ReadFile(filepath.Join( + "../../", + "github", + "actions", + "testdata", + "rootCA.crt", + )) + Expect(err).NotTo(HaveOccurred(), "failed to read root CA cert") + rootCAConfigMap = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "root-ca-configmap", + Namespace: autoscalingNS.Name, + }, + Data: map[string]string{ + "rootCA.crt": string(cert), + }, + } + err = k8sClient.Create(ctx, rootCAConfigMap) + Expect(err).NotTo(HaveOccurred(), "failed to create configmap with root CAs") + + controller = &AutoscalingRunnerSetReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + ControllerNamespace: autoscalingNS.Name, + DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", + ActionsClient: fake.NewMultiClient(), + } + err = controller.SetupWithManager(mgr) + Expect(err).NotTo(HaveOccurred(), "failed to setup controller") + + startManagers(GinkgoT(), mgr) + }) + + It("should be able to make requests to a server using root CAs", func() { + controller.ActionsClient = actions.NewMultiClient("test", logr.Discard()) + + certsFolder := filepath.Join( + "../../", + "github", + "actions", + "testdata", + ) + certPath := filepath.Join(certsFolder, "server.crt") + keyPath := filepath.Join(certsFolder, "server.key") + + serverSuccessfullyCalled := false + server := testserver.NewUnstarted(GinkgoT(), http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + serverSuccessfullyCalled = true + w.WriteHeader(http.StatusOK) + })) + cert, err := tls.LoadX509KeyPair(certPath, keyPath) + Expect(err).NotTo(HaveOccurred(), "failed to load server cert") + + server.TLS = &tls.Config{Certificates: []tls.Certificate{cert}} + server.StartTLS() + + min := 1 + max := 10 + autoscalingRunnerSet := &v1alpha1.AutoscalingRunnerSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-asrs", + Namespace: autoscalingNS.Name, + }, + Spec: v1alpha1.AutoscalingRunnerSetSpec{ + GitHubConfigUrl: server.ConfigURLForOrg("my-org"), + GitHubConfigSecret: configSecret.Name, + GitHubServerTLS: &v1alpha1.GitHubServerTLSConfig{ + CertificateFrom: &v1alpha1.TLSCertificateSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: rootCAConfigMap.Name, + }, + Key: "rootCA.crt", + }, + }, + }, + MaxRunners: &max, + MinRunners: &min, + RunnerGroup: "testgroup", + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "runner", + Image: "ghcr.io/actions/runner", + }, + }, + }, + }, + }, + } + + err = k8sClient.Create(ctx, autoscalingRunnerSet) + Expect(err).NotTo(HaveOccurred(), "failed to create AutoScalingRunnerSet") + + // wait for server to be called + Eventually( + func() (bool, error) { + return serverSuccessfullyCalled, nil + }, + autoscalingRunnerSetTestTimeout, + 1*time.Nanosecond, + ).Should(BeTrue(), "server was not called") + }) + + It("it creates a listener referencing the right configmap for TLS", func() { + min := 1 + max := 10 + autoscalingRunnerSet := &v1alpha1.AutoscalingRunnerSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-asrs", + Namespace: autoscalingNS.Name, + }, + Spec: v1alpha1.AutoscalingRunnerSetSpec{ + GitHubConfigUrl: "https://github.com/owner/repo", + GitHubConfigSecret: configSecret.Name, + GitHubServerTLS: &v1alpha1.GitHubServerTLSConfig{ + CertificateFrom: &v1alpha1.TLSCertificateSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: rootCAConfigMap.Name, + }, + Key: "rootCA.crt", + }, + }, + }, + MaxRunners: &max, + MinRunners: &min, + RunnerGroup: "testgroup", + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "runner", + Image: "ghcr.io/actions/runner", + }, + }, + }, + }, + }, + } + + err := k8sClient.Create(ctx, autoscalingRunnerSet) + Expect(err).NotTo(HaveOccurred(), "failed to create AutoScalingRunnerSet") + + Eventually( + func(g Gomega) { + listener := new(v1alpha1.AutoscalingListener) + err := k8sClient.Get( + ctx, + client.ObjectKey{ + Name: scaleSetListenerName(autoscalingRunnerSet), + Namespace: autoscalingRunnerSet.Namespace, + }, + listener, + ) + g.Expect(err).NotTo(HaveOccurred(), "failed to get listener") + + g.Expect(listener.Spec.GitHubServerTLS).NotTo(BeNil(), "listener does not have TLS config") + g.Expect(listener.Spec.GitHubServerTLS).To(BeEquivalentTo(autoscalingRunnerSet.Spec.GitHubServerTLS), "listener does not have TLS config") + }, + autoscalingRunnerSetTestTimeout, + autoscalingListenerTestInterval, + ).Should(Succeed(), "tls config is incorrect") + }) + + It("it creates an ephemeral runner set referencing the right configmap for TLS", func() { + min := 1 + max := 10 + autoscalingRunnerSet := &v1alpha1.AutoscalingRunnerSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-asrs", + Namespace: autoscalingNS.Name, + }, + Spec: v1alpha1.AutoscalingRunnerSetSpec{ + GitHubConfigUrl: "https://github.com/owner/repo", + GitHubConfigSecret: configSecret.Name, + GitHubServerTLS: &v1alpha1.GitHubServerTLSConfig{ + CertificateFrom: &v1alpha1.TLSCertificateSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: rootCAConfigMap.Name, + }, + Key: "rootCA.crt", + }, + }, + }, + MaxRunners: &max, + MinRunners: &min, + RunnerGroup: "testgroup", + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "runner", + Image: "ghcr.io/actions/runner", + }, + }, + }, + }, + }, + } + + err := k8sClient.Create(ctx, autoscalingRunnerSet) + Expect(err).NotTo(HaveOccurred(), "failed to create AutoScalingRunnerSet") + + Eventually( + func(g Gomega) { + runnerSetList := new(v1alpha1.EphemeralRunnerSetList) + err := k8sClient.List(ctx, runnerSetList, client.InNamespace(autoscalingRunnerSet.Namespace)) + g.Expect(err).NotTo(HaveOccurred(), "failed to list EphemeralRunnerSet") + g.Expect(runnerSetList.Items).To(HaveLen(1), "expected 1 EphemeralRunnerSet to be created") + + runnerSet := &runnerSetList.Items[0] + + g.Expect(runnerSet.Spec.EphemeralRunnerSpec.GitHubServerTLS).NotTo(BeNil(), "expected EphemeralRunnerSpec.GitHubServerTLS to be set") + g.Expect(runnerSet.Spec.EphemeralRunnerSpec.GitHubServerTLS).To(BeEquivalentTo(autoscalingRunnerSet.Spec.GitHubServerTLS), "EphemeralRunnerSpec does not have TLS config") + }, + autoscalingRunnerSetTestTimeout, + autoscalingListenerTestInterval, + ).Should(Succeed()) + }) + }) }) diff --git a/controllers/actions.github.com/ephemeralrunner_controller.go b/controllers/actions.github.com/ephemeralrunner_controller.go index 516526a3..f697091b 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller.go +++ b/controllers/actions.github.com/ephemeralrunner_controller.go @@ -680,6 +680,21 @@ func (r *EphemeralRunnerReconciler) actionsClientFor(ctx context.Context, runner return nil, fmt.Errorf("failed to get secret: %w", err) } + opts, err := r.actionsClientOptionsFor(ctx, runner) + if err != nil { + return nil, fmt.Errorf("failed to get actions client options: %w", err) + } + + return r.ActionsClient.GetClientFromSecret( + ctx, + runner.Spec.GitHubConfigUrl, + runner.Namespace, + secret.Data, + opts..., + ) +} + +func (r *EphemeralRunnerReconciler) actionsClientOptionsFor(ctx context.Context, runner *v1alpha1.EphemeralRunner) ([]actions.ClientOption, error) { var opts []actions.ClientOption if runner.Spec.Proxy != nil { proxyFunc, err := runner.Spec.Proxy.ProxyFunc(func(s string) (*corev1.Secret, error) { @@ -698,13 +713,32 @@ func (r *EphemeralRunnerReconciler) actionsClientFor(ctx context.Context, runner opts = append(opts, actions.WithProxy(proxyFunc)) } - return r.ActionsClient.GetClientFromSecret( - ctx, - runner.Spec.GitHubConfigUrl, - runner.Namespace, - secret.Data, - opts..., - ) + tlsConfig := runner.Spec.GitHubServerTLS + if tlsConfig != nil { + pool, err := tlsConfig.ToCertPool(func(name, key string) ([]byte, error) { + var configmap corev1.ConfigMap + err := r.Get( + ctx, + types.NamespacedName{ + Namespace: runner.Namespace, + Name: name, + }, + &configmap, + ) + if err != nil { + return nil, fmt.Errorf("failed to get configmap %s: %w", name, err) + } + + return []byte(configmap.Data[key]), nil + }) + if err != nil { + return nil, fmt.Errorf("failed to get tls config: %w", err) + } + + opts = append(opts, actions.WithRootCAs(pool)) + } + + return opts, nil } // runnerRegisteredWithService checks if the runner is still registered with the service diff --git a/controllers/actions.github.com/ephemeralrunner_controller_test.go b/controllers/actions.github.com/ephemeralrunner_controller_test.go index b5e064b1..03086099 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunner_controller_test.go @@ -2,10 +2,13 @@ package actionsgithubcom import ( "context" + "crypto/tls" "encoding/base64" "fmt" "net/http" "net/http/httptest" + "os" + "path/filepath" "strings" "time" @@ -14,6 +17,7 @@ import ( "github.com/go-logr/logr" "github.com/actions/actions-runner-controller/github/actions/fake" + "github.com/actions/actions-runner-controller/github/actions/testserver" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" @@ -841,4 +845,100 @@ var _ = Describe("EphemeralRunner", func() { })) }) }) + + Describe("TLS config", func() { + var ctx context.Context + var mgr ctrl.Manager + var autoScalingNS *corev1.Namespace + var configSecret *corev1.Secret + var controller *EphemeralRunnerReconciler + var rootCAConfigMap *corev1.ConfigMap + + BeforeEach(func() { + ctx = context.Background() + autoScalingNS, mgr = createNamespace(GinkgoT(), k8sClient) + configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoScalingNS.Name) + + cert, err := os.ReadFile(filepath.Join( + "../../", + "github", + "actions", + "testdata", + "rootCA.crt", + )) + Expect(err).NotTo(HaveOccurred(), "failed to read root CA cert") + rootCAConfigMap = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "root-ca-configmap", + Namespace: autoScalingNS.Name, + }, + Data: map[string]string{ + "rootCA.crt": string(cert), + }, + } + err = k8sClient.Create(ctx, rootCAConfigMap) + Expect(err).NotTo(HaveOccurred(), "failed to create configmap with root CAs") + + controller = &EphemeralRunnerReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + ActionsClient: fake.NewMultiClient(), + } + + err = controller.SetupWithManager(mgr) + Expect(err).To(BeNil(), "failed to setup controller") + + startManagers(GinkgoT(), mgr) + }) + + It("should be able to make requests to a server using root CAs", func() { + certsFolder := filepath.Join( + "../../", + "github", + "actions", + "testdata", + ) + certPath := filepath.Join(certsFolder, "server.crt") + keyPath := filepath.Join(certsFolder, "server.key") + + serverSuccessfullyCalled := false + server := testserver.NewUnstarted(GinkgoT(), http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + serverSuccessfullyCalled = true + w.WriteHeader(http.StatusOK) + })) + cert, err := tls.LoadX509KeyPair(certPath, keyPath) + Expect(err).NotTo(HaveOccurred(), "failed to load server cert") + + server.TLS = &tls.Config{Certificates: []tls.Certificate{cert}} + server.StartTLS() + + // Use an actual client + controller.ActionsClient = actions.NewMultiClient("test", logr.Discard()) + + ephemeralRunner := newExampleRunner("test-runner", autoScalingNS.Name, configSecret.Name) + ephemeralRunner.Spec.GitHubConfigUrl = server.ConfigURLForOrg("my-org") + ephemeralRunner.Spec.GitHubServerTLS = &v1alpha1.GitHubServerTLSConfig{ + CertificateFrom: &v1alpha1.TLSCertificateSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: rootCAConfigMap.Name, + }, + Key: "rootCA.crt", + }, + }, + } + + err = k8sClient.Create(ctx, ephemeralRunner) + Expect(err).To(BeNil(), "failed to create ephemeral runner") + + Eventually( + func() bool { + return serverSuccessfullyCalled + }, + 2*time.Second, + interval, + ).Should(BeTrue(), "failed to contact server") + }) + }) }) diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller.go b/controllers/actions.github.com/ephemeralrunnerset_controller.go index 29f40e0d..0db58409 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller.go @@ -450,6 +450,22 @@ func (r *EphemeralRunnerSetReconciler) actionsClientFor(ctx context.Context, rs if err := r.Get(ctx, types.NamespacedName{Namespace: rs.Namespace, Name: rs.Spec.EphemeralRunnerSpec.GitHubConfigSecret}, secret); err != nil { return nil, fmt.Errorf("failed to get secret: %w", err) } + + opts, err := r.actionsClientOptionsFor(ctx, rs) + if err != nil { + return nil, fmt.Errorf("failed to get actions client options: %w", err) + } + + return r.ActionsClient.GetClientFromSecret( + ctx, + rs.Spec.EphemeralRunnerSpec.GitHubConfigUrl, + rs.Namespace, + secret.Data, + opts..., + ) +} + +func (r *EphemeralRunnerSetReconciler) actionsClientOptionsFor(ctx context.Context, rs *v1alpha1.EphemeralRunnerSet) ([]actions.ClientOption, error) { var opts []actions.ClientOption if rs.Spec.EphemeralRunnerSpec.Proxy != nil { proxyFunc, err := rs.Spec.EphemeralRunnerSpec.Proxy.ProxyFunc(func(s string) (*corev1.Secret, error) { @@ -468,13 +484,32 @@ func (r *EphemeralRunnerSetReconciler) actionsClientFor(ctx context.Context, rs opts = append(opts, actions.WithProxy(proxyFunc)) } - return r.ActionsClient.GetClientFromSecret( - ctx, - rs.Spec.EphemeralRunnerSpec.GitHubConfigUrl, - rs.Namespace, - secret.Data, - opts..., - ) + tlsConfig := rs.Spec.EphemeralRunnerSpec.GitHubServerTLS + if tlsConfig != nil { + pool, err := tlsConfig.ToCertPool(func(name, key string) ([]byte, error) { + var configmap corev1.ConfigMap + err := r.Get( + ctx, + types.NamespacedName{ + Namespace: rs.Namespace, + Name: name, + }, + &configmap, + ) + if err != nil { + return nil, fmt.Errorf("failed to get configmap %s: %w", name, err) + } + + return []byte(configmap.Data[key]), nil + }) + if err != nil { + return nil, fmt.Errorf("failed to get tls config: %w", err) + } + + opts = append(opts, actions.WithRootCAs(pool)) + } + + return opts, nil } // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go index e2ad842b..d51698ab 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go @@ -2,10 +2,13 @@ package actionsgithubcom import ( "context" + "crypto/tls" "encoding/base64" "fmt" "net/http" "net/http/httptest" + "os" + "path/filepath" "strings" "time" @@ -24,6 +27,7 @@ import ( v1alpha1 "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" "github.com/actions/actions-runner-controller/github/actions" "github.com/actions/actions-runner-controller/github/actions/fake" + "github.com/actions/actions-runner-controller/github/actions/testserver" ) const ( @@ -834,3 +838,148 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func( ).Should(BeEquivalentTo(true)) }) }) + +var _ = Describe("Test EphemeralRunnerSet controller with custom root CA", func() { + var ctx context.Context + var mgr ctrl.Manager + var autoscalingNS *corev1.Namespace + var ephemeralRunnerSet *actionsv1alpha1.EphemeralRunnerSet + var configSecret *corev1.Secret + var rootCAConfigMap *corev1.ConfigMap + + BeforeEach(func() { + ctx = context.Background() + autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient) + configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) + + cert, err := os.ReadFile(filepath.Join( + "../../", + "github", + "actions", + "testdata", + "rootCA.crt", + )) + Expect(err).NotTo(HaveOccurred(), "failed to read root CA cert") + rootCAConfigMap = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "root-ca-configmap", + Namespace: autoscalingNS.Name, + }, + Data: map[string]string{ + "rootCA.crt": string(cert), + }, + } + err = k8sClient.Create(ctx, rootCAConfigMap) + Expect(err).NotTo(HaveOccurred(), "failed to create configmap with root CAs") + + controller := &EphemeralRunnerSetReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + ActionsClient: actions.NewMultiClient("test", logr.Discard()), + } + err = controller.SetupWithManager(mgr) + Expect(err).NotTo(HaveOccurred(), "failed to setup controller") + + startManagers(GinkgoT(), mgr) + }) + + It("should be able to make requests to a server using root CAs", func() { + certsFolder := filepath.Join( + "../../", + "github", + "actions", + "testdata", + ) + certPath := filepath.Join(certsFolder, "server.crt") + keyPath := filepath.Join(certsFolder, "server.key") + + serverSuccessfullyCalled := false + server := testserver.NewUnstarted(GinkgoT(), http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + serverSuccessfullyCalled = true + w.WriteHeader(http.StatusOK) + })) + cert, err := tls.LoadX509KeyPair(certPath, keyPath) + Expect(err).NotTo(HaveOccurred(), "failed to load server cert") + + server.TLS = &tls.Config{Certificates: []tls.Certificate{cert}} + server.StartTLS() + + ephemeralRunnerSet = &actionsv1alpha1.EphemeralRunnerSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-asrs", + Namespace: autoscalingNS.Name, + }, + Spec: actionsv1alpha1.EphemeralRunnerSetSpec{ + Replicas: 1, + EphemeralRunnerSpec: actionsv1alpha1.EphemeralRunnerSpec{ + GitHubConfigUrl: server.ConfigURLForOrg("my-org"), + GitHubConfigSecret: configSecret.Name, + GitHubServerTLS: &actionsv1alpha1.GitHubServerTLSConfig{ + CertificateFrom: &v1alpha1.TLSCertificateSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: rootCAConfigMap.Name, + }, + Key: "rootCA.crt", + }, + }, + }, + RunnerScaleSetId: 100, + PodTemplateSpec: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "runner", + Image: "ghcr.io/actions/runner", + }, + }, + }, + }, + }, + }, + } + + err = k8sClient.Create(ctx, ephemeralRunnerSet) + Expect(err).NotTo(HaveOccurred(), "failed to create EphemeralRunnerSet") + + runnerList := new(actionsv1alpha1.EphemeralRunnerList) + Eventually(func() (int, error) { + err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + if err != nil { + return -1, err + } + + return len(runnerList.Items), nil + }, + ephemeralRunnerSetTestTimeout, + ephemeralRunnerSetTestInterval, + ).Should(BeEquivalentTo(1), "failed to create ephemeral runner") + + runner := runnerList.Items[0].DeepCopy() + Expect(runner.Spec.GitHubServerTLS).NotTo(BeNil(), "runner tls config should not be nil") + Expect(runner.Spec.GitHubServerTLS).To(BeEquivalentTo(ephemeralRunnerSet.Spec.EphemeralRunnerSpec.GitHubServerTLS), "runner tls config should be correct") + + runner.Status.Phase = corev1.PodRunning + runner.Status.RunnerId = 100 + err = k8sClient.Status().Patch(ctx, runner, client.MergeFrom(&runnerList.Items[0])) + Expect(err).NotTo(HaveOccurred(), "failed to update ephemeral runner status") + + updatedRunnerSet := new(actionsv1alpha1.EphemeralRunnerSet) + err = k8sClient.Get(ctx, client.ObjectKey{Namespace: ephemeralRunnerSet.Namespace, Name: ephemeralRunnerSet.Name}, updatedRunnerSet) + Expect(err).NotTo(HaveOccurred(), "failed to get EphemeralRunnerSet") + + updatedRunnerSet.Spec.Replicas = 0 + err = k8sClient.Update(ctx, updatedRunnerSet) + Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunnerSet") + + // wait for server to be called + Eventually( + func() bool { + return serverSuccessfullyCalled + }, + autoscalingRunnerSetTestTimeout, + 1*time.Nanosecond, + ).Should(BeTrue(), "server was not called") + }) +}) diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index b8f7512d..dd555289 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/controllers/actions.github.com/resourcebuilder.go @@ -307,6 +307,7 @@ func (b *resourceBuilder) newAutoScalingListener(autoscalingRunnerSet *v1alpha1. Image: image, ImagePullSecrets: imagePullSecrets, Proxy: autoscalingRunnerSet.Spec.Proxy, + GitHubServerTLS: autoscalingRunnerSet.Spec.GitHubServerTLS, }, } diff --git a/controllers/actions.github.com/suite_test.go b/controllers/actions.github.com/suite_test.go index c0a12a37..80fb4196 100644 --- a/controllers/actions.github.com/suite_test.go +++ b/controllers/actions.github.com/suite_test.go @@ -39,9 +39,11 @@ import ( // These tests use Ginkgo (BDD-style Go testing framework). Refer to // http://onsi.github.io/ginkgo/ to learn more about Ginkgo. -var cfg *rest.Config -var k8sClient client.Client -var testEnv *envtest.Environment +var ( + cfg *rest.Config + k8sClient client.Client + testEnv *envtest.Environment +) func TestAPIs(t *testing.T) { RegisterFailHandler(Fail) diff --git a/github/actions/actions_server_test.go b/github/actions/actions_server_test.go index 2638435a..e2580bd4 100644 --- a/github/actions/actions_server_test.go +++ b/github/actions/actions_server_test.go @@ -58,12 +58,6 @@ func newActionsServer(t *testing.T, handler http.Handler, options ...actionsServ type actionsServerOption func(*actionsServer) -func withActionsToken(token string) actionsServerOption { - return func(s *actionsServer) { - s.token = token - } -} - type actionsServer struct { *httptest.Server diff --git a/github/actions/client.go b/github/actions/client.go index 4574b354..7d68bd39 100644 --- a/github/actions/client.go +++ b/github/actions/client.go @@ -198,6 +198,12 @@ func (c *Client) Identifier() string { ) } + if c.rootCAs != nil { + // ignoring because this cert pool is intended not to come from SystemCertPool + // nolint:staticcheck + identifier += fmt.Sprintf("rootCAs:%q", c.rootCAs.Subjects()) + } + return uuid.NewHash(sha256.New(), uuid.NameSpaceOID, []byte(identifier), 6).String() } diff --git a/github/actions/client_tls_test.go b/github/actions/client_tls_test.go index 5e7190b5..297339c0 100644 --- a/github/actions/client_tls_test.go +++ b/github/actions/client_tls_test.go @@ -95,8 +95,8 @@ func TestServerWithSelfSignedCertificates(t *testing.T) { cert, err := os.ReadFile(filepath.Join("testdata", "rootCA.crt")) require.NoError(t, err) - pool, err := actions.RootCAsFromConfigMap(map[string][]byte{"cert": cert}) - require.NoError(t, err) + pool := x509.NewCertPool() + require.True(t, pool.AppendCertsFromPEM(cert)) client, err := actions.NewClient(configURL, auth, actions.WithRootCAs(pool)) require.NoError(t, err) @@ -123,8 +123,8 @@ func TestServerWithSelfSignedCertificates(t *testing.T) { cert, err := os.ReadFile(filepath.Join("testdata", "intermediate.pem")) require.NoError(t, err) - pool, err := actions.RootCAsFromConfigMap(map[string][]byte{"cert": cert}) - require.NoError(t, err) + pool := x509.NewCertPool() + require.True(t, pool.AppendCertsFromPEM(cert)) client, err := actions.NewClient(configURL, auth, actions.WithRootCAs(pool), actions.WithRetryMax(0)) require.NoError(t, err) diff --git a/github/actions/github_api_request_test.go b/github/actions/github_api_request_test.go index 3a378149..fef7b58f 100644 --- a/github/actions/github_api_request_test.go +++ b/github/actions/github_api_request_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/actions/actions-runner-controller/github/actions" + "github.com/actions/actions-runner-controller/github/actions/testserver" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -95,9 +96,9 @@ func TestNewActionsServiceRequest(t *testing.T) { t.Run("manages authentication", func(t *testing.T) { t.Run("client is brand new", func(t *testing.T) { token := defaultActionsToken(t) - server := newActionsServer(t, nil, withActionsToken(token)) + server := testserver.New(t, nil, testserver.WithActionsToken(token)) - client, err := actions.NewClient(server.configURLForOrg("my-org"), defaultCreds) + client, err := actions.NewClient(server.ConfigURLForOrg("my-org"), defaultCreds) require.NoError(t, err) req, err := client.NewActionsServiceRequest(ctx, http.MethodGet, "my-path", nil) @@ -108,9 +109,9 @@ func TestNewActionsServiceRequest(t *testing.T) { t.Run("admin token is about to expire", func(t *testing.T) { newToken := defaultActionsToken(t) - server := newActionsServer(t, nil, withActionsToken(newToken)) + server := testserver.New(t, nil, testserver.WithActionsToken(newToken)) - client, err := actions.NewClient(server.configURLForOrg("my-org"), defaultCreds) + client, err := actions.NewClient(server.ConfigURLForOrg("my-org"), defaultCreds) require.NoError(t, err) client.ActionsServiceAdminToken = "expiring-token" client.ActionsServiceAdminTokenExpiresAt = time.Now().Add(59 * time.Second) @@ -123,9 +124,9 @@ func TestNewActionsServiceRequest(t *testing.T) { t.Run("token is currently valid", func(t *testing.T) { tokenThatShouldNotBeFetched := defaultActionsToken(t) - server := newActionsServer(t, nil, withActionsToken(tokenThatShouldNotBeFetched)) + server := testserver.New(t, nil, testserver.WithActionsToken(tokenThatShouldNotBeFetched)) - client, err := actions.NewClient(server.configURLForOrg("my-org"), defaultCreds) + client, err := actions.NewClient(server.ConfigURLForOrg("my-org"), defaultCreds) require.NoError(t, err) client.ActionsServiceAdminToken = "healthy-token" client.ActionsServiceAdminTokenExpiresAt = time.Now().Add(1 * time.Hour) @@ -138,9 +139,9 @@ func TestNewActionsServiceRequest(t *testing.T) { }) t.Run("builds the right URL including api version", func(t *testing.T) { - server := newActionsServer(t, nil) + server := testserver.New(t, nil) - client, err := actions.NewClient(server.configURLForOrg("my-org"), defaultCreds) + client, err := actions.NewClient(server.ConfigURLForOrg("my-org"), defaultCreds) require.NoError(t, err) req, err := client.NewActionsServiceRequest(ctx, http.MethodGet, "/my/path?name=banana", nil) @@ -157,9 +158,9 @@ func TestNewActionsServiceRequest(t *testing.T) { }) t.Run("populates header", func(t *testing.T) { - server := newActionsServer(t, nil) + server := testserver.New(t, nil) - client, err := actions.NewClient(server.configURLForOrg("my-org"), defaultCreds, actions.WithUserAgent("my-agent")) + client, err := actions.NewClient(server.ConfigURLForOrg("my-org"), defaultCreds, actions.WithUserAgent("my-agent")) require.NoError(t, err) req, err := client.NewActionsServiceRequest(ctx, http.MethodGet, "/my/path", nil) diff --git a/github/actions/identifier_test.go b/github/actions/identifier_test.go index 0a184f86..60c08f3b 100644 --- a/github/actions/identifier_test.go +++ b/github/actions/identifier_test.go @@ -1,6 +1,9 @@ package actions_test import ( + "crypto/x509" + "os" + "path/filepath" "testing" "github.com/actions/actions-runner-controller/github/actions" @@ -108,4 +111,48 @@ func TestClient_Identifier(t *testing.T) { }) } }) + + t.Run("changes in TLS config", func(t *testing.T) { + configURL := "https://github.com/org/repo" + defaultCreds := &actions.ActionsAuth{ + Token: "token", + } + + noTlS, err := actions.NewClient(configURL, defaultCreds) + require.NoError(t, err) + + poolFromCert := func(t *testing.T, path string) *x509.CertPool { + t.Helper() + f, err := os.ReadFile(path) + require.NoError(t, err) + pool := x509.NewCertPool() + require.True(t, pool.AppendCertsFromPEM(f)) + return pool + } + + root, err := actions.NewClient( + configURL, + defaultCreds, + actions.WithRootCAs(poolFromCert(t, filepath.Join("testdata", "rootCA.crt"))), + ) + require.NoError(t, err) + + chain, err := actions.NewClient( + configURL, + defaultCreds, + actions.WithRootCAs(poolFromCert(t, filepath.Join("testdata", "intermediate.pem"))), + ) + require.NoError(t, err) + + clients := []*actions.Client{ + noTlS, + root, + chain, + } + identifiers := map[string]struct{}{} + for _, client := range clients { + identifiers[client.Identifier()] = struct{}{} + } + assert.Len(t, identifiers, len(clients), "all clients should have a unique identifier") + }) } diff --git a/github/actions/multi_client.go b/github/actions/multi_client.go index 37316870..eff3fd65 100644 --- a/github/actions/multi_client.go +++ b/github/actions/multi_client.go @@ -2,7 +2,6 @@ package actions import ( "context" - "crypto/x509" "fmt" "strconv" "sync" @@ -84,7 +83,7 @@ func (m *multiClient) GetClientFor(ctx context.Context, githubConfigURL string, } cachedClient, has := m.clients[key] - if has { + if has && cachedClient.rootCAs.Equal(client.rootCAs) { m.logger.Info("using cache client", "githubConfigURL", githubConfigURL, "namespace", namespace) return cachedClient, nil } @@ -141,19 +140,3 @@ func (m *multiClient) GetClientFromSecret(ctx context.Context, githubConfigURL, auth.AppCreds = &GitHubAppAuth{AppID: parsedAppID, AppInstallationID: parsedAppInstallationID, AppPrivateKey: appPrivateKey} return m.GetClientFor(ctx, githubConfigURL, auth, namespace, options...) } - -func RootCAsFromConfigMap(configMapData map[string][]byte) (*x509.CertPool, error) { - caCertPool, err := x509.SystemCertPool() - if err != nil { - caCertPool = x509.NewCertPool() - } - - for key, certData := range configMapData { - ok := caCertPool.AppendCertsFromPEM(certData) - if !ok { - return nil, fmt.Errorf("no certificates successfully parsed from key %s", key) - } - } - - return caCertPool, nil -}