diff --git a/apis/actions.github.com/v1alpha1/appconfig/appconfig.go b/apis/actions.github.com/v1alpha1/appconfig/appconfig.go index c6cceeda..b8c31ca9 100644 --- a/apis/actions.github.com/v1alpha1/appconfig/appconfig.go +++ b/apis/actions.github.com/v1alpha1/appconfig/appconfig.go @@ -32,6 +32,9 @@ func (c *AppConfig) tidy() *AppConfig { } func (c *AppConfig) Validate() error { + if c == nil { + return fmt.Errorf("missing app config") + } hasToken := len(c.Token) > 0 hasGitHubAppAuth := c.hasGitHubAppAuth() if hasToken && hasGitHubAppAuth { diff --git a/cmd/ghalistener/config/config.go b/cmd/ghalistener/config/config.go index ec0a5176..cee8a20a 100644 --- a/cmd/ghalistener/config/config.go +++ b/cmd/ghalistener/config/config.go @@ -116,7 +116,7 @@ func (c *Config) Validate() error { return fmt.Errorf("VaultLookupKey is required when VaultType is set to %q", c.VaultType) } - if c.VaultType != "" && c.VaultLookupKey == "" { + if c.VaultType == "" && c.VaultLookupKey == "" { if err := c.AppConfig.Validate(); err != nil { return fmt.Errorf("AppConfig validation failed: %w", err) } diff --git a/cmd/ghalistener/config/config_validation_test.go b/cmd/ghalistener/config/config_validation_test.go index 2e3d5bc5..1d97f3a2 100644 --- a/cmd/ghalistener/config/config_validation_test.go +++ b/cmd/ghalistener/config/config_validation_test.go @@ -33,7 +33,7 @@ func TestConfigValidationMissingToken(t *testing.T) { RunnerScaleSetId: 1, } err := config.Validate() - expectedError := fmt.Sprintf(`GitHub auth credential is missing, token length: "%d", appId: %q, installationId: "%d", private key length: "%d"`, len(config.Token), config.AppID, config.AppInstallationID, len(config.AppPrivateKey)) + expectedError := fmt.Sprintf("AppConfig validation failed: missing app config") assert.ErrorContains(t, err, expectedError, "Expected error about missing auth") } diff --git a/controllers/actions.github.com/autoscalinglistener_controller_test.go b/controllers/actions.github.com/autoscalinglistener_controller_test.go index 9466fed1..330214c1 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller_test.go +++ b/controllers/actions.github.com/autoscalinglistener_controller_test.go @@ -15,6 +15,7 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" ghalistenerconfig "github.com/actions/actions-runner-controller/cmd/ghalistener/config" + "github.com/actions/actions-runner-controller/github/actions/fake" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" kerrors "k8s.io/apimachinery/pkg/api/errors" @@ -43,10 +44,17 @@ var _ = Describe("Test AutoScalingListener controller", func() { autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient) configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) + secretResolver := NewSecretResolver(mgr.GetClient(), fake.NewMultiClient()) + + rb := ResourceBuilder{ + SecretResolver: secretResolver, + } + controller := &AutoscalingListenerReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Log: logf.Log, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + ResourceBuilder: rb, } err := controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") @@ -426,10 +434,17 @@ var _ = Describe("Test AutoScalingListener customization", func() { autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient) configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) + secretResolver := NewSecretResolver(mgr.GetClient(), fake.NewMultiClient()) + + rb := ResourceBuilder{ + SecretResolver: secretResolver, + } + controller := &AutoscalingListenerReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Log: logf.Log, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + ResourceBuilder: rb, } err := controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") @@ -699,11 +714,17 @@ var _ = Describe("Test AutoScalingListener controller with proxy", func() { ctx = context.Background() autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient) configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) + secretResolver := NewSecretResolver(mgr.GetClient(), fake.NewMultiClient()) + + rb := ResourceBuilder{ + SecretResolver: secretResolver, + } controller := &AutoscalingListenerReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Log: logf.Log, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + ResourceBuilder: rb, } err := controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") @@ -896,10 +917,17 @@ var _ = Describe("Test AutoScalingListener controller with template modification autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient) configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) + secretResolver := NewSecretResolver(mgr.GetClient(), fake.NewMultiClient()) + + rb := ResourceBuilder{ + SecretResolver: secretResolver, + } + controller := &AutoscalingListenerReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Log: logf.Log, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + ResourceBuilder: rb, } err := controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") @@ -992,6 +1020,12 @@ var _ = Describe("Test GitHub Server TLS configuration", func() { autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient) configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) + secretResolver := NewSecretResolver(mgr.GetClient(), fake.NewMultiClient()) + + rb := ResourceBuilder{ + SecretResolver: secretResolver, + } + cert, err := os.ReadFile(filepath.Join( "../../", "github", @@ -1013,9 +1047,10 @@ var _ = Describe("Test GitHub Server TLS configuration", func() { Expect(err).NotTo(HaveOccurred(), "failed to create configmap with root CAs") controller := &AutoscalingListenerReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Log: logf.Log, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + ResourceBuilder: rb, } err = controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") diff --git a/controllers/actions.github.com/secret_resolver.go b/controllers/actions.github.com/secret_resolver.go index ea5a603e..7e3fafff 100644 --- a/controllers/actions.github.com/secret_resolver.go +++ b/controllers/actions.github.com/secret_resolver.go @@ -37,17 +37,17 @@ func NewSecretResolver(k8sClient client.Client, multiClient actions.MultiClient, panic("k8sClient must not be nil") } - pool := &SecretResolver{ + secretResolver := &SecretResolver{ k8sClient: k8sClient, multiClient: multiClient, vaultResolvers: make(map[vault.VaultType]resolver), } for _, opt := range opts { - opt(pool) + opt(secretResolver) } - return pool + return secretResolver } type ActionsGitHubObject interface { @@ -162,12 +162,12 @@ func (sr *SecretResolver) GetActionsService(ctx context.Context, obj ActionsGitH ) } -func (p *SecretResolver) resolverForObject(obj ActionsGitHubObject) (resolver, error) { +func (sr *SecretResolver) resolverForObject(obj ActionsGitHubObject) (resolver, error) { ty, ok := obj.GetAnnotations()[AnnotationKeyGitHubVaultType] if !ok { return &k8sResolver{ namespace: obj.GetNamespace(), - client: p.k8sClient, + client: sr.k8sClient, }, nil } @@ -176,7 +176,7 @@ func (p *SecretResolver) resolverForObject(obj ActionsGitHubObject) (resolver, e return nil, fmt.Errorf("invalid vault type %q: %v", ty, err) } - vault, ok := p.vaultResolvers[vaultType] + vault, ok := sr.vaultResolvers[vaultType] if !ok { return nil, fmt.Errorf("unknown vault resolver %q", ty) }