From 610071e0d5dd45471269d323f8b969b71b7efe3b Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Mon, 9 Jun 2025 00:25:56 +0200 Subject: [PATCH] remove proxyconfig and add proxy func --- .../v1alpha1/autoscalinglistener_types.go | 16 --- .../v1alpha1/autoscalingrunnerset_types.go | 23 +-- .../v1alpha1/ephemeralrunner_types.go | 13 +- .../v1alpha1/ephemeralrunnerset_types.go | 13 +- .../actions.github.com/secret_resolver.go | 28 +++- proxyconfig/proxyconfig.go | 133 ------------------ proxyconfig/proxyconfig_test.go | 104 -------------- vault/azurekeyvault/config.go | 24 ++-- vault/azurekeyvault/config_test.go | 43 +----- 9 files changed, 71 insertions(+), 326 deletions(-) delete mode 100644 proxyconfig/proxyconfig.go delete mode 100644 proxyconfig/proxyconfig_test.go diff --git a/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go b/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go index 987cccba..3943c6f6 100644 --- a/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go @@ -89,22 +89,6 @@ type AutoscalingListener struct { Status AutoscalingListenerStatus `json:"status,omitempty"` } -func (l *AutoscalingListener) GitHubConfigSecret() string { - return l.Spec.GitHubConfigSecret -} - -func (l *AutoscalingListener) GitHubConfigUrl() string { - return l.Spec.GitHubConfigUrl -} - -func (l *AutoscalingListener) Proxy() *ProxyConfig { - return l.Spec.Proxy -} - -func (l *AutoscalingListener) GitHubServerTLS() *TLSConfig { - return l.Spec.GitHubServerTLS -} - // +kubebuilder:object:root=true // AutoscalingListenerList contains a list of AutoscalingListener type AutoscalingListenerList struct { diff --git a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go index 534d3ab0..fcbf38eb 100644 --- a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go @@ -149,7 +149,7 @@ type ProxyConfig struct { NoProxy []string `json:"noProxy,omitempty"` } -func (c *ProxyConfig) toHTTPProxyConfig(secretFetcher func(string) (*corev1.Secret, error)) (*httpproxy.Config, error) { +func (c *ProxyConfig) ToHTTPProxyConfig(secretFetcher func(string) (*corev1.Secret, error)) (*httpproxy.Config, error) { config := &httpproxy.Config{ NoProxy: strings.Join(c.NoProxy, ","), } @@ -208,7 +208,7 @@ func (c *ProxyConfig) toHTTPProxyConfig(secretFetcher func(string) (*corev1.Secr } func (c *ProxyConfig) ToSecretData(secretFetcher func(string) (*corev1.Secret, error)) (map[string][]byte, error) { - config, err := c.toHTTPProxyConfig(secretFetcher) + config, err := c.ToHTTPProxyConfig(secretFetcher) if err != nil { return nil, err } @@ -222,7 +222,7 @@ func (c *ProxyConfig) ToSecretData(secretFetcher func(string) (*corev1.Secret, e } func (c *ProxyConfig) ProxyFunc(secretFetcher func(string) (*corev1.Secret, error)) (func(*http.Request) (*url.URL, error), error) { - config, err := c.toHTTPProxyConfig(secretFetcher) + config, err := c.ToHTTPProxyConfig(secretFetcher) if err != nil { return nil, err } @@ -320,11 +320,7 @@ func (ars *AutoscalingRunnerSet) GitHubConfigUrl() string { return ars.Spec.GitHubConfigUrl } -func (ars *AutoscalingRunnerSet) VaultConfig() *VaultConfig { - return ars.Spec.VaultConfig -} - -func (ars *AutoscalingRunnerSet) Proxy() *ProxyConfig { +func (ars *AutoscalingRunnerSet) GitHubProxy() *ProxyConfig { return ars.Spec.Proxy } @@ -332,6 +328,17 @@ func (ars *AutoscalingRunnerSet) GitHubServerTLS() *TLSConfig { return ars.Spec.GitHubServerTLS } +func (ars *AutoscalingRunnerSet) VaultConfig() *VaultConfig { + return ars.Spec.VaultConfig +} + +func (ars *AutoscalingRunnerSet) VaultProxy() *ProxyConfig { + if ars.Spec.VaultConfig != nil { + return ars.Spec.VaultConfig.Proxy + } + return nil +} + func (ars *AutoscalingRunnerSet) RunnerSetSpecHash() string { type runnerSetSpec struct { GitHubConfigUrl string diff --git a/apis/actions.github.com/v1alpha1/ephemeralrunner_types.go b/apis/actions.github.com/v1alpha1/ephemeralrunner_types.go index 41b99ef7..9381a84a 100644 --- a/apis/actions.github.com/v1alpha1/ephemeralrunner_types.go +++ b/apis/actions.github.com/v1alpha1/ephemeralrunner_types.go @@ -75,7 +75,7 @@ func (er *EphemeralRunner) GitHubConfigUrl() string { return er.Spec.GitHubConfigUrl } -func (er *EphemeralRunner) Proxy() *ProxyConfig { +func (er *EphemeralRunner) GitHubProxy() *ProxyConfig { return er.Spec.Proxy } @@ -83,8 +83,15 @@ func (er *EphemeralRunner) GitHubServerTLS() *TLSConfig { return er.Spec.GitHubServerTLS } -func (ars *EphemeralRunner) VaultConfig() *VaultConfig { - return ars.Spec.VaultConfig +func (er *EphemeralRunner) VaultConfig() *VaultConfig { + return er.Spec.VaultConfig +} + +func (er *EphemeralRunner) VaultProxy() *ProxyConfig { + if er.Spec.VaultConfig != nil { + return er.Spec.VaultConfig.Proxy + } + return nil } // EphemeralRunnerSpec defines the desired state of EphemeralRunner diff --git a/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go b/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go index da2a9f02..fc9ef2d7 100644 --- a/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go +++ b/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go @@ -68,7 +68,7 @@ func (ers *EphemeralRunnerSet) GitHubConfigUrl() string { return ers.Spec.EphemeralRunnerSpec.GitHubConfigUrl } -func (ers *EphemeralRunnerSet) Proxy() *ProxyConfig { +func (ers *EphemeralRunnerSet) GitHubProxy() *ProxyConfig { return ers.Spec.EphemeralRunnerSpec.Proxy } @@ -76,8 +76,15 @@ func (ers *EphemeralRunnerSet) GitHubServerTLS() *TLSConfig { return ers.Spec.EphemeralRunnerSpec.GitHubServerTLS } -func (ars *EphemeralRunnerSet) VaultConfig() *VaultConfig { - return ars.Spec.EphemeralRunnerSpec.VaultConfig +func (ers *EphemeralRunnerSet) VaultConfig() *VaultConfig { + return ers.Spec.EphemeralRunnerSpec.VaultConfig +} + +func (ers *EphemeralRunnerSet) VaultProxy() *ProxyConfig { + if ers.Spec.EphemeralRunnerSpec.VaultConfig != nil { + return ers.Spec.EphemeralRunnerSpec.VaultConfig.Proxy + } + return nil } // EphemeralRunnerSetList contains a list of EphemeralRunnerSet diff --git a/controllers/actions.github.com/secret_resolver.go b/controllers/actions.github.com/secret_resolver.go index 88c4851c..6a6dc62b 100644 --- a/controllers/actions.github.com/secret_resolver.go +++ b/controllers/actions.github.com/secret_resolver.go @@ -47,13 +47,14 @@ type ActionsGitHubObject interface { client.Object GitHubConfigUrl() string GitHubConfigSecret() string - Proxy() *v1alpha1.ProxyConfig + GitHubProxy() *v1alpha1.ProxyConfig GitHubServerTLS() *v1alpha1.TLSConfig VaultConfig() *v1alpha1.VaultConfig + VaultProxy() *v1alpha1.ProxyConfig } func (sr *SecretResolver) GetAppConfig(ctx context.Context, obj ActionsGitHubObject) (*appconfig.AppConfig, error) { - resolver, err := sr.resolverForObject(obj) + resolver, err := sr.resolverForObject(ctx, obj) if err != nil { return nil, fmt.Errorf("failed to get resolver for object: %v", err) } @@ -67,7 +68,7 @@ func (sr *SecretResolver) GetAppConfig(ctx context.Context, obj ActionsGitHubObj } func (sr *SecretResolver) GetActionsService(ctx context.Context, obj ActionsGitHubObject) (actions.ActionsService, error) { - resolver, err := sr.resolverForObject(obj) + resolver, err := sr.resolverForObject(ctx, obj) if err != nil { return nil, fmt.Errorf("failed to get resolver for object: %v", err) } @@ -78,7 +79,7 @@ func (sr *SecretResolver) GetActionsService(ctx context.Context, obj ActionsGitH } var clientOptions []actions.ClientOption - if proxy := obj.Proxy(); proxy != nil { + if proxy := obj.GitHubProxy(); proxy != nil { config := &httpproxy.Config{ NoProxy: strings.Join(proxy.NoProxy, ","), } @@ -156,7 +157,7 @@ func (sr *SecretResolver) GetActionsService(ctx context.Context, obj ActionsGitH ) } -func (sr *SecretResolver) resolverForObject(obj ActionsGitHubObject) (resolver, error) { +func (sr *SecretResolver) resolverForObject(ctx context.Context, obj ActionsGitHubObject) (resolver, error) { vaultConfig := obj.VaultConfig() if vaultConfig == nil || vaultConfig.Type == "" { return &k8sResolver{ @@ -165,6 +166,22 @@ func (sr *SecretResolver) resolverForObject(obj ActionsGitHubObject) (resolver, }, nil } + var proxy *httpproxy.Config + if vaultProxy := obj.VaultProxy(); vaultProxy != nil { + p, err := vaultProxy.ToHTTPProxyConfig(func(s string) (*corev1.Secret, error) { + var secret corev1.Secret + err := sr.k8sClient.Get(ctx, types.NamespacedName{Name: s, Namespace: obj.GetNamespace()}, &secret) + if err != nil { + return nil, fmt.Errorf("failed to get secret %s: %w", s, err) + } + return &secret, nil + }) + if err != nil { + return nil, fmt.Errorf("failed to create proxy config: %v", err) + } + proxy = p + } + switch vaultConfig.Type { case vault.VaultTypeAzureKeyVault: akv, err := azurekeyvault.New(azurekeyvault.Config{ @@ -172,6 +189,7 @@ func (sr *SecretResolver) resolverForObject(obj ActionsGitHubObject) (resolver, ClientID: vaultConfig.AzureKeyVault.ClientID, URL: vaultConfig.AzureKeyVault.URL, CertificatePath: vaultConfig.AzureKeyVault.CertificatePath, + Proxy: proxy, }) if err != nil { return nil, fmt.Errorf("failed to create Azure Key Vault client: %v", err) diff --git a/proxyconfig/proxyconfig.go b/proxyconfig/proxyconfig.go deleted file mode 100644 index a706fa08..00000000 --- a/proxyconfig/proxyconfig.go +++ /dev/null @@ -1,133 +0,0 @@ -package proxyconfig - -import ( - "fmt" - "net/url" - "os" - "strings" - - "golang.org/x/net/http/httpproxy" -) - -type ProxyConfig struct { - HTTP *ProxyServerConfig `json:"http,omitempty"` - HTTPS *ProxyServerConfig `json:"https,omitempty"` - NoProxy []string `json:"no_proxy,omitempty"` -} - -func (pc *ProxyConfig) Validate() error { - if pc == nil { - return nil - } - - if pc.HTTP != nil { - _, err := url.ParseRequestURI(pc.HTTP.URL) - if err != nil { - return fmt.Errorf("proxy http set with invalid url: %v", err) - } - } - if pc.HTTPS != nil { - _, err := url.ParseRequestURI(pc.HTTPS.URL) - if err != nil { - return fmt.Errorf("proxy https set with invalid url: %v", err) - } - } - - for _, u := range pc.NoProxy { - if _, err := url.ParseRequestURI(u); err != nil { - return fmt.Errorf("proxy no_proxy set with invalid url: %v", err) - } - } - return nil -} - -func (c *ProxyConfig) ProxyConfig() (*httpproxy.Config, error) { - if c == nil { - return nil, nil - } - config := &httpproxy.Config{ - NoProxy: strings.Join(c.NoProxy, ","), - } - if c.HTTP != nil { - u, err := c.HTTP.proxyURL() - if err != nil { - return nil, fmt.Errorf("failed to create proxy http url: %w", err) - } - config.HTTPProxy = u.String() - } - - if c.HTTPS != nil { - u, err := c.HTTPS.proxyURL() - if err != nil { - return nil, fmt.Errorf("failed to create proxy https url: %w", err) - } - config.HTTPSProxy = u.String() - } - - return config, nil -} - -type ProxyServerConfig struct { - URL string `json:"url"` - Username string `json:"username"` - Password string `json:"password"` -} - -func (c *ProxyServerConfig) proxyURL() (*url.URL, error) { - u, err := url.Parse(c.URL) - if err != nil { - return nil, fmt.Errorf("failed to parse proxy url %q: %w", c.URL, err) - } - - u.User = url.UserPassword( - c.Username, - c.Password, - ) - - return u, nil -} - -func ReadFromEnv(prefix string) (*ProxyConfig, error) { - url := os.Getenv(prefix + "HTTP_URL") - username := os.Getenv(prefix + "HTTP_USERNAME") - password := os.Getenv(prefix + "HTTP_PASSWORD") - - var http *ProxyServerConfig - if url != "" || username != "" || password != "" { - http = &ProxyServerConfig{ - URL: url, - Username: username, - Password: password, - } - } - - url = os.Getenv(prefix + "HTTPS_URL") - username = os.Getenv(prefix + "HTTPS_USERNAME") - password = os.Getenv(prefix + "HTTPS_PASSWORD") - - var https *ProxyServerConfig - if url != "" || username != "" || password != "" { - https = &ProxyServerConfig{ - URL: url, - Username: username, - Password: password, - } - } - - noProxyRaw := os.Getenv(prefix + "NO_PROXY") - - if http == nil && https == nil && noProxyRaw == "" { - return nil, nil - } - - var noProxy []string - if noProxyRaw != "" { - noProxy = strings.Split(noProxyRaw, ",") - } - - return &ProxyConfig{ - HTTP: http, - HTTPS: https, - NoProxy: noProxy, - }, nil -} diff --git a/proxyconfig/proxyconfig_test.go b/proxyconfig/proxyconfig_test.go deleted file mode 100644 index 6d71c434..00000000 --- a/proxyconfig/proxyconfig_test.go +++ /dev/null @@ -1,104 +0,0 @@ -package proxyconfig - -import ( - "os" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -type kv struct { - key string - value string -} - -func TestReadFromEnvNoPrefix(t *testing.T) { - var ( - url = "example.com" - username = "user" - password = "password" - - noProxy = "test.com,other.com" - ) - tt := map[string]struct { - envs []*kv - want *ProxyConfig - }{ - "no envs": {}, - "http only": { - envs: []*kv{ - {"HTTP_URL", url}, - {"HTTP_USERNAME", username}, - {"HTTP_PASSWORD", password}, - }, - want: &ProxyConfig{ - HTTP: &ProxyServerConfig{ - URL: url, - Username: username, - Password: password, - }, - }, - }, - "https only": { - envs: []*kv{ - {"HTTPS_URL", url}, - {"HTTPS_USERNAME", username}, - {"HTTPS_PASSWORD", password}, - }, - want: &ProxyConfig{ - HTTPS: &ProxyServerConfig{ - URL: url, - Username: username, - Password: password, - }, - }, - }, - "no proxy only": { - envs: []*kv{ - {"NO_PROXY", noProxy}, - }, - want: &ProxyConfig{ - NoProxy: strings.Split(noProxy, ","), - }, - }, - "all set": { - envs: []*kv{ - {"HTTP_URL", url}, - {"HTTP_USERNAME", username}, - {"HTTP_PASSWORD", password}, - {"HTTPS_URL", url}, - {"HTTPS_USERNAME", username}, - {"HTTPS_PASSWORD", password}, - {"NO_PROXY", noProxy}, - }, - want: &ProxyConfig{ - HTTP: &ProxyServerConfig{ - URL: url, - Username: username, - Password: password, - }, - HTTPS: &ProxyServerConfig{ - URL: url, - Username: username, - Password: password, - }, - NoProxy: strings.Split(noProxy, ","), - }, - }, - } - - for name, tc := range tt { - t.Run(name, func(t *testing.T) { - os.Clearenv() - for _, kv := range tc.envs { - os.Setenv(kv.key, kv.value) - } - - got, err := ReadFromEnv("") - require.NoError(t, err) - assert.Equal(t, tc.want, got) - }) - } -} diff --git a/vault/azurekeyvault/config.go b/vault/azurekeyvault/config.go index 68eb7adb..415f5acf 100644 --- a/vault/azurekeyvault/config.go +++ b/vault/azurekeyvault/config.go @@ -11,17 +11,17 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets" - "github.com/actions/actions-runner-controller/proxyconfig" "github.com/hashicorp/go-retryablehttp" + "golang.org/x/net/http/httpproxy" ) // AzureKeyVault is a struct that holds the Azure Key Vault client. type Config struct { - TenantID string `json:"tenant_id"` - ClientID string `json:"client_id"` - URL string `json:"url"` - CertificatePath string `json:"certificate_path"` - Proxy *proxyconfig.ProxyConfig `json:"proxy,omitempty"` + TenantID string `json:"tenant_id"` + ClientID string `json:"client_id"` + URL string `json:"url"` + CertificatePath string `json:"certificate_path"` + Proxy *httpproxy.Config `json:"proxy,omitempty"` } func (c *Config) Validate() error { @@ -43,8 +43,10 @@ func (c *Config) Validate() error { return fmt.Errorf("cert path %q does not exist: %v", c.CertificatePath, err) } - if err := c.Proxy.Validate(); err != nil { - return fmt.Errorf("proxy validation failed: %v", err) + if c.Proxy != nil { + if c.Proxy.HTTPProxy == "" && c.Proxy.HTTPSProxy == "" && c.Proxy.NoProxy == "" { + return errors.New("proxy configuration is empty, at least one proxy must be set") + } } return nil @@ -109,12 +111,8 @@ func (c *Config) httpClient() (*http.Client, error) { return nil, fmt.Errorf("failed to get http transport") } if c.Proxy != nil { - pc, err := c.Proxy.ProxyConfig() - if err != nil { - return nil, fmt.Errorf("failed to create proxy config: %v", err) - } transport.Proxy = func(req *http.Request) (*url.URL, error) { - return pc.ProxyFunc()(req.URL) + return c.Proxy.ProxyFunc()(req.URL) } } diff --git a/vault/azurekeyvault/config_test.go b/vault/azurekeyvault/config_test.go index 2b3166c3..e0f06c21 100644 --- a/vault/azurekeyvault/config_test.go +++ b/vault/azurekeyvault/config_test.go @@ -5,8 +5,8 @@ import ( "path/filepath" "testing" - "github.com/actions/actions-runner-controller/proxyconfig" "github.com/stretchr/testify/require" + "golang.org/x/net/http/httpproxy" ) func TestConfigValidate_invalid(t *testing.T) { @@ -24,22 +24,6 @@ func TestConfigValidate_invalid(t *testing.T) { os.Remove(certPath) }) - proxy := &proxyconfig.ProxyConfig{ - HTTP: &proxyconfig.ProxyServerConfig{ - URL: "http://httpconfig.com", - Username: "user", - Password: "pass", - }, - HTTPS: &proxyconfig.ProxyServerConfig{ - URL: "https://httpsconfig.com", - Username: "user", - Password: "pass", - }, - NoProxy: []string{ - "http://noproxy.com", - }, - } - tt := map[string]*Config{ "empty": {}, "no tenant id": { @@ -47,37 +31,31 @@ func TestConfigValidate_invalid(t *testing.T) { ClientID: clientID, URL: url, CertificatePath: certPath, - Proxy: proxy, }, "no client id": { TenantID: tenantID, ClientID: "", URL: url, CertificatePath: certPath, - Proxy: proxy, }, "no url": { TenantID: tenantID, ClientID: clientID, URL: "", CertificatePath: certPath, - Proxy: proxy, }, "no jwt and no cert path": { TenantID: tenantID, ClientID: clientID, URL: url, CertificatePath: "", - Proxy: proxy, }, "invalid proxy": { TenantID: tenantID, ClientID: clientID, URL: url, CertificatePath: certPath, - Proxy: &proxyconfig.ProxyConfig{ - HTTP: &proxyconfig.ProxyServerConfig{}, - }, + Proxy: &httpproxy.Config{}, }, } @@ -94,22 +72,6 @@ func TestValidate_valid(t *testing.T) { clientID := "clientID" url := "https://example.com" - proxy := &proxyconfig.ProxyConfig{ - HTTP: &proxyconfig.ProxyServerConfig{ - URL: "http://httpconfig.com", - Username: "user", - Password: "pass", - }, - HTTPS: &proxyconfig.ProxyServerConfig{ - URL: "https://httpsconfig.com", - Username: "user", - Password: "pass", - }, - NoProxy: []string{ - "http://noproxy.com", - }, - } - certPath, err := filepath.Abs("testdata/server.crt") require.NoError(t, err) @@ -119,7 +81,6 @@ func TestValidate_valid(t *testing.T) { ClientID: clientID, URL: url, CertificatePath: certPath, - Proxy: proxy, }, "without proxy": { TenantID: tenantID,