diff --git a/apis/actions.github.com/v1alpha1/appconfig/appconfig.go b/apis/actions.github.com/v1alpha1/appconfig/appconfig.go new file mode 100644 index 00000000..28179ae8 --- /dev/null +++ b/apis/actions.github.com/v1alpha1/appconfig/appconfig.go @@ -0,0 +1,89 @@ +package appconfig + +import ( + "bytes" + "encoding/json" + "fmt" + "strconv" + + corev1 "k8s.io/api/core/v1" +) + +type AppConfig struct { + AppID string `json:"github_app_id"` + AppInstallationID int64 `json:"github_app_installation_id"` + AppPrivateKey string `json:"github_app_private_key"` + + Token string `json:"github_token"` +} + +func (c *AppConfig) tidy() *AppConfig { + if len(c.Token) > 0 { + return &AppConfig{ + Token: c.Token, + } + } + + return &AppConfig{ + AppID: c.AppID, + AppInstallationID: c.AppInstallationID, + AppPrivateKey: c.AppPrivateKey, + } +} + +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 { + return fmt.Errorf("both PAT and GitHub App credentials provided. should only provide one") + } + if !hasToken && !hasGitHubAppAuth { + return fmt.Errorf("no credentials provided: either a PAT or GitHub App credentials should be provided") + } + + return nil +} + +func (c *AppConfig) hasGitHubAppAuth() bool { + return len(c.AppID) > 0 && c.AppInstallationID > 0 && len(c.AppPrivateKey) > 0 +} + +func FromSecret(secret *corev1.Secret) (*AppConfig, error) { + var appInstallationID int64 + if v := string(secret.Data["github_app_installation_id"]); v != "" { + val, err := strconv.ParseInt(v, 10, 64) + if err != nil { + return nil, err + } + appInstallationID = val + } + + cfg := &AppConfig{ + Token: string(secret.Data["github_token"]), + AppID: string(secret.Data["github_app_id"]), + AppInstallationID: appInstallationID, + AppPrivateKey: string(secret.Data["github_app_private_key"]), + } + + if err := cfg.Validate(); err != nil { + return nil, fmt.Errorf("failed to validate config: %v", err) + } + + return cfg.tidy(), nil +} + +func FromJSONString(v string) (*AppConfig, error) { + var appConfig AppConfig + if err := json.NewDecoder(bytes.NewBufferString(v)).Decode(&appConfig); err != nil { + return nil, err + } + + if err := appConfig.Validate(); err != nil { + return nil, fmt.Errorf("failed to validate app config decoded from string: %w", err) + } + + return appConfig.tidy(), nil +} diff --git a/apis/actions.github.com/v1alpha1/appconfig/appconfig_test.go b/apis/actions.github.com/v1alpha1/appconfig/appconfig_test.go new file mode 100644 index 00000000..c9009bc6 --- /dev/null +++ b/apis/actions.github.com/v1alpha1/appconfig/appconfig_test.go @@ -0,0 +1,152 @@ +package appconfig + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" +) + +func TestAppConfigValidate_invalid(t *testing.T) { + tt := map[string]*AppConfig{ + "empty": {}, + "token and app config": { + AppID: "1", + AppInstallationID: 2, + AppPrivateKey: "private key", + Token: "token", + }, + "app id not set": { + AppInstallationID: 2, + AppPrivateKey: "private key", + }, + "app installation id not set": { + AppID: "2", + AppPrivateKey: "private key", + }, + "private key empty": { + AppID: "2", + AppInstallationID: 1, + AppPrivateKey: "", + }, + } + + for name, cfg := range tt { + t.Run(name, func(t *testing.T) { + err := cfg.Validate() + require.Error(t, err) + }) + } +} + +func TestAppConfigValidate_valid(t *testing.T) { + tt := map[string]*AppConfig{ + "token": { + Token: "token", + }, + "app ID": { + AppID: "1", + AppInstallationID: 2, + AppPrivateKey: "private key", + }, + } + + for name, cfg := range tt { + t.Run(name, func(t *testing.T) { + err := cfg.Validate() + require.NoError(t, err) + }) + } +} + +func TestAppConfigFromSecret_invalid(t *testing.T) { + tt := map[string]map[string]string{ + "empty": {}, + "token and app provided": { + "github_token": "token", + "github_app_id": "2", + "githu_app_installation_id": "3", + "github_app_private_key": "private key", + }, + "invalid app id": { + "github_app_id": "abc", + "githu_app_installation_id": "3", + "github_app_private_key": "private key", + }, + "invalid app installation_id": { + "github_app_id": "1", + "githu_app_installation_id": "abc", + "github_app_private_key": "private key", + }, + "empty private key": { + "github_app_id": "1", + "githu_app_installation_id": "2", + "github_app_private_key": "", + }, + } + + for name, data := range tt { + t.Run(name, func(t *testing.T) { + secret := &corev1.Secret{ + StringData: data, + } + + appConfig, err := FromSecret(secret) + assert.Error(t, err) + assert.Nil(t, appConfig) + }) + } +} + +func TestAppConfigFromSecret_valid(t *testing.T) { + tt := map[string]map[string]string{ + "with token": { + "github_token": "token", + }, + "app config": { + "github_app_id": "2", + "githu_app_installation_id": "3", + "github_app_private_key": "private key", + }, + } + + for name, data := range tt { + t.Run(name, func(t *testing.T) { + secret := &corev1.Secret{ + StringData: data, + } + + appConfig, err := FromSecret(secret) + assert.Error(t, err) + assert.Nil(t, appConfig) + }) + } +} + +func TestAppConfigFromString_valid(t *testing.T) { + tt := map[string]*AppConfig{ + "token": { + Token: "token", + }, + "app ID": { + AppID: "1", + AppInstallationID: 2, + AppPrivateKey: "private key", + }, + } + + for name, cfg := range tt { + t.Run(name, func(t *testing.T) { + bytes, err := json.Marshal(cfg) + require.NoError(t, err) + + got, err := FromJSONString(string(bytes)) + require.NoError(t, err) + + want := cfg.tidy() + assert.Equal(t, want, got) + }) + } +} diff --git a/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go b/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go index 57363dba..3943c6f6 100644 --- a/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go @@ -59,7 +59,10 @@ type AutoscalingListenerSpec struct { Proxy *ProxyConfig `json:"proxy,omitempty"` // +optional - GitHubServerTLS *GitHubServerTLSConfig `json:"githubServerTLS,omitempty"` + GitHubServerTLS *TLSConfig `json:"githubServerTLS,omitempty"` + + // +optional + VaultConfig *VaultConfig `json:"vaultConfig,omitempty"` // +optional Metrics *MetricsConfig `json:"metrics,omitempty"` @@ -87,7 +90,6 @@ type AutoscalingListener struct { } // +kubebuilder:object:root=true - // AutoscalingListenerList contains a list of AutoscalingListener type AutoscalingListenerList struct { metav1.TypeMeta `json:",inline"` diff --git a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go index 2d7946f9..ecb01b58 100644 --- a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go @@ -24,6 +24,7 @@ import ( "strings" "github.com/actions/actions-runner-controller/hash" + "github.com/actions/actions-runner-controller/vault" "golang.org/x/net/http/httpproxy" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -69,7 +70,10 @@ type AutoscalingRunnerSetSpec struct { Proxy *ProxyConfig `json:"proxy,omitempty"` // +optional - GitHubServerTLS *GitHubServerTLSConfig `json:"githubServerTLS,omitempty"` + GitHubServerTLS *TLSConfig `json:"githubServerTLS,omitempty"` + + // +optional + VaultConfig *VaultConfig `json:"vaultConfig,omitempty"` // Required Template corev1.PodTemplateSpec `json:"template,omitempty"` @@ -89,12 +93,12 @@ type AutoscalingRunnerSetSpec struct { MinRunners *int `json:"minRunners,omitempty"` } -type GitHubServerTLSConfig struct { +type TLSConfig struct { // Required CertificateFrom *TLSCertificateSource `json:"certificateFrom,omitempty"` } -func (c *GitHubServerTLSConfig) ToCertPool(keyFetcher func(name, key string) ([]byte, error)) (*x509.CertPool, error) { +func (c *TLSConfig) ToCertPool(keyFetcher func(name, key string) ([]byte, error)) (*x509.CertPool, error) { if c.CertificateFrom == nil { return nil, fmt.Errorf("certificateFrom not specified") } @@ -142,7 +146,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, ","), } @@ -201,7 +205,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 } @@ -215,7 +219,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 } @@ -235,6 +239,26 @@ type ProxyServerConfig struct { CredentialSecretRef string `json:"credentialSecretRef,omitempty"` } +type VaultConfig struct { + // +optional + Type vault.VaultType `json:"type,omitempty"` + // +optional + AzureKeyVault *AzureKeyVaultConfig `json:"azureKeyVault,omitempty"` + // +optional + Proxy *ProxyConfig `json:"proxy,omitempty"` +} + +type AzureKeyVaultConfig struct { + // +required + URL string `json:"url,omitempty"` + // +required + TenantID string `json:"tenantId,omitempty"` + // +required + ClientID string `json:"clientId,omitempty"` + // +required + CertificatePath string `json:"certificatePath,omitempty"` +} + // MetricsConfig holds configuration parameters for each metric type type MetricsConfig struct { // +optional @@ -285,6 +309,33 @@ func (ars *AutoscalingRunnerSet) ListenerSpecHash() string { return hash.ComputeTemplateHash(&spec) } +func (ars *AutoscalingRunnerSet) GitHubConfigSecret() string { + return ars.Spec.GitHubConfigSecret +} + +func (ars *AutoscalingRunnerSet) GitHubConfigUrl() string { + return ars.Spec.GitHubConfigUrl +} + +func (ars *AutoscalingRunnerSet) GitHubProxy() *ProxyConfig { + return ars.Spec.Proxy +} + +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 @@ -292,7 +343,7 @@ func (ars *AutoscalingRunnerSet) RunnerSetSpecHash() string { RunnerGroup string RunnerScaleSetName string Proxy *ProxyConfig - GitHubServerTLS *GitHubServerTLSConfig + GitHubServerTLS *TLSConfig Template corev1.PodTemplateSpec } spec := &runnerSetSpec{ diff --git a/apis/actions.github.com/v1alpha1/ephemeralrunner_types.go b/apis/actions.github.com/v1alpha1/ephemeralrunner_types.go index 0c13bf7c..7667174f 100644 --- a/apis/actions.github.com/v1alpha1/ephemeralrunner_types.go +++ b/apis/actions.github.com/v1alpha1/ephemeralrunner_types.go @@ -67,6 +67,33 @@ func (er *EphemeralRunner) HasContainerHookConfigured() bool { return false } +func (er *EphemeralRunner) GitHubConfigSecret() string { + return er.Spec.GitHubConfigSecret +} + +func (er *EphemeralRunner) GitHubConfigUrl() string { + return er.Spec.GitHubConfigUrl +} + +func (er *EphemeralRunner) GitHubProxy() *ProxyConfig { + return er.Spec.Proxy +} + +func (er *EphemeralRunner) GitHubServerTLS() *TLSConfig { + return er.Spec.GitHubServerTLS +} + +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 type EphemeralRunnerSpec struct { // +required @@ -75,6 +102,9 @@ type EphemeralRunnerSpec struct { // +required GitHubConfigSecret string `json:"githubConfigSecret,omitempty"` + // +optional + GitHubServerTLS *TLSConfig `json:"githubServerTLS,omitempty"` + // +required RunnerScaleSetId int `json:"runnerScaleSetId,omitempty"` @@ -85,7 +115,7 @@ type EphemeralRunnerSpec struct { ProxySecretRef string `json:"proxySecretRef,omitempty"` // +optional - GitHubServerTLS *GitHubServerTLSConfig `json:"githubServerTLS,omitempty"` + VaultConfig *VaultConfig `json:"vaultConfig,omitempty"` corev1.PodTemplateSpec `json:",inline"` } diff --git a/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go b/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go index f74edfb0..fc9ef2d7 100644 --- a/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go +++ b/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go @@ -60,9 +60,35 @@ type EphemeralRunnerSet struct { Status EphemeralRunnerSetStatus `json:"status,omitempty"` } -// +kubebuilder:object:root=true +func (ers *EphemeralRunnerSet) GitHubConfigSecret() string { + return ers.Spec.EphemeralRunnerSpec.GitHubConfigSecret +} + +func (ers *EphemeralRunnerSet) GitHubConfigUrl() string { + return ers.Spec.EphemeralRunnerSpec.GitHubConfigUrl +} + +func (ers *EphemeralRunnerSet) GitHubProxy() *ProxyConfig { + return ers.Spec.EphemeralRunnerSpec.Proxy +} + +func (ers *EphemeralRunnerSet) GitHubServerTLS() *TLSConfig { + return ers.Spec.EphemeralRunnerSpec.GitHubServerTLS +} + +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 +// +kubebuilder:object:root=true type EphemeralRunnerSetList struct { metav1.TypeMeta `json:",inline"` metav1.ListMeta `json:"metadata,omitempty"` diff --git a/apis/actions.github.com/v1alpha1/tls_config_test.go b/apis/actions.github.com/v1alpha1/tls_config_test.go index c3a74bf7..e05bf81b 100644 --- a/apis/actions.github.com/v1alpha1/tls_config_test.go +++ b/apis/actions.github.com/v1alpha1/tls_config_test.go @@ -17,7 +17,7 @@ import ( func TestGitHubServerTLSConfig_ToCertPool(t *testing.T) { t.Run("returns an error if CertificateFrom not specified", func(t *testing.T) { - c := &v1alpha1.GitHubServerTLSConfig{ + c := &v1alpha1.TLSConfig{ CertificateFrom: nil, } @@ -29,7 +29,7 @@ func TestGitHubServerTLSConfig_ToCertPool(t *testing.T) { }) t.Run("returns an error if CertificateFrom.ConfigMapKeyRef not specified", func(t *testing.T) { - c := &v1alpha1.GitHubServerTLSConfig{ + c := &v1alpha1.TLSConfig{ CertificateFrom: &v1alpha1.TLSCertificateSource{}, } @@ -41,7 +41,7 @@ func TestGitHubServerTLSConfig_ToCertPool(t *testing.T) { }) t.Run("returns a valid cert pool with correct configuration", func(t *testing.T) { - c := &v1alpha1.GitHubServerTLSConfig{ + c := &v1alpha1.TLSConfig{ CertificateFrom: &v1alpha1.TLSCertificateSource{ ConfigMapKeyRef: &v1.ConfigMapKeySelector{ LocalObjectReference: v1.LocalObjectReference{ diff --git a/apis/actions.github.com/v1alpha1/zz_generated.deepcopy.go b/apis/actions.github.com/v1alpha1/zz_generated.deepcopy.go index b0947659..f50acc08 100644 --- a/apis/actions.github.com/v1alpha1/zz_generated.deepcopy.go +++ b/apis/actions.github.com/v1alpha1/zz_generated.deepcopy.go @@ -100,7 +100,12 @@ func (in *AutoscalingListenerSpec) DeepCopyInto(out *AutoscalingListenerSpec) { } if in.GitHubServerTLS != nil { in, out := &in.GitHubServerTLS, &out.GitHubServerTLS - *out = new(GitHubServerTLSConfig) + *out = new(TLSConfig) + (*in).DeepCopyInto(*out) + } + if in.VaultConfig != nil { + in, out := &in.VaultConfig, &out.VaultConfig + *out = new(VaultConfig) (*in).DeepCopyInto(*out) } if in.Metrics != nil { @@ -209,7 +214,12 @@ func (in *AutoscalingRunnerSetSpec) DeepCopyInto(out *AutoscalingRunnerSetSpec) } if in.GitHubServerTLS != nil { in, out := &in.GitHubServerTLS, &out.GitHubServerTLS - *out = new(GitHubServerTLSConfig) + *out = new(TLSConfig) + (*in).DeepCopyInto(*out) + } + if in.VaultConfig != nil { + in, out := &in.VaultConfig, &out.VaultConfig + *out = new(VaultConfig) (*in).DeepCopyInto(*out) } in.Template.DeepCopyInto(&out.Template) @@ -260,6 +270,21 @@ func (in *AutoscalingRunnerSetStatus) DeepCopy() *AutoscalingRunnerSetStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AzureKeyVaultConfig) DeepCopyInto(out *AzureKeyVaultConfig) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureKeyVaultConfig. +func (in *AzureKeyVaultConfig) DeepCopy() *AzureKeyVaultConfig { + if in == nil { + return nil + } + out := new(AzureKeyVaultConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CounterMetric) DeepCopyInto(out *CounterMetric) { *out = *in @@ -432,14 +457,19 @@ func (in *EphemeralRunnerSetStatus) DeepCopy() *EphemeralRunnerSetStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *EphemeralRunnerSpec) DeepCopyInto(out *EphemeralRunnerSpec) { *out = *in + if in.GitHubServerTLS != nil { + in, out := &in.GitHubServerTLS, &out.GitHubServerTLS + *out = new(TLSConfig) + (*in).DeepCopyInto(*out) + } if in.Proxy != nil { in, out := &in.Proxy, &out.Proxy *out = new(ProxyConfig) (*in).DeepCopyInto(*out) } - if in.GitHubServerTLS != nil { - in, out := &in.GitHubServerTLS, &out.GitHubServerTLS - *out = new(GitHubServerTLSConfig) + if in.VaultConfig != nil { + in, out := &in.VaultConfig, &out.VaultConfig + *out = new(VaultConfig) (*in).DeepCopyInto(*out) } in.PodTemplateSpec.DeepCopyInto(&out.PodTemplateSpec) @@ -497,26 +527,6 @@ func (in *GaugeMetric) DeepCopy() *GaugeMetric { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *GitHubServerTLSConfig) DeepCopyInto(out *GitHubServerTLSConfig) { - *out = *in - if in.CertificateFrom != nil { - in, out := &in.CertificateFrom, &out.CertificateFrom - *out = new(TLSCertificateSource) - (*in).DeepCopyInto(*out) - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitHubServerTLSConfig. -func (in *GitHubServerTLSConfig) DeepCopy() *GitHubServerTLSConfig { - if in == nil { - return nil - } - out := new(GitHubServerTLSConfig) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HistogramMetric) DeepCopyInto(out *HistogramMetric) { *out = *in @@ -669,3 +679,48 @@ func (in *TLSCertificateSource) DeepCopy() *TLSCertificateSource { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TLSConfig) DeepCopyInto(out *TLSConfig) { + *out = *in + if in.CertificateFrom != nil { + in, out := &in.CertificateFrom, &out.CertificateFrom + *out = new(TLSCertificateSource) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TLSConfig. +func (in *TLSConfig) DeepCopy() *TLSConfig { + if in == nil { + return nil + } + out := new(TLSConfig) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VaultConfig) DeepCopyInto(out *VaultConfig) { + *out = *in + if in.AzureKeyVault != nil { + in, out := &in.AzureKeyVault, &out.AzureKeyVault + *out = new(AzureKeyVaultConfig) + **out = **in + } + if in.Proxy != nil { + in, out := &in.Proxy, &out.Proxy + *out = new(ProxyConfig) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VaultConfig. +func (in *VaultConfig) DeepCopy() *VaultConfig { + if in == nil { + return nil + } + out := new(VaultConfig) + in.DeepCopyInto(out) + return out +} diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml index 7af045bd..6b894061 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml @@ -7863,6 +7863,53 @@ spec: - containers type: object type: object + vaultConfig: + properties: + azureKeyVault: + properties: + certificatePath: + type: string + clientId: + type: string + tenantId: + type: string + url: + type: string + required: + - certificatePath + - clientId + - tenantId + - url + type: object + proxy: + properties: + http: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + https: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + noProxy: + items: + type: string + type: array + type: object + type: + description: |- + 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: string + type: object type: object status: description: AutoscalingListenerStatus defines the observed state of AutoscalingListener diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml index bf6f60cc..12ba4a95 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml @@ -15504,6 +15504,53 @@ spec: - containers type: object type: object + vaultConfig: + properties: + azureKeyVault: + properties: + certificatePath: + type: string + clientId: + type: string + tenantId: + type: string + url: + type: string + required: + - certificatePath + - clientId + - tenantId + - url + type: object + proxy: + properties: + http: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + https: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + noProxy: + items: + type: string + type: array + type: object + type: + description: |- + 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: string + type: object type: object status: description: AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml index f7cf1139..b45d9ae8 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml @@ -7784,6 +7784,53 @@ spec: required: - containers type: object + vaultConfig: + properties: + azureKeyVault: + properties: + certificatePath: + type: string + clientId: + type: string + tenantId: + type: string + url: + type: string + required: + - certificatePath + - clientId + - tenantId + - url + type: object + proxy: + properties: + http: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + https: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + noProxy: + items: + type: string + type: array + type: object + type: + description: |- + 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: string + type: object required: - githubConfigSecret - githubConfigUrl diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml index 09d89ce7..3a1a4733 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml @@ -7778,6 +7778,53 @@ spec: required: - containers type: object + vaultConfig: + properties: + azureKeyVault: + properties: + certificatePath: + type: string + clientId: + type: string + tenantId: + type: string + url: + type: string + required: + - certificatePath + - clientId + - tenantId + - url + type: object + proxy: + properties: + http: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + https: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + noProxy: + items: + type: string + type: array + type: object + type: + description: |- + 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: string + type: object required: - githubConfigSecret - githubConfigUrl diff --git a/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml b/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml index 3008b28c..5244e0fc 100644 --- a/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml +++ b/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml @@ -45,6 +45,7 @@ metadata: {{- if and (ne $containerMode.type "kubernetes") (not .Values.template.spec.serviceAccountName) }} actions.github.com/cleanup-no-permission-service-account-name: {{ include "gha-runner-scale-set.noPermissionServiceAccountName" . }} {{- end }} + spec: githubConfigUrl: {{ required ".Values.githubConfigUrl is required" (trimSuffix "/" .Values.githubConfigUrl) }} githubConfigSecret: {{ include "gha-runner-scale-set.githubsecret" . }} @@ -65,6 +66,24 @@ spec: {{- end }} {{- end }} + {{- if and .Values.keyVault .Values.keyVault.type }} + vaultConfig: + type: {{ .Values.keyVault.type }} + {{- if .Values.keyVault.proxy }} + proxy: {{- toYaml .Values.keyVault.proxy | nindent 6 }} + {{- end }} + {{- if eq .Values.keyVault.type "azure_key_vault" }} + azureKeyVault: + url: {{ .Values.keyVault.azureKeyVault.url }} + tenantId: {{ .Values.keyVault.azureKeyVault.tenantId }} + clientId: {{ .Values.keyVault.azureKeyVault.clientId }} + certificatePath: {{ .Values.keyVault.azureKeyVault.certificatePath }} + secretKey: {{ .Values.keyVault.azureKeyVault.secretKey }} + {{- else }} + {{- fail "Unsupported keyVault type: " .Values.keyVault.type }} + {{- end }} + {{- end }} + {{- if .Values.proxy }} proxy: {{- if .Values.proxy.http }} diff --git a/charts/gha-runner-scale-set/tests/template_test.go b/charts/gha-runner-scale-set/tests/template_test.go index 72220866..c08f9d3e 100644 --- a/charts/gha-runner-scale-set/tests/template_test.go +++ b/charts/gha-runner-scale-set/tests/template_test.go @@ -1158,7 +1158,7 @@ func TestTemplateRenderedWithTLS(t *testing.T) { ars := render(t, options) require.NotNil(t, ars.Spec.GitHubServerTLS) - expected := &v1alpha1.GitHubServerTLSConfig{ + expected := &v1alpha1.TLSConfig{ CertificateFrom: &v1alpha1.TLSCertificateSource{ ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ @@ -1218,7 +1218,7 @@ func TestTemplateRenderedWithTLS(t *testing.T) { ars := render(t, options) require.NotNil(t, ars.Spec.GitHubServerTLS) - expected := &v1alpha1.GitHubServerTLSConfig{ + expected := &v1alpha1.TLSConfig{ CertificateFrom: &v1alpha1.TLSCertificateSource{ ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ @@ -1278,7 +1278,7 @@ func TestTemplateRenderedWithTLS(t *testing.T) { ars := render(t, options) require.NotNil(t, ars.Spec.GitHubServerTLS) - expected := &v1alpha1.GitHubServerTLSConfig{ + expected := &v1alpha1.TLSConfig{ CertificateFrom: &v1alpha1.TLSCertificateSource{ ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ @@ -1338,7 +1338,7 @@ func TestTemplateRenderedWithTLS(t *testing.T) { ars := render(t, options) require.NotNil(t, ars.Spec.GitHubServerTLS) - expected := &v1alpha1.GitHubServerTLSConfig{ + expected := &v1alpha1.TLSConfig{ CertificateFrom: &v1alpha1.TLSCertificateSource{ ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ @@ -1394,7 +1394,7 @@ func TestTemplateRenderedWithTLS(t *testing.T) { ars := render(t, options) require.NotNil(t, ars.Spec.GitHubServerTLS) - expected := &v1alpha1.GitHubServerTLSConfig{ + expected := &v1alpha1.TLSConfig{ CertificateFrom: &v1alpha1.TLSCertificateSource{ ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ @@ -1450,7 +1450,7 @@ func TestTemplateRenderedWithTLS(t *testing.T) { ars := render(t, options) require.NotNil(t, ars.Spec.GitHubServerTLS) - expected := &v1alpha1.GitHubServerTLSConfig{ + expected := &v1alpha1.TLSConfig{ CertificateFrom: &v1alpha1.TLSCertificateSource{ ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ @@ -2468,3 +2468,43 @@ func TestNamespaceOverride(t *testing.T) { }) } } + +func TestAutoscalingRunnerSetCustomAnnotationsAndLabelsApplied(t *testing.T) { + t.Parallel() + + // Path to the helm chart we will test + helmChartPath, err := filepath.Abs("../../gha-runner-scale-set") + require.NoError(t, err) + + releaseName := "test-runners" + namespaceName := "test-" + strings.ToLower(random.UniqueId()) + + options := &helm.Options{ + Logger: logger.Discard, + SetValues: map[string]string{ + "githubConfigUrl": "https://github.com/actions", + "githubConfigSecret.github_token": "gh_token12345", + "controllerServiceAccount.name": "arc", + "controllerServiceAccount.namespace": "arc-system", + "annotations.actions\\.github\\.com/vault": "azure_key_vault", + "annotations.actions\\.github\\.com/cleanup-manager-role-name": "not-propagated", + "labels.custom": "custom", + "labels.app\\.kubernetes\\.io/component": "not-propagated", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + } + + output := helm.RenderTemplate(t, options, helmChartPath, releaseName, []string{"templates/autoscalingrunnerset.yaml"}) + + var autoscalingRunnerSet v1alpha1.AutoscalingRunnerSet + helm.UnmarshalK8SYaml(t, output, &autoscalingRunnerSet) + + vault := autoscalingRunnerSet.Annotations["actions.github.com/vault"] + assert.Equal(t, "azure_key_vault", vault) + + custom := autoscalingRunnerSet.Labels["custom"] + assert.Equal(t, "custom", custom) + + assert.NotEqual(t, "not-propagated", autoscalingRunnerSet.Annotations["actions.github.com/cleanup-manager-role-name"]) + assert.NotEqual(t, "not-propagated", autoscalingRunnerSet.Labels["app.kubernetes.io/component"]) +} diff --git a/charts/gha-runner-scale-set/values.yaml b/charts/gha-runner-scale-set/values.yaml index 45e5591a..60a2d1a1 100644 --- a/charts/gha-runner-scale-set/values.yaml +++ b/charts/gha-runner-scale-set/values.yaml @@ -6,7 +6,7 @@ githubConfigUrl: "" ## You can choose to supply: ## A) a PAT token, ## B) a GitHub App, or -## C) a pre-defined Kubernetes secret. +## C) a pre-defined secret. ## The syntax for each of these variations is documented below. ## (Variation A) When using a PAT token, the syntax is as follows: githubConfigSecret: @@ -28,8 +28,11 @@ githubConfigSecret: # . # private key line N # -## (Variation C) When using a pre-defined Kubernetes secret in the same namespace that the gha-runner-scale-set is going to deploy, -## the syntax is as follows: +## (Variation C) When using a pre-defined secret. +## The secret can be pulled either directly from Kubernetes, or from the vault, depending on configuration. +## Kubernetes secret in the same namespace that the gha-runner-scale-set is going to deploy. +## On the other hand, if the vault is configured, secret name will be used to fetch the app configuration. +## The syntax is as follows: # githubConfigSecret: pre-defined-secret ## Notes on using pre-defined Kubernetes secrets: ## You need to make sure your predefined secret has all the required secret data set properly. @@ -85,6 +88,26 @@ githubConfigSecret: # key: ca.crt # runnerMountPath: /usr/local/share/ca-certificates/ +# keyVault: + # Available values: "azure_key_vault" + # type: "" + # Configuration related to azure key vault + # azure_key_vault: + # url: "" + # client_id: "" + # tenant_id: "" + # certificate_path: "" + # proxy: + # http: + # url: http://proxy.com:1234 + # credentialSecretRef: proxy-auth # a secret with `username` and `password` keys + # https: + # url: http://proxy.com:1234 + # credentialSecretRef: proxy-auth # a secret with `username` and `password` keys + # noProxy: + # - example.com + # - example.org + ## Container mode is an object that provides out-of-box configuration ## for dind and kubernetes mode. Template will be modified as documented under the ## template object. diff --git a/cmd/ghalistener/app/app.go b/cmd/ghalistener/app/app.go index 02807f6a..004898a7 100644 --- a/cmd/ghalistener/app/app.go +++ b/cmd/ghalistener/app/app.go @@ -17,7 +17,7 @@ import ( // App is responsible for initializing required components and running the app. type App struct { // configured fields - config config.Config + config *config.Config logger logr.Logger // initialized fields @@ -38,8 +38,12 @@ type Worker interface { } func New(config config.Config) (*App, error) { + if err := config.Validate(); err != nil { + return nil, fmt.Errorf("failed to validate config: %w", err) + } + app := &App{ - config: config, + config: &config, } ghConfig, err := actions.ParseGitHubConfigFromURL(config.ConfigureUrl) diff --git a/cmd/ghalistener/config/config.go b/cmd/ghalistener/config/config.go index 905249aa..0df638bc 100644 --- a/cmd/ghalistener/config/config.go +++ b/cmd/ghalistener/config/config.go @@ -1,6 +1,7 @@ package config import ( + "context" "crypto/x509" "encoding/json" "fmt" @@ -9,20 +10,26 @@ import ( "os" "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/build" "github.com/actions/actions-runner-controller/github/actions" "github.com/actions/actions-runner-controller/logging" + "github.com/actions/actions-runner-controller/vault" + "github.com/actions/actions-runner-controller/vault/azurekeyvault" "github.com/go-logr/logr" "golang.org/x/net/http/httpproxy" ) type Config struct { - ConfigureUrl string `json:"configure_url"` - // AppID can be an ID of the app or the client ID - AppID string `json:"app_id"` - AppInstallationID int64 `json:"app_installation_id"` - AppPrivateKey string `json:"app_private_key"` - Token string `json:"token"` + ConfigureUrl string `json:"configure_url"` + VaultType vault.VaultType `json:"vault_type"` + VaultLookupKey string `json:"vault_lookup_key"` + // If the VaultType is set to "azure_key_vault", this field must be populated. + AzureKeyVaultConfig *azurekeyvault.Config `json:"azure_key_vault,omitempty"` + // 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"` @@ -37,23 +44,58 @@ type Config struct { Metrics *v1alpha1.MetricsConfig `json:"metrics"` } -func Read(path string) (Config, error) { - f, err := os.Open(path) +func Read(ctx context.Context, configPath string) (*Config, error) { + f, err := os.Open(configPath) if err != nil { - return Config{}, err + return nil, err } defer f.Close() var config Config if err := json.NewDecoder(f).Decode(&config); err != nil { - return Config{}, fmt.Errorf("failed to decode config: %w", err) + return nil, fmt.Errorf("failed to decode config: %w", err) } + var vault vault.Vault + switch config.VaultType { + case "": + if err := config.Validate(); err != nil { + return nil, fmt.Errorf("failed to validate configuration: %v", err) + } + + return &config, nil + case "azure_key_vault": + akv, err := azurekeyvault.New(*config.AzureKeyVaultConfig) + if err != nil { + return nil, fmt.Errorf("failed to create Azure Key Vault client: %w", err) + } + + vault = akv + default: + return nil, fmt.Errorf("unsupported vault type: %s", config.VaultType) + } + + appConfigRaw, err := vault.GetSecret(ctx, config.VaultLookupKey) + if err != nil { + return nil, fmt.Errorf("failed to get app config from vault: %w", err) + } + + appConfig, err := appconfig.FromJSONString(appConfigRaw) + if err != nil { + return nil, fmt.Errorf("failed to read app config from string: %v", err) + } + + config.AppConfig = appConfig + if err := config.Validate(); err != nil { - return Config{}, fmt.Errorf("failed to validate config: %w", err) + return nil, fmt.Errorf("config validation failed: %w", err) } - return config, nil + if ctx.Err() != nil { + return nil, ctx.Err() + } + + return &config, nil } // Validate checks the configuration for errors. @@ -74,15 +116,19 @@ func (c *Config) Validate() error { return fmt.Errorf(`MinRunners "%d" cannot be greater than MaxRunners "%d"`, c.MinRunners, c.MaxRunners) } - hasToken := len(c.Token) > 0 - hasPrivateKeyConfig := len(c.AppID) > 0 && c.AppPrivateKey != "" - - if !hasToken && !hasPrivateKeyConfig { - return fmt.Errorf(`GitHub auth credential is missing, token length: "%d", appId: %q, installationId: "%d", private key length: "%d"`, len(c.Token), c.AppID, c.AppInstallationID, len(c.AppPrivateKey)) + if c.VaultType != "" { + if err := c.VaultType.Validate(); err != nil { + return fmt.Errorf("VaultType validation failed: %w", err) + } + if c.VaultLookupKey == "" { + return fmt.Errorf("VaultLookupKey is required when VaultType is set to %q", c.VaultType) + } } - if hasToken && hasPrivateKeyConfig { - return fmt.Errorf(`only one GitHub auth method supported at a time. Have both PAT and App auth: token length: "%d", appId: %q, installationId: "%d", private key length: "%d"`, len(c.Token), c.AppID, c.AppInstallationID, len(c.AppPrivateKey)) + 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 f2dfa3d9..4fc37341 100644 --- a/cmd/ghalistener/config/config_client_test.go +++ b/cmd/ghalistener/config/config_client_test.go @@ -9,6 +9,7 @@ import ( "path/filepath" "testing" + "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1/appconfig" "github.com/actions/actions-runner-controller/cmd/ghalistener/config" "github.com/actions/actions-runner-controller/github/actions" "github.com/actions/actions-runner-controller/github/actions/testserver" @@ -53,7 +54,9 @@ func TestCustomerServerRootCA(t *testing.T) { config := config.Config{ ConfigureUrl: server.ConfigURLForOrg("myorg"), ServerRootCA: certsString, - Token: "token", + AppConfig: &appconfig.AppConfig{ + Token: "token", + }, } client, err := config.ActionsClient(logr.Discard()) @@ -80,7 +83,9 @@ func TestProxySettings(t *testing.T) { config := config.Config{ ConfigureUrl: "https://github.com/org/repo", - Token: "token", + AppConfig: &appconfig.AppConfig{ + Token: "token", + }, } client, err := config.ActionsClient(logr.Discard()) @@ -110,7 +115,9 @@ func TestProxySettings(t *testing.T) { config := config.Config{ ConfigureUrl: "https://github.com/org/repo", - Token: "token", + AppConfig: &appconfig.AppConfig{ + Token: "token", + }, } client, err := config.ActionsClient(logr.Discard(), actions.WithRetryMax(0)) @@ -145,7 +152,9 @@ func TestProxySettings(t *testing.T) { config := config.Config{ ConfigureUrl: "https://github.com/org/repo", - Token: "token", + AppConfig: &appconfig.AppConfig{ + Token: "token", + }, } client, err := config.ActionsClient(logr.Discard()) diff --git a/cmd/ghalistener/config/config_test.go b/cmd/ghalistener/config/config_validation_test.go similarity index 50% rename from cmd/ghalistener/config/config_test.go rename to cmd/ghalistener/config/config_validation_test.go index f62b4b73..18551f66 100644 --- a/cmd/ghalistener/config/config_test.go +++ b/cmd/ghalistener/config/config_validation_test.go @@ -1,9 +1,10 @@ package config import ( - "fmt" "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" ) @@ -15,7 +16,9 @@ func TestConfigValidationMinMax(t *testing.T) { RunnerScaleSetId: 1, MinRunners: 5, MaxRunners: 2, - Token: "token", + AppConfig: &appconfig.AppConfig{ + Token: "token", + }, } err := config.Validate() assert.ErrorContains(t, err, `MinRunners "5" cannot be greater than MaxRunners "2"`, "Expected error about MinRunners > MaxRunners") @@ -29,7 +32,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 := "AppConfig validation failed: missing app config" assert.ErrorContains(t, err, expectedError, "Expected error about missing auth") } @@ -39,47 +42,53 @@ func TestConfigValidationAppKey(t *testing.T) { t.Run("app id integer", func(t *testing.T) { t.Parallel() config := &Config{ - AppID: "1", - AppInstallationID: 10, + AppConfig: &appconfig.AppConfig{ + AppID: "1", + AppInstallationID: 10, + }, ConfigureUrl: "github.com/some_org/some_repo", EphemeralRunnerSetNamespace: "namespace", EphemeralRunnerSetName: "deployment", 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 := "AppConfig validation failed: no credentials provided: either a PAT or GitHub App credentials should be provided" assert.ErrorContains(t, err, expectedError, "Expected error about missing auth") }) t.Run("app id as client id", func(t *testing.T) { t.Parallel() config := &Config{ - AppID: "Iv23f8doAlphaNumer1c", - AppInstallationID: 10, + AppConfig: &appconfig.AppConfig{ + AppID: "Iv23f8doAlphaNumer1c", + AppInstallationID: 10, + }, ConfigureUrl: "github.com/some_org/some_repo", EphemeralRunnerSetNamespace: "namespace", EphemeralRunnerSetName: "deployment", 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 := "AppConfig validation failed: no credentials provided: either a PAT or GitHub App credentials should be provided" assert.ErrorContains(t, err, expectedError, "Expected error about missing auth") }) } func TestConfigValidationOnlyOneTypeOfCredentials(t *testing.T) { config := &Config{ - AppID: "1", - AppInstallationID: 10, - AppPrivateKey: "asdf", - Token: "asdf", + AppConfig: &appconfig.AppConfig{ + AppID: "1", + AppInstallationID: 10, + AppPrivateKey: "asdf", + Token: "asdf", + }, ConfigureUrl: "github.com/some_org/some_repo", EphemeralRunnerSetNamespace: "namespace", EphemeralRunnerSetName: "deployment", RunnerScaleSetId: 1, } err := config.Validate() - expectedError := fmt.Sprintf(`only one GitHub auth method supported at a time. Have both PAT and App auth: token length: "%d", appId: %q, installationId: "%d", private key length: "%d"`, len(config.Token), config.AppID, config.AppInstallationID, len(config.AppPrivateKey)) + expectedError := "AppConfig validation failed: both PAT and GitHub App credentials provided. should only provide one" assert.ErrorContains(t, err, expectedError, "Expected error about missing auth") } @@ -91,7 +100,9 @@ func TestConfigValidation(t *testing.T) { RunnerScaleSetId: 1, MinRunners: 1, MaxRunners: 5, - Token: "asdf", + AppConfig: &appconfig.AppConfig{ + Token: "asdf", + }, } err := config.Validate() @@ -110,3 +121,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, `VaultLookupKey is required when VaultType is set to "azure_key_vault"`, "Expected error for vault type without lookup key") + }) +} diff --git a/cmd/ghalistener/main.go b/cmd/ghalistener/main.go index 10436b30..26fcb511 100644 --- a/cmd/ghalistener/main.go +++ b/cmd/ghalistener/main.go @@ -13,26 +13,27 @@ import ( ) func main() { + ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) + defer stop() + configPath, ok := os.LookupEnv("LISTENER_CONFIG_PATH") if !ok { fmt.Fprintf(os.Stderr, "Error: LISTENER_CONFIG_PATH environment variable is not set\n") os.Exit(1) } - config, err := config.Read(configPath) + + config, err := config.Read(ctx, configPath) if err != nil { log.Printf("Failed to read config: %v", err) os.Exit(1) } - app, err := app.New(config) + app, err := app.New(*config) if err != nil { log.Printf("Failed to initialize app: %v", err) os.Exit(1) } - ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) - defer stop() - if err := app.Run(ctx); err != nil { log.Printf("Application returned an error: %v", err) os.Exit(1) diff --git a/config/crd/bases/actions.github.com_autoscalinglisteners.yaml b/config/crd/bases/actions.github.com_autoscalinglisteners.yaml index 7af045bd..6b894061 100644 --- a/config/crd/bases/actions.github.com_autoscalinglisteners.yaml +++ b/config/crd/bases/actions.github.com_autoscalinglisteners.yaml @@ -7863,6 +7863,53 @@ spec: - containers type: object type: object + vaultConfig: + properties: + azureKeyVault: + properties: + certificatePath: + type: string + clientId: + type: string + tenantId: + type: string + url: + type: string + required: + - certificatePath + - clientId + - tenantId + - url + type: object + proxy: + properties: + http: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + https: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + noProxy: + items: + type: string + type: array + type: object + type: + description: |- + 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: string + type: object type: object status: description: AutoscalingListenerStatus defines the observed state of AutoscalingListener diff --git a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml index bf6f60cc..12ba4a95 100644 --- a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml +++ b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml @@ -15504,6 +15504,53 @@ spec: - containers type: object type: object + vaultConfig: + properties: + azureKeyVault: + properties: + certificatePath: + type: string + clientId: + type: string + tenantId: + type: string + url: + type: string + required: + - certificatePath + - clientId + - tenantId + - url + type: object + proxy: + properties: + http: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + https: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + noProxy: + items: + type: string + type: array + type: object + type: + description: |- + 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: string + type: object type: object status: description: AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet diff --git a/config/crd/bases/actions.github.com_ephemeralrunners.yaml b/config/crd/bases/actions.github.com_ephemeralrunners.yaml index f7cf1139..b45d9ae8 100644 --- a/config/crd/bases/actions.github.com_ephemeralrunners.yaml +++ b/config/crd/bases/actions.github.com_ephemeralrunners.yaml @@ -7784,6 +7784,53 @@ spec: required: - containers type: object + vaultConfig: + properties: + azureKeyVault: + properties: + certificatePath: + type: string + clientId: + type: string + tenantId: + type: string + url: + type: string + required: + - certificatePath + - clientId + - tenantId + - url + type: object + proxy: + properties: + http: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + https: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + noProxy: + items: + type: string + type: array + type: object + type: + description: |- + 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: string + type: object required: - githubConfigSecret - githubConfigUrl diff --git a/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml b/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml index 09d89ce7..3a1a4733 100644 --- a/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml +++ b/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml @@ -7778,6 +7778,53 @@ spec: required: - containers type: object + vaultConfig: + properties: + azureKeyVault: + properties: + certificatePath: + type: string + clientId: + type: string + tenantId: + type: string + url: + type: string + required: + - certificatePath + - clientId + - tenantId + - url + type: object + proxy: + properties: + http: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + https: + properties: + credentialSecretRef: + type: string + url: + description: Required + type: string + type: object + noProxy: + items: + type: string + type: array + type: object + type: + description: |- + 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: string + type: object required: - githubConfigSecret - githubConfigUrl diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index 1b16eadf..33486e6b 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,36 +129,19 @@ 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 } - // Create a mirror secret in the same namespace as the AutoscalingListener - mirrorSecret := new(corev1.Secret) - if err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Namespace, Name: autoscalingListener.Name}, mirrorSecret); err != nil { - if !kerrors.IsNotFound(err) { - log.Error(err, "Unable to get listener secret mirror", "namespace", autoscalingListener.Namespace, "name", autoscalingListener.Name) - return ctrl.Result{}, err - } - - // Create a mirror secret for the listener pod in the Controller namespace for listener pod to use - log.Info("Creating a mirror listener secret for the listener pod") - return r.createSecretsForListener(ctx, autoscalingListener, secret, log) - } - - // make sure the mirror secret is up to date - mirrorSecretDataHash := mirrorSecret.Labels["secret-data-hash"] - secretDataHash := hash.ComputeTemplateHash(secret.Data) - if mirrorSecretDataHash != secretDataHash { - log.Info("Updating mirror listener secret for the listener pod", "mirrorSecretDataHash", mirrorSecretDataHash, "secretDataHash", secretDataHash) - return r.updateSecretsForListener(ctx, secret, mirrorSecret, log) - } - // Make sure the runner scale set listener service account is created for the listener pod in the controller namespace serviceAccount := new(corev1.ServiceAccount) if err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingListener.Namespace, Name: autoscalingListener.Name}, serviceAccount); err != nil { @@ -239,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, mirrorSecret, log) + return r.createListenerPod(ctx, &autoscalingRunnerSet, autoscalingListener, serviceAccount, appConfig, log) } cs := listenerContainerStatus(listenerPod) @@ -411,7 +395,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{ @@ -480,7 +464,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 @@ -499,7 +483,7 @@ func (r *AutoscalingListenerReconciler) createListenerPod(ctx context.Context, a return ctrl.Result{Requeue: true}, nil } - newPod, err := r.newScaleSetListenerPod(autoscalingListener, &podConfig, serviceAccount, secret, metricsConfig, envs...) + newPod, err := r.newScaleSetListenerPod(autoscalingListener, &podConfig, serviceAccount, metricsConfig, envs...) if err != nil { logger.Error(err, "Failed to build listener pod") return ctrl.Result{}, err @@ -558,23 +542,6 @@ func (r *AutoscalingListenerReconciler) certificate(ctx context.Context, autosca return certificate, nil } -func (r *AutoscalingListenerReconciler) createSecretsForListener(ctx context.Context, autoscalingListener *v1alpha1.AutoscalingListener, secret *corev1.Secret, logger logr.Logger) (ctrl.Result, error) { - newListenerSecret := r.newScaleSetListenerSecretMirror(autoscalingListener, secret) - - if err := ctrl.SetControllerReference(autoscalingListener, newListenerSecret, r.Scheme); err != nil { - return ctrl.Result{}, err - } - - logger.Info("Creating listener secret", "namespace", newListenerSecret.Namespace, "name", newListenerSecret.Name) - if err := r.Create(ctx, newListenerSecret); err != nil { - logger.Error(err, "Unable to create listener secret", "namespace", newListenerSecret.Namespace, "name", newListenerSecret.Name) - return ctrl.Result{}, err - } - - logger.Info("Created listener secret", "namespace", newListenerSecret.Namespace, "name", newListenerSecret.Name) - return ctrl.Result{Requeue: true}, nil -} - func (r *AutoscalingListenerReconciler) createProxySecret(ctx context.Context, autoscalingListener *v1alpha1.AutoscalingListener, logger logr.Logger) (ctrl.Result, error) { data, err := autoscalingListener.Spec.Proxy.ToSecretData(func(s string) (*corev1.Secret, error) { var secret corev1.Secret @@ -614,22 +581,6 @@ func (r *AutoscalingListenerReconciler) createProxySecret(ctx context.Context, a return ctrl.Result{Requeue: true}, nil } -func (r *AutoscalingListenerReconciler) updateSecretsForListener(ctx context.Context, secret *corev1.Secret, mirrorSecret *corev1.Secret, logger logr.Logger) (ctrl.Result, error) { - dataHash := hash.ComputeTemplateHash(secret.Data) - updatedMirrorSecret := mirrorSecret.DeepCopy() - updatedMirrorSecret.Labels["secret-data-hash"] = dataHash - updatedMirrorSecret.Data = secret.Data - - logger.Info("Updating listener mirror secret", "namespace", updatedMirrorSecret.Namespace, "name", updatedMirrorSecret.Name, "hash", dataHash) - if err := r.Update(ctx, updatedMirrorSecret); err != nil { - logger.Error(err, "Unable to update listener mirror secret", "namespace", updatedMirrorSecret.Namespace, "name", updatedMirrorSecret.Name) - return ctrl.Result{}, err - } - - logger.Info("Updated listener mirror secret", "namespace", updatedMirrorSecret.Namespace, "name", updatedMirrorSecret.Name, "hash", dataHash) - return ctrl.Result{Requeue: true}, nil -} - func (r *AutoscalingListenerReconciler) createRoleForListener(ctx context.Context, autoscalingListener *v1alpha1.AutoscalingListener, logger logr.Logger) (ctrl.Result, error) { newRole := r.newScaleSetListenerRole(autoscalingListener) diff --git a/controllers/actions.github.com/autoscalinglistener_controller_test.go b/controllers/actions.github.com/autoscalinglistener_controller_test.go index b5b957d0..407dd12a 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller_test.go +++ b/controllers/actions.github.com/autoscalinglistener_controller_test.go @@ -14,7 +14,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" - listenerconfig "github.com/actions/actions-runner-controller/cmd/ghalistener/config" + 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") @@ -452,10 +460,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") @@ -725,11 +740,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") @@ -922,10 +943,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") @@ -1018,6 +1046,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", @@ -1039,9 +1073,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") @@ -1056,7 +1091,7 @@ var _ = Describe("Test GitHub Server TLS configuration", func() { Spec: v1alpha1.AutoscalingRunnerSetSpec{ GitHubConfigUrl: "https://github.com/owner/repo", GitHubConfigSecret: configSecret.Name, - GitHubServerTLS: &v1alpha1.GitHubServerTLSConfig{ + GitHubServerTLS: &v1alpha1.TLSConfig{ CertificateFrom: &v1alpha1.TLSCertificateSource{ ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ @@ -1092,7 +1127,7 @@ var _ = Describe("Test GitHub Server TLS configuration", func() { Spec: v1alpha1.AutoscalingListenerSpec{ GitHubConfigUrl: "https://github.com/owner/repo", GitHubConfigSecret: configSecret.Name, - GitHubServerTLS: &v1alpha1.GitHubServerTLSConfig{ + GitHubServerTLS: &v1alpha1.TLSConfig{ CertificateFrom: &v1alpha1.TLSCertificateSource{ ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ @@ -1136,7 +1171,7 @@ var _ = Describe("Test GitHub Server TLS configuration", func() { g.Expect(config.Data["config.json"]).ToNot(BeEmpty(), "listener configuration file should not be empty") - var listenerConfig listenerconfig.Config + var listenerConfig ghalistenerconfig.Config err = json.Unmarshal(config.Data["config.json"], &listenerConfig) g.Expect(err).NotTo(HaveOccurred(), "failed to parse listener configuration file") diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go index 21740ff6..1a7a83b9 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -207,14 +207,6 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl return r.updateRunnerScaleSetName(ctx, autoscalingRunnerSet, log) } - secret := new(corev1.Secret) - if err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingRunnerSet.Namespace, Name: autoscalingRunnerSet.Spec.GitHubConfigSecret}, secret); err != nil { - log.Error(err, "Failed to find GitHub config secret.", - "namespace", autoscalingRunnerSet.Namespace, - "name", autoscalingRunnerSet.Spec.GitHubConfigSecret) - return ctrl.Result{}, err - } - existingRunnerSets, err := r.listEphemeralRunnerSets(ctx, autoscalingRunnerSet) if err != nil { log.Error(err, "Failed to list existing ephemeral runner sets") @@ -402,12 +394,12 @@ 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.actionsClientFor(ctx, autoscalingRunnerSet) + actionsClient, err := r.GetActionsService(ctx, autoscalingRunnerSet) if len(autoscalingRunnerSet.Spec.RunnerScaleSetName) == 0 { autoscalingRunnerSet.Spec.RunnerScaleSetName = autoscalingRunnerSet.Name } if err != nil { - logger.Error(err, "Failed to initialize Actions service client for creating a new runner scale set") + logger.Error(err, "Failed to initialize Actions service client for creating a new runner scale set", "error", err.Error()) return ctrl.Result{}, err } @@ -498,7 +490,7 @@ func (r *AutoscalingRunnerSetReconciler) updateRunnerScaleSetRunnerGroup(ctx con return ctrl.Result{}, err } - actionsClient, err := r.actionsClientFor(ctx, autoscalingRunnerSet) + actionsClient, err := r.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 @@ -546,7 +538,7 @@ func (r *AutoscalingRunnerSetReconciler) updateRunnerScaleSetName(ctx context.Co return ctrl.Result{}, nil } - actionsClient, err := r.actionsClientFor(ctx, autoscalingRunnerSet) + actionsClient, err := r.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 @@ -597,7 +589,7 @@ func (r *AutoscalingRunnerSetReconciler) deleteRunnerScaleSet(ctx context.Contex return nil } - actionsClient, err := r.actionsClientFor(ctx, autoscalingRunnerSet) + actionsClient, err := r.GetActionsService(ctx, autoscalingRunnerSet) if err != nil { logger.Error(err, "Failed to initialize Actions service client for updating a existing runner scale set") return err @@ -676,74 +668,6 @@ func (r *AutoscalingRunnerSetReconciler) listEphemeralRunnerSets(ctx context.Con return &EphemeralRunnerSets{list: list}, nil } -func (r *AutoscalingRunnerSetReconciler) actionsClientFor(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet) (actions.ActionsService, error) { - var configSecret corev1.Secret - if err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingRunnerSet.Namespace, Name: autoscalingRunnerSet.Spec.GitHubConfigSecret}, &configSecret); err != nil { - return nil, fmt.Errorf("failed to find GitHub config secret: %w", err) - } - - opts, err := r.actionsClientOptionsFor(ctx, autoscalingRunnerSet) - if err != nil { - return nil, fmt.Errorf("failed to get actions client options: %w", err) - } - - return r.ActionsClient.GetClientFromSecret( - ctx, - autoscalingRunnerSet.Spec.GitHubConfigUrl, - autoscalingRunnerSet.Namespace, - configSecret.Data, - opts..., - ) -} - -func (r *AutoscalingRunnerSetReconciler) actionsClientOptionsFor(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet) ([]actions.ClientOption, error) { - var options []actions.ClientOption - - if autoscalingRunnerSet.Spec.Proxy != nil { - proxyFunc, err := autoscalingRunnerSet.Spec.Proxy.ProxyFunc(func(s string) (*corev1.Secret, error) { - var secret corev1.Secret - err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingRunnerSet.Namespace, Name: s}, &secret) - if err != nil { - return nil, fmt.Errorf("failed to get proxy secret %s: %w", s, err) - } - - return &secret, nil - }) - if err != nil { - return nil, fmt.Errorf("failed to get proxy func: %w", err) - } - - options = append(options, actions.WithProxy(proxyFunc)) - } - - tlsConfig := autoscalingRunnerSet.Spec.GitHubServerTLS - if tlsConfig != nil { - pool, err := tlsConfig.ToCertPool(func(name, key string) ([]byte, error) { - var configmap corev1.ConfigMap - err := r.Get( - ctx, - types.NamespacedName{ - Namespace: autoscalingRunnerSet.Namespace, - Name: name, - }, - &configmap, - ) - if err != nil { - return nil, fmt.Errorf("failed to get configmap %s: %w", name, err) - } - - return []byte(configmap.Data[key]), nil - }) - if err != nil { - return nil, fmt.Errorf("failed to get tls config: %w", err) - } - - options = append(options, actions.WithRootCAs(pool)) - } - - return options, nil -} - // SetupWithManager sets up the controller with the Manager. func (r *AutoscalingRunnerSetReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go index b3002470..7411b6b9 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go @@ -70,7 +70,12 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { Log: logf.Log, ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", - ActionsClient: fake.NewMultiClient(), + ResourceBuilder: ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: k8sClient, + multiClient: fake.NewMultiClient(), + }, + }, } err := controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") @@ -677,33 +682,40 @@ var _ = Describe("Test AutoScalingController updates", Ordered, func() { autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient) configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) + multiClient := fake.NewMultiClient( + fake.WithDefaultClient( + fake.NewFakeClient( + fake.WithUpdateRunnerScaleSet( + &actions.RunnerScaleSet{ + Id: 1, + Name: "testset_update", + RunnerGroupId: 1, + RunnerGroupName: "testgroup", + Labels: []actions.Label{{Type: "test", Name: "test"}}, + RunnerSetting: actions.RunnerSetting{}, + CreatedOn: time.Now(), + RunnerJitConfigUrl: "test.test.test", + Statistics: nil, + }, + nil, + ), + ), + nil, + ), + ) + controller := &AutoscalingRunnerSetReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), Log: logf.Log, ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", - ActionsClient: fake.NewMultiClient( - fake.WithDefaultClient( - fake.NewFakeClient( - fake.WithUpdateRunnerScaleSet( - &actions.RunnerScaleSet{ - Id: 1, - Name: "testset_update", - RunnerGroupId: 1, - RunnerGroupName: "testgroup", - Labels: []actions.Label{{Type: "test", Name: "test"}}, - RunnerSetting: actions.RunnerSetting{}, - CreatedOn: time.Now(), - RunnerJitConfigUrl: "test.test.test", - Statistics: nil, - }, - nil, - ), - ), - nil, - ), - ), + ResourceBuilder: ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: k8sClient, + multiClient: multiClient, + }, + }, } err := controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") @@ -818,7 +830,12 @@ var _ = Describe("Test AutoscalingController creation failures", Ordered, func() Log: logf.Log, ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", - ActionsClient: fake.NewMultiClient(), + ResourceBuilder: ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: k8sClient, + multiClient: fake.NewMultiClient(), + }, + }, } err := controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") @@ -937,14 +954,19 @@ var _ = Describe("Test client optional configuration", Ordered, func() { ctx = context.Background() autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient) configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) - + multiClient := actions.NewMultiClient(logr.Discard()) controller = &AutoscalingRunnerSetReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), Log: logf.Log, ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", - ActionsClient: actions.NewMultiClient(logr.Discard()), + ResourceBuilder: ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: k8sClient, + multiClient: multiClient, + }, + }, } err := controller.SetupWithManager(mgr) @@ -1127,7 +1149,12 @@ var _ = Describe("Test client optional configuration", Ordered, func() { Log: logf.Log, ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", - ActionsClient: fake.NewMultiClient(), + ResourceBuilder: ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: k8sClient, + multiClient: fake.NewMultiClient(), + }, + }, } err = controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") @@ -1136,7 +1163,10 @@ var _ = Describe("Test client optional configuration", Ordered, func() { }) It("should be able to make requests to a server using root CAs", func() { - controller.ActionsClient = actions.NewMultiClient(logr.Discard()) + controller.SecretResolver = &SecretResolver{ + k8sClient: k8sClient, + multiClient: actions.NewMultiClient(logr.Discard()), + } certsFolder := filepath.Join( "../../", @@ -1171,7 +1201,7 @@ var _ = Describe("Test client optional configuration", Ordered, func() { Spec: v1alpha1.AutoscalingRunnerSetSpec{ GitHubConfigUrl: server.ConfigURLForOrg("my-org"), GitHubConfigSecret: configSecret.Name, - GitHubServerTLS: &v1alpha1.GitHubServerTLSConfig{ + GitHubServerTLS: &v1alpha1.TLSConfig{ CertificateFrom: &v1alpha1.TLSCertificateSource{ ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ @@ -1224,7 +1254,7 @@ var _ = Describe("Test client optional configuration", Ordered, func() { Spec: v1alpha1.AutoscalingRunnerSetSpec{ GitHubConfigUrl: "https://github.com/owner/repo", GitHubConfigSecret: configSecret.Name, - GitHubServerTLS: &v1alpha1.GitHubServerTLSConfig{ + GitHubServerTLS: &v1alpha1.TLSConfig{ CertificateFrom: &v1alpha1.TLSCertificateSource{ ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ @@ -1288,7 +1318,7 @@ var _ = Describe("Test client optional configuration", Ordered, func() { Spec: v1alpha1.AutoscalingRunnerSetSpec{ GitHubConfigUrl: "https://github.com/owner/repo", GitHubConfigSecret: configSecret.Name, - GitHubServerTLS: &v1alpha1.GitHubServerTLSConfig{ + GitHubServerTLS: &v1alpha1.TLSConfig{ CertificateFrom: &v1alpha1.TLSCertificateSource{ ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ @@ -1361,7 +1391,12 @@ var _ = Describe("Test external permissions cleanup", Ordered, func() { Log: logf.Log, ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", - ActionsClient: fake.NewMultiClient(), + ResourceBuilder: ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: k8sClient, + multiClient: fake.NewMultiClient(), + }, + }, } err := controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") @@ -1519,7 +1554,12 @@ var _ = Describe("Test external permissions cleanup", Ordered, func() { Log: logf.Log, ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", - ActionsClient: fake.NewMultiClient(), + ResourceBuilder: ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: k8sClient, + multiClient: fake.NewMultiClient(), + }, + }, } err := controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") @@ -1727,7 +1767,12 @@ var _ = Describe("Test resource version and build version mismatch", func() { Log: logf.Log, ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", - ActionsClient: fake.NewMultiClient(), + ResourceBuilder: ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: k8sClient, + multiClient: fake.NewMultiClient(), + }, + }, } err := controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") diff --git a/controllers/actions.github.com/ephemeralrunner_controller.go b/controllers/actions.github.com/ephemeralrunner_controller.go index 5ac80df7..9ca3f52e 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller.go +++ b/controllers/actions.github.com/ephemeralrunner_controller.go @@ -45,9 +45,8 @@ const ( // EphemeralRunnerReconciler reconciles a EphemeralRunner object type EphemeralRunnerReconciler struct { client.Client - Log logr.Logger - Scheme *runtime.Scheme - ActionsClient actions.MultiClient + Log logr.Logger + Scheme *runtime.Scheme ResourceBuilder } @@ -529,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.actionsClientFor(ctx, ephemeralRunner) + actionsClient, err := r.GetActionsService(ctx, ephemeralRunner) if err != nil { return &ctrl.Result{}, fmt.Errorf("failed to get actions client for generating JIT config: %w", err) } @@ -753,77 +752,10 @@ func (r *EphemeralRunnerReconciler) updateRunStatusFromPod(ctx context.Context, return nil } -func (r *EphemeralRunnerReconciler) actionsClientFor(ctx context.Context, runner *v1alpha1.EphemeralRunner) (actions.ActionsService, error) { - secret := new(corev1.Secret) - if err := r.Get(ctx, types.NamespacedName{Namespace: runner.Namespace, Name: runner.Spec.GitHubConfigSecret}, secret); err != nil { - return nil, fmt.Errorf("failed to get secret: %w", err) - } - - opts, err := r.actionsClientOptionsFor(ctx, runner) - if err != nil { - return nil, fmt.Errorf("failed to get actions client options: %w", err) - } - - return r.ActionsClient.GetClientFromSecret( - ctx, - runner.Spec.GitHubConfigUrl, - runner.Namespace, - secret.Data, - opts..., - ) -} - -func (r *EphemeralRunnerReconciler) actionsClientOptionsFor(ctx context.Context, runner *v1alpha1.EphemeralRunner) ([]actions.ClientOption, error) { - var opts []actions.ClientOption - if runner.Spec.Proxy != nil { - proxyFunc, err := runner.Spec.Proxy.ProxyFunc(func(s string) (*corev1.Secret, error) { - var secret corev1.Secret - err := r.Get(ctx, types.NamespacedName{Namespace: runner.Namespace, Name: s}, &secret) - if err != nil { - return nil, fmt.Errorf("failed to get proxy secret %s: %w", s, err) - } - - return &secret, nil - }) - if err != nil { - return nil, fmt.Errorf("failed to get proxy func: %w", err) - } - - opts = append(opts, actions.WithProxy(proxyFunc)) - } - - tlsConfig := runner.Spec.GitHubServerTLS - if tlsConfig != nil { - pool, err := tlsConfig.ToCertPool(func(name, key string) ([]byte, error) { - var configmap corev1.ConfigMap - err := r.Get( - ctx, - types.NamespacedName{ - Namespace: runner.Namespace, - Name: name, - }, - &configmap, - ) - if err != nil { - return nil, fmt.Errorf("failed to get configmap %s: %w", name, err) - } - - return []byte(configmap.Data[key]), nil - }) - if err != nil { - return nil, fmt.Errorf("failed to get tls config: %w", err) - } - - opts = append(opts, actions.WithRootCAs(pool)) - } - - return opts, nil -} - // 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.actionsClientFor(ctx, runner) + actionsClient, err := r.GetActionsService(ctx, runner) if err != nil { return false, fmt.Errorf("failed to get Actions client for ScaleSet: %w", err) } @@ -850,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.actionsClientFor(ctx, ephemeralRunner) + client, err := r.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 0b842d5c..26d66814 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunner_controller_test.go @@ -107,10 +107,15 @@ var _ = Describe("EphemeralRunner", func() { configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) controller = &EphemeralRunnerReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Log: logf.Log, - ActionsClient: fake.NewMultiClient(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + ResourceBuilder: ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: mgr.GetClient(), + multiClient: fake.NewMultiClient(), + }, + }, } err := controller.SetupWithManager(mgr) @@ -789,22 +794,27 @@ var _ = Describe("EphemeralRunner", func() { Client: mgr.GetClient(), Scheme: mgr.GetScheme(), Log: logf.Log, - ActionsClient: fake.NewMultiClient( - fake.WithDefaultClient( - fake.NewFakeClient( - fake.WithGetRunner( + ResourceBuilder: ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: mgr.GetClient(), + multiClient: fake.NewMultiClient( + fake.WithDefaultClient( + fake.NewFakeClient( + fake.WithGetRunner( + nil, + &actions.ActionsError{ + StatusCode: http.StatusNotFound, + Err: &actions.ActionsExceptionError{ + ExceptionName: "AgentNotFoundException", + }, + }, + ), + ), nil, - &actions.ActionsError{ - StatusCode: http.StatusNotFound, - Err: &actions.ActionsExceptionError{ - ExceptionName: "AgentNotFoundException", - }, - }, ), ), - nil, - ), - ), + }, + }, } err := controller.SetupWithManager(mgr) Expect(err).To(BeNil(), "failed to setup controller") @@ -861,10 +871,15 @@ var _ = Describe("EphemeralRunner", func() { configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoScalingNS.Name) controller = &EphemeralRunnerReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Log: logf.Log, - ActionsClient: fake.NewMultiClient(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + ResourceBuilder: ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: mgr.GetClient(), + multiClient: fake.NewMultiClient(), + }, + }, } err := controller.SetupWithManager(mgr) Expect(err).To(BeNil(), "failed to setup controller") @@ -874,7 +889,12 @@ var _ = Describe("EphemeralRunner", func() { It("uses an actions client with proxy transport", func() { // Use an actual client - controller.ActionsClient = actions.NewMultiClient(logr.Discard()) + controller.ResourceBuilder = ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: mgr.GetClient(), + multiClient: actions.NewMultiClient(logr.Discard()), + }, + } proxySuccessfulllyCalled := false proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -1025,10 +1045,15 @@ var _ = Describe("EphemeralRunner", func() { Expect(err).NotTo(HaveOccurred(), "failed to create configmap with root CAs") controller = &EphemeralRunnerReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Log: logf.Log, - ActionsClient: fake.NewMultiClient(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + ResourceBuilder: ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: mgr.GetClient(), + multiClient: fake.NewMultiClient(), + }, + }, } err = controller.SetupWithManager(mgr) @@ -1059,11 +1084,16 @@ var _ = Describe("EphemeralRunner", func() { server.StartTLS() // Use an actual client - controller.ActionsClient = actions.NewMultiClient(logr.Discard()) + controller.ResourceBuilder = ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: mgr.GetClient(), + multiClient: actions.NewMultiClient(logr.Discard()), + }, + } ephemeralRunner := newExampleRunner("test-runner", autoScalingNS.Name, configSecret.Name) ephemeralRunner.Spec.GitHubConfigUrl = server.ConfigURLForOrg("my-org") - ephemeralRunner.Spec.GitHubServerTLS = &v1alpha1.GitHubServerTLSConfig{ + ephemeralRunner.Spec.GitHubServerTLS = &v1alpha1.TLSConfig{ CertificateFrom: &v1alpha1.TLSCertificateSource{ ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller.go b/controllers/actions.github.com/ephemeralrunnerset_controller.go index 773e1286..2a09a1c5 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.actionsClientFor(ctx, ephemeralRunnerSet) + actionsClient, err := r.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.actionsClientFor(ctx, ephemeralRunnerSet) + actionsClient, err := r.GetActionsService(ctx, ephemeralRunnerSet) if err != nil { return fmt.Errorf("failed to create actions client for ephemeral runner replica set: %w", err) } @@ -502,73 +502,6 @@ func (r *EphemeralRunnerSetReconciler) deleteEphemeralRunnerWithActionsClient(ct return true, nil } -func (r *EphemeralRunnerSetReconciler) actionsClientFor(ctx context.Context, rs *v1alpha1.EphemeralRunnerSet) (actions.ActionsService, error) { - secret := new(corev1.Secret) - if err := r.Get(ctx, types.NamespacedName{Namespace: rs.Namespace, Name: rs.Spec.EphemeralRunnerSpec.GitHubConfigSecret}, secret); err != nil { - return nil, fmt.Errorf("failed to get secret: %w", err) - } - - opts, err := r.actionsClientOptionsFor(ctx, rs) - if err != nil { - return nil, fmt.Errorf("failed to get actions client options: %w", err) - } - - return r.ActionsClient.GetClientFromSecret( - ctx, - rs.Spec.EphemeralRunnerSpec.GitHubConfigUrl, - rs.Namespace, - secret.Data, - opts..., - ) -} - -func (r *EphemeralRunnerSetReconciler) actionsClientOptionsFor(ctx context.Context, rs *v1alpha1.EphemeralRunnerSet) ([]actions.ClientOption, error) { - var opts []actions.ClientOption - if rs.Spec.EphemeralRunnerSpec.Proxy != nil { - proxyFunc, err := rs.Spec.EphemeralRunnerSpec.Proxy.ProxyFunc(func(s string) (*corev1.Secret, error) { - var secret corev1.Secret - err := r.Get(ctx, types.NamespacedName{Namespace: rs.Namespace, Name: s}, &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 get proxy func: %w", err) - } - - opts = append(opts, actions.WithProxy(proxyFunc)) - } - - tlsConfig := rs.Spec.EphemeralRunnerSpec.GitHubServerTLS - if tlsConfig != nil { - pool, err := tlsConfig.ToCertPool(func(name, key string) ([]byte, error) { - var configmap corev1.ConfigMap - err := r.Get( - ctx, - types.NamespacedName{ - Namespace: rs.Namespace, - Name: name, - }, - &configmap, - ) - if err != nil { - return nil, fmt.Errorf("failed to get configmap %s: %w", name, err) - } - - return []byte(configmap.Data[key]), nil - }) - if err != nil { - return nil, fmt.Errorf("failed to get tls config: %w", err) - } - - opts = append(opts, actions.WithRootCAs(pool)) - } - - return opts, nil -} - // SetupWithManager sets up the controller with the Manager. func (r *EphemeralRunnerSetReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go index 665279e8..a23d537a 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go @@ -54,10 +54,15 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) controller := &EphemeralRunnerSetReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Log: logf.Log, - ActionsClient: fake.NewMultiClient(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + ResourceBuilder: ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: mgr.GetClient(), + multiClient: fake.NewMultiClient(), + }, + }, } err := controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") @@ -1104,10 +1109,15 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func( configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) controller := &EphemeralRunnerSetReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Log: logf.Log, - ActionsClient: actions.NewMultiClient(logr.Discard()), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + ResourceBuilder: ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: mgr.GetClient(), + multiClient: actions.NewMultiClient(logr.Discard()), + }, + }, } err := controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") @@ -1403,10 +1413,15 @@ var _ = Describe("Test EphemeralRunnerSet controller with custom root CA", func( Expect(err).NotTo(HaveOccurred(), "failed to create configmap with root CAs") controller := &EphemeralRunnerSetReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Log: logf.Log, - ActionsClient: actions.NewMultiClient(logr.Discard()), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + ResourceBuilder: ResourceBuilder{ + SecretResolver: &SecretResolver{ + k8sClient: mgr.GetClient(), + multiClient: actions.NewMultiClient(logr.Discard()), + }, + }, } err = controller.SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred(), "failed to setup controller") @@ -1445,7 +1460,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with custom root CA", func( EphemeralRunnerSpec: v1alpha1.EphemeralRunnerSpec{ GitHubConfigUrl: server.ConfigURLForOrg("my-org"), GitHubConfigSecret: configSecret.Name, - GitHubServerTLS: &v1alpha1.GitHubServerTLSConfig{ + GitHubServerTLS: &v1alpha1.TLSConfig{ CertificateFrom: &v1alpha1.TLSCertificateSource{ ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index 7fe8febf..abdb0706 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/controllers/actions.github.com/resourcebuilder.go @@ -12,11 +12,13 @@ import ( "strings" "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/build" - listenerconfig "github.com/actions/actions-runner-controller/cmd/ghalistener/config" + ghalistenerconfig "github.com/actions/actions-runner-controller/cmd/ghalistener/config" "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/azurekeyvault" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -72,6 +74,7 @@ func SetListenerEntrypoint(entrypoint string) { type ResourceBuilder struct { ExcludeLabelPropagationPrefixes []string + *SecretResolver } // boolPtr returns a pointer to a bool value @@ -121,6 +124,7 @@ func (b *ResourceBuilder) newAutoScalingListener(autoscalingRunnerSet *v1alpha1. Spec: v1alpha1.AutoscalingListenerSpec{ GitHubConfigUrl: autoscalingRunnerSet.Spec.GitHubConfigUrl, GitHubConfigSecret: autoscalingRunnerSet.Spec.GitHubConfigSecret, + VaultConfig: autoscalingRunnerSet.VaultConfig(), RunnerScaleSetId: runnerScaleSetId, AutoscalingRunnerSetNamespace: autoscalingRunnerSet.Namespace, AutoscalingRunnerSetName: autoscalingRunnerSet.Name, @@ -160,7 +164,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 = "" @@ -170,21 +174,8 @@ func (b *ResourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha metricsEndpoint = metricsConfig.endpoint } - var appInstallationID int64 - if id, ok := secret.Data["github_app_installation_id"]; ok { - var err error - appInstallationID, err = strconv.ParseInt(string(id), 10, 64) - if err != nil { - return nil, fmt.Errorf("failed to convert github_app_installation_id to int: %v", err) - } - } - - config := listenerconfig.Config{ + config := ghalistenerconfig.Config{ ConfigureUrl: autoscalingListener.Spec.GitHubConfigUrl, - AppID: string(secret.Data["github_app_id"]), - AppInstallationID: appInstallationID, - AppPrivateKey: string(secret.Data["github_app_private_key"]), - Token: string(secret.Data["github_token"]), EphemeralRunnerSetNamespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, EphemeralRunnerSetName: autoscalingListener.Spec.EphemeralRunnerSetName, MaxRunners: autoscalingListener.Spec.MaxRunners, @@ -199,6 +190,20 @@ func (b *ResourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha Metrics: autoscalingListener.Spec.Metrics, } + vault := autoscalingListener.Spec.VaultConfig + if vault == nil { + config.AppConfig = appConfig + } else { + config.VaultType = vault.Type + config.VaultLookupKey = autoscalingListener.Spec.GitHubConfigSecret + config.AzureKeyVaultConfig = &azurekeyvault.Config{ + TenantID: vault.AzureKeyVault.TenantID, + ClientID: vault.AzureKeyVault.ClientID, + URL: vault.AzureKeyVault.URL, + CertificatePath: vault.AzureKeyVault.CertificatePath, + } + } + if err := config.Validate(); err != nil { return nil, fmt.Errorf("invalid listener config: %w", err) } @@ -219,7 +224,7 @@ func (b *ResourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha }, nil } -func (b *ResourceBuilder) newScaleSetListenerPod(autoscalingListener *v1alpha1.AutoscalingListener, podConfig *corev1.Secret, serviceAccount *corev1.ServiceAccount, secret *corev1.Secret, metricsConfig *listenerMetricsServerConfig, envs ...corev1.EnvVar) (*corev1.Pod, error) { +func (b *ResourceBuilder) newScaleSetListenerPod(autoscalingListener *v1alpha1.AutoscalingListener, podConfig *corev1.Secret, serviceAccount *corev1.ServiceAccount, metricsConfig *listenerMetricsServerConfig, envs ...corev1.EnvVar) (*corev1.Pod, error) { listenerEnv := []corev1.EnvVar{ { Name: "LISTENER_CONFIG_PATH", @@ -490,25 +495,6 @@ func (b *ResourceBuilder) newScaleSetListenerRoleBinding(autoscalingListener *v1 return newRoleBinding } -func (b *ResourceBuilder) newScaleSetListenerSecretMirror(autoscalingListener *v1alpha1.AutoscalingListener, secret *corev1.Secret) *corev1.Secret { - dataHash := hash.ComputeTemplateHash(&secret.Data) - - newListenerSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: autoscalingListener.Name, - Namespace: autoscalingListener.Namespace, - Labels: b.mergeLabels(autoscalingListener.Labels, map[string]string{ - LabelKeyGitHubScaleSetNamespace: autoscalingListener.Spec.AutoscalingRunnerSetNamespace, - LabelKeyGitHubScaleSetName: autoscalingListener.Spec.AutoscalingRunnerSetName, - "secret-data-hash": dataHash, - }), - }, - Data: secret.DeepCopy().Data, - } - - return newListenerSecret -} - func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet) (*v1alpha1.EphemeralRunnerSet, error) { runnerScaleSetId, err := strconv.Atoi(autoscalingRunnerSet.Annotations[runnerScaleSetIdAnnotationKey]) if err != nil { @@ -561,6 +547,7 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A Proxy: autoscalingRunnerSet.Spec.Proxy, GitHubServerTLS: autoscalingRunnerSet.Spec.GitHubServerTLS, PodTemplateSpec: autoscalingRunnerSet.Spec.Template, + VaultConfig: autoscalingRunnerSet.VaultConfig(), }, }, } @@ -582,6 +569,7 @@ func (b *ResourceBuilder) newEphemeralRunner(ephemeralRunnerSet *v1alpha1.Epheme for key, val := range ephemeralRunnerSet.Annotations { annotations[key] = val } + annotations[AnnotationKeyPatchID] = strconv.Itoa(ephemeralRunnerSet.Spec.PatchID) return &v1alpha1.EphemeralRunner{ TypeMeta: metav1.TypeMeta{}, diff --git a/controllers/actions.github.com/resourcebuilder_test.go b/controllers/actions.github.com/resourcebuilder_test.go index a7dcdf75..12e50533 100644 --- a/controllers/actions.github.com/resourcebuilder_test.go +++ b/controllers/actions.github.com/resourcebuilder_test.go @@ -82,12 +82,7 @@ func TestLabelPropagation(t *testing.T) { Name: "test", }, } - listenerSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - } - listenerPod, err := b.newScaleSetListenerPod(listener, &corev1.Secret{}, listenerServiceAccount, listenerSecret, nil) + listenerPod, err := b.newScaleSetListenerPod(listener, &corev1.Secret{}, listenerServiceAccount, nil) require.NoError(t, err) assert.Equal(t, listenerPod.Labels, listener.Labels) diff --git a/controllers/actions.github.com/secret_resolver.go b/controllers/actions.github.com/secret_resolver.go new file mode 100644 index 00000000..d5a4af08 --- /dev/null +++ b/controllers/actions.github.com/secret_resolver.go @@ -0,0 +1,280 @@ +package actionsgithubcom + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/url" + "strings" + + "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/github/actions" + "github.com/actions/actions-runner-controller/vault" + "github.com/actions/actions-runner-controller/vault/azurekeyvault" + "golang.org/x/net/http/httpproxy" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type SecretResolver struct { + k8sClient client.Client + multiClient actions.MultiClient +} + +type SecretResolverOption func(*SecretResolver) + +func NewSecretResolver(k8sClient client.Client, multiClient actions.MultiClient, opts ...SecretResolverOption) *SecretResolver { + if k8sClient == nil { + panic("k8sClient must not be nil") + } + + secretResolver := &SecretResolver{ + k8sClient: k8sClient, + multiClient: multiClient, + } + + for _, opt := range opts { + opt(secretResolver) + } + + return secretResolver +} + +type ActionsGitHubObject interface { + client.Object + GitHubConfigUrl() string + GitHubConfigSecret() string + 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(ctx, 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(ctx, 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) + } + + var clientOptions []actions.ClientOption + if proxy := obj.GitHubProxy(); proxy != nil { + config := &httpproxy.Config{ + NoProxy: strings.Join(proxy.NoProxy, ","), + } + + if proxy.HTTP != nil { + u, err := url.Parse(proxy.HTTP.Url) + if err != nil { + return nil, fmt.Errorf("failed to parse proxy http url %q: %w", proxy.HTTP.Url, err) + } + + if ref := proxy.HTTP.CredentialSecretRef; ref != "" { + u.User, err = resolver.proxyCredentials(ctx, ref) + if err != nil { + return nil, fmt.Errorf("failed to resolve proxy credentials: %v", err) + } + } + + config.HTTPProxy = u.String() + } + + if proxy.HTTPS != nil { + u, err := url.Parse(proxy.HTTPS.Url) + if err != nil { + return nil, fmt.Errorf("failed to parse proxy https url %q: %w", proxy.HTTPS.Url, err) + } + + if ref := proxy.HTTPS.CredentialSecretRef; ref != "" { + u.User, err = resolver.proxyCredentials(ctx, ref) + if err != nil { + return nil, fmt.Errorf("failed to resolve proxy credentials: %v", err) + } + } + + config.HTTPSProxy = u.String() + } + + proxyFunc := func(req *http.Request) (*url.URL, error) { + return config.ProxyFunc()(req.URL) + } + + clientOptions = append(clientOptions, actions.WithProxy(proxyFunc)) + } + + tlsConfig := obj.GitHubServerTLS() + if tlsConfig != nil { + pool, err := tlsConfig.ToCertPool(func(name, key string) ([]byte, error) { + var configmap corev1.ConfigMap + err := sr.k8sClient.Get( + ctx, + types.NamespacedName{ + Namespace: obj.GetNamespace(), + Name: name, + }, + &configmap, + ) + if err != nil { + return nil, fmt.Errorf("failed to get configmap %s: %w", name, err) + } + + return []byte(configmap.Data[key]), nil + }) + if err != nil { + return nil, fmt.Errorf("failed to get tls config: %w", err) + } + + clientOptions = append(clientOptions, actions.WithRootCAs(pool)) + } + + return sr.multiClient.GetClientFor( + ctx, + obj.GitHubConfigUrl(), + appConfig, + obj.GetNamespace(), + clientOptions..., + ) +} + +func (sr *SecretResolver) resolverForObject(ctx context.Context, obj ActionsGitHubObject) (resolver, error) { + vaultConfig := obj.VaultConfig() + if vaultConfig == nil || vaultConfig.Type == "" { + return &k8sResolver{ + namespace: obj.GetNamespace(), + client: sr.k8sClient, + }, 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{ + TenantID: vaultConfig.AzureKeyVault.TenantID, + 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) + } + return &vaultResolver{ + vault: akv, + }, nil + + default: + return nil, fmt.Errorf("unknown vault type %q", vaultConfig.Type) + } +} + +type resolver interface { + appConfig(ctx context.Context, key string) (*appconfig.AppConfig, error) + proxyCredentials(ctx context.Context, key string) (*url.Userinfo, error) +} + +type k8sResolver struct { + namespace string + client client.Client +} + +func (r *k8sResolver) appConfig(ctx context.Context, key string) (*appconfig.AppConfig, error) { + nsName := types.NamespacedName{ + Namespace: r.namespace, + Name: key, + } + secret := new(corev1.Secret) + if err := r.client.Get( + ctx, + nsName, + secret, + ); err != nil { + return nil, fmt.Errorf("failed to get kubernetes secret: %q", nsName.String()) + } + + return appconfig.FromSecret(secret) +} + +func (r *k8sResolver) proxyCredentials(ctx context.Context, key string) (*url.Userinfo, error) { + nsName := types.NamespacedName{Namespace: r.namespace, Name: key} + secret := new(corev1.Secret) + if err := r.client.Get( + ctx, + nsName, + secret, + ); err != nil { + return nil, fmt.Errorf("failed to get kubernetes secret: %q", nsName.String()) + } + + return url.UserPassword( + string(secret.Data["username"]), + string(secret.Data["password"]), + ), nil +} + +type vaultResolver struct { + vault vault.Vault +} + +func (r *vaultResolver) appConfig(ctx context.Context, key string) (*appconfig.AppConfig, error) { + val, err := r.vault.GetSecret(ctx, key) + if err != nil { + return nil, fmt.Errorf("failed to resolve secret: %v", err) + } + + return appconfig.FromJSONString(val) +} + +func (r *vaultResolver) proxyCredentials(ctx context.Context, key string) (*url.Userinfo, error) { + val, err := r.vault.GetSecret(ctx, key) + if err != nil { + return nil, fmt.Errorf("failed to resolve secret: %v", err) + } + + type info struct { + Username string `json:"username"` + Password string `json:"password"` + } + + var i info + if err := json.Unmarshal([]byte(val), &i); err != nil { + return nil, fmt.Errorf("failed to unmarshal info: %v", err) + } + + return url.UserPassword(i.Username, i.Password), nil +} diff --git a/controllers/actions.summerwind.net/autoscaling.go b/controllers/actions.summerwind.net/autoscaling.go index 677041c7..1804bf28 100644 --- a/controllers/actions.summerwind.net/autoscaling.go +++ b/controllers/actions.summerwind.net/autoscaling.go @@ -130,7 +130,7 @@ func (r *HorizontalRunnerAutoscalerReconciler) suggestReplicasByQueuedAndInProgr jobs, resp, err := ghc.Actions.ListWorkflowJobs(context.TODO(), user, repoName, runID, &opt) if err != nil { r.Log.Error(err, "Error listing workflow jobs") - return //err + return // err } allJobs = append(allJobs, jobs.Jobs...) if resp.NextPage == 0 { diff --git a/github/actions/fake/multi_client.go b/github/actions/fake/multi_client.go index e6625102..62d6a0e5 100644 --- a/github/actions/fake/multi_client.go +++ b/github/actions/fake/multi_client.go @@ -3,6 +3,7 @@ package fake import ( "context" + "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1/appconfig" "github.com/actions/actions-runner-controller/github/actions" ) @@ -34,10 +35,6 @@ func NewMultiClient(opts ...MultiClientOption) actions.MultiClient { return f } -func (f *fakeMultiClient) GetClientFor(ctx context.Context, githubConfigURL string, creds actions.ActionsAuth, namespace string, options ...actions.ClientOption) (actions.ActionsService, error) { - return f.defaultClient, f.defaultErr -} - -func (f *fakeMultiClient) GetClientFromSecret(ctx context.Context, githubConfigURL, namespace string, secretData actions.KubernetesSecretData, options ...actions.ClientOption) (actions.ActionsService, error) { +func (f *fakeMultiClient) GetClientFor(ctx context.Context, githubConfigURL string, appConfig *appconfig.AppConfig, namespace string, options ...actions.ClientOption) (actions.ActionsService, error) { return f.defaultClient, f.defaultErr } diff --git a/github/actions/multi_client.go b/github/actions/multi_client.go index f8e16071..b9ed5873 100644 --- a/github/actions/multi_client.go +++ b/github/actions/multi_client.go @@ -3,15 +3,14 @@ package actions import ( "context" "fmt" - "strconv" "sync" + "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1/appconfig" "github.com/go-logr/logr" ) type MultiClient interface { - GetClientFor(ctx context.Context, githubConfigURL string, creds ActionsAuth, namespace string, options ...ClientOption) (ActionsService, error) - GetClientFromSecret(ctx context.Context, githubConfigURL, namespace string, secretData KubernetesSecretData, options ...ClientOption) (ActionsService, error) + GetClientFor(ctx context.Context, githubConfigURL string, appConfig *appconfig.AppConfig, namespace string, options ...ClientOption) (ActionsService, error) } type multiClient struct { @@ -50,15 +49,22 @@ func NewMultiClient(logger logr.Logger) MultiClient { } } -func (m *multiClient) GetClientFor(ctx context.Context, githubConfigURL string, creds ActionsAuth, namespace string, options ...ClientOption) (ActionsService, error) { +func (m *multiClient) GetClientFor(ctx context.Context, githubConfigURL string, appConfig *appconfig.AppConfig, namespace string, options ...ClientOption) (ActionsService, error) { m.logger.Info("retrieve actions client", "githubConfigURL", githubConfigURL, "namespace", namespace) - if creds.Token == "" && creds.AppCreds == nil { - return nil, fmt.Errorf("no credentials provided. either a PAT or GitHub App credentials should be provided") + if err := appConfig.Validate(); err != nil { + return nil, fmt.Errorf("failed to validate app config: %w", err) } - if creds.Token != "" && creds.AppCreds != nil { - return nil, fmt.Errorf("both PAT and GitHub App credentials provided. should only provide one") + var creds ActionsAuth + if len(appConfig.Token) > 0 { + creds.Token = appConfig.Token + } else { + creds.AppCreds = &GitHubAppAuth{ + AppID: appConfig.AppID, + AppInstallationID: appConfig.AppInstallationID, + AppPrivateKey: appConfig.AppPrivateKey, + } } client, err := NewClient( @@ -69,7 +75,7 @@ func (m *multiClient) GetClientFor(ctx context.Context, githubConfigURL string, }, options...)..., ) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to instantiate new client: %w", err) } m.mu.Lock() @@ -94,42 +100,3 @@ func (m *multiClient) GetClientFor(ctx context.Context, githubConfigURL string, return client, nil } - -type KubernetesSecretData map[string][]byte - -func (m *multiClient) GetClientFromSecret(ctx context.Context, githubConfigURL, namespace string, secretData KubernetesSecretData, options ...ClientOption) (ActionsService, error) { - if len(secretData) == 0 { - return nil, fmt.Errorf("must provide secret data with either PAT or GitHub App Auth") - } - - token := string(secretData["github_token"]) - hasToken := len(token) > 0 - - appID := string(secretData["github_app_id"]) - appInstallationID := string(secretData["github_app_installation_id"]) - appPrivateKey := string(secretData["github_app_private_key"]) - hasGitHubAppAuth := len(appID) > 0 && len(appInstallationID) > 0 && len(appPrivateKey) > 0 - - if hasToken && hasGitHubAppAuth { - return nil, fmt.Errorf("must provide secret with only PAT or GitHub App Auth to avoid ambiguity in client behavior") - } - - if !hasToken && !hasGitHubAppAuth { - return nil, fmt.Errorf("neither PAT nor GitHub App Auth credentials provided in secret") - } - - auth := ActionsAuth{} - - if hasToken { - auth.Token = token - return m.GetClientFor(ctx, githubConfigURL, auth, namespace, options...) - } - - parsedAppInstallationID, err := strconv.ParseInt(appInstallationID, 10, 64) - if err != nil { - return nil, err - } - - auth.AppCreds = &GitHubAppAuth{AppID: appID, AppInstallationID: parsedAppInstallationID, AppPrivateKey: appPrivateKey} - return m.GetClientFor(ctx, githubConfigURL, auth, namespace, options...) -} diff --git a/github/actions/multi_client_test.go b/github/actions/multi_client_test.go index 57589857..95a65572 100644 --- a/github/actions/multi_client_test.go +++ b/github/actions/multi_client_test.go @@ -5,6 +5,7 @@ import ( "fmt" "testing" + "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1/appconfig" "github.com/go-logr/logr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -23,10 +24,13 @@ func TestMultiClientCaching(t *testing.T) { defaultNamespace := "default" defaultConfigURL := "https://github.com/org/repo" - defaultCreds := &ActionsAuth{ + defaultCreds := &appconfig.AppConfig{ Token: "token", } - client, err := NewClient(defaultConfigURL, defaultCreds) + defaultAuth := ActionsAuth{ + Token: defaultCreds.Token, + } + client, err := NewClient(defaultConfigURL, &defaultAuth) require.NoError(t, err) multiClient.clients[ActionsClientKey{client.Identifier(), defaultNamespace}] = client @@ -35,7 +39,7 @@ func TestMultiClientCaching(t *testing.T) { cachedClient, err := multiClient.GetClientFor( ctx, defaultConfigURL, - *defaultCreds, + defaultCreds, defaultNamespace, ) require.NoError(t, err) @@ -47,7 +51,7 @@ func TestMultiClientCaching(t *testing.T) { newClient, err := multiClient.GetClientFor( ctx, defaultConfigURL, - *defaultCreds, + defaultCreds, otherNamespace, ) require.NoError(t, err) @@ -63,7 +67,7 @@ func TestMultiClientOptions(t *testing.T) { defaultConfigURL := "https://github.com/org/repo" t.Run("GetClientFor", func(t *testing.T) { - defaultCreds := &ActionsAuth{ + defaultCreds := &appconfig.AppConfig{ Token: "token", } @@ -71,7 +75,7 @@ func TestMultiClientOptions(t *testing.T) { service, err := multiClient.GetClientFor( ctx, defaultConfigURL, - *defaultCreds, + defaultCreds, defaultNamespace, ) service.SetUserAgent(testUserAgent) @@ -83,27 +87,6 @@ func TestMultiClientOptions(t *testing.T) { require.NoError(t, err) assert.Equal(t, testUserAgent.String(), req.Header.Get("User-Agent")) }) - - t.Run("GetClientFromSecret", func(t *testing.T) { - secret := map[string][]byte{ - "github_token": []byte("token"), - } - - multiClient := NewMultiClient(logger) - service, err := multiClient.GetClientFromSecret( - ctx, - defaultConfigURL, - defaultNamespace, - secret, - ) - service.SetUserAgent(testUserAgent) - require.NoError(t, err) - - client := service.(*Client) - req, err := client.NewGitHubAPIRequest(ctx, "GET", "/test", nil) - require.NoError(t, err) - assert.Equal(t, testUserAgent.String(), req.Header.Get("User-Agent")) - }) } func TestCreateJWT(t *testing.T) { diff --git a/go.mod b/go.mod index e8d699f5..5f7fb05d 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,9 @@ module github.com/actions/actions-runner-controller go 1.24.3 require ( + github.com/Azure/azure-sdk-for-go/sdk/azcore v1.17.0 + github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.1 + github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.3.0 github.com/bradleyfalzon/ghinstallation/v2 v2.14.0 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc github.com/evanphx/json-patch v5.9.11+incompatible @@ -39,6 +42,9 @@ require ( require ( filippo.io/edwards25519 v1.1.0 // indirect + github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 // indirect + github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.1.0 // indirect + github.com/AzureAD/microsoft-authentication-library-for-go v1.3.2 // indirect github.com/BurntSushi/toml v1.4.0 // indirect github.com/ProtonMail/go-crypto v1.1.6 // indirect github.com/aws/aws-sdk-go-v2 v1.36.3 // indirect @@ -95,6 +101,7 @@ require ( github.com/go-sql-driver/mysql v1.9.0 // indirect github.com/go-task/slim-sprig/v3 v3.0.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect + github.com/golang-jwt/jwt/v5 v5.2.1 // indirect github.com/golang/protobuf v1.5.4 // indirect github.com/gonvenience/bunt v1.4.0 // indirect github.com/gonvenience/idem v0.0.1 // indirect @@ -121,6 +128,7 @@ require ( github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/klauspost/compress v1.18.0 // indirect + github.com/kylelemons/godebug v1.1.0 // indirect github.com/lucasb-eyer/go-colorful v1.2.0 // indirect github.com/mailru/easyjson v0.9.0 // indirect github.com/mattn/go-ciede2000 v0.0.0-20170301095244-782e8c62fec3 // indirect @@ -134,6 +142,7 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f // indirect + github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/pquerna/otp v1.4.0 // indirect diff --git a/go.sum b/go.sum index f50716a0..9a9cbead 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,22 @@ filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA= filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4= +github.com/Azure/azure-sdk-for-go v51.0.0+incompatible h1:p7blnyJSjJqf5jflHbSGhIhEpXIgIFmYZNg5uwqweso= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.17.0 h1:g0EZJwz7xkXQiZAI5xi9f3WWFYBlX1CPTrR+NDToRkQ= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.17.0/go.mod h1:XCW7KnZet0Opnr7HccfUw1PLc4CjHqpcaxW8DHklNkQ= +github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.1 h1:1mvYtZfWQAnwNah/C+Z+Jb9rQH95LPE2vlmMuWAHJk8= +github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.1/go.mod h1:75I/mXtme1JyWFtz8GocPHVFyH421IBoZErnO16dd0k= +github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.1 h1:Bk5uOhSAenHyR5P61D/NzeQCv+4fEVV8mOkJ82NqpWw= +github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.1/go.mod h1:QZ4pw3or1WPmRBxf0cHd1tknzrT54WPBOQoGutCPvSU= +github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 h1:ywEEhmNahHBihViHepv3xPBn1663uRv2t2q/ESv9seY= +github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0/go.mod h1:iZDifYGJTIgIIkYRNWPENUnqx6bJ2xnSDFI2tjwZNuY= +github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.3.0 h1:WLUIpeyv04H0RCcQHaA4TNoyrQ39Ox7V+re+iaqzTe0= +github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.3.0/go.mod h1:hd8hTTIY3VmUVPRHNH7GVCHO3SHgXkJKZHReby/bnUQ= +github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.1.0 h1:eXnN9kaS8TiDwXjoie3hMRLuwdUBUMW9KRgOqB3mCaw= +github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.1.0/go.mod h1:XIpam8wumeZ5rVMuhdDQLMfIPDf1WO3IzrCRO3e3e3o= +github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1 h1:WJTmL004Abzc5wDB5VtZG2PJk5ndYDgVacGqfirKxjM= +github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1/go.mod h1:tCcJZ0uHAmvjsVYzEFivsRTN00oz5BEsRgQHu5JZ9WE= +github.com/AzureAD/microsoft-authentication-library-for-go v1.3.2 h1:kYRSnvJju5gYVyhkij+RTJ/VR6QIUaCfWeaFm2ycsjQ= +github.com/AzureAD/microsoft-authentication-library-for-go v1.3.2/go.mod h1:wP83P5OoQ5p6ip3ScPr0BAq0BvuPAvacpEuSzyouqAI= github.com/BurntSushi/toml v1.4.0 h1:kuoIxZQy2WRRk1pttg9asf+WVv6tWQuBNVmK8+nqPr0= github.com/BurntSushi/toml v1.4.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= github.com/ProtonMail/go-crypto v1.1.6 h1:ZcV+Ropw6Qn0AX9brlQLAUXfqLBc7Bl+f/DmNxpLfdw= @@ -97,6 +114,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/rVNCu3HqELle0jiPLLBs70cWOduZpkS1E78= +github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc= github.com/emicklei/go-restful/v3 v3.12.2 h1:DhwDP0vY3k8ZzE0RunuJy8GhNpPL6zqLkDf9B/a0/xU= github.com/emicklei/go-restful/v3 v3.12.2/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= github.com/evanphx/json-patch v5.9.11+incompatible h1:ixHHqfcGvxhWkniF1tWxBHA0yb4Z+d1UQi45df52xW8= @@ -134,6 +153,8 @@ github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= github.com/golang-jwt/jwt/v4 v4.5.2 h1:YtQM7lnr8iZ+j5q71MGKkNw9Mn7AjHM68uc9g5fXeUI= github.com/golang-jwt/jwt/v4 v4.5.2/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= +github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk= +github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.4.0-rc.1/go.mod h1:ceaxUfeHdC40wWswd/P6IGgMaK3YpKi5j83Wpe3EHw8= github.com/golang/protobuf v1.4.0-rc.1.0.20200221234624-67d41d38c208/go.mod h1:xKAWHe0F5eneWXFV3EuXVDTCmh+JuBKY0li0aMyXATA= @@ -215,6 +236,8 @@ github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnr github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/kelseyhightower/envconfig v1.4.0 h1:Im6hONhd3pLkfDFsbRgu68RDNkGF1r3dvMUtDTo2cv8= github.com/kelseyhightower/envconfig v1.4.0/go.mod h1:cccZRl6mQpaq41TPp5QxidR+Sa3axMbJDNb//FQX6Gg= +github.com/keybase/go-keychain v0.0.0-20231219164618-57a3676c3af6 h1:IsMZxCuZqKuao2vNdfD82fjjgPLfyHLpR41Z88viRWs= +github.com/keybase/go-keychain v0.0.0-20231219164618-57a3676c3af6/go.mod h1:3VeWNIJaW+O5xpRQbPp0Ybqu1vJd/pm7s2F473HRrkw= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo= @@ -269,6 +292,8 @@ github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7J github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= github.com/onsi/gomega v1.36.3 h1:hID7cr8t3Wp26+cYnfcjR6HpJ00fdogN6dqZ1t6IylU= github.com/onsi/gomega v1.36.3/go.mod h1:8D9+Txp43QWKhM24yyOBEdpkzN8FvJyAwecBgsU4KU0= +github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c h1:+mdjkGKdHQG3305AYmdv1U2eRNDiU2ErMBj1gwrq8eQ= +github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c/go.mod h1:7rwL4CYBLnjLxUqIJNnCWiEdr3bn6IUYi15bNlnbCCU= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -284,6 +309,8 @@ github.com/prometheus/common v0.62.0 h1:xasJaQlnWAeyHdUBeGjXmutelfJHWMRr+Fg4QszZ github.com/prometheus/common v0.62.0/go.mod h1:vyBcEuLSvWos9B1+CyL7JZ2up+uFzXhkqml0W5zIY1I= github.com/prometheus/procfs v0.15.1 h1:YagwOFzUgYfKKHX6Dr+sHT7km/hxC76UB0learggepc= github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoGhij/e3PBqk= +github.com/redis/go-redis/v9 v9.7.0 h1:HhLSs+B6O021gwzl+locl0zEDnyNkxMtf/Z3NNBMa9E= +github.com/redis/go-redis/v9 v9.7.0/go.mod h1:f6zhXITC7JUJIlPEiBOTXxJgPLdZcA93GewI7inzyWw= github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8= github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= @@ -355,6 +382,7 @@ golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210112080510-489259a85091/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.31.0 h1:ioabZlmFYtWhL+TRYpcnNlLwhyxaM9kWTDEmfnprqik= golang.org/x/sys v0.31.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= diff --git a/main.go b/main.go index 61b68a9e..8f0025f9 100644 --- a/main.go +++ b/main.go @@ -274,10 +274,18 @@ func main() { log.WithName("actions-clients"), ) + secretResolver := actionsgithubcom.NewSecretResolver( + mgr.GetClient(), + actionsMultiClient, + ) + rb := actionsgithubcom.ResourceBuilder{ ExcludeLabelPropagationPrefixes: excludeLabelPropagationPrefixes, + SecretResolver: secretResolver, } + log.Info("Resource builder initializing") + if err = (&actionsgithubcom.AutoscalingRunnerSetReconciler{ Client: mgr.GetClient(), Log: log.WithName("AutoscalingRunnerSet").WithValues("version", build.Version), @@ -297,7 +305,6 @@ func main() { Client: mgr.GetClient(), Log: log.WithName("EphemeralRunner").WithValues("version", build.Version), Scheme: mgr.GetScheme(), - ActionsClient: actionsMultiClient, ResourceBuilder: rb, }).SetupWithManager(mgr, actionsgithubcom.WithMaxConcurrentReconciles(opts.RunnerMaxConcurrentReconciles)); err != nil { log.Error(err, "unable to create controller", "controller", "EphemeralRunner") @@ -308,7 +315,6 @@ func main() { Client: mgr.GetClient(), Log: log.WithName("EphemeralRunnerSet").WithValues("version", build.Version), Scheme: mgr.GetScheme(), - ActionsClient: actionsMultiClient, PublishMetrics: metricsAddr != "0", ResourceBuilder: rb, }).SetupWithManager(mgr); err != nil { diff --git a/test_e2e_arc/arc_jobs_test.go b/test_e2e_arc/arc_jobs_test.go index 39682c87..04991e44 100644 --- a/test_e2e_arc/arc_jobs_test.go +++ b/test_e2e_arc/arc_jobs_test.go @@ -126,7 +126,6 @@ func TestARCJobs(t *testing.T) { if !success { t.Fatal("Expected pods count did not match available pods count during job run.") } - }, ) t.Run("Get available pods after job run", func(t *testing.T) { diff --git a/vault/azurekeyvault/azurekeyvault.go b/vault/azurekeyvault/azurekeyvault.go new file mode 100644 index 00000000..1ec06403 --- /dev/null +++ b/vault/azurekeyvault/azurekeyvault.go @@ -0,0 +1,39 @@ +package azurekeyvault + +import ( + "context" + "fmt" + + "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets" +) + +// AzureKeyVault is a struct that holds the Azure Key Vault client. +type AzureKeyVault struct { + client *azsecrets.Client +} + +func New(cfg Config) (*AzureKeyVault, error) { + if err := cfg.Validate(); err != nil { + return nil, fmt.Errorf("failed to validate config: %v", err) + } + + client, err := cfg.Client() + if err != nil { + return nil, fmt.Errorf("failed to create azsecrets client from config: %v", err) + } + + return &AzureKeyVault{client: client}, nil +} + +// GetSecret retrieves a secret from Azure Key Vault. +func (v *AzureKeyVault) GetSecret(ctx context.Context, name string) (string, error) { + secret, err := v.client.GetSecret(ctx, name, "", nil) + if err != nil { + return "", fmt.Errorf("failed to get secret: %w", err) + } + if secret.Value == nil { + return "", fmt.Errorf("secret value is nil") + } + + return *secret.Value, nil +} diff --git a/vault/azurekeyvault/config.go b/vault/azurekeyvault/config.go new file mode 100644 index 00000000..415f5acf --- /dev/null +++ b/vault/azurekeyvault/config.go @@ -0,0 +1,120 @@ +package azurekeyvault + +import ( + "errors" + "fmt" + "net/http" + "net/url" + "os" + "time" + + "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/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 *httpproxy.Config `json:"proxy,omitempty"` +} + +func (c *Config) Validate() error { + if c.TenantID == "" { + return errors.New("tenant_id is not set") + } + if c.ClientID == "" { + return errors.New("client_id is not set") + } + if _, err := url.ParseRequestURI(c.URL); err != nil { + return fmt.Errorf("failed to parse url: %v", err) + } + + if c.CertificatePath == "" { + return errors.New("cert path must be provided") + } + + if _, err := os.Stat(c.CertificatePath); err != nil { + return fmt.Errorf("cert path %q does not exist: %v", c.CertificatePath, 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 +} + +// Client creates a new Azure Key Vault client using the provided configuration. +func (c *Config) Client() (*azsecrets.Client, error) { + return c.certClient() +} + +func (c *Config) certClient() (*azsecrets.Client, error) { + data, err := os.ReadFile(c.CertificatePath) + if err != nil { + return nil, fmt.Errorf("failed to read cert file from path %q: %v", c.CertificatePath, err) + } + + certs, key, err := azidentity.ParseCertificates(data, nil) + if err != nil { + return nil, fmt.Errorf("failed to parse certificates: %w", err) + } + + httpClient, err := c.httpClient() + if err != nil { + return nil, fmt.Errorf("failed to instantiate http client: %v", err) + } + + cred, err := azidentity.NewClientCertificateCredential( + c.TenantID, + c.ClientID, + certs, + key, + &azidentity.ClientCertificateCredentialOptions{ + ClientOptions: policy.ClientOptions{ + Transport: httpClient, + }, + }, + ) + if err != nil { + return nil, fmt.Errorf("failed to create client certificate credential: %v", err) + } + + client, err := azsecrets.NewClient(c.URL, cred, &azsecrets.ClientOptions{ + ClientOptions: policy.ClientOptions{ + Transport: httpClient, + }, + }) + if err != nil { + return nil, fmt.Errorf("failed to instantiate client for azsecrets: %v", err) + } + + return client, nil +} + +func (c *Config) httpClient() (*http.Client, error) { + retryClient := retryablehttp.NewClient() + retryClient.RetryMax = 4 + retryClient.RetryWaitMax = 30 * time.Second + retryClient.HTTPClient.Timeout = 5 * time.Minute + + transport, ok := retryClient.HTTPClient.Transport.(*http.Transport) + if !ok { + return nil, fmt.Errorf("failed to get http transport") + } + if c.Proxy != nil { + transport.Proxy = func(req *http.Request) (*url.URL, error) { + return c.Proxy.ProxyFunc()(req.URL) + } + } + + return retryClient.StandardClient(), nil +} diff --git a/vault/azurekeyvault/config_test.go b/vault/azurekeyvault/config_test.go new file mode 100644 index 00000000..e0f06c21 --- /dev/null +++ b/vault/azurekeyvault/config_test.go @@ -0,0 +1,99 @@ +package azurekeyvault + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "golang.org/x/net/http/httpproxy" +) + +func TestConfigValidate_invalid(t *testing.T) { + tenantID := "tenantID" + clientID := "clientID" + url := "https://example.com" + + cp, err := os.CreateTemp("", "") + require.NoError(t, err) + err = cp.Close() + require.NoError(t, err) + certPath := cp.Name() + + t.Cleanup(func() { + os.Remove(certPath) + }) + + tt := map[string]*Config{ + "empty": {}, + "no tenant id": { + TenantID: "", + ClientID: clientID, + URL: url, + CertificatePath: certPath, + }, + "no client id": { + TenantID: tenantID, + ClientID: "", + URL: url, + CertificatePath: certPath, + }, + "no url": { + TenantID: tenantID, + ClientID: clientID, + URL: "", + CertificatePath: certPath, + }, + "no jwt and no cert path": { + TenantID: tenantID, + ClientID: clientID, + URL: url, + CertificatePath: "", + }, + "invalid proxy": { + TenantID: tenantID, + ClientID: clientID, + URL: url, + CertificatePath: certPath, + Proxy: &httpproxy.Config{}, + }, + } + + for name, cfg := range tt { + t.Run(name, func(t *testing.T) { + err := cfg.Validate() + require.Error(t, err) + }) + } +} + +func TestValidate_valid(t *testing.T) { + tenantID := "tenantID" + clientID := "clientID" + url := "https://example.com" + + certPath, err := filepath.Abs("testdata/server.crt") + require.NoError(t, err) + + tt := map[string]*Config{ + "with cert": { + TenantID: tenantID, + ClientID: clientID, + URL: url, + CertificatePath: certPath, + }, + "without proxy": { + TenantID: tenantID, + ClientID: clientID, + URL: url, + CertificatePath: certPath, + }, + } + + for name, cfg := range tt { + t.Run(name, func(t *testing.T) { + err := cfg.Validate() + require.NoError(t, err) + }) + } +} diff --git a/vault/azurekeyvault/testdata/server.crt b/vault/azurekeyvault/testdata/server.crt new file mode 100644 index 00000000..60930054 --- /dev/null +++ b/vault/azurekeyvault/testdata/server.crt @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDOjCCAiKgAwIBAgIUQr7R8yN5+2and6ucUOPF6oIbD48wDQYJKoZIhvcNAQEL +BQAwFzEVMBMGA1UEAwwMVGVzdCBSb290IENBMB4XDTI1MDIyODEyMDEzMFoXDTI2 +MDcxMzEyMDEzMFowFDESMBAGA1UEAwwJbG9jYWxob3N0MIIBIjANBgkqhkiG9w0B +AQEFAAOCAQ8AMIIBCgKCAQEA4oL2hAPQlDVaNJru5fIstkpoVSuam0vpswC7ciRc +XQRjF3q8kjtIA7+jdySsKJqOLGnybDX3awvRyKMEjq11IfnZLjZc+FzTlA+x4z0h +MHb0GiBFXKNzrExGI9F0KEPtFxcMIqZ119LY2ReexxWkZBQYlgTepaevp71za4c2 +n4Zy1+0iS5+uklZ4ANKMTBGlN76Qgt530VnpNiIeUbiUzY58Vx4q7kFcUv/oSz8p +rbXr+/GGpAjrOc6/JsezRE8YK2po60dvV80TJ2Jt6pduvF7OSQnq/v4mJl1xuXKl +Byo9HLbeu3BuVRWQs2/EwEzx5kX3Ugysl9Bm44K2yKe9/QIDAQABo4GAMH4wHwYD +VR0jBBgwFoAUfd/q0BY4fkVBV3X+HWzXH0toW08wCQYDVR0TBAIwADALBgNVHQ8E +BAMCBaAwEwYDVR0lBAwwCgYIKwYBBQUHAwEwDwYDVR0RBAgwBocEfwAAATAdBgNV +HQ4EFgQUe0rTTfWjho3hgeLTnajTCpddo2MwDQYJKoZIhvcNAQELBQADggEBAIR2 +5zkA7rPnddxCunsz8Jjq3wyhR/KiAFz+RGeFeiXDkF2fWr7QIQ9KbFbv8tpfXR7P +B75bY0sXwutHMB2sZDi92cH5sthNBfp19fI35cxcU4oTPxp4UZJKEiA3Qx8y73CX +NJu1009nPdOJNlIboDGAFdZ5SH6RCh+YcQZ68kjHPWBIpXxLbs9FN3QmpbAvtLh1 +PoPaSy7IjKmxm1u+Lf6tyIn2IiB3MiynaB3OKvbkLCseM/5SZKMk6WKSDWopOCJr +xciPOc+yeLz5I2Omn0uViOIIciqjlgxncWAyNtDgvJcecwqB2cPiIhk6GY0QZ1uM +e7KoqGzWXvWLqJ13a9U= +-----END CERTIFICATE----- diff --git a/vault/vault.go b/vault/vault.go new file mode 100644 index 00000000..6880a93d --- /dev/null +++ b/vault/vault.go @@ -0,0 +1,38 @@ +package vault + +import ( + "context" + "fmt" + + "github.com/actions/actions-runner-controller/vault/azurekeyvault" +) + +// Vault is the interface every vault implementation needs to adhere to +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 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)