organization and repository are now exclusive

api/v1alpha1/runner_types.go

@@ -23,12 +23,12 @@ import (
 
 // RunnerSpec defines the desired state of Runner
 type RunnerSpec struct {
-	// +kubebuilder:validation:MinLength=3
+	// +optional
 	// +kubebuilder:validation:Pattern=`^[^/]+$`
-	Organization string `json:"organization"`
+	Organization string `json:"organization,omitempty"`
 
 	// +optional
-	// +kubebuilder:validation:Pattern=`^[^/]*$`
+	// +kubebuilder:validation:Pattern=`^[^/]+/[^/]+$`
 	Repository string `json:"repository,omitempty"`
 
 	// +optional
@@ -85,7 +85,7 @@ type RunnerStatus struct {
 
 // RunnerStatusRegistration contains runner registration status
 type RunnerStatusRegistration struct {
-	Organization string      `json:"organization"`
+	Organization string      `json:"organization,omitempty"`
 	Repository   string      `json:"repository,omitempty"`
 	Labels       []string    `json:"labels,omitempty"`
 	Token        string      `json:"token"`

config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml

@@ -4120,11 +4120,10 @@ spec:
                         type: string
                       type: object
                     organization:
-                      minLength: 3
                       pattern: ^[^/]+$
                       type: string
                     repository:
-                      pattern: ^[^/]*$
+                      pattern: ^[^/]+/[^/]+$
                       type: string
                     resources:
                       description: ResourceRequirements describes the compute resource
@@ -6715,8 +6714,6 @@ spec:
                         - name
                         type: object
                       type: array
-                  required:
-                  - organization
                   type: object
               type: object
           required:

config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml

@@ -4120,11 +4120,10 @@ spec:
                         type: string
                       type: object
                     organization:
-                      minLength: 3
                       pattern: ^[^/]+$
                       type: string
                     repository:
-                      pattern: ^[^/]*$
+                      pattern: ^[^/]+/[^/]+$
                       type: string
                     resources:
                       description: ResourceRequirements describes the compute resource
@@ -6715,8 +6714,6 @@ spec:
                         - name
                         type: object
                       type: array
-                  required:
-                  - organization
                   type: object
               type: object
           required:

config/crd/bases/actions.summerwind.dev_runners.yaml

@@ -3869,11 +3869,10 @@ spec:
                 type: string
               type: object
             organization:
-              minLength: 3
               pattern: ^[^/]+$
               type: string
             repository:
-              pattern: ^[^/]*$
+              pattern: ^[^/]+/[^/]+$
               type: string
             resources:
               description: ResourceRequirements describes the compute resource requirements.
@@ -6305,8 +6304,6 @@ spec:
                 - name
                 type: object
               type: array
-          required:
-          - organization
           type: object
         status:
           description: RunnerStatus defines the observed state of Runner
@@ -6335,7 +6332,6 @@ spec:
                   type: string
               required:
               - expiresAt
-              - organization
               - token
               type: object
           required:

controllers/runner_controller.go

@@ -66,6 +66,12 @@ func (r *RunnerReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
+	err := validateRunnerSpec(&runner.Spec)
+	if err != nil {
+		log.Info("Failed to validate runner spec", "error", err.Error())
+		return ctrl.Result{}, nil
+	}
+
 	if runner.ObjectMeta.DeletionTimestamp.IsZero() {
 		finalizers, added := addFinalizer(runner.ObjectMeta.Finalizers)
 
@@ -439,3 +445,15 @@ func removeFinalizer(finalizers []string) ([]string, bool) {
 
 	return result, removed
 }
+
+// organization & repository are both exclusive - however this cannot be checked with kubebuilder
+// therefore have an additional check here to log an error in case spec is invalid
+func validateRunnerSpec(spec *v1alpha1.RunnerSpec) error {
+	if len(spec.Organization) == 0 && len(spec.Repository) == 0 {
+		return fmt.Errorf("RunnerSpec needs organization or repository")
+	}
+	if len(spec.Organization) > 0 && len(spec.Repository) > 0 {
+		return fmt.Errorf("RunnerSpec cannot have both organization and repository")
+	}
+	return nil
+}

github/github.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"strings"
 	"sync"
 	"time"
 
@@ -47,31 +48,25 @@ func NewClientWithAccessToken(token string) (*Client, error) {
 }
 
 // GetRegistrationToken returns a registration token tied with the name of repository and runner.
-func (c *Client) GetRegistrationToken(ctx context.Context, owner, repo, name string) (*github.RegistrationToken, error) {
+func (c *Client) GetRegistrationToken(ctx context.Context, org, repo, name string) (*github.RegistrationToken, error) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	key := owner
-	if len(repo) > 0 {
-		key = fmt.Sprintf("%s/%s", repo, name)
-	}
-
-	var rt *github.RegistrationToken
-
+	key := getRegistrationKey(org, repo)
 	rt, ok := c.regTokens[key]
+
 	if ok && rt.GetExpiresAt().After(time.Now().Add(-10*time.Minute)) {
 		return rt, nil
 	}
 
-	var res *github.Response
-	var err error
+	owner, repo, err := getOwnerAndRepo(org, repo)
 
-	if len(repo) > 0 {
-		rt, res, err = c.Client.Actions.CreateRegistrationToken(ctx, owner, repo)
-	} else {
-		rt, res, err = CreateOrganizationRegistrationToken(ctx, c, owner)
+	if err != nil {
+		return rt, err
 	}
 
+	rt, res, err := c.createRegistrationToken(ctx, owner, repo)
+
 	if err != nil {
 		return nil, fmt.Errorf("failed to create registration token: %v", err)
 	}
@@ -89,16 +84,15 @@ func (c *Client) GetRegistrationToken(ctx context.Context, owner, repo, name str
 }
 
 // RemoveRunner removes a runner with specified runner ID from repocitory.
-func (c *Client) RemoveRunner(ctx context.Context, owner, repo string, runnerID int64) error {
-	var res *github.Response
-	var err error
+func (c *Client) RemoveRunner(ctx context.Context, org, repo string, runnerID int64) error {
+	owner, repo, err := getOwnerAndRepo(org, repo)
 
-	if len(repo) > 0 {
-		res, err = c.Client.Actions.RemoveRunner(ctx, owner, repo, runnerID)
-	} else {
-		res, err = RemoveOrganizationRunner(ctx, c, owner, runnerID)
+	if err != nil {
+		return err
 	}
 
+	res, err := c.removeRunner(ctx, owner, repo, runnerID)
+
 	if err != nil {
 		return fmt.Errorf("failed to remove runner: %v", err)
 	}
@@ -111,20 +105,18 @@ func (c *Client) RemoveRunner(ctx context.Context, owner, repo string, runnerID
 }
 
 // ListRunners returns a list of runners of specified owner/repository name.
-func (c *Client) ListRunners(ctx context.Context, owner, repo string) ([]*github.Runner, error) {
+func (c *Client) ListRunners(ctx context.Context, org, repo string) ([]*github.Runner, error) {
+	owner, repo, err := getOwnerAndRepo(org, repo)
+
+	if err != nil {
+		return nil, err
+	}
+
 	var runners []*github.Runner
 
 	opts := github.ListOptions{PerPage: 10}
 	for {
-		list := &github.Runners{}
-		var res *github.Response
-		var err error
-
-		if len(repo) > 0 {
-			list, res, err = c.Client.Actions.ListRunners(ctx, owner, repo, &opts)
-		} else {
-			list, res, err = ListOrganizationRunners(ctx, c, owner, &opts)
-		}
+		list, res, err := c.listRunners(ctx, owner, repo, &opts)
 
 		if err != nil {
 			return runners, fmt.Errorf("failed to remove runner: %v", err)
@@ -151,3 +143,57 @@ func (c *Client) cleanup() {
 		}
 	}
 }
+
+// wrappers for github functions (switch between organization/repository mode)
+// so the calling functions don't need to switch and their code is a bit cleaner
+
+func (c *Client) createRegistrationToken(ctx context.Context, owner, repo string) (*github.RegistrationToken, *github.Response, error) {
+	if len(repo) > 0 {
+		return c.Client.Actions.CreateRegistrationToken(ctx, owner, repo)
+	} else {
+		return CreateOrganizationRegistrationToken(ctx, c, owner)
+	}
+}
+
+func (c *Client) removeRunner(ctx context.Context, owner, repo string, runnerID int64) (*github.Response, error) {
+	if len(repo) > 0 {
+		return c.Client.Actions.RemoveRunner(ctx, owner, repo, runnerID)
+	} else {
+		return RemoveOrganizationRunner(ctx, c, owner, runnerID)
+	}
+}
+
+func (c *Client) listRunners(ctx context.Context, owner, repo string, opts *github.ListOptions) (*github.Runners, *github.Response, error) {
+	if len(repo) > 0 {
+		return c.Client.Actions.ListRunners(ctx, owner, repo, opts)
+	} else {
+		return ListOrganizationRunners(ctx, c, owner, opts)
+	}
+}
+
+// Validates owner and repo arguments. Both are optional, but at least one should be specified
+func getOwnerAndRepo(org, repo string) (string, string, error) {
+	if len(repo) > 0 {
+		return splitOwnerAndRepo(repo)
+	}
+	if len(org) > 0 {
+		return org, "", nil
+	}
+	return "", "", fmt.Errorf("organization and repository are both empty")
+}
+
+func getRegistrationKey(org, repo string) string {
+	if len(org) > 0 {
+		return org
+	} else {
+		return repo
+	}
+}
+
+func splitOwnerAndRepo(repo string) (string, string, error) {
+	chunk := strings.Split(repo, "/")
+	if len(chunk) != 2 {
+		return "", "", fmt.Errorf("invalid repository name: '%s'", repo)
+	}
+	return chunk[0], chunk[1], nil
+}