diff --git a/.github/workflows/validate-gha-chart.yaml b/.github/workflows/validate-gha-chart.yaml index 0d54f6e2..645b32e9 100644 --- a/.github/workflows/validate-gha-chart.yaml +++ b/.github/workflows/validate-gha-chart.yaml @@ -71,7 +71,7 @@ jobs: git clone https://github.com/helm/chart-testing cd chart-testing unset CT_CONFIG_DIR - goreleaser build --clean --skip-validate + goreleaser build --clean --skip-validate ./dist/chart-testing_linux_amd64_v1/ct version echo 'Adding ct directory to PATH...' echo "$RUNNER_TEMP/chart-testing/dist/chart-testing_linux_amd64_v1" >> "$GITHUB_PATH" @@ -107,7 +107,7 @@ jobs: load: true build-args: | DOCKER_IMAGE_NAME=test-arc - VERSION=dev + VERSION=dev tags: | test-arc:dev cache-from: type=gha diff --git a/charts/gha-runner-scale-set-controller/templates/manager_cluster_role.yaml b/charts/gha-runner-scale-set-controller/templates/manager_cluster_role.yaml index 3ea31279..0ee3bb53 100644 --- a/charts/gha-runner-scale-set-controller/templates/manager_cluster_role.yaml +++ b/charts/gha-runner-scale-set-controller/templates/manager_cluster_role.yaml @@ -133,4 +133,5 @@ rules: verbs: - list - watch + - patch {{- end }} diff --git a/charts/gha-runner-scale-set-controller/templates/manager_single_namespace_controller_role.yaml b/charts/gha-runner-scale-set-controller/templates/manager_single_namespace_controller_role.yaml index a72dc738..7fd6e988 100644 --- a/charts/gha-runner-scale-set-controller/templates/manager_single_namespace_controller_role.yaml +++ b/charts/gha-runner-scale-set-controller/templates/manager_single_namespace_controller_role.yaml @@ -81,4 +81,4 @@ rules: verbs: - list - watch -{{- end }} \ No newline at end of file +{{- end }} diff --git a/charts/gha-runner-scale-set-controller/templates/manager_single_namespace_watch_role.yaml b/charts/gha-runner-scale-set-controller/templates/manager_single_namespace_watch_role.yaml index bf840bcf..f195da55 100644 --- a/charts/gha-runner-scale-set-controller/templates/manager_single_namespace_watch_role.yaml +++ b/charts/gha-runner-scale-set-controller/templates/manager_single_namespace_watch_role.yaml @@ -114,4 +114,5 @@ rules: verbs: - list - watch -{{- end }} \ No newline at end of file + - patch +{{- end }} diff --git a/charts/gha-runner-scale-set/templates/_helpers.tpl b/charts/gha-runner-scale-set/templates/_helpers.tpl index 45e77945..202bb04d 100644 --- a/charts/gha-runner-scale-set/templates/_helpers.tpl +++ b/charts/gha-runner-scale-set/templates/_helpers.tpl @@ -11,17 +11,9 @@ We truncate at 63 chars because some Kubernetes name fields are limited to this If release name contains chart name it will be used as a full name. */}} {{- define "gha-runner-scale-set.fullname" -}} -{{- if .Values.fullnameOverride }} -{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }} -{{- else }} -{{- $name := default .Chart.Name .Values.nameOverride }} -{{- if contains $name .Release.Name }} -{{- .Release.Name | trunc 63 | trimSuffix "-" }} -{{- else }} +{{- $name := default .Chart.Name }} {{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }} {{- end }} -{{- end }} -{{- end }} {{/* Create chart name and version as used by the chart label. @@ -41,6 +33,8 @@ app.kubernetes.io/version: {{ .Chart.AppVersion | quote }} {{- end }} app.kubernetes.io/managed-by: {{ .Release.Service }} app.kubernetes.io/part-of: gha-runner-scale-set +actions.github.com/scale-set-name: {{ .Release.Name }} +actions.github.com/scale-set-namespace: {{ .Release.Namespace }} {{- end }} {{/* @@ -71,6 +65,10 @@ app.kubernetes.io/instance: {{ .Release.Name }} {{- include "gha-runner-scale-set.fullname" . }}-kube-mode-role {{- end }} +{{- define "gha-runner-scale-set.kubeModeRoleBindingName" -}} +{{- include "gha-runner-scale-set.fullname" . }}-kube-mode-role-binding +{{- end }} + {{- define "gha-runner-scale-set.kubeModeServiceAccountName" -}} {{- include "gha-runner-scale-set.fullname" . }}-kube-mode-service-account {{- end }} @@ -433,7 +431,7 @@ volumeMounts: {{- include "gha-runner-scale-set.fullname" . }}-manager-role {{- end }} -{{- define "gha-runner-scale-set.managerRoleBinding" -}} +{{- define "gha-runner-scale-set.managerRoleBindingName" -}} {{- include "gha-runner-scale-set.fullname" . }}-manager-role-binding {{- end }} diff --git a/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml b/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml index df43ffbb..9e52e249 100644 --- a/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml +++ b/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml @@ -12,6 +12,21 @@ metadata: labels: app.kubernetes.io/component: "autoscaling-runner-set" {{- include "gha-runner-scale-set.labels" . | nindent 4 }} + annotations: + {{- $containerMode := .Values.containerMode }} + {{- if not (kindIs "string" .Values.githubConfigSecret) }} + actions.github.com/cleanup-github-secret-name: {{ include "gha-runner-scale-set.githubsecret" . }} + {{- end }} + actions.github.com/cleanup-manager-role-binding: {{ include "gha-runner-scale-set.managerRoleBindingName" . }} + actions.github.com/cleanup-manager-role-name: {{ include "gha-runner-scale-set.managerRoleName" . }} + {{- if and $containerMode (eq $containerMode.type "kubernetes") (not .Values.template.spec.serviceAccountName) }} + actions.github.com/cleanup-kubernetes-mode-role-binding-name: {{ include "gha-runner-scale-set.kubeModeRoleBindingName" . }} + actions.github.com/cleanup-kubernetes-mode-role-name: {{ include "gha-runner-scale-set.kubeModeRoleName" . }} + actions.github.com/cleanup-kubernetes-mode-service-account-name: {{ include "gha-runner-scale-set.kubeModeServiceAccountName" . }} + {{- end }} + {{- if and (ne $containerMode.type "kubernetes") (not .Values.template.spec.serviceAccountName) }} + actions.github.com/cleanup-no-permission-service-account-name: {{ include "gha-runner-scale-set.noPermissionServiceAccountName" . }} + {{- end }} spec: githubConfigUrl: {{ required ".Values.githubConfigUrl is required" (trimSuffix "/" .Values.githubConfigUrl) }} githubConfigSecret: {{ include "gha-runner-scale-set.githubsecret" . }} diff --git a/charts/gha-runner-scale-set/templates/githubsecret.yaml b/charts/gha-runner-scale-set/templates/githubsecret.yaml index 03411486..67282c18 100644 --- a/charts/gha-runner-scale-set/templates/githubsecret.yaml +++ b/charts/gha-runner-scale-set/templates/githubsecret.yaml @@ -7,7 +7,7 @@ metadata: labels: {{- include "gha-runner-scale-set.labels" . | nindent 4 }} finalizers: - - actions.github.com/secret-protection + - actions.github.com/cleanup-protection data: {{- $hasToken := false }} {{- $hasAppId := false }} @@ -36,4 +36,4 @@ data: {{- if and $hasAppId (or (not $hasInstallationId) (not $hasPrivateKey)) }} {{- fail "A valid .Values.githubConfigSecret is required for setting auth with GitHub server, provide .Values.githubConfigSecret.github_app_installation_id and .Values.githubConfigSecret.github_app_private_key." }} {{- end }} -{{- end}} \ No newline at end of file +{{- end}} diff --git a/charts/gha-runner-scale-set/templates/kube_mode_role.yaml b/charts/gha-runner-scale-set/templates/kube_mode_role.yaml index 9f98e0dc..e82d7b77 100644 --- a/charts/gha-runner-scale-set/templates/kube_mode_role.yaml +++ b/charts/gha-runner-scale-set/templates/kube_mode_role.yaml @@ -6,6 +6,8 @@ kind: Role metadata: name: {{ include "gha-runner-scale-set.kubeModeRoleName" . }} namespace: {{ .Release.Namespace }} + finalizers: + - actions.github.com/cleanup-protection rules: - apiGroups: [""] resources: ["pods"] diff --git a/charts/gha-runner-scale-set/templates/kube_mode_role_binding.yaml b/charts/gha-runner-scale-set/templates/kube_mode_role_binding.yaml index fb04e7a3..060b9399 100644 --- a/charts/gha-runner-scale-set/templates/kube_mode_role_binding.yaml +++ b/charts/gha-runner-scale-set/templates/kube_mode_role_binding.yaml @@ -3,8 +3,10 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding metadata: - name: {{ include "gha-runner-scale-set.kubeModeRoleName" . }} + name: {{ include "gha-runner-scale-set.kubeModeRoleBindingName" . }} namespace: {{ .Release.Namespace }} + finalizers: + - actions.github.com/cleanup-protection roleRef: apiGroup: rbac.authorization.k8s.io kind: Role diff --git a/charts/gha-runner-scale-set/templates/kube_mode_serviceaccount.yaml b/charts/gha-runner-scale-set/templates/kube_mode_serviceaccount.yaml index 13b31ba2..a32eceef 100644 --- a/charts/gha-runner-scale-set/templates/kube_mode_serviceaccount.yaml +++ b/charts/gha-runner-scale-set/templates/kube_mode_serviceaccount.yaml @@ -5,6 +5,8 @@ kind: ServiceAccount metadata: name: {{ include "gha-runner-scale-set.kubeModeServiceAccountName" . }} namespace: {{ .Release.Namespace }} + finalizers: + - actions.github.com/cleanup-protection labels: {{- include "gha-runner-scale-set.labels" . | nindent 4 }} {{- end }} diff --git a/charts/gha-runner-scale-set/templates/manager_role.yaml b/charts/gha-runner-scale-set/templates/manager_role.yaml index 6f4cd9a6..f6a1e493 100644 --- a/charts/gha-runner-scale-set/templates/manager_role.yaml +++ b/charts/gha-runner-scale-set/templates/manager_role.yaml @@ -3,6 +3,11 @@ kind: Role metadata: name: {{ include "gha-runner-scale-set.managerRoleName" . }} namespace: {{ .Release.Namespace }} + labels: + {{- include "gha-runner-scale-set.labels" . | nindent 4 }} + app.kubernetes.io/component: manager-role + finalizers: + - actions.github.com/cleanup-protection rules: - apiGroups: - "" @@ -29,6 +34,17 @@ rules: - list - patch - update +- apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - create + - delete + - get + - list + - patch + - update - apiGroups: - rbac.authorization.k8s.io resources: @@ -56,4 +72,4 @@ rules: - configmaps verbs: - get -{{- end }} \ No newline at end of file +{{- end }} diff --git a/charts/gha-runner-scale-set/templates/manager_role_binding.yaml b/charts/gha-runner-scale-set/templates/manager_role_binding.yaml index ba38ab0f..ce212f77 100644 --- a/charts/gha-runner-scale-set/templates/manager_role_binding.yaml +++ b/charts/gha-runner-scale-set/templates/manager_role_binding.yaml @@ -1,8 +1,13 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding metadata: - name: {{ include "gha-runner-scale-set.managerRoleBinding" . }} + name: {{ include "gha-runner-scale-set.managerRoleBindingName" . }} namespace: {{ .Release.Namespace }} + labels: + {{- include "gha-runner-scale-set.labels" . | nindent 4 }} + app.kubernetes.io/component: manager-role-binding + finalizers: + - actions.github.com/cleanup-protection roleRef: apiGroup: rbac.authorization.k8s.io kind: Role @@ -10,4 +15,4 @@ roleRef: subjects: - kind: ServiceAccount name: {{ include "gha-runner-scale-set.managerServiceAccountName" . | nindent 4 }} - namespace: {{ include "gha-runner-scale-set.managerServiceAccountNamespace" . | nindent 4 }} \ No newline at end of file + namespace: {{ include "gha-runner-scale-set.managerServiceAccountNamespace" . | nindent 4 }} diff --git a/charts/gha-runner-scale-set/templates/no_permission_serviceaccount.yaml b/charts/gha-runner-scale-set/templates/no_permission_serviceaccount.yaml index 8175c874..f7c9700f 100644 --- a/charts/gha-runner-scale-set/templates/no_permission_serviceaccount.yaml +++ b/charts/gha-runner-scale-set/templates/no_permission_serviceaccount.yaml @@ -7,4 +7,6 @@ metadata: namespace: {{ .Release.Namespace }} labels: {{- include "gha-runner-scale-set.labels" . | nindent 4 }} + finalizers: + - actions.github.com/cleanup-protection {{- end }} diff --git a/charts/gha-runner-scale-set/tests/template_test.go b/charts/gha-runner-scale-set/tests/template_test.go index 9c3692ee..8676da66 100644 --- a/charts/gha-runner-scale-set/tests/template_test.go +++ b/charts/gha-runner-scale-set/tests/template_test.go @@ -1,11 +1,13 @@ package tests import ( + "fmt" "path/filepath" "strings" "testing" v1alpha1 "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" + actionsgithubcom "github.com/actions/actions-runner-controller/controllers/actions.github.com" "github.com/gruntwork-io/terratest/modules/helm" "github.com/gruntwork-io/terratest/modules/k8s" "github.com/gruntwork-io/terratest/modules/random" @@ -43,7 +45,7 @@ func TestTemplateRenderedGitHubSecretWithGitHubToken(t *testing.T) { assert.Equal(t, namespaceName, githubSecret.Namespace) assert.Equal(t, "test-runners-gha-runner-scale-set-github-secret", githubSecret.Name) assert.Equal(t, "gh_token12345", string(githubSecret.Data["github_token"])) - assert.Equal(t, "actions.github.com/secret-protection", githubSecret.Finalizers[0]) + assert.Equal(t, "actions.github.com/cleanup-protection", githubSecret.Finalizers[0]) } func TestTemplateRenderedGitHubSecretWithGitHubApp(t *testing.T) { @@ -188,6 +190,7 @@ func TestTemplateRenderedSetServiceAccountToNoPermission(t *testing.T) { helm.UnmarshalK8SYaml(t, output, &ars) assert.Equal(t, "test-runners-gha-runner-scale-set-no-permission-service-account", ars.Spec.Template.Spec.ServiceAccountName) + assert.Empty(t, ars.Annotations[actionsgithubcom.AnnotationKeyKubernetesModeServiceAccountName]) // no finalizer protections in place } func TestTemplateRenderedSetServiceAccountToKubeMode(t *testing.T) { @@ -217,6 +220,7 @@ func TestTemplateRenderedSetServiceAccountToKubeMode(t *testing.T) { assert.Equal(t, namespaceName, serviceAccount.Namespace) assert.Equal(t, "test-runners-gha-runner-scale-set-kube-mode-service-account", serviceAccount.Name) + assert.Equal(t, "actions.github.com/cleanup-protection", serviceAccount.Finalizers[0]) output = helm.RenderTemplate(t, options, helmChartPath, releaseName, []string{"templates/kube_mode_role.yaml"}) var role rbacv1.Role @@ -224,6 +228,9 @@ func TestTemplateRenderedSetServiceAccountToKubeMode(t *testing.T) { assert.Equal(t, namespaceName, role.Namespace) assert.Equal(t, "test-runners-gha-runner-scale-set-kube-mode-role", role.Name) + + assert.Equal(t, "actions.github.com/cleanup-protection", role.Finalizers[0]) + assert.Len(t, role.Rules, 5, "kube mode role should have 5 rules") assert.Equal(t, "pods", role.Rules[0].Resources[0]) assert.Equal(t, "pods/exec", role.Rules[1].Resources[0]) @@ -236,18 +243,21 @@ func TestTemplateRenderedSetServiceAccountToKubeMode(t *testing.T) { helm.UnmarshalK8SYaml(t, output, &roleBinding) assert.Equal(t, namespaceName, roleBinding.Namespace) - assert.Equal(t, "test-runners-gha-runner-scale-set-kube-mode-role", roleBinding.Name) + assert.Equal(t, "test-runners-gha-runner-scale-set-kube-mode-role-binding", roleBinding.Name) assert.Len(t, roleBinding.Subjects, 1) assert.Equal(t, "test-runners-gha-runner-scale-set-kube-mode-service-account", roleBinding.Subjects[0].Name) assert.Equal(t, namespaceName, roleBinding.Subjects[0].Namespace) assert.Equal(t, "test-runners-gha-runner-scale-set-kube-mode-role", roleBinding.RoleRef.Name) assert.Equal(t, "Role", roleBinding.RoleRef.Kind) + assert.Equal(t, "actions.github.com/cleanup-protection", serviceAccount.Finalizers[0]) output = helm.RenderTemplate(t, options, helmChartPath, releaseName, []string{"templates/autoscalingrunnerset.yaml"}) var ars v1alpha1.AutoscalingRunnerSet helm.UnmarshalK8SYaml(t, output, &ars) - assert.Equal(t, "test-runners-gha-runner-scale-set-kube-mode-service-account", ars.Spec.Template.Spec.ServiceAccountName) + expectedServiceAccountName := "test-runners-gha-runner-scale-set-kube-mode-service-account" + assert.Equal(t, expectedServiceAccountName, ars.Spec.Template.Spec.ServiceAccountName) + assert.Equal(t, expectedServiceAccountName, ars.Annotations[actionsgithubcom.AnnotationKeyKubernetesModeServiceAccountName]) } func TestTemplateRenderedUserProvideSetServiceAccount(t *testing.T) { @@ -279,6 +289,7 @@ func TestTemplateRenderedUserProvideSetServiceAccount(t *testing.T) { helm.UnmarshalK8SYaml(t, output, &ars) assert.Equal(t, "test-service-account", ars.Spec.Template.Spec.ServiceAccountName) + assert.Empty(t, ars.Annotations[actionsgithubcom.AnnotationKeyKubernetesModeServiceAccountName]) } func TestTemplateRenderedAutoScalingRunnerSet(t *testing.T) { @@ -1458,7 +1469,11 @@ func TestTemplate_CreateManagerRole(t *testing.T) { assert.Equal(t, namespaceName, managerRole.Namespace, "namespace should match the namespace of the Helm release") assert.Equal(t, "test-runners-gha-runner-scale-set-manager-role", managerRole.Name) - assert.Equal(t, 5, len(managerRole.Rules)) + assert.Equal(t, "actions.github.com/cleanup-protection", managerRole.Finalizers[0]) + assert.Equal(t, 6, len(managerRole.Rules)) + + var ars v1alpha1.AutoscalingRunnerSet + helm.UnmarshalK8SYaml(t, output, &ars) } func TestTemplate_CreateManagerRole_UseConfigMaps(t *testing.T) { @@ -1489,8 +1504,9 @@ func TestTemplate_CreateManagerRole_UseConfigMaps(t *testing.T) { assert.Equal(t, namespaceName, managerRole.Namespace, "namespace should match the namespace of the Helm release") assert.Equal(t, "test-runners-gha-runner-scale-set-manager-role", managerRole.Name) - assert.Equal(t, 6, len(managerRole.Rules)) - assert.Equal(t, "configmaps", managerRole.Rules[5].Resources[0]) + assert.Equal(t, "actions.github.com/cleanup-protection", managerRole.Finalizers[0]) + assert.Equal(t, 7, len(managerRole.Rules)) + assert.Equal(t, "configmaps", managerRole.Rules[6].Resources[0]) } func TestTemplate_CreateManagerRoleBinding(t *testing.T) { @@ -1521,6 +1537,7 @@ func TestTemplate_CreateManagerRoleBinding(t *testing.T) { assert.Equal(t, namespaceName, managerRoleBinding.Namespace, "namespace should match the namespace of the Helm release") assert.Equal(t, "test-runners-gha-runner-scale-set-manager-role-binding", managerRoleBinding.Name) assert.Equal(t, "test-runners-gha-runner-scale-set-manager-role", managerRoleBinding.RoleRef.Name) + assert.Equal(t, "actions.github.com/cleanup-protection", managerRoleBinding.Finalizers[0]) assert.Equal(t, "arc", managerRoleBinding.Subjects[0].Name) assert.Equal(t, "arc-system", managerRoleBinding.Subjects[0].Namespace) } @@ -1692,3 +1709,103 @@ func TestTemplateRenderedAutoScalingRunnerSet_KubeModeMergePodSpec(t *testing.T) assert.Equal(t, "others", ars.Spec.Template.Spec.Containers[0].VolumeMounts[1].Name, "VolumeMount name should be others") assert.Equal(t, "/others", ars.Spec.Template.Spec.Containers[0].VolumeMounts[1].MountPath, "VolumeMount mountPath should be /others") } + +func TestTemplateRenderedAutoscalingRunnerSetAnnotation_GitHubSecret(t *testing.T) { + t.Parallel() + + // Path to the helm chart we will test + helmChartPath, err := filepath.Abs("../../gha-runner-scale-set") + require.NoError(t, err) + + releaseName := "test-runners" + namespaceName := "test-" + strings.ToLower(random.UniqueId()) + + annotationExpectedTests := map[string]*helm.Options{ + "GitHub token": { + SetValues: map[string]string{ + "githubConfigUrl": "https://github.com/actions", + "githubConfigSecret.github_token": "gh_token12345", + "controllerServiceAccount.name": "arc", + "controllerServiceAccount.namespace": "arc-system", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + }, + "GitHub app": { + SetValues: map[string]string{ + "githubConfigUrl": "https://github.com/actions", + "githubConfigSecret.github_app_id": "10", + "githubConfigSecret.github_app_installation_id": "100", + "githubConfigSecret.github_app_private_key": "private_key", + "controllerServiceAccount.name": "arc", + "controllerServiceAccount.namespace": "arc-system", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + }, + } + + for name, options := range annotationExpectedTests { + t.Run("Annotation set: "+name, func(t *testing.T) { + output := helm.RenderTemplate(t, options, helmChartPath, releaseName, []string{"templates/autoscalingrunnerset.yaml"}) + var autoscalingRunnerSet v1alpha1.AutoscalingRunnerSet + helm.UnmarshalK8SYaml(t, output, &autoscalingRunnerSet) + + assert.NotEmpty(t, autoscalingRunnerSet.Annotations[actionsgithubcom.AnnotationKeyGitHubSecretName]) + }) + } + + t.Run("Annotation should not be set", func(t *testing.T) { + options := &helm.Options{ + SetValues: map[string]string{ + "githubConfigUrl": "https://github.com/actions", + "githubConfigSecret": "pre-defined-secret", + "controllerServiceAccount.name": "arc", + "controllerServiceAccount.namespace": "arc-system", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + } + output := helm.RenderTemplate(t, options, helmChartPath, releaseName, []string{"templates/autoscalingrunnerset.yaml"}) + var autoscalingRunnerSet v1alpha1.AutoscalingRunnerSet + helm.UnmarshalK8SYaml(t, output, &autoscalingRunnerSet) + + assert.Empty(t, autoscalingRunnerSet.Annotations[actionsgithubcom.AnnotationKeyGitHubSecretName]) + }) +} + +func TestTemplateRenderedAutoscalingRunnerSetAnnotation_KubernetesModeCleanup(t *testing.T) { + t.Parallel() + + // Path to the helm chart we will test + helmChartPath, err := filepath.Abs("../../gha-runner-scale-set") + require.NoError(t, err) + + releaseName := "test-runners" + namespaceName := "test-" + strings.ToLower(random.UniqueId()) + + options := &helm.Options{ + SetValues: map[string]string{ + "githubConfigUrl": "https://github.com/actions", + "githubConfigSecret.github_token": "gh_token12345", + "controllerServiceAccount.name": "arc", + "controllerServiceAccount.namespace": "arc-system", + "containerMode.type": "kubernetes", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + } + + output := helm.RenderTemplate(t, options, helmChartPath, releaseName, []string{"templates/autoscalingrunnerset.yaml"}) + var autoscalingRunnerSet v1alpha1.AutoscalingRunnerSet + helm.UnmarshalK8SYaml(t, output, &autoscalingRunnerSet) + + annotationValues := map[string]string{ + actionsgithubcom.AnnotationKeyGitHubSecretName: "test-runners-gha-runner-scale-set-github-secret", + actionsgithubcom.AnnotationKeyManagerRoleName: "test-runners-gha-runner-scale-set-manager-role", + actionsgithubcom.AnnotationKeyManagerRoleBindingName: "test-runners-gha-runner-scale-set-manager-role-binding", + actionsgithubcom.AnnotationKeyKubernetesModeServiceAccountName: "test-runners-gha-runner-scale-set-kube-mode-service-account", + actionsgithubcom.AnnotationKeyKubernetesModeRoleName: "test-runners-gha-runner-scale-set-kube-mode-role", + actionsgithubcom.AnnotationKeyKubernetesModeRoleBindingName: "test-runners-gha-runner-scale-set-kube-mode-role-binding", + } + + for annotation, value := range annotationValues { + assert.Equal(t, value, autoscalingRunnerSet.Annotations[annotation], fmt.Sprintf("Annotation %q does not match the expected value", annotation)) + } +} diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go index 43c13823..903acb06 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -27,6 +27,7 @@ import ( "github.com/actions/actions-runner-controller/github/actions" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -42,11 +43,12 @@ import ( const ( // TODO: Replace with shared image. - autoscalingRunnerSetOwnerKey = ".metadata.controller" - LabelKeyRunnerSpecHash = "runner-spec-hash" - autoscalingRunnerSetFinalizerName = "autoscalingrunnerset.actions.github.com/finalizer" - runnerScaleSetIdAnnotationKey = "runner-scale-set-id" - runnerScaleSetNameAnnotationKey = "runner-scale-set-name" + autoscalingRunnerSetOwnerKey = ".metadata.controller" + LabelKeyRunnerSpecHash = "runner-spec-hash" + autoscalingRunnerSetFinalizerName = "autoscalingrunnerset.actions.github.com/finalizer" + runnerScaleSetIdAnnotationKey = "runner-scale-set-id" + runnerScaleSetNameAnnotationKey = "runner-scale-set-name" + autoscalingRunnerSetCleanupFinalizerName = "actions.github.com/cleanup-protection" ) // AutoscalingRunnerSetReconciler reconciles a AutoscalingRunnerSet object @@ -113,6 +115,17 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl return ctrl.Result{}, err } + requeue, err := r.removeFinalizersFromDependentResources(ctx, autoscalingRunnerSet, log) + if err != nil { + log.Error(err, "Failed to remove finalizers on dependent resources") + return ctrl.Result{}, err + } + + if requeue { + log.Info("Waiting for dependent resources to be deleted") + return ctrl.Result{Requeue: true}, nil + } + log.Info("Removing finalizer") err = patch(ctx, r.Client, autoscalingRunnerSet, func(obj *v1alpha1.AutoscalingRunnerSet) { controllerutil.RemoveFinalizer(obj, autoscalingRunnerSetFinalizerName) @@ -305,6 +318,29 @@ func (r *AutoscalingRunnerSetReconciler) deleteEphemeralRunnerSets(ctx context.C return nil } +func (r *AutoscalingRunnerSetReconciler) removeFinalizersFromDependentResources(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, logger logr.Logger) (requeue bool, err error) { + c := autoscalingRunnerSetFinalizerDependencyCleaner{ + client: r.Client, + autoscalingRunnerSet: autoscalingRunnerSet, + logger: logger, + } + + c.removeKubernetesModeRoleBindingFinalizer(ctx) + c.removeKubernetesModeRoleFinalizer(ctx) + c.removeKubernetesModeServiceAccountFinalizer(ctx) + c.removeNoPermissionServiceAccountFinalizer(ctx) + c.removeGitHubSecretFinalizer(ctx) + c.removeManagerRoleBindingFinalizer(ctx) + c.removeManagerRoleFinalizer(ctx) + + requeue, err = c.result() + if err != nil { + logger.Error(err, "Failed to cleanup finalizer from dependent resource") + return true, err + } + return requeue, nil +} + func (r *AutoscalingRunnerSetReconciler) createRunnerScaleSet(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, logger logr.Logger) (ctrl.Result, error) { logger.Info("Creating a new runner scale set") actionsClient, err := r.actionsClientFor(ctx, autoscalingRunnerSet) @@ -467,12 +503,28 @@ func (r *AutoscalingRunnerSetReconciler) updateRunnerScaleSetName(ctx context.Co } func (r *AutoscalingRunnerSetReconciler) deleteRunnerScaleSet(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, logger logr.Logger) error { + scaleSetId, ok := autoscalingRunnerSet.Annotations[runnerScaleSetIdAnnotationKey] + if !ok { + // Annotation not being present can occur in 3 scenarios + // 1. Scale set is never created. + // In this case, we don't need to fetch the actions client to delete the scale set that does not exist + // + // 2. The scale set has been deleted by the controller. + // In that case, the controller will clean up annotation because the scale set does not exist anymore. + // Removal of the scale set id is also useful because permission cleanup will eventually lose permission + // assigned to it on a GitHub secret, causing actions client from secret to result in permission denied + // + // 3. Annotation is removed manually. + // In this case, the controller will treat this as if the scale set is being removed from the actions service + // Then, manual deletion of the scale set is required. + return nil + } logger.Info("Deleting the runner scale set from Actions service") - runnerScaleSetId, err := strconv.Atoi(autoscalingRunnerSet.Annotations[runnerScaleSetIdAnnotationKey]) + runnerScaleSetId, err := strconv.Atoi(scaleSetId) if err != nil { - // If the annotation is not set correctly, or if it does not exist, we are going to get stuck in a loop trying to parse the scale set id. - // If the configuration is invalid (secret does not exist for example), we never get to the point to create runner set. But then, manual cleanup - // would get stuck finalizing the resource trying to parse annotation indefinitely + // If the annotation is not set correctly, we are going to get stuck in a loop trying to parse the scale set id. + // If the configuration is invalid (secret does not exist for example), we never got to the point to create runner set. + // But then, manual cleanup would get stuck finalizing the resource trying to parse annotation indefinitely logger.Info("autoscaling runner set does not have annotation describing scale set id. Skip deletion", "err", err.Error()) return nil } @@ -489,6 +541,14 @@ func (r *AutoscalingRunnerSetReconciler) deleteRunnerScaleSet(ctx context.Contex return err } + err = patch(ctx, r.Client, autoscalingRunnerSet, func(obj *v1alpha1.AutoscalingRunnerSet) { + delete(obj.Annotations, runnerScaleSetIdAnnotationKey) + }) + if err != nil { + logger.Error(err, "Failed to patch autoscaling runner set with annotation removed", "annotation", runnerScaleSetIdAnnotationKey) + return err + } + logger.Info("Deleted the runner scale set from Actions service") return nil } @@ -658,6 +718,328 @@ func (r *AutoscalingRunnerSetReconciler) SetupWithManager(mgr ctrl.Manager) erro Complete(r) } +type autoscalingRunnerSetFinalizerDependencyCleaner struct { + // configuration fields + client client.Client + autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet + logger logr.Logger + + // fields to operate on + requeue bool + err error +} + +func (c *autoscalingRunnerSetFinalizerDependencyCleaner) result() (requeue bool, err error) { + return c.requeue, c.err +} + +func (c *autoscalingRunnerSetFinalizerDependencyCleaner) removeKubernetesModeRoleBindingFinalizer(ctx context.Context) { + if c.requeue || c.err != nil { + return + } + + roleBindingName, ok := c.autoscalingRunnerSet.Annotations[AnnotationKeyKubernetesModeRoleBindingName] + if !ok { + c.logger.Info( + "Skipping cleaning up kubernetes mode service account", + "reason", + fmt.Sprintf("annotation key %q not present", AnnotationKeyKubernetesModeRoleBindingName), + ) + return + } + + c.logger.Info("Removing finalizer from container mode kubernetes role binding", "name", roleBindingName) + + roleBinding := new(rbacv1.RoleBinding) + err := c.client.Get(ctx, types.NamespacedName{Name: roleBindingName, Namespace: c.autoscalingRunnerSet.Namespace}, roleBinding) + switch { + case err == nil: + if !controllerutil.ContainsFinalizer(roleBinding, autoscalingRunnerSetCleanupFinalizerName) { + c.logger.Info("Kubernetes mode role binding finalizer has already been removed", "name", roleBindingName) + return + } + err = patch(ctx, c.client, roleBinding, func(obj *rbacv1.RoleBinding) { + controllerutil.RemoveFinalizer(obj, autoscalingRunnerSetCleanupFinalizerName) + }) + if err != nil { + c.err = fmt.Errorf("failed to patch kubernetes mode role binding without finalizer: %w", err) + return + } + c.requeue = true + c.logger.Info("Removed finalizer from container mode kubernetes role binding", "name", roleBindingName) + return + case err != nil && !kerrors.IsNotFound(err): + c.err = fmt.Errorf("failed to fetch kubernetes mode role binding: %w", err) + return + default: + c.logger.Info("Container mode kubernetes role binding has already been deleted", "name", roleBindingName) + return + } +} + +func (c *autoscalingRunnerSetFinalizerDependencyCleaner) removeKubernetesModeRoleFinalizer(ctx context.Context) { + if c.requeue || c.err != nil { + return + } + + roleName, ok := c.autoscalingRunnerSet.Annotations[AnnotationKeyKubernetesModeRoleName] + if !ok { + c.logger.Info( + "Skipping cleaning up kubernetes mode role", + "reason", + fmt.Sprintf("annotation key %q not present", AnnotationKeyKubernetesModeRoleName), + ) + return + } + + c.logger.Info("Removing finalizer from container mode kubernetes role", "name", roleName) + role := new(rbacv1.Role) + err := c.client.Get(ctx, types.NamespacedName{Name: roleName, Namespace: c.autoscalingRunnerSet.Namespace}, role) + switch { + case err == nil: + if !controllerutil.ContainsFinalizer(role, autoscalingRunnerSetCleanupFinalizerName) { + c.logger.Info("Kubernetes mode role finalizer has already been removed", "name", roleName) + return + } + err = patch(ctx, c.client, role, func(obj *rbacv1.Role) { + controllerutil.RemoveFinalizer(obj, autoscalingRunnerSetCleanupFinalizerName) + }) + if err != nil { + c.err = fmt.Errorf("failed to patch kubernetes mode role without finalizer: %w", err) + return + } + c.requeue = true + c.logger.Info("Removed finalizer from container mode kubernetes role") + return + case err != nil && !kerrors.IsNotFound(err): + c.err = fmt.Errorf("failed to fetch kubernetes mode role: %w", err) + return + default: + c.logger.Info("Container mode kubernetes role has already been deleted", "name", roleName) + return + } +} + +func (c *autoscalingRunnerSetFinalizerDependencyCleaner) removeKubernetesModeServiceAccountFinalizer(ctx context.Context) { + if c.requeue || c.err != nil { + return + } + + serviceAccountName, ok := c.autoscalingRunnerSet.Annotations[AnnotationKeyKubernetesModeServiceAccountName] + if !ok { + c.logger.Info( + "Skipping cleaning up kubernetes mode role binding", + "reason", + fmt.Sprintf("annotation key %q not present", AnnotationKeyKubernetesModeServiceAccountName), + ) + return + } + + c.logger.Info("Removing finalizer from container mode kubernetes service account", "name", serviceAccountName) + + serviceAccount := new(corev1.ServiceAccount) + err := c.client.Get(ctx, types.NamespacedName{Name: serviceAccountName, Namespace: c.autoscalingRunnerSet.Namespace}, serviceAccount) + switch { + case err == nil: + if !controllerutil.ContainsFinalizer(serviceAccount, autoscalingRunnerSetCleanupFinalizerName) { + c.logger.Info("Kubernetes mode service account finalizer has already been removed", "name", serviceAccountName) + return + } + err = patch(ctx, c.client, serviceAccount, func(obj *corev1.ServiceAccount) { + controllerutil.RemoveFinalizer(obj, autoscalingRunnerSetCleanupFinalizerName) + }) + if err != nil { + c.err = fmt.Errorf("failed to patch kubernetes mode service account without finalizer: %w", err) + return + } + c.requeue = true + c.logger.Info("Removed finalizer from container mode kubernetes service account") + return + case err != nil && !kerrors.IsNotFound(err): + c.err = fmt.Errorf("failed to fetch kubernetes mode service account: %w", err) + return + default: + c.logger.Info("Container mode kubernetes service account has already been deleted", "name", serviceAccountName) + return + } +} + +func (c *autoscalingRunnerSetFinalizerDependencyCleaner) removeNoPermissionServiceAccountFinalizer(ctx context.Context) { + if c.requeue || c.err != nil { + return + } + + serviceAccountName, ok := c.autoscalingRunnerSet.Annotations[AnnotationKeyNoPermissionServiceAccountName] + if !ok { + c.logger.Info( + "Skipping cleaning up no permission service account", + "reason", + fmt.Sprintf("annotation key %q not present", AnnotationKeyNoPermissionServiceAccountName), + ) + return + } + + c.logger.Info("Removing finalizer from no permission service account", "name", serviceAccountName) + + serviceAccount := new(corev1.ServiceAccount) + err := c.client.Get(ctx, types.NamespacedName{Name: serviceAccountName, Namespace: c.autoscalingRunnerSet.Namespace}, serviceAccount) + switch { + case err == nil: + if !controllerutil.ContainsFinalizer(serviceAccount, autoscalingRunnerSetCleanupFinalizerName) { + c.logger.Info("No permission service account finalizer has already been removed", "name", serviceAccountName) + return + } + err = patch(ctx, c.client, serviceAccount, func(obj *corev1.ServiceAccount) { + controllerutil.RemoveFinalizer(obj, autoscalingRunnerSetCleanupFinalizerName) + }) + if err != nil { + c.err = fmt.Errorf("failed to patch service account without finalizer: %w", err) + return + } + c.requeue = true + c.logger.Info("Removed finalizer from no permission service account", "name", serviceAccountName) + return + case err != nil && !kerrors.IsNotFound(err): + c.err = fmt.Errorf("failed to fetch service account: %w", err) + return + default: + c.logger.Info("No permission service account has already been deleted", "name", serviceAccountName) + return + } +} + +func (c *autoscalingRunnerSetFinalizerDependencyCleaner) removeGitHubSecretFinalizer(ctx context.Context) { + if c.requeue || c.err != nil { + return + } + + githubSecretName, ok := c.autoscalingRunnerSet.Annotations[AnnotationKeyGitHubSecretName] + if !ok { + c.logger.Info( + "Skipping cleaning up no permission service account", + "reason", + fmt.Sprintf("annotation key %q not present", AnnotationKeyGitHubSecretName), + ) + return + } + + c.logger.Info("Removing finalizer from GitHub secret", "name", githubSecretName) + + githubSecret := new(corev1.Secret) + err := c.client.Get(ctx, types.NamespacedName{Name: githubSecretName, Namespace: c.autoscalingRunnerSet.Namespace}, githubSecret) + switch { + case err == nil: + if !controllerutil.ContainsFinalizer(githubSecret, autoscalingRunnerSetCleanupFinalizerName) { + c.logger.Info("GitHub secret finalizer has already been removed", "name", githubSecretName) + return + } + err = patch(ctx, c.client, githubSecret, func(obj *corev1.Secret) { + controllerutil.RemoveFinalizer(obj, autoscalingRunnerSetCleanupFinalizerName) + }) + if err != nil { + c.err = fmt.Errorf("failed to patch GitHub secret without finalizer: %w", err) + return + } + c.requeue = true + c.logger.Info("Removed finalizer from GitHub secret", "name", githubSecretName) + return + case err != nil && !kerrors.IsNotFound(err) && !kerrors.IsForbidden(err): + c.err = fmt.Errorf("failed to fetch GitHub secret: %w", err) + return + default: + c.logger.Info("GitHub secret has already been deleted", "name", githubSecretName) + return + } +} + +func (c *autoscalingRunnerSetFinalizerDependencyCleaner) removeManagerRoleBindingFinalizer(ctx context.Context) { + if c.requeue || c.err != nil { + return + } + + managerRoleBindingName, ok := c.autoscalingRunnerSet.Annotations[AnnotationKeyManagerRoleBindingName] + if !ok { + c.logger.Info( + "Skipping cleaning up manager role binding", + "reason", + fmt.Sprintf("annotation key %q not present", AnnotationKeyManagerRoleBindingName), + ) + return + } + + c.logger.Info("Removing finalizer from manager role binding", "name", managerRoleBindingName) + + roleBinding := new(rbacv1.RoleBinding) + err := c.client.Get(ctx, types.NamespacedName{Name: managerRoleBindingName, Namespace: c.autoscalingRunnerSet.Namespace}, roleBinding) + switch { + case err == nil: + if !controllerutil.ContainsFinalizer(roleBinding, autoscalingRunnerSetCleanupFinalizerName) { + c.logger.Info("Manager role binding finalizer has already been removed", "name", managerRoleBindingName) + return + } + err = patch(ctx, c.client, roleBinding, func(obj *rbacv1.RoleBinding) { + controllerutil.RemoveFinalizer(obj, autoscalingRunnerSetCleanupFinalizerName) + }) + if err != nil { + c.err = fmt.Errorf("failed to patch manager role binding without finalizer: %w", err) + return + } + c.requeue = true + c.logger.Info("Removed finalizer from manager role binding", "name", managerRoleBindingName) + return + case err != nil && !kerrors.IsNotFound(err): + c.err = fmt.Errorf("failed to fetch manager role binding: %w", err) + return + default: + c.logger.Info("Manager role binding has already been deleted", "name", managerRoleBindingName) + return + } +} + +func (c *autoscalingRunnerSetFinalizerDependencyCleaner) removeManagerRoleFinalizer(ctx context.Context) { + if c.requeue || c.err != nil { + return + } + + managerRoleName, ok := c.autoscalingRunnerSet.Annotations[AnnotationKeyManagerRoleName] + if !ok { + c.logger.Info( + "Skipping cleaning up manager role", + "reason", + fmt.Sprintf("annotation key %q not present", AnnotationKeyManagerRoleName), + ) + return + } + + c.logger.Info("Removing finalizer from manager role", "name", managerRoleName) + + role := new(rbacv1.Role) + err := c.client.Get(ctx, types.NamespacedName{Name: managerRoleName, Namespace: c.autoscalingRunnerSet.Namespace}, role) + switch { + case err == nil: + if !controllerutil.ContainsFinalizer(role, autoscalingRunnerSetCleanupFinalizerName) { + c.logger.Info("Manager role finalizer has already been removed", "name", managerRoleName) + return + } + err = patch(ctx, c.client, role, func(obj *rbacv1.Role) { + controllerutil.RemoveFinalizer(obj, autoscalingRunnerSetCleanupFinalizerName) + }) + if err != nil { + c.err = fmt.Errorf("failed to patch manager role without finalizer: %w", err) + return + } + c.requeue = true + c.logger.Info("Removed finalizer from manager role", "name", managerRoleName) + return + case err != nil && !kerrors.IsNotFound(err): + c.err = fmt.Errorf("failed to fetch manager role: %w", err) + return + default: + c.logger.Info("Manager role has already been deleted", "name", managerRoleName) + return + } +} + // NOTE: if this is logic should be used for other resources, // consider using generics type EphemeralRunnerSets struct { diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go index 2fd0e61b..6ad2f18a 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go @@ -13,6 +13,7 @@ import ( "time" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -23,6 +24,7 @@ import ( . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" "github.com/actions/actions-runner-controller/github/actions" @@ -571,6 +573,7 @@ var _ = Describe("Test AutoScalingController updates", func() { update := autoscalingRunnerSet.DeepCopy() update.Spec.RunnerScaleSetName = "testset_update" + err = k8sClient.Patch(ctx, update, client.MergeFrom(autoscalingRunnerSet)) Expect(err).NotTo(HaveOccurred(), "failed to update AutoScalingRunnerSet") @@ -1036,7 +1039,7 @@ var _ = Describe("Test Client optional configuration", func() { g.Expect(listener.Spec.GitHubServerTLS).To(BeEquivalentTo(autoscalingRunnerSet.Spec.GitHubServerTLS), "listener does not have TLS config") }, autoscalingRunnerSetTestTimeout, - autoscalingListenerTestInterval, + autoscalingRunnerSetTestInterval, ).Should(Succeed(), "tls config is incorrect") }) @@ -1093,8 +1096,372 @@ var _ = Describe("Test Client optional configuration", func() { g.Expect(runnerSet.Spec.EphemeralRunnerSpec.GitHubServerTLS).To(BeEquivalentTo(autoscalingRunnerSet.Spec.GitHubServerTLS), "EphemeralRunnerSpec does not have TLS config") }, autoscalingRunnerSetTestTimeout, - autoscalingListenerTestInterval, + autoscalingRunnerSetTestInterval, ).Should(Succeed()) }) }) }) + +var _ = Describe("Test external permissions cleanup", func() { + It("Should clean up kubernetes mode permissions", func() { + ctx := context.Background() + autoscalingNS, mgr := createNamespace(GinkgoT(), k8sClient) + + configSecret := createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) + + 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) + + min := 1 + max := 10 + autoscalingRunnerSet := &v1alpha1.AutoscalingRunnerSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-asrs", + Namespace: autoscalingNS.Name, + Labels: map[string]string{ + "app.kubernetes.io/name": "gha-runner-scale-set", + }, + Annotations: map[string]string{ + AnnotationKeyKubernetesModeRoleBindingName: "kube-mode-role-binding", + AnnotationKeyKubernetesModeRoleName: "kube-mode-role", + AnnotationKeyKubernetesModeServiceAccountName: "kube-mode-service-account", + }, + }, + Spec: v1alpha1.AutoscalingRunnerSetSpec{ + GitHubConfigUrl: "https://github.com/owner/repo", + GitHubConfigSecret: configSecret.Name, + MaxRunners: &max, + MinRunners: &min, + RunnerGroup: "testgroup", + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "runner", + Image: "ghcr.io/actions/runner", + }, + }, + }, + }, + }, + } + + role := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: autoscalingRunnerSet.Annotations[AnnotationKeyKubernetesModeRoleName], + Namespace: autoscalingRunnerSet.Namespace, + Finalizers: []string{autoscalingRunnerSetCleanupFinalizerName}, + }, + } + + err = k8sClient.Create(ctx, role) + Expect(err).NotTo(HaveOccurred(), "failed to create kubernetes mode role") + + serviceAccount := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: autoscalingRunnerSet.Annotations[AnnotationKeyKubernetesModeServiceAccountName], + Namespace: autoscalingRunnerSet.Namespace, + Finalizers: []string{autoscalingRunnerSetCleanupFinalizerName}, + }, + } + + err = k8sClient.Create(ctx, serviceAccount) + Expect(err).NotTo(HaveOccurred(), "failed to create kubernetes mode service account") + + roleBinding := &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: autoscalingRunnerSet.Annotations[AnnotationKeyKubernetesModeRoleBindingName], + Namespace: autoscalingRunnerSet.Namespace, + Finalizers: []string{autoscalingRunnerSetCleanupFinalizerName}, + }, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + Name: serviceAccount.Name, + Namespace: serviceAccount.Namespace, + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + // Kind is the type of resource being referenced + Kind: "Role", + Name: role.Name, + }, + } + + err = k8sClient.Create(ctx, roleBinding) + Expect(err).NotTo(HaveOccurred(), "failed to create kubernetes mode role binding") + + err = k8sClient.Create(ctx, autoscalingRunnerSet) + Expect(err).NotTo(HaveOccurred(), "failed to create AutoScalingRunnerSet") + + Eventually( + func() (string, error) { + created := new(v1alpha1.AutoscalingRunnerSet) + err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, created) + if err != nil { + return "", err + } + if len(created.Finalizers) == 0 { + return "", nil + } + return created.Finalizers[0], nil + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(BeEquivalentTo(autoscalingRunnerSetFinalizerName), "AutoScalingRunnerSet should have a finalizer") + + err = k8sClient.Delete(ctx, autoscalingRunnerSet) + Expect(err).NotTo(HaveOccurred(), "failed to delete autoscaling runner set") + + err = k8sClient.Delete(ctx, roleBinding) + Expect(err).NotTo(HaveOccurred(), "failed to delete kubernetes mode role binding") + + err = k8sClient.Delete(ctx, role) + Expect(err).NotTo(HaveOccurred(), "failed to delete kubernetes mode role") + + err = k8sClient.Delete(ctx, serviceAccount) + Expect(err).NotTo(HaveOccurred(), "failed to delete kubernetes mode service account") + + Eventually( + func() bool { + r := new(rbacv1.RoleBinding) + err := k8sClient.Get(ctx, types.NamespacedName{ + Name: roleBinding.Name, + Namespace: roleBinding.Namespace, + }, r) + + return errors.IsNotFound(err) + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(BeTrue(), "Expected role binding to be cleaned up") + + Eventually( + func() bool { + r := new(rbacv1.Role) + err := k8sClient.Get(ctx, types.NamespacedName{ + Name: role.Name, + Namespace: role.Namespace, + }, r) + + return errors.IsNotFound(err) + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(BeTrue(), "Expected role to be cleaned up") + }) + + It("Should clean up manager permissions and no-permission service account", func() { + ctx := context.Background() + autoscalingNS, mgr := createNamespace(GinkgoT(), k8sClient) + + 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) + + min := 1 + max := 10 + autoscalingRunnerSet := &v1alpha1.AutoscalingRunnerSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-asrs", + Namespace: autoscalingNS.Name, + Labels: map[string]string{ + "app.kubernetes.io/name": "gha-runner-scale-set", + }, + Annotations: map[string]string{ + AnnotationKeyManagerRoleName: "manager-role", + AnnotationKeyManagerRoleBindingName: "manager-role-binding", + AnnotationKeyGitHubSecretName: "gh-secret-name", + AnnotationKeyNoPermissionServiceAccountName: "no-permission-sa", + }, + }, + Spec: v1alpha1.AutoscalingRunnerSetSpec{ + GitHubConfigUrl: "https://github.com/owner/repo", + MaxRunners: &max, + MinRunners: &min, + RunnerGroup: "testgroup", + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "runner", + Image: "ghcr.io/actions/runner", + }, + }, + }, + }, + }, + } + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: autoscalingRunnerSet.Annotations[AnnotationKeyGitHubSecretName], + Namespace: autoscalingRunnerSet.Namespace, + Finalizers: []string{autoscalingRunnerSetCleanupFinalizerName}, + }, + Data: map[string][]byte{ + "github_token": []byte(defaultGitHubToken), + }, + } + + err = k8sClient.Create(context.Background(), secret) + Expect(err).NotTo(HaveOccurred(), "failed to create github secret") + + autoscalingRunnerSet.Spec.GitHubConfigSecret = secret.Name + + role := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: autoscalingRunnerSet.Annotations[AnnotationKeyManagerRoleName], + Namespace: autoscalingRunnerSet.Namespace, + Finalizers: []string{autoscalingRunnerSetCleanupFinalizerName}, + }, + } + + err = k8sClient.Create(ctx, role) + Expect(err).NotTo(HaveOccurred(), "failed to create manager role") + + roleBinding := &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: autoscalingRunnerSet.Annotations[AnnotationKeyManagerRoleBindingName], + Namespace: autoscalingRunnerSet.Namespace, + Finalizers: []string{autoscalingRunnerSetCleanupFinalizerName}, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "Role", + Name: role.Name, + }, + } + + err = k8sClient.Create(ctx, roleBinding) + Expect(err).NotTo(HaveOccurred(), "failed to create manager role binding") + + noPermissionServiceAccount := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: autoscalingRunnerSet.Annotations[AnnotationKeyNoPermissionServiceAccountName], + Namespace: autoscalingRunnerSet.Namespace, + Finalizers: []string{autoscalingRunnerSetCleanupFinalizerName}, + }, + } + + err = k8sClient.Create(ctx, noPermissionServiceAccount) + Expect(err).NotTo(HaveOccurred(), "failed to create no permission service account") + + err = k8sClient.Create(ctx, autoscalingRunnerSet) + Expect(err).NotTo(HaveOccurred(), "failed to create AutoScalingRunnerSet") + + Eventually( + func() (string, error) { + created := new(v1alpha1.AutoscalingRunnerSet) + err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, created) + if err != nil { + return "", err + } + if len(created.Finalizers) == 0 { + return "", nil + } + return created.Finalizers[0], nil + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(BeEquivalentTo(autoscalingRunnerSetFinalizerName), "AutoScalingRunnerSet should have a finalizer") + + err = k8sClient.Delete(ctx, autoscalingRunnerSet) + Expect(err).NotTo(HaveOccurred(), "failed to delete autoscaling runner set") + + err = k8sClient.Delete(ctx, noPermissionServiceAccount) + Expect(err).NotTo(HaveOccurred(), "failed to delete no permission service account") + + err = k8sClient.Delete(ctx, secret) + Expect(err).NotTo(HaveOccurred(), "failed to delete GitHub secret") + + err = k8sClient.Delete(ctx, roleBinding) + Expect(err).NotTo(HaveOccurred(), "failed to delete manager role binding") + + err = k8sClient.Delete(ctx, role) + Expect(err).NotTo(HaveOccurred(), "failed to delete manager role") + + Eventually( + func() bool { + r := new(corev1.ServiceAccount) + err := k8sClient.Get( + ctx, + types.NamespacedName{ + Name: noPermissionServiceAccount.Name, + Namespace: noPermissionServiceAccount.Namespace, + }, + r, + ) + return errors.IsNotFound(err) + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(BeTrue(), "Expected no permission service account to be cleaned up") + + Eventually( + func() bool { + r := new(corev1.Secret) + err := k8sClient.Get(ctx, types.NamespacedName{ + Name: secret.Name, + Namespace: secret.Namespace, + }, r) + + return errors.IsNotFound(err) + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(BeTrue(), "Expected role binding to be cleaned up") + + Eventually( + func() bool { + r := new(rbacv1.RoleBinding) + err := k8sClient.Get(ctx, types.NamespacedName{ + Name: roleBinding.Name, + Namespace: roleBinding.Namespace, + }, r) + + return errors.IsNotFound(err) + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(BeTrue(), "Expected role binding to be cleaned up") + + Eventually( + func() bool { + r := new(rbacv1.Role) + err := k8sClient.Get( + ctx, + types.NamespacedName{ + Name: role.Name, + Namespace: role.Namespace, + }, + r, + ) + + return errors.IsNotFound(err) + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(BeTrue(), "Expected role to be cleaned up") + }) +}) diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index 4ca6abc6..0ea81461 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/controllers/actions.github.com/resourcebuilder.go @@ -43,6 +43,17 @@ const ( labelKeyListenerNamespace = "auto-scaling-listener-namespace" ) +// Annotations applied for later cleanup of resources +const ( + AnnotationKeyManagerRoleBindingName = "actions.github.com/cleanup-manager-role-binding" + AnnotationKeyManagerRoleName = "actions.github.com/cleanup-manager-role-name" + AnnotationKeyKubernetesModeRoleName = "actions.github.com/cleanup-kubernetes-mode-role-name" + AnnotationKeyKubernetesModeRoleBindingName = "actions.github.com/cleanup-kubernetes-mode-role-binding-name" + AnnotationKeyKubernetesModeServiceAccountName = "actions.github.com/cleanup-kubernetes-mode-service-account-name" + AnnotationKeyGitHubSecretName = "actions.github.com/cleanup-github-secret-name" + AnnotationKeyNoPermissionServiceAccountName = "actions.github.com/cleanup-no-permission-service-account-name" +) + var commonLabelKeys = [...]string{ LabelKeyKubernetesPartOf, LabelKeyKubernetesComponent,