Github-app auth-mode

README.md

@@ -29,6 +29,35 @@ helm install github-actions-runner-operator evryfs-oss/github-actions-runner-ope
 
 Declare a resource like [in the example](config/samples/garo_v1alpha1_githubactionrunner.yaml)
 
+## Authentication modes
+
+The operator's authentication towards GitHub can work in different two modes:
+
+1.  As a [github app](https://docs.github.com/en/free-pro-team@latest/developers/apps/creating-a-github-app).
+
+This is the preferred mode as it provides enhanced security and increased API quota, and avoids exposure of tokens to runner pods. 
+You are advised to install the operator into its own namespace for the same reason.
+
+Follow the guide, no need for defining callback url or webhook secret as they are not in use.
+Give the app read/write permission for [self-hosted runners](https://docs.github.com/en/free-pro-team@latest/rest/reference/permissions-required-for-github-apps#permission-on-self-hosted-runners).
+Deploy the operator with the [environment variables](https://github.com/palantir/go-githubapp/blob/develop/githubapp/config.go#L47) defining the secrets:
+
+````yaml
+env:
+- name: GITHUB_APP_INTEGRATION_ID
+  value: ....
+- name: GITHUB_APP_PRIVATE_KEY
+  value: |
+    -----BEGIN RSA PRIVATE KEY-----
+    .....
+    -----END RSA PRIVATE KEY-----
+````
+
+2.  Using [Personal Access Tokens (PAT)](https://docs.github.com/en/free-pro-team@latest/github/authenticating-to-github/creating-a-personal-access-token)
+
+Define a secret containing the token and refer it from the [custom-resource](config/crd/bases/garo.tietoevry.com_githubactionrunners.yaml#L6311)
+The two modes can be combined, if a PAT is defined on the CR it will take precedence over the github-app auth mode.
+
 ## Weaknesses
 
   * There is a theoretical possibility that a runner pod can be deleted while running a build,

api/v1alpha1/githubactionrunner_types.go

@@ -33,7 +33,8 @@ type GithubActionRunnerSpec struct {
 	// +kubebuilder:validation:Required
 	PodTemplateSpec v1.PodTemplateSpec `json:"podTemplateSpec"`
 
-	// +kubebuilder:validation:Required
+	// PAT to un/register runners. Required if the operator is not running in github-application mode.
+	// +kubebuilder:validation:Optional
 	TokenRef v1.SecretKeySelector `json:"tokenRef"`
 
 	// How often to reconcile/check the runner pool. If undefined the controller uses a default of 1m

codecov.yaml

@@ -8,5 +8,5 @@ coverage:
         threshold: 10%
     patch:
       default:
-        target: 50%
+        target: 10%
         threshold: 10%

config/crd/bases/garo.tietoevry.com_githubactionrunners.yaml

@@ -6309,7 +6309,8 @@ spec:
                 description: Optional Github repository name, if repo scoped.
                 type: string
               tokenRef:
-                description: SecretKeySelector selects a key of a Secret.
+                description: PAT to un/register runners. Required if the operator
+                  is not running in github-application mode.
                 properties:
                   key:
                     description: The key of the secret to select from.  Must be a
@@ -6330,7 +6331,6 @@ spec:
             - minRunners
             - organization
             - podTemplateSpec
-            - tokenRef
             type: object
           status:
             description: GithubActionRunnerStatus defines the observed state of GithubActionRunner

controllers/githubactionrunner_controller.go

@@ -179,13 +179,17 @@ func (r *GithubActionRunnerReconciler) SetupWithManager(mgr ctrl.Manager) error
 		Complete(r)
 }
 
+func (r *GithubActionRunnerReconciler) getRegistrationSecretObjectKey(instance *garov1alpha1.GithubActionRunner) client.ObjectKey {
+	return client.ObjectKey{Namespace: instance.GetNamespace(), Name: fmt.Sprintf("%s-%s", instance.GetName(), regTokenPostfix)}
+}
+
 func (r *GithubActionRunnerReconciler) createOrUpdateRegistrationTokenSecret(ctx context.Context, instance *garov1alpha1.GithubActionRunner) error {
 	logger := logr.FromContext(ctx)
 	secret := &corev1.Secret{
 		TypeMeta:   metav1.TypeMeta{},
 		ObjectMeta: metav1.ObjectMeta{},
 	}
-	err := r.GetClient().Get(ctx, client.ObjectKeyFromObject(instance), secret)
+	err := r.GetClient().Get(ctx, r.getRegistrationSecretObjectKey(instance), secret)
 
 	// not found - create
 	if apierrors.IsNotFound(err) {
@@ -214,8 +218,9 @@ func (r *GithubActionRunnerReconciler) createOrUpdateRegistrationTokenSecret(ctx
 }
 
 func (r *GithubActionRunnerReconciler) updateRegistrationToken(ctx context.Context, instance *garov1alpha1.GithubActionRunner, secret *corev1.Secret) error {
-	secret.GetObjectMeta().SetName(fmt.Sprintf("%s-%s", instance.GetName(), regTokenPostfix))
-	secret.GetObjectMeta().SetNamespace(instance.GetNamespace())
+	objectKey := r.getRegistrationSecretObjectKey(instance)
+	secret.GetObjectMeta().SetName(objectKey.Name)
+	secret.GetObjectMeta().SetNamespace(objectKey.Namespace)
 	apiToken, err := r.tokenForRef(ctx, instance)
 	if err != nil {
 		return err
@@ -317,16 +322,20 @@ func (r *GithubActionRunnerReconciler) listRelatedPods(ctx context.Context, cr *
 func (r *GithubActionRunnerReconciler) unregisterRunners(ctx context.Context, cr *garov1alpha1.GithubActionRunner, list podRunnerPairList) error {
 	for _, item := range list.getPodsBeingDeleted() {
 		if util.HasFinalizer(&item.pod, finalizer) {
-			logr.FromContext(ctx).Info("Unregistering runner", "name", item.runner.GetName(), "id", item.runner.GetID())
-			token, err := r.tokenForRef(ctx, cr)
-			if err != nil {
-				return err
-			}
-			if err = r.GithubAPI.UnregisterRunner(ctx, cr.Spec.Organization, cr.Spec.Repository, token, *item.runner.ID); err != nil {
-				return err
+
+			if item.runner.GetName() != "" && item.runner.GetID() != 0 {
+				logr.FromContext(ctx).Info("Unregistering runner", "name", item.runner.GetName(), "id", item.runner.GetID())
+				token, err := r.tokenForRef(ctx, cr)
+				if err != nil {
+					return err
+				}
+				if err = r.GithubAPI.UnregisterRunner(ctx, cr.Spec.Organization, cr.Spec.Repository, token, *item.runner.ID); err != nil {
+					return err
+				}
 			}
+
 			util.RemoveFinalizer(&item.pod, finalizer)
-			if err = r.GetClient().Update(ctx, &item.pod); err != nil {
+			if err := r.GetClient().Update(ctx, &item.pod); err != nil {
 				return err
 			}
 		}
@@ -338,11 +347,15 @@ func (r *GithubActionRunnerReconciler) unregisterRunners(ctx context.Context, cr
 // tokenForRef returns the token referenced from the GithubActionRunner Spec.TokenRef
 func (r *GithubActionRunnerReconciler) tokenForRef(ctx context.Context, cr *garov1alpha1.GithubActionRunner) (string, error) {
 	var secret corev1.Secret
-	if err := r.GetClient().Get(ctx, client.ObjectKey{Name: cr.Spec.TokenRef.Name, Namespace: cr.Namespace}, &secret); err != nil {
-		return "", err
+	if cr.Spec.TokenRef.Name != "" {
+		if err := r.GetClient().Get(ctx, client.ObjectKey{Name: cr.Spec.TokenRef.Name, Namespace: cr.Namespace}, &secret); err != nil {
+			return "", err
+		}
+
+		return string(secret.Data[cr.Spec.TokenRef.Key]), nil
 	}
 
-	return string(secret.Data[cr.Spec.TokenRef.Key]), nil
+	return "", nil
 }
 
 // getPodRunnerPairs returns a struct podRunnerPairList with pods and runners

controllers/githubapi/runnerapi.go

@@ -3,7 +3,8 @@ package githubapi
 import (
 	"context"
 	"github.com/google/go-github/v33/github"
-	"golang.org/x/oauth2"
+	"github.com/gregjones/httpcache"
+	"github.com/palantir/go-githubapp/githubapp"
 )
 
 //IRunnerAPI is a service towards GitHubs runners
@@ -14,26 +15,53 @@ type IRunnerAPI interface {
 }
 
 type runnerAPI struct {
+	clientCreator githubapp.ClientCreator
 }
 
 //NewRunnerAPI gets a new instance of the API.
-func NewRunnerAPI() runnerAPI {
-	return runnerAPI{}
+func NewRunnerAPI() (runnerAPI, error) {
+	config := githubapp.Config{
+		V3APIURL: "https://api.github.com",
+		V4APIURL: "https://api.github.com",
+	}
+	config.SetValuesFromEnv("")
+
+	clientCreator, err := githubapp.NewDefaultCachingClientCreator(config,
+		githubapp.WithClientUserAgent("evryfs/garo"),
+		githubapp.WithClientCaching(true, func() httpcache.Cache { return httpcache.NewMemoryCache() }),
+	)
+
+	return runnerAPI{
+		clientCreator: clientCreator,
+	}, err
 }
 
-func getClient(ctx context.Context, token string) *github.Client {
-	ts := oauth2.StaticTokenSource(&(oauth2.Token{
-		AccessToken: token,
-	}))
-	tc := oauth2.NewClient(ctx, ts)
-	client := github.NewClient(tc)
+func (r runnerAPI) getClient(ctx context.Context, organization string, token string) (*github.Client, error) {
+	if token != "" {
+		return r.clientCreator.NewTokenClient(token)
+	}
+
+	client, err := r.clientCreator.NewAppClient()
+	if err != nil {
+		return nil, err
+	}
+
+	installationsService := githubapp.NewInstallationsService(client)
+	installation, err := installationsService.GetByOwner(ctx, organization)
+	if err != nil {
+		return nil, err
+	}
 
-	return client
+	return r.clientCreator.NewInstallationClient(installation.ID)
 }
 
 // Return all runners for the org
 func (r runnerAPI) GetRunners(ctx context.Context, organization string, repository string, token string) ([]*github.Runner, error) {
-	client := getClient(ctx, token)
+	client, err := r.getClient(ctx, organization, token)
+	if err != nil {
+		return nil, err
+	}
+
 	var allRunners []*github.Runner
 	opts := &github.ListOptions{PerPage: 30}
 
@@ -61,18 +89,26 @@ func (r runnerAPI) GetRunners(ctx context.Context, organization string, reposito
 }
 
 func (r runnerAPI) UnregisterRunner(ctx context.Context, organization string, repository string, token string, runnerID int64) error {
-	client := getClient(ctx, token)
+	client, err := r.getClient(ctx, organization, token)
+	if err != nil {
+		return err
+	}
+
 	if repository != "" {
 		_, err := client.Actions.RemoveRunner(ctx, organization, repository, runnerID)
 		return err
 	}
-	_, err := client.Actions.RemoveOrganizationRunner(ctx, organization, runnerID)
+	_, err = client.Actions.RemoveOrganizationRunner(ctx, organization, runnerID)
 
 	return err
 }
 
 func (r runnerAPI) CreateRegistrationToken(ctx context.Context, organization string, repository string, token string) (*github.RegistrationToken, error) {
-	client := getClient(ctx, token)
+	client, err := r.getClient(ctx, organization, token)
+	if err != nil {
+		return nil, err
+	}
+
 	if repository != "" {
 		regToken, _, err := client.Actions.CreateRegistrationToken(ctx, organization, repository)
 		return regToken, err

go.mod

@@ -6,13 +6,14 @@ require (
 	github.com/go-logr/logr v0.3.0
 	github.com/google/go-github/v33 v33.0.0
 	github.com/gophercloud/gophercloud v0.15.0
+	github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79
 	github.com/imdario/mergo v0.3.11
 	github.com/onsi/ginkgo v1.14.2
 	github.com/onsi/gomega v1.10.4
+	github.com/palantir/go-githubapp v0.6.0
 	github.com/redhat-cop/operator-utils v1.1.1
 	github.com/stretchr/testify v1.6.1
 	github.com/thoas/go-funk v0.7.0
-	golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d
 	k8s.io/api v0.20.1
 	k8s.io/apimachinery v0.20.1
 	k8s.io/client-go v0.20.1