diff --git a/cmd/ghalistener/config/config.go b/cmd/ghalistener/config/config.go index 51c91de2..ec0a5176 100644 --- a/cmd/ghalistener/config/config.go +++ b/cmd/ghalistener/config/config.go @@ -20,10 +20,13 @@ import ( ) type Config struct { - ConfigureUrl string `json:"configure_url"` - VaultType string `json:"vault_type"` - VaultLookupKey string `json:"vault_lookup_key"` - appconfig.AppConfig + ConfigureUrl string `json:"configure_url"` + VaultType vault.VaultType `json:"vault_type"` + VaultLookupKey string `json:"vault_lookup_key"` + // AppConfig contains the GitHub App configuration. + // It is initially set to nil if VaultType is set. + // Otherwise, it is populated with the GitHub App credentials from the GitHub secret. + *appconfig.AppConfig EphemeralRunnerSetNamespace string `json:"ephemeral_runner_set_namespace"` EphemeralRunnerSetName string `json:"ephemeral_runner_set_name"` MaxRunners int `json:"max_runners"` @@ -82,7 +85,7 @@ func Read(ctx context.Context, configPath string) (*Config, error) { return nil, fmt.Errorf("failed to read app config from string: %v", err) } - config.AppConfig = *appConfig + config.AppConfig = appConfig if err := config.Validate(); err != nil { return nil, fmt.Errorf("config validation failed: %w", err) @@ -109,8 +112,14 @@ func (c *Config) Validate() error { return fmt.Errorf(`MinRunners "%d" cannot be greater than MaxRunners "%d"`, c.MinRunners, c.MaxRunners) } - if err := c.AppConfig.Validate(); err != nil { - return fmt.Errorf("AppConfig validation failed: %w", err) + if c.VaultType != "" && c.VaultLookupKey == "" { + return fmt.Errorf("VaultLookupKey is required when VaultType is set to %q", c.VaultType) + } + + if c.VaultType != "" && c.VaultLookupKey == "" { + if err := c.AppConfig.Validate(); err != nil { + return fmt.Errorf("AppConfig validation failed: %w", err) + } } return nil diff --git a/cmd/ghalistener/config/config_client_test.go b/cmd/ghalistener/config/config_client_test.go index 8e8f4eca..4fc37341 100644 --- a/cmd/ghalistener/config/config_client_test.go +++ b/cmd/ghalistener/config/config_client_test.go @@ -54,7 +54,7 @@ func TestCustomerServerRootCA(t *testing.T) { config := config.Config{ ConfigureUrl: server.ConfigURLForOrg("myorg"), ServerRootCA: certsString, - AppConfig: appconfig.AppConfig{ + AppConfig: &appconfig.AppConfig{ Token: "token", }, } @@ -83,7 +83,7 @@ func TestProxySettings(t *testing.T) { config := config.Config{ ConfigureUrl: "https://github.com/org/repo", - AppConfig: appconfig.AppConfig{ + AppConfig: &appconfig.AppConfig{ Token: "token", }, } @@ -115,7 +115,7 @@ func TestProxySettings(t *testing.T) { config := config.Config{ ConfigureUrl: "https://github.com/org/repo", - AppConfig: appconfig.AppConfig{ + AppConfig: &appconfig.AppConfig{ Token: "token", }, } @@ -152,7 +152,7 @@ func TestProxySettings(t *testing.T) { config := config.Config{ ConfigureUrl: "https://github.com/org/repo", - AppConfig: appconfig.AppConfig{ + AppConfig: &appconfig.AppConfig{ Token: "token", }, } diff --git a/cmd/ghalistener/config/config_test.go b/cmd/ghalistener/config/config_validation_test.go similarity index 67% rename from cmd/ghalistener/config/config_test.go rename to cmd/ghalistener/config/config_validation_test.go index ecb711f9..2e3d5bc5 100644 --- a/cmd/ghalistener/config/config_test.go +++ b/cmd/ghalistener/config/config_validation_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1/appconfig" + "github.com/actions/actions-runner-controller/vault" "github.com/stretchr/testify/assert" ) @@ -16,7 +17,7 @@ func TestConfigValidationMinMax(t *testing.T) { RunnerScaleSetId: 1, MinRunners: 5, MaxRunners: 2, - AppConfig: appconfig.AppConfig{ + AppConfig: &appconfig.AppConfig{ Token: "token", }, } @@ -42,7 +43,7 @@ func TestConfigValidationAppKey(t *testing.T) { t.Run("app id integer", func(t *testing.T) { t.Parallel() config := &Config{ - AppConfig: appconfig.AppConfig{ + AppConfig: &appconfig.AppConfig{ AppID: "1", AppInstallationID: 10, }, @@ -59,7 +60,7 @@ func TestConfigValidationAppKey(t *testing.T) { t.Run("app id as client id", func(t *testing.T) { t.Parallel() config := &Config{ - AppConfig: appconfig.AppConfig{ + AppConfig: &appconfig.AppConfig{ AppID: "Iv23f8doAlphaNumer1c", AppInstallationID: 10, }, @@ -76,7 +77,7 @@ func TestConfigValidationAppKey(t *testing.T) { func TestConfigValidationOnlyOneTypeOfCredentials(t *testing.T) { config := &Config{ - AppConfig: appconfig.AppConfig{ + AppConfig: &appconfig.AppConfig{ AppID: "1", AppInstallationID: 10, AppPrivateKey: "asdf", @@ -100,7 +101,7 @@ func TestConfigValidation(t *testing.T) { RunnerScaleSetId: 1, MinRunners: 1, MaxRunners: 5, - AppConfig: appconfig.AppConfig{ + AppConfig: &appconfig.AppConfig{ Token: "asdf", }, } @@ -121,3 +122,50 @@ func TestConfigValidationConfigUrl(t *testing.T) { assert.ErrorContains(t, err, "GitHubConfigUrl is not provided", "Expected error about missing ConfigureUrl") } + +func TestConfigValidationWithVaultConfig(t *testing.T) { + t.Run("valid", func(t *testing.T) { + config := &Config{ + ConfigureUrl: "https://github.com/actions", + EphemeralRunnerSetNamespace: "namespace", + EphemeralRunnerSetName: "deployment", + RunnerScaleSetId: 1, + MinRunners: 1, + MaxRunners: 5, + VaultType: vault.VaultTypeAzureKeyVault, + VaultLookupKey: "testkey", + } + err := config.Validate() + assert.NoError(t, err, "Expected no error for valid vault type") + }) + + t.Run("invalid vault type", func(t *testing.T) { + config := &Config{ + ConfigureUrl: "https://github.com/actions", + EphemeralRunnerSetNamespace: "namespace", + EphemeralRunnerSetName: "deployment", + RunnerScaleSetId: 1, + MinRunners: 1, + MaxRunners: 5, + VaultType: vault.VaultType("invalid_vault_type"), + VaultLookupKey: "testkey", + } + err := config.Validate() + assert.ErrorContains(t, err, `unknown vault type: "invalid_vault_type"`, "Expected error for invalid vault type") + }) + + t.Run("vault type set without lookup key", func(t *testing.T) { + config := &Config{ + ConfigureUrl: "https://github.com/actions", + EphemeralRunnerSetNamespace: "namespace", + EphemeralRunnerSetName: "deployment", + RunnerScaleSetId: 1, + MinRunners: 1, + MaxRunners: 5, + VaultType: vault.VaultTypeAzureKeyVault, + VaultLookupKey: "", + } + err := config.Validate() + assert.ErrorContains(t, err, `vault type set to "invalid_vault_type", but lookup key is empty`, "Expected error for vault type without lookup key") + }) +} diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index b064648b..142cb2a7 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller.go +++ b/controllers/actions.github.com/autoscalinglistener_controller.go @@ -32,6 +32,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" v1alpha1 "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" + "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1/appconfig" "github.com/actions/actions-runner-controller/controllers/actions.github.com/metrics" "github.com/actions/actions-runner-controller/github/actions" hash "github.com/actions/actions-runner-controller/hash" @@ -128,12 +129,16 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. return ctrl.Result{}, err } - // Check if the GitHub config secret exists - secret := new(corev1.Secret) - if err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, Name: autoscalingListener.Spec.GitHubConfigSecret}, secret); err != nil { - log.Error(err, "Failed to find GitHub config secret.", - "namespace", autoscalingListener.Spec.AutoscalingRunnerSetNamespace, - "name", autoscalingListener.Spec.GitHubConfigSecret) + appConfig, err := r.GetAppConfig(ctx, &autoscalingRunnerSet) + if err != nil { + log.Error( + err, + "Failed to get app config for AutoscalingRunnerSet.", + "namespace", + autoscalingRunnerSet.Namespace, + "name", + autoscalingRunnerSet.GitHubConfigSecret, + ) return ctrl.Result{}, err } @@ -218,7 +223,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. // Create a listener pod in the controller namespace log.Info("Creating a listener pod") - return r.createListenerPod(ctx, &autoscalingRunnerSet, autoscalingListener, serviceAccount, secret, log) + return r.createListenerPod(ctx, &autoscalingRunnerSet, autoscalingListener, serviceAccount, appConfig, log) } cs := listenerContainerStatus(listenerPod) @@ -377,7 +382,7 @@ func (r *AutoscalingListenerReconciler) createServiceAccountForListener(ctx cont return ctrl.Result{}, nil } -func (r *AutoscalingListenerReconciler) createListenerPod(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, autoscalingListener *v1alpha1.AutoscalingListener, serviceAccount *corev1.ServiceAccount, secret *corev1.Secret, logger logr.Logger) (ctrl.Result, error) { +func (r *AutoscalingListenerReconciler) createListenerPod(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, autoscalingListener *v1alpha1.AutoscalingListener, serviceAccount *corev1.ServiceAccount, appConfig *appconfig.AppConfig, logger logr.Logger) (ctrl.Result, error) { var envs []corev1.EnvVar if autoscalingListener.Spec.Proxy != nil { httpURL := corev1.EnvVar{ @@ -446,7 +451,7 @@ func (r *AutoscalingListenerReconciler) createListenerPod(ctx context.Context, a logger.Info("Creating listener config secret") - podConfig, err := r.newScaleSetListenerConfig(autoscalingListener, secret, metricsConfig, cert) + podConfig, err := r.newScaleSetListenerConfig(autoscalingListener, appConfig, metricsConfig, cert) if err != nil { logger.Error(err, "Failed to build listener config secret") return ctrl.Result{}, err diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go index c2e7b2c8..ba663ffb 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -394,7 +394,7 @@ func (r *AutoscalingRunnerSetReconciler) removeFinalizersFromDependentResources( 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.ActionsClientPool.Get(ctx, autoscalingRunnerSet) + actionsClient, err := r.SecretResolver.GetActionsService(ctx, autoscalingRunnerSet) if len(autoscalingRunnerSet.Spec.RunnerScaleSetName) == 0 { autoscalingRunnerSet.Spec.RunnerScaleSetName = autoscalingRunnerSet.Name } @@ -490,7 +490,7 @@ func (r *AutoscalingRunnerSetReconciler) updateRunnerScaleSetRunnerGroup(ctx con return ctrl.Result{}, err } - actionsClient, err := r.ActionsClientPool.Get(ctx, autoscalingRunnerSet) + actionsClient, err := r.SecretResolver.GetActionsService(ctx, autoscalingRunnerSet) if err != nil { logger.Error(err, "Failed to initialize Actions service client for updating a existing runner scale set") return ctrl.Result{}, err @@ -538,7 +538,7 @@ func (r *AutoscalingRunnerSetReconciler) updateRunnerScaleSetName(ctx context.Co return ctrl.Result{}, nil } - actionsClient, err := r.ActionsClientPool.Get(ctx, autoscalingRunnerSet) + actionsClient, err := r.SecretResolver.GetActionsService(ctx, autoscalingRunnerSet) if err != nil { logger.Error(err, "Failed to initialize Actions service client for updating a existing runner scale set") return ctrl.Result{}, err @@ -589,7 +589,7 @@ func (r *AutoscalingRunnerSetReconciler) deleteRunnerScaleSet(ctx context.Contex return nil } - actionsClient, err := r.ActionsClientPool.Get(ctx, autoscalingRunnerSet) + actionsClient, err := r.SecretResolver.GetActionsService(ctx, autoscalingRunnerSet) if err != nil { logger.Error(err, "Failed to initialize Actions service client for updating a existing runner scale set") return err diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go index 9b149447..1bcae3fc 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go @@ -71,7 +71,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: k8sClient, multiClient: fake.NewMultiClient(), }, @@ -711,7 +711,7 @@ var _ = Describe("Test AutoScalingController updates", Ordered, func() { ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: k8sClient, multiClient: multiClient, }, @@ -831,7 +831,7 @@ var _ = Describe("Test AutoscalingController creation failures", Ordered, func() ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: k8sClient, multiClient: fake.NewMultiClient(), }, @@ -962,7 +962,7 @@ var _ = Describe("Test client optional configuration", Ordered, func() { ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: k8sClient, multiClient: multiClient, }, @@ -1150,7 +1150,7 @@ var _ = Describe("Test client optional configuration", Ordered, func() { ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: k8sClient, multiClient: fake.NewMultiClient(), }, @@ -1163,7 +1163,7 @@ var _ = Describe("Test client optional configuration", Ordered, func() { }) It("should be able to make requests to a server using root CAs", func() { - controller.ActionsClientPool = &ActionsClientPool{ + controller.SecretResolver = &SecretResolver{ k8sClient: k8sClient, multiClient: actions.NewMultiClient(logr.Discard()), } @@ -1392,7 +1392,7 @@ var _ = Describe("Test external permissions cleanup", Ordered, func() { ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: k8sClient, multiClient: fake.NewMultiClient(), }, @@ -1555,7 +1555,7 @@ var _ = Describe("Test external permissions cleanup", Ordered, func() { ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: k8sClient, multiClient: fake.NewMultiClient(), }, @@ -1768,7 +1768,7 @@ var _ = Describe("Test resource version and build version mismatch", func() { ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: k8sClient, multiClient: fake.NewMultiClient(), }, diff --git a/controllers/actions.github.com/ephemeralrunner_controller.go b/controllers/actions.github.com/ephemeralrunner_controller.go index e915220b..f6c62849 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller.go +++ b/controllers/actions.github.com/ephemeralrunner_controller.go @@ -528,7 +528,7 @@ func (r *EphemeralRunnerReconciler) deletePodAsFailed(ctx context.Context, ephem func (r *EphemeralRunnerReconciler) updateStatusWithRunnerConfig(ctx context.Context, ephemeralRunner *v1alpha1.EphemeralRunner, log logr.Logger) (*ctrl.Result, error) { // Runner is not registered with the service. We need to register it first log.Info("Creating ephemeral runner JIT config") - actionsClient, err := r.ActionsClientPool.Get(ctx, ephemeralRunner) + actionsClient, err := r.SecretResolver.GetActionsService(ctx, ephemeralRunner) if err != nil { return &ctrl.Result{}, fmt.Errorf("failed to get actions client for generating JIT config: %w", err) } @@ -755,7 +755,7 @@ func (r *EphemeralRunnerReconciler) updateRunStatusFromPod(ctx context.Context, // runnerRegisteredWithService checks if the runner is still registered with the service // Returns found=false and err=nil if ephemeral runner does not exist in GitHub service and should be deleted func (r EphemeralRunnerReconciler) runnerRegisteredWithService(ctx context.Context, runner *v1alpha1.EphemeralRunner, log logr.Logger) (found bool, err error) { - actionsClient, err := r.ActionsClientPool.Get(ctx, runner) + actionsClient, err := r.SecretResolver.GetActionsService(ctx, runner) if err != nil { return false, fmt.Errorf("failed to get Actions client for ScaleSet: %w", err) } @@ -782,7 +782,7 @@ func (r EphemeralRunnerReconciler) runnerRegisteredWithService(ctx context.Conte } func (r *EphemeralRunnerReconciler) deleteRunnerFromService(ctx context.Context, ephemeralRunner *v1alpha1.EphemeralRunner, log logr.Logger) error { - client, err := r.ActionsClientPool.Get(ctx, ephemeralRunner) + client, err := r.SecretResolver.GetActionsService(ctx, ephemeralRunner) if err != nil { return fmt.Errorf("failed to get actions client for runner: %w", err) } diff --git a/controllers/actions.github.com/ephemeralrunner_controller_test.go b/controllers/actions.github.com/ephemeralrunner_controller_test.go index c6a19142..a2563e0f 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunner_controller_test.go @@ -111,7 +111,7 @@ var _ = Describe("EphemeralRunner", func() { Scheme: mgr.GetScheme(), Log: logf.Log, ResourceBuilder: ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: mgr.GetClient(), multiClient: fake.NewMultiClient(), }, @@ -795,7 +795,7 @@ var _ = Describe("EphemeralRunner", func() { Scheme: mgr.GetScheme(), Log: logf.Log, ResourceBuilder: ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: mgr.GetClient(), multiClient: fake.NewMultiClient( fake.WithDefaultClient( @@ -875,7 +875,7 @@ var _ = Describe("EphemeralRunner", func() { Scheme: mgr.GetScheme(), Log: logf.Log, ResourceBuilder: ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: mgr.GetClient(), multiClient: fake.NewMultiClient(), }, @@ -890,7 +890,7 @@ var _ = Describe("EphemeralRunner", func() { It("uses an actions client with proxy transport", func() { // Use an actual client controller.ResourceBuilder = ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: mgr.GetClient(), multiClient: actions.NewMultiClient(logr.Discard()), }, @@ -1049,7 +1049,7 @@ var _ = Describe("EphemeralRunner", func() { Scheme: mgr.GetScheme(), Log: logf.Log, ResourceBuilder: ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: mgr.GetClient(), multiClient: fake.NewMultiClient(), }, @@ -1085,7 +1085,7 @@ var _ = Describe("EphemeralRunner", func() { // Use an actual client controller.ResourceBuilder = ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: mgr.GetClient(), multiClient: actions.NewMultiClient(logr.Discard()), }, diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller.go b/controllers/actions.github.com/ephemeralrunnerset_controller.go index 2d2b8bcd..95bf898f 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller.go @@ -331,7 +331,7 @@ func (r *EphemeralRunnerSetReconciler) cleanUpEphemeralRunners(ctx context.Conte return false, nil } - actionsClient, err := r.ActionsClientPool.Get(ctx, ephemeralRunnerSet) + actionsClient, err := r.SecretResolver.GetActionsService(ctx, ephemeralRunnerSet) if err != nil { return false, err } @@ -439,7 +439,7 @@ func (r *EphemeralRunnerSetReconciler) deleteIdleEphemeralRunners(ctx context.Co log.Info("No pending or running ephemeral runners running at this time for scale down") return nil } - actionsClient, err := r.ActionsClientPool.Get(ctx, ephemeralRunnerSet) + actionsClient, err := r.SecretResolver.GetActionsService(ctx, ephemeralRunnerSet) if err != nil { return fmt.Errorf("failed to create actions client for ephemeral runner replica set: %w", err) } diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go index a955ea8f..50fac910 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go @@ -58,7 +58,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { Scheme: mgr.GetScheme(), Log: logf.Log, ResourceBuilder: ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: mgr.GetClient(), multiClient: fake.NewMultiClient(), }, @@ -1113,7 +1113,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func( Scheme: mgr.GetScheme(), Log: logf.Log, ResourceBuilder: ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: mgr.GetClient(), multiClient: actions.NewMultiClient(logr.Discard()), }, @@ -1417,7 +1417,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with custom root CA", func( Scheme: mgr.GetScheme(), Log: logf.Log, ResourceBuilder: ResourceBuilder{ - ActionsClientPool: &ActionsClientPool{ + SecretResolver: &SecretResolver{ k8sClient: mgr.GetClient(), multiClient: actions.NewMultiClient(logr.Discard()), }, diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index dac23ac4..28fd91e4 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/controllers/actions.github.com/resourcebuilder.go @@ -18,6 +18,7 @@ import ( "github.com/actions/actions-runner-controller/github/actions" "github.com/actions/actions-runner-controller/hash" "github.com/actions/actions-runner-controller/logging" + "github.com/actions/actions-runner-controller/vault" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -73,7 +74,7 @@ func SetListenerEntrypoint(entrypoint string) { type ResourceBuilder struct { ExcludeLabelPropagationPrefixes []string - *ActionsClientPool + *SecretResolver } // boolPtr returns a pointer to a bool value @@ -166,7 +167,7 @@ func (lm *listenerMetricsServerConfig) containerPort() (corev1.ContainerPort, er }, nil } -func (b *ResourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha1.AutoscalingListener, secret *corev1.Secret, metricsConfig *listenerMetricsServerConfig, cert string) (*corev1.Secret, error) { +func (b *ResourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha1.AutoscalingListener, appConfig *appconfig.AppConfig, metricsConfig *listenerMetricsServerConfig, cert string) (*corev1.Secret, error) { var ( metricsAddr = "" metricsEndpoint = "" @@ -193,13 +194,13 @@ func (b *ResourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha } if ty, ok := autoscalingListener.Annotations[AnnotationKeyGitHubVaultType]; !ok { - appConfig, err := appconfig.FromSecret(secret) - if err != nil { - return nil, fmt.Errorf("failed to parse appconfig from secret: %v", err) - } - config.AppConfig = *appConfig + config.AppConfig = appConfig } else { - config.VaultType = ty + vaultType := vault.VaultType(ty) + if err := vaultType.Validate(); err != nil { + return nil, fmt.Errorf("vault type validation error: %w", err) + } + config.VaultType = vaultType config.VaultLookupKey = autoscalingListener.Spec.GitHubConfigSecret } diff --git a/controllers/actions.github.com/actions_client_pool.go b/controllers/actions.github.com/secret_resolver.go similarity index 79% rename from controllers/actions.github.com/actions_client_pool.go rename to controllers/actions.github.com/secret_resolver.go index 2be944a0..ea5a603e 100644 --- a/controllers/actions.github.com/actions_client_pool.go +++ b/controllers/actions.github.com/secret_resolver.go @@ -18,29 +18,29 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -type ActionsClientPool struct { +type SecretResolver struct { k8sClient client.Client - vaultResolvers map[string]resolver + vaultResolvers map[vault.VaultType]resolver multiClient actions.MultiClient } -type ActionsClientPoolOption func(*ActionsClientPool) +type SecretResolverOption func(*SecretResolver) -func WithVault(ty string, vault vault.Vault) ActionsClientPoolOption { - return func(pool *ActionsClientPool) { +func WithVault(ty vault.VaultType, vault vault.Vault) SecretResolverOption { + return func(pool *SecretResolver) { pool.vaultResolvers[ty] = &vaultResolver{vault} } } -func NewActionsClientPool(k8sClient client.Client, multiClient actions.MultiClient, opts ...ActionsClientPoolOption) *ActionsClientPool { +func NewSecretResolver(k8sClient client.Client, multiClient actions.MultiClient, opts ...SecretResolverOption) *SecretResolver { if k8sClient == nil { panic("k8sClient must not be nil") } - pool := &ActionsClientPool{ + pool := &SecretResolver{ k8sClient: k8sClient, multiClient: multiClient, - vaultResolvers: make(map[string]resolver), + vaultResolvers: make(map[vault.VaultType]resolver), } for _, opt := range opts { @@ -58,8 +58,22 @@ type ActionsGitHubObject interface { GitHubServerTLS() *v1alpha1.GitHubServerTLSConfig } -func (p *ActionsClientPool) Get(ctx context.Context, obj ActionsGitHubObject) (actions.ActionsService, error) { - resolver, err := p.resolverForObject(obj) +func (sr *SecretResolver) GetAppConfig(ctx context.Context, obj ActionsGitHubObject) (*appconfig.AppConfig, error) { + resolver, err := sr.resolverForObject(obj) + if err != nil { + return nil, fmt.Errorf("failed to get resolver for object: %v", err) + } + + appConfig, err := resolver.appConfig(ctx, obj.GitHubConfigSecret()) + if err != nil { + return nil, fmt.Errorf("failed to resolve app config: %v", err) + } + + return appConfig, nil +} + +func (sr *SecretResolver) GetActionsService(ctx context.Context, obj ActionsGitHubObject) (actions.ActionsService, error) { + resolver, err := sr.resolverForObject(obj) if err != nil { return nil, fmt.Errorf("failed to get resolver for object: %v", err) } @@ -118,7 +132,7 @@ func (p *ActionsClientPool) Get(ctx context.Context, obj ActionsGitHubObject) (a if tlsConfig != nil { pool, err := tlsConfig.ToCertPool(func(name, key string) ([]byte, error) { var configmap corev1.ConfigMap - err := p.k8sClient.Get( + err := sr.k8sClient.Get( ctx, types.NamespacedName{ Namespace: obj.GetNamespace(), @@ -139,7 +153,7 @@ func (p *ActionsClientPool) Get(ctx context.Context, obj ActionsGitHubObject) (a clientOptions = append(clientOptions, actions.WithRootCAs(pool)) } - return p.multiClient.GetClientFor( + return sr.multiClient.GetClientFor( ctx, obj.GitHubConfigUrl(), appConfig, @@ -148,7 +162,7 @@ func (p *ActionsClientPool) Get(ctx context.Context, obj ActionsGitHubObject) (a ) } -func (p *ActionsClientPool) resolverForObject(obj ActionsGitHubObject) (resolver, error) { +func (p *SecretResolver) resolverForObject(obj ActionsGitHubObject) (resolver, error) { ty, ok := obj.GetAnnotations()[AnnotationKeyGitHubVaultType] if !ok { return &k8sResolver{ @@ -157,7 +171,12 @@ func (p *ActionsClientPool) resolverForObject(obj ActionsGitHubObject) (resolver }, nil } - vault, ok := p.vaultResolvers[ty] + vaultType := vault.VaultType(ty) + if err := vaultType.Validate(); err != nil { + return nil, fmt.Errorf("invalid vault type %q: %v", ty, err) + } + + vault, ok := p.vaultResolvers[vaultType] if !ok { return nil, fmt.Errorf("unknown vault resolver %q", ty) } diff --git a/main.go b/main.go index 7fb076c3..dddb6d27 100644 --- a/main.go +++ b/main.go @@ -281,20 +281,20 @@ func main() { os.Exit(1) } - var poolOptions []actionsgithubcom.ActionsClientPoolOption + var secretResolverOptions []actionsgithubcom.SecretResolverOption for name, vault := range vaults { - poolOptions = append(poolOptions, actionsgithubcom.WithVault(name, vault)) + secretResolverOptions = append(secretResolverOptions, actionsgithubcom.WithVault(name, vault)) } - clientPool := actionsgithubcom.NewActionsClientPool( + secretResolver := actionsgithubcom.NewSecretResolver( mgr.GetClient(), actionsMultiClient, - poolOptions..., + secretResolverOptions..., ) rb := actionsgithubcom.ResourceBuilder{ ExcludeLabelPropagationPrefixes: excludeLabelPropagationPrefixes, - ActionsClientPool: clientPool, + SecretResolver: secretResolver, } log.Info("Resource builder initializing") diff --git a/vault/vault.go b/vault/vault.go index c5f11c10..6042c42f 100644 --- a/vault/vault.go +++ b/vault/vault.go @@ -14,11 +14,28 @@ type Vault interface { GetSecret(ctx context.Context, name string) (string, error) } +// VaultType represents the type of vault that can be used in the application. +// It is used to identify which vault integration should be used to resolve secrets. +type VaultType string + // VaultType is the type of vault supported const ( - VaultTypeAzureKeyVault = "azure_key_vault" + VaultTypeAzureKeyVault VaultType = "azure_key_vault" ) +func (t VaultType) String() string { + return string(t) +} + +func (t VaultType) Validate() error { + switch t { + case VaultTypeAzureKeyVault: + return nil + default: + return fmt.Errorf("unknown vault type: %q", t) + } +} + // Compile-time checks var _ Vault = (*azurekeyvault.AzureKeyVault)(nil) @@ -31,10 +48,10 @@ var _ Vault = (*azurekeyvault.AzureKeyVault)(nil) // // For example, listener has prefix "LISTENER_", has "AZURE_KEY_VAULT_" configured, // and should read the vault URL. The environment variable will be "LISTENER_AZURE_KEY_VAULT_URL". -func InitAll(prefix string) (map[string]Vault, error) { +func InitAll(prefix string) (map[VaultType]Vault, error) { envs := os.Environ() - result := make(map[string]Vault) + result := make(map[VaultType]Vault) for _, env := range envs { if strings.HasPrefix(env, prefix+"AZURE_KEY_VAULT_") { akv, err := azurekeyvault.FromEnv(prefix + "AZURE_KEY_VAULT_")