diff --git a/charts/gha-runner-scale-set/values.yaml b/charts/gha-runner-scale-set/values.yaml index e3f69992..35922f84 100644 --- a/charts/gha-runner-scale-set/values.yaml +++ b/charts/gha-runner-scale-set/values.yaml @@ -17,6 +17,7 @@ githubConfigSecret: ## (Variation B) When using a GitHub App, the syntax is as follows: # githubConfigSecret: # # NOTE: IDs MUST be strings, use quotes +# # The github_app_id can be an app_id or the client_id # github_app_id: "" # github_app_installation_id: "" # github_app_private_key: | diff --git a/cmd/ghalistener/config/config.go b/cmd/ghalistener/config/config.go index b2fa0acd..905249aa 100644 --- a/cmd/ghalistener/config/config.go +++ b/cmd/ghalistener/config/config.go @@ -17,8 +17,9 @@ import ( ) type Config struct { - ConfigureUrl string `json:"configure_url"` - AppID int64 `json:"app_id"` + 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"` @@ -62,26 +63,26 @@ func (c *Config) Validate() error { } if len(c.EphemeralRunnerSetNamespace) == 0 || len(c.EphemeralRunnerSetName) == 0 { - return fmt.Errorf("EphemeralRunnerSetNamespace '%s' or EphemeralRunnerSetName '%s' is missing", c.EphemeralRunnerSetNamespace, c.EphemeralRunnerSetName) + return fmt.Errorf("EphemeralRunnerSetNamespace %q or EphemeralRunnerSetName %q is missing", c.EphemeralRunnerSetNamespace, c.EphemeralRunnerSetName) } if c.RunnerScaleSetId == 0 { - return fmt.Errorf("RunnerScaleSetId '%d' is missing", c.RunnerScaleSetId) + return fmt.Errorf(`RunnerScaleSetId "%d" is missing`, c.RunnerScaleSetId) } if c.MaxRunners < c.MinRunners { - return fmt.Errorf("MinRunners '%d' cannot be greater than MaxRunners '%d'", c.MinRunners, c.MaxRunners) + return fmt.Errorf(`MinRunners "%d" cannot be greater than MaxRunners "%d"`, c.MinRunners, c.MaxRunners) } hasToken := len(c.Token) > 0 - hasPrivateKeyConfig := c.AppID > 0 && c.AppPrivateKey != "" + hasPrivateKeyConfig := len(c.AppID) > 0 && c.AppPrivateKey != "" if !hasToken && !hasPrivateKeyConfig { - return fmt.Errorf("GitHub auth credential is missing, token length: '%d', appId: '%d', installationId: '%d', private key length: '%d", len(c.Token), c.AppID, c.AppInstallationID, len(c.AppPrivateKey)) + 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 hasToken && hasPrivateKeyConfig { - return fmt.Errorf("only one GitHub auth method supported at a time. Have both PAT and App auth: token length: '%d', appId: '%d', installationId: '%d', private key length: '%d", len(c.Token), c.AppID, c.AppInstallationID, len(c.AppPrivateKey)) + 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)) } return nil diff --git a/cmd/ghalistener/config/config_test.go b/cmd/ghalistener/config/config_test.go index fba4f17c..f62b4b73 100644 --- a/cmd/ghalistener/config/config_test.go +++ b/cmd/ghalistener/config/config_test.go @@ -18,7 +18,7 @@ func TestConfigValidationMinMax(t *testing.T) { Token: "token", } err := config.Validate() - assert.ErrorContains(t, err, "MinRunners '5' cannot be greater than MaxRunners '2", "Expected error about MinRunners > MaxRunners") + assert.ErrorContains(t, err, `MinRunners "5" cannot be greater than MaxRunners "2"`, "Expected error about MinRunners > MaxRunners") } func TestConfigValidationMissingToken(t *testing.T) { @@ -29,27 +29,47 @@ func TestConfigValidationMissingToken(t *testing.T) { RunnerScaleSetId: 1, } err := config.Validate() - expectedError := fmt.Sprintf("GitHub auth credential is missing, token length: '%d', appId: '%d', installationId: '%d', private key length: '%d", len(config.Token), config.AppID, config.AppInstallationID, len(config.AppPrivateKey)) + 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)) assert.ErrorContains(t, err, expectedError, "Expected error about missing auth") } func TestConfigValidationAppKey(t *testing.T) { - config := &Config{ - 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: '%d', installationId: '%d', private key length: '%d", len(config.Token), config.AppID, config.AppInstallationID, len(config.AppPrivateKey)) - assert.ErrorContains(t, err, expectedError, "Expected error about missing auth") + t.Parallel() + + t.Run("app id integer", func(t *testing.T) { + t.Parallel() + config := &Config{ + 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)) + 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, + 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)) + assert.ErrorContains(t, err, expectedError, "Expected error about missing auth") + }) } func TestConfigValidationOnlyOneTypeOfCredentials(t *testing.T) { config := &Config{ - AppID: 1, + AppID: "1", AppInstallationID: 10, AppPrivateKey: "asdf", Token: "asdf", @@ -59,7 +79,7 @@ func TestConfigValidationOnlyOneTypeOfCredentials(t *testing.T) { 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: '%d', installationId: '%d', private key length: '%d", len(config.Token), config.AppID, config.AppInstallationID, len(config.AppPrivateKey)) + 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)) assert.ErrorContains(t, err, expectedError, "Expected error about missing auth") } diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index 0726b82d..7fe8febf 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/controllers/actions.github.com/resourcebuilder.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + "maps" "math" "net" "strconv" @@ -169,15 +170,6 @@ func (b *ResourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha metricsEndpoint = metricsConfig.endpoint } - var appID int64 - if id, ok := secret.Data["github_app_id"]; ok { - var err error - appID, err = strconv.ParseInt(string(id), 10, 64) - if err != nil { - return nil, fmt.Errorf("failed to convert github_app_id to int: %v", err) - } - } - var appInstallationID int64 if id, ok := secret.Data["github_app_installation_id"]; ok { var err error @@ -189,7 +181,7 @@ func (b *ResourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha config := listenerconfig.Config{ ConfigureUrl: autoscalingListener.Spec.GitHubConfigUrl, - AppID: appID, + AppID: string(secret.Data["github_app_id"]), AppInstallationID: appInstallationID, AppPrivateKey: string(secret.Data["github_app_private_key"]), Token: string(secret.Data["github_token"]), @@ -207,6 +199,10 @@ func (b *ResourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha Metrics: autoscalingListener.Spec.Metrics, } + if err := config.Validate(); err != nil { + return nil, fmt.Errorf("invalid listener config: %w", err) + } + var buf bytes.Buffer if err := json.NewEncoder(&buf).Encode(config); err != nil { return nil, fmt.Errorf("failed to encode config: %w", err) @@ -278,9 +274,7 @@ func (b *ResourceBuilder) newScaleSetListenerPod(autoscalingListener *v1alpha1.A } labels := make(map[string]string, len(autoscalingListener.Labels)) - for key, val := range autoscalingListener.Labels { - labels[key] = val - } + maps.Copy(labels, autoscalingListener.Labels) newRunnerScaleSetListenerPod := &corev1.Pod{ TypeMeta: metav1.TypeMeta{ diff --git a/github/actions/client.go b/github/actions/client.go index 607551c0..9f6f8886 100644 --- a/github/actions/client.go +++ b/github/actions/client.go @@ -1212,7 +1212,7 @@ func createJWTForGitHubApp(appAuth *GitHubAppAuth) (string, error) { claims := &jwt.RegisteredClaims{ IssuedAt: jwt.NewNumericDate(issuedAt), ExpiresAt: jwt.NewNumericDate(expiresAt), - Issuer: strconv.FormatInt(appAuth.AppID, 10), + Issuer: appAuth.AppID, } token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) diff --git a/github/actions/identifier_test.go b/github/actions/identifier_test.go index 5604d894..528e0521 100644 --- a/github/actions/identifier_test.go +++ b/github/actions/identifier_test.go @@ -57,7 +57,7 @@ func TestClient_Identifier(t *testing.T) { } defaultAppCreds := &actions.ActionsAuth{ AppCreds: &actions.GitHubAppAuth{ - AppID: 123, + AppID: "123", AppInstallationID: 123, AppPrivateKey: "private key", }, @@ -90,7 +90,7 @@ func TestClient_Identifier(t *testing.T) { old: defaultAppCreds, new: &actions.ActionsAuth{ AppCreds: &actions.GitHubAppAuth{ - AppID: 456, + AppID: "456", AppInstallationID: 456, AppPrivateKey: "new private key", }, diff --git a/github/actions/multi_client.go b/github/actions/multi_client.go index 01cb7abf..f8e16071 100644 --- a/github/actions/multi_client.go +++ b/github/actions/multi_client.go @@ -23,7 +23,8 @@ type multiClient struct { } type GitHubAppAuth struct { - AppID int64 + // AppID is the ID or the Client ID of the application + AppID string AppInstallationID int64 AppPrivateKey string } @@ -124,16 +125,11 @@ func (m *multiClient) GetClientFromSecret(ctx context.Context, githubConfigURL, return m.GetClientFor(ctx, githubConfigURL, auth, namespace, options...) } - parsedAppID, err := strconv.ParseInt(appID, 10, 64) - if err != nil { - return nil, err - } - parsedAppInstallationID, err := strconv.ParseInt(appInstallationID, 10, 64) if err != nil { return nil, err } - auth.AppCreds = &GitHubAppAuth{AppID: parsedAppID, AppInstallationID: parsedAppInstallationID, AppPrivateKey: appPrivateKey} + 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 665df7ad..57589857 100644 --- a/github/actions/multi_client_test.go +++ b/github/actions/multi_client_test.go @@ -137,7 +137,7 @@ etFcaQuTHEZyRhhJ4BU= -----END PRIVATE KEY-----` auth := &GitHubAppAuth{ - AppID: 123, + AppID: "123", AppPrivateKey: key, } jwt, err := createJWTForGitHubApp(auth)