Configure listener pod with the secret instead of env (#2965)

Co-authored-by: Bassem Dghaidi <568794+Link-@users.noreply.github.com>
This commit is contained in:
Nikola Jokic
2023-10-19 12:29:32 +02:00
committed by GitHub
parent e1edb84abe
commit 2117fd1892
9 changed files with 401 additions and 321 deletions

View File

@@ -287,6 +287,21 @@ func (r *AutoscalingListenerReconciler) cleanupResources(ctx context.Context, au
}
logger.Info("Listener pod is deleted")
var secret corev1.Secret
err = r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Namespace, Name: scaleSetListenerConfigName(autoscalingListener)}, &secret)
switch {
case err == nil:
if secret.ObjectMeta.DeletionTimestamp.IsZero() {
logger.Info("Deleting the listener config secret")
if err := r.Delete(ctx, &secret); err != nil {
return false, fmt.Errorf("failed to delete listener config secret: %v", err)
}
}
return false, nil
case err != nil && !kerrors.IsNotFound(err):
return false, fmt.Errorf("failed to get listener config secret: %v", err)
}
if autoscalingListener.Spec.Proxy != nil {
logger.Info("Cleaning up the listener proxy secret")
proxySecret := new(corev1.Secret)
@@ -418,13 +433,13 @@ func (r *AutoscalingListenerReconciler) createListenerPod(ctx context.Context, a
}
}
cert := ""
if autoscalingListener.Spec.GitHubServerTLS != nil {
env, err := r.certificateEnvVarForListener(ctx, autoscalingRunnerSet, autoscalingListener)
var err error
cert, err = r.certificate(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)
}
var metricsConfig *listenerMetricsServerConfig
@@ -435,7 +450,35 @@ func (r *AutoscalingListenerReconciler) createListenerPod(ctx context.Context, a
}
}
newPod, err := r.resourceBuilder.newScaleSetListenerPod(autoscalingListener, serviceAccount, secret, metricsConfig, envs...)
var podConfig corev1.Secret
if err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Namespace, Name: scaleSetListenerConfigName(autoscalingListener)}, &podConfig); err != nil {
if !kerrors.IsNotFound(err) {
logger.Error(err, "Unable to get listener config secret", "namespace", autoscalingListener.Namespace, "name", scaleSetListenerConfigName(autoscalingListener))
return ctrl.Result{Requeue: true}, err
}
logger.Info("Creating listener config secret")
podConfig, err := r.resourceBuilder.newScaleSetListenerConfig(autoscalingListener, secret, metricsConfig, cert)
if err != nil {
logger.Error(err, "Failed to build listener config secret")
return ctrl.Result{}, err
}
if err := ctrl.SetControllerReference(autoscalingListener, podConfig, r.Scheme); err != nil {
logger.Error(err, "Failed to set controller reference")
return ctrl.Result{}, err
}
if err := r.Create(ctx, podConfig); err != nil {
logger.Error(err, "Unable to create listener config secret", "namespace", podConfig.Namespace, "name", podConfig.Name)
return ctrl.Result{}, err
}
return ctrl.Result{Requeue: true}, nil
}
newPod, err := r.resourceBuilder.newScaleSetListenerPod(autoscalingListener, &podConfig, serviceAccount, secret, metricsConfig, envs...)
if err != nil {
logger.Error(err, "Failed to build listener pod")
return ctrl.Result{}, err
@@ -456,13 +499,13 @@ 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) {
func (r *AutoscalingListenerReconciler) certificate(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, autoscalingListener *v1alpha1.AutoscalingListener) (string, error) {
if autoscalingListener.Spec.GitHubServerTLS.CertificateFrom == nil {
return corev1.EnvVar{}, fmt.Errorf("githubServerTLS.certificateFrom is not specified")
return "", 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")
return "", fmt.Errorf("githubServerTLS.certificateFrom.configMapKeyRef is not specified")
}
var configmap corev1.ConfigMap
@@ -475,7 +518,7 @@ func (r *AutoscalingListenerReconciler) certificateEnvVarForListener(ctx context
&configmap,
)
if err != nil {
return corev1.EnvVar{}, fmt.Errorf(
return "", fmt.Errorf(
"failed to get configmap %s: %w",
autoscalingListener.Spec.GitHubServerTLS.CertificateFrom.ConfigMapKeyRef.Name,
err,
@@ -484,17 +527,14 @@ func (r *AutoscalingListenerReconciler) certificateEnvVarForListener(ctx context
certificate, ok := configmap.Data[autoscalingListener.Spec.GitHubServerTLS.CertificateFrom.ConfigMapKeyRef.Key]
if !ok {
return corev1.EnvVar{}, fmt.Errorf(
return "", 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
return certificate, nil
}
func (r *AutoscalingListenerReconciler) createSecretsForListener(ctx context.Context, autoscalingListener *v1alpha1.AutoscalingListener, secret *corev1.Secret, logger logr.Logger) (ctrl.Result, error) {

View File

@@ -2,6 +2,7 @@ package actionsgithubcom
import (
"context"
"encoding/json"
"fmt"
"os"
"path/filepath"
@@ -13,6 +14,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"
listenerconfig "github.com/actions/actions-runner-controller/cmd/githubrunnerscalesetlistener/config"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
kerrors "k8s.io/apimachinery/pkg/api/errors"
@@ -103,6 +105,19 @@ var _ = Describe("Test AutoScalingListener controller", func() {
Context("When creating a new AutoScalingListener", func() {
It("It should create/add all required resources for a new AutoScalingListener (finalizer, secret, service account, role, rolebinding, pod)", func() {
config := new(corev1.Secret)
Eventually(
func() error {
err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerConfigName(autoscalingListener), Namespace: configSecret.Namespace}, config)
if err != nil {
return err
}
return nil
},
autoscalingListenerTestTimeout,
autoscalingListenerTestInterval,
).Should(Succeed(), "Config secret should be created")
// Check if finalizer is added
created := new(actionsv1alpha1.AutoscalingListener)
Eventually(
@@ -251,6 +266,17 @@ var _ = Describe("Test AutoScalingListener controller", func() {
autoscalingListenerTestInterval,
).Should(BeTrue(), "failed to delete role")
// Cleanup the listener config
Eventually(
func() bool {
config := new(corev1.Secret)
err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerConfigName(autoscalingListener), Namespace: autoscalingListener.Namespace}, config)
return kerrors.IsNotFound(err)
},
autoscalingListenerTestTimeout,
autoscalingListenerTestInterval,
).Should(BeTrue(), "failed to delete config secret")
// Cleanup the listener service account
Eventually(
func() error {
@@ -501,6 +527,17 @@ var _ = Describe("Test AutoScalingListener customization", func() {
autoscalingListenerTestTimeout,
autoscalingListenerTestInterval).Should(BeEquivalentTo(autoscalingListenerFinalizerName), "AutoScalingListener should have a finalizer")
// Check if config is created
config := new(corev1.Secret)
Eventually(
func() error {
err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerConfigName(autoscalingListener), Namespace: autoscalingListener.Namespace}, config)
return err
},
autoscalingListenerTestTimeout,
autoscalingListenerTestInterval,
).Should(Succeed(), "Config secret should be created")
// Check if pod is created
pod := new(corev1.Pod)
Eventually(
@@ -989,31 +1026,26 @@ var _ = Describe("Test GitHub Server TLS configuration", func() {
})
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)
It("It should set the certificates in the config of the pod", func() {
config := new(corev1.Secret)
Eventually(
func(g Gomega) {
err := k8sClient.Get(
ctx,
client.ObjectKey{
Name: autoscalingListener.Name,
Name: scaleSetListenerConfigName(autoscalingListener),
Namespace: autoscalingListener.Namespace,
},
pod,
config,
)
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")
g.Expect(config.Data["config.json"]).ToNot(BeEmpty(), "listener configuration file should not be empty")
var listenerConfig listenerconfig.Config
err = json.Unmarshal(config.Data["config.json"], &listenerConfig)
g.Expect(err).NotTo(HaveOccurred(), "failed to parse listener configuration file")
cert, err := os.ReadFile(filepath.Join(
"../../",
@@ -1024,7 +1056,7 @@ var _ = Describe("Test GitHub Server TLS configuration", func() {
))
g.Expect(err).NotTo(HaveOccurred(), "failed to read rootCA.crt")
g.Expect(env.Value).To(
g.Expect(listenerConfig.ServerRootCA).To(
BeEquivalentTo(string(cert)),
"GITHUB_SERVER_ROOT_CA should be the rootCA.crt",
)

View File

@@ -1,7 +1,9 @@
package actionsgithubcom
import (
"bytes"
"context"
"encoding/json"
"fmt"
"math"
"net"
@@ -9,6 +11,7 @@ import (
"github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1"
"github.com/actions/actions-runner-controller/build"
listenerconfig "github.com/actions/actions-runner-controller/cmd/githubrunnerscalesetlistener/config"
"github.com/actions/actions-runner-controller/github/actions"
"github.com/actions/actions-runner-controller/hash"
"github.com/actions/actions-runner-controller/logging"
@@ -33,21 +36,6 @@ var commonLabelKeys = [...]string{
LabelKeyGitHubRepository,
}
var reservedListenerContainerEnvKeys = map[string]struct{}{
"GITHUB_CONFIGURE_URL": {},
"GITHUB_EPHEMERAL_RUNNER_SET_NAMESPACE": {},
"GITHUB_EPHEMERAL_RUNNER_SET_NAME": {},
"GITHUB_MAX_RUNNERS": {},
"GITHUB_MIN_RUNNERS": {},
"GITHUB_RUNNER_SCALE_SET_ID": {},
"GITHUB_RUNNER_LOG_LEVEL": {},
"GITHUB_RUNNER_LOG_FORMAT": {},
"GITHUB_TOKEN": {},
"GITHUB_APP_ID": {},
"GITHUB_APP_INSTALLATION_ID": {},
"GITHUB_APP_PRIVATE_KEY": {},
}
const labelValueKubernetesPartOf = "gha-runner-scale-set"
var scaleSetListenerLogLevel = DefaultScaleSetListenerLogLevel
@@ -144,133 +132,101 @@ type listenerMetricsServerConfig struct {
endpoint string
}
func (b *resourceBuilder) newScaleSetListenerPod(autoscalingListener *v1alpha1.AutoscalingListener, serviceAccount *corev1.ServiceAccount, secret *corev1.Secret, metricsConfig *listenerMetricsServerConfig, envs ...corev1.EnvVar) (*corev1.Pod, error) {
func (lm *listenerMetricsServerConfig) containerPort() (corev1.ContainerPort, error) {
_, portStr, err := net.SplitHostPort(lm.addr)
if err != nil {
return corev1.ContainerPort{}, err
}
port, err := strconv.ParseInt(portStr, 10, 32)
if err != nil {
return corev1.ContainerPort{}, err
}
return corev1.ContainerPort{
ContainerPort: int32(port),
Protocol: corev1.ProtocolTCP,
Name: "metrics",
}, nil
}
func (b *resourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha1.AutoscalingListener, secret *corev1.Secret, metricsConfig *listenerMetricsServerConfig, cert string) (*corev1.Secret, error) {
var (
metricsAddr = ""
metricsEndpoint = ""
)
if metricsConfig != nil {
metricsAddr = metricsConfig.addr
metricsEndpoint = metricsConfig.endpoint
}
var appID int64
if id, ok := secret.Data["github_app_id"]; ok {
var err error
appID, err = strconv.ParseInt(string(id), 10, 64)
if err != nil {
return nil, fmt.Errorf("failed to convert github_app_id to int: %v", err)
}
}
var appInstallationID int64
if id, ok := secret.Data["github_app_installation_id"]; ok {
var err error
appInstallationID, err = strconv.ParseInt(string(id), 10, 64)
if err != nil {
return nil, fmt.Errorf("failed to convert github_app_installation_id to int: %v", err)
}
}
config := listenerconfig.Config{
ConfigureUrl: autoscalingListener.Spec.GitHubConfigUrl,
AppID: appID,
AppInstallationID: appInstallationID,
AppPrivateKey: string(secret.Data["github_app_private_key"]),
Token: string(secret.Data["github_token"]),
EphemeralRunnerSetNamespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace,
EphemeralRunnerSetName: autoscalingListener.Spec.EphemeralRunnerSetName,
MaxRunners: autoscalingListener.Spec.MaxRunners,
MinRunners: autoscalingListener.Spec.MinRunners,
RunnerScaleSetId: autoscalingListener.Spec.RunnerScaleSetId,
RunnerScaleSetName: autoscalingListener.Spec.AutoscalingRunnerSetName,
ServerRootCA: cert,
LogLevel: scaleSetListenerLogLevel,
LogFormat: scaleSetListenerLogFormat,
MetricsAddr: metricsAddr,
MetricsEndpoint: metricsEndpoint,
}
var buf bytes.Buffer
if err := json.NewEncoder(&buf).Encode(config); err != nil {
return nil, fmt.Errorf("failed to encode config: %w", err)
}
return &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: scaleSetListenerConfigName(autoscalingListener),
Namespace: autoscalingListener.Namespace,
},
Data: map[string][]byte{
"config.json": buf.Bytes(),
},
}, nil
}
func (b *resourceBuilder) newScaleSetListenerPod(autoscalingListener *v1alpha1.AutoscalingListener, podConfig *corev1.Secret, serviceAccount *corev1.ServiceAccount, secret *corev1.Secret, metricsConfig *listenerMetricsServerConfig, envs ...corev1.EnvVar) (*corev1.Pod, error) {
listenerEnv := []corev1.EnvVar{
{
Name: "GITHUB_CONFIGURE_URL",
Value: autoscalingListener.Spec.GitHubConfigUrl,
},
{
Name: "GITHUB_EPHEMERAL_RUNNER_SET_NAMESPACE",
Value: autoscalingListener.Spec.AutoscalingRunnerSetNamespace,
},
{
Name: "GITHUB_EPHEMERAL_RUNNER_SET_NAME",
Value: autoscalingListener.Spec.EphemeralRunnerSetName,
},
{
Name: "GITHUB_MAX_RUNNERS",
Value: strconv.Itoa(autoscalingListener.Spec.MaxRunners),
},
{
Name: "GITHUB_MIN_RUNNERS",
Value: strconv.Itoa(autoscalingListener.Spec.MinRunners),
},
{
Name: "GITHUB_RUNNER_SCALE_SET_ID",
Value: strconv.Itoa(autoscalingListener.Spec.RunnerScaleSetId),
},
{
Name: "GITHUB_RUNNER_SCALE_SET_NAME",
Value: autoscalingListener.Spec.AutoscalingRunnerSetName,
},
{
Name: "GITHUB_RUNNER_LOG_LEVEL",
Value: scaleSetListenerLogLevel,
},
{
Name: "GITHUB_RUNNER_LOG_FORMAT",
Value: scaleSetListenerLogFormat,
Name: "LISTENER_CONFIG_PATH",
Value: "/etc/gha-listener/config.json",
},
}
listenerEnv = append(listenerEnv, envs...)
if _, ok := secret.Data["github_token"]; ok {
listenerEnv = append(listenerEnv, corev1.EnvVar{
Name: "GITHUB_TOKEN",
ValueFrom: &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: secret.Name,
},
Key: "github_token",
},
},
})
}
if _, ok := secret.Data["github_app_id"]; ok {
listenerEnv = append(listenerEnv, corev1.EnvVar{
Name: "GITHUB_APP_ID",
ValueFrom: &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: secret.Name,
},
Key: "github_app_id",
},
},
})
}
if _, ok := secret.Data["github_app_installation_id"]; ok {
listenerEnv = append(listenerEnv, corev1.EnvVar{
Name: "GITHUB_APP_INSTALLATION_ID",
ValueFrom: &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: secret.Name,
},
Key: "github_app_installation_id",
},
},
})
}
if _, ok := secret.Data["github_app_private_key"]; ok {
listenerEnv = append(listenerEnv, corev1.EnvVar{
Name: "GITHUB_APP_PRIVATE_KEY",
ValueFrom: &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: secret.Name,
},
Key: "github_app_private_key",
},
},
})
}
var ports []corev1.ContainerPort
if metricsConfig != nil && len(metricsConfig.addr) != 0 {
listenerEnv = append(
listenerEnv,
corev1.EnvVar{
Name: "GITHUB_METRICS_ADDR",
Value: metricsConfig.addr,
},
corev1.EnvVar{
Name: "GITHUB_METRICS_ENDPOINT",
Value: metricsConfig.endpoint,
},
)
_, portStr, err := net.SplitHostPort(metricsConfig.addr)
port, err := metricsConfig.containerPort()
if err != nil {
return nil, fmt.Errorf("failed to split host:port for metrics address: %v", err)
return nil, fmt.Errorf("failed to convert metrics server address to container port: %v", err)
}
port, err := strconv.ParseInt(portStr, 10, 32)
if err != nil {
return nil, fmt.Errorf("failed to convert port %q to int32: %v", portStr, err)
}
ports = append(
ports,
corev1.ContainerPort{
ContainerPort: int32(port),
Protocol: corev1.ProtocolTCP,
Name: "metrics",
},
)
ports = append(ports, port)
}
podSpec := corev1.PodSpec{
@@ -285,6 +241,23 @@ func (b *resourceBuilder) newScaleSetListenerPod(autoscalingListener *v1alpha1.A
"/github-runnerscaleset-listener",
},
Ports: ports,
VolumeMounts: []corev1.VolumeMount{
{
Name: "listener-config",
MountPath: "/etc/gha-listener",
ReadOnly: true,
},
},
},
},
Volumes: []corev1.Volume{
{
Name: "listener-config",
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: podConfig.Name,
},
},
},
},
ImagePullSecrets: autoscalingListener.Spec.ImagePullSecrets,
@@ -348,7 +321,7 @@ func mergeListenerPodWithTemplate(pod *corev1.Pod, tmpl *corev1.PodTemplateSpec)
}
// apply pod related spec
// NOTE: fields that should be gnored
// NOTE: fields that should be ignored
// - service account based fields
if tmpl.Spec.RestartPolicy != "" {
@@ -359,7 +332,7 @@ func mergeListenerPodWithTemplate(pod *corev1.Pod, tmpl *corev1.PodTemplateSpec)
pod.Spec.ImagePullSecrets = tmpl.Spec.ImagePullSecrets
}
pod.Spec.Volumes = tmpl.Spec.Volumes
pod.Spec.Volumes = append(pod.Spec.Volumes, tmpl.Spec.Volumes...)
pod.Spec.InitContainers = tmpl.Spec.InitContainers
pod.Spec.EphemeralContainers = tmpl.Spec.EphemeralContainers
pod.Spec.TerminationGracePeriodSeconds = tmpl.Spec.TerminationGracePeriodSeconds
@@ -405,11 +378,7 @@ func mergeListenerContainer(base, from *corev1.Container) {
base.Command = from.Command
}
for _, v := range from.Env {
if _, ok := reservedListenerContainerEnvKeys[v.Name]; !ok {
base.Env = append(base.Env, v)
}
}
base.Env = append(base.Env, from.Env...)
if from.ImagePullPolicy != "" {
base.ImagePullPolicy = from.ImagePullPolicy
@@ -682,6 +651,10 @@ func (b *resourceBuilder) newEphemeralRunnerJitSecret(ephemeralRunner *v1alpha1.
}
}
func scaleSetListenerConfigName(autoscalingListener *v1alpha1.AutoscalingListener) string {
return fmt.Sprintf("%s-config", autoscalingListener.Name)
}
func scaleSetListenerName(autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet) string {
namespaceHash := hash.FNVHashString(autoscalingRunnerSet.Namespace)
if len(namespaceHash) > 8 {

View File

@@ -68,7 +68,7 @@ func TestLabelPropagation(t *testing.T) {
Name: "test",
},
}
listenerPod, err := b.newScaleSetListenerPod(listener, listenerServiceAccount, listenerSecret, nil)
listenerPod, err := b.newScaleSetListenerPod(listener, &corev1.Secret{}, listenerServiceAccount, listenerSecret, nil)
require.NoError(t, err)
assert.Equal(t, listenerPod.Labels, listener.Labels)