Refactor secret resolver implementation, add vault type and modify autoscaling listener controller to search for app config instead of secret
This commit is contained in:
		
							parent
							
								
									74ed8e6838
								
							
						
					
					
						commit
						379f466d8f
					
				|  | @ -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 | ||||
|  |  | |||
|  | @ -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", | ||||
| 			}, | ||||
| 		} | ||||
|  |  | |||
|  | @ -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") | ||||
| 	}) | ||||
| } | ||||
|  | @ -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 | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
|  | @ -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(), | ||||
| 				}, | ||||
|  |  | |||
|  | @ -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) | ||||
| 	} | ||||
|  |  | |||
|  | @ -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()), | ||||
| 				}, | ||||
|  |  | |||
|  | @ -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) | ||||
| 	} | ||||
|  |  | |||
|  | @ -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()), | ||||
| 				}, | ||||
|  |  | |||
|  | @ -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 | ||||
| 	} | ||||
| 
 | ||||
|  |  | |||
|  | @ -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) | ||||
| 	} | ||||
							
								
								
									
										10
									
								
								main.go
								
								
								
								
							
							
						
						
									
										10
									
								
								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") | ||||
|  |  | |||
|  | @ -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_") | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue