diff --git a/api/v1alpha1/runner_types.go b/api/v1alpha1/runner_types.go index 3a9f2ffe..252b5c39 100644 --- a/api/v1alpha1/runner_types.go +++ b/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"` diff --git a/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml b/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml index 5bca12b6..da31694c 100644 --- a/config/crd/bases/actions.summerwind.dev_runnerdeployments.yaml +++ b/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: diff --git a/config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml b/config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml index 193b1c33..06216a1e 100644 --- a/config/crd/bases/actions.summerwind.dev_runnerreplicasets.yaml +++ b/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: diff --git a/config/crd/bases/actions.summerwind.dev_runners.yaml b/config/crd/bases/actions.summerwind.dev_runners.yaml index 2de67d7d..4450d325 100644 --- a/config/crd/bases/actions.summerwind.dev_runners.yaml +++ b/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: diff --git a/controllers/runner_controller.go b/controllers/runner_controller.go index be6863ed..7cb9fd14 100644 --- a/controllers/runner_controller.go +++ b/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 +} diff --git a/github/github.go b/github/github.go index 6bcd5518..a4de3ac1 100644 --- a/github/github.go +++ b/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 +}