diff --git a/.github/workflows/golangci-lint.yaml b/.github/workflows/golangci-lint.yaml new file mode 100644 index 00000000..26d1bdb1 --- /dev/null +++ b/.github/workflows/golangci-lint.yaml @@ -0,0 +1,23 @@ +name: golangci-lint +on: + push: + branches: + - master + pull_request: +permissions: + contents: read + pull-requests: read +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/setup-go@v3 + with: + go-version: 1.19 + - uses: actions/checkout@v3 + - name: golangci-lint + uses: golangci/golangci-lint-action@v3 + with: + only-new-issues: true + version: v1.49.0 diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 00000000..a3957802 --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,17 @@ +run: + timeout: 3m +output: + format: github-actions +linters-settings: + errcheck: + exclude-functions: + - (net/http.ResponseWriter).Write + - (*net/http.Server).Shutdown + - (*github.com/actions-runner-controller/actions-runner-controller/simulator.VisibleRunnerGroups).Add + - (*github.com/actions-runner-controller/actions-runner-controller/testing.Kind).Stop +issues: + exclude-rules: + - path: controllers/suite_test.go + linters: + - staticcheck + text: "SA1019" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c19d350c..49efbab5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -155,3 +155,7 @@ GINKGO_FOCUS='[It] should create a new Runner resource from the specified templa #### Helm Version Bumps In general we ask you not to bump the version in your PR, the maintainers in general manage the publishing of a new chart. + +#### Running linters locally + +To run the `golangci-lint` tool locally, we recommend you use `make lint` to run the tool using a Docker container matching the CI version. \ No newline at end of file diff --git a/Makefile b/Makefile index 1db89f7c..ea0e64b1 100644 --- a/Makefile +++ b/Makefile @@ -51,6 +51,9 @@ endif all: manager +lint: + docker run --rm -v $(PWD):/app -w /app golangci/golangci-lint:v1.49.0 golangci-lint run + GO_TEST_ARGS ?= -short # Run tests diff --git a/api/v1alpha1/runner_types.go b/api/v1alpha1/runner_types.go index 38b4d8e6..bda324b8 100644 --- a/api/v1alpha1/runner_types.go +++ b/api/v1alpha1/runner_types.go @@ -334,11 +334,7 @@ func (r Runner) IsRegisterable() bool { } now := metav1.Now() - if r.Status.Registration.ExpiresAt.Before(&now) { - return false - } - - return true + return !r.Status.Registration.ExpiresAt.Before(&now) } // +kubebuilder:object:root=true diff --git a/controllers/horizontal_runner_autoscaler_batch_scale.go b/controllers/horizontal_runner_autoscaler_batch_scale.go index d89d85cd..f6317ba5 100644 --- a/controllers/horizontal_runner_autoscaler_batch_scale.go +++ b/controllers/horizontal_runner_autoscaler_batch_scale.go @@ -79,7 +79,6 @@ func (s *batchScaler) Add(st *ScaleTarget) { for { select { case <-after: - after = nil break batch case st := <-s.queue: nsName := types.NamespacedName{ diff --git a/controllers/horizontal_runner_autoscaler_webhook.go b/controllers/horizontal_runner_autoscaler_webhook.go index 02ad9855..c500860d 100644 --- a/controllers/horizontal_runner_autoscaler_webhook.go +++ b/controllers/horizontal_runner_autoscaler_webhook.go @@ -20,7 +20,7 @@ import ( "context" "encoding/json" "fmt" - "io/ioutil" + "io" "net/http" "strings" "sync" @@ -75,10 +75,8 @@ type HorizontalRunnerAutoscalerGitHubWebhook struct { // A scale target is enqueued on each retrieval of each eligible webhook event, so that it is processed asynchronously. QueueLimit int - worker *worker - workerInit sync.Once - workerStart sync.Once - batchCh chan *ScaleTarget + worker *worker + workerInit sync.Once } func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) Reconcile(_ context.Context, request reconcile.Request) (reconcile.Result, error) { @@ -133,7 +131,7 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) Handle(w http.Respons return } } else { - payload, err = ioutil.ReadAll(r.Body) + payload, err = io.ReadAll(r.Body) if err != nil { autoscaler.Log.Error(err, "error reading request body") diff --git a/controllers/horizontal_runner_autoscaler_webhook_test.go b/controllers/horizontal_runner_autoscaler_webhook_test.go index 4db5db79..d1a336e6 100644 --- a/controllers/horizontal_runner_autoscaler_webhook_test.go +++ b/controllers/horizontal_runner_autoscaler_webhook_test.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "io" - "io/ioutil" "net/http" "net/http/httptest" "net/url" @@ -504,7 +503,7 @@ func testServerWithInitObjs(t *testing.T, eventType string, event interface{}, w hraWebhook := &HorizontalRunnerAutoscalerGitHubWebhook{} - client := fake.NewFakeClientWithScheme(sc, initObjs...) + client := fake.NewClientBuilder().WithScheme(sc).WithRuntimeObjects(initObjs...).Build() logs := installTestLogger(hraWebhook) @@ -537,7 +536,7 @@ func testServerWithInitObjs(t *testing.T, eventType string, event interface{}, w t.Error("status:", resp.StatusCode) } - respBody, err := ioutil.ReadAll(resp.Body) + respBody, err := io.ReadAll(resp.Body) if err != nil { t.Fatal(err) } @@ -575,7 +574,7 @@ func sendWebhook(server *httptest.Server, eventType string, event interface{}) ( "X-GitHub-Event": {eventType}, "Content-Type": {"application/json"}, }, - Body: ioutil.NopCloser(bytes.NewBuffer(reqBody)), + Body: io.NopCloser(bytes.NewBuffer(reqBody)), } return http.DefaultClient.Do(req) @@ -607,7 +606,7 @@ func (l *testLogSink) Info(_ int, msg string, kvs ...interface{}) { fmt.Fprintf(l.writer, "\n") } -func (_ *testLogSink) Enabled(level int) bool { +func (*testLogSink) Enabled(level int) bool { return true } diff --git a/controllers/metrics/runnerset.go b/controllers/metrics/runnerset.go index ece2b2db..39384559 100644 --- a/controllers/metrics/runnerset.go +++ b/controllers/metrics/runnerset.go @@ -10,12 +10,6 @@ const ( rsNamespace = "namespace" ) -var ( - runnerSetMetrics = []prometheus.Collector{ - runnerSetReplicas, - } -) - var ( runnerSetReplicas = prometheus.NewGaugeVec( prometheus.GaugeOpts{ diff --git a/controllers/multi_githubclient.go b/controllers/multi_githubclient.go index cd873aff..3bcdccdd 100644 --- a/controllers/multi_githubclient.go +++ b/controllers/multi_githubclient.go @@ -151,14 +151,6 @@ func (c *MultiGitHubClient) DeinitForRunnerSet(rs *v1alpha1.RunnerSet) { c.derefClient(rs.Namespace, secretName, refFromRunnerSet(rs)) } -func (c *MultiGitHubClient) deinitClientForRunnerReplicaSet(rs *v1alpha1.RunnerReplicaSet) { - c.derefClient(rs.Namespace, rs.Spec.Template.Spec.GitHubAPICredentialsFrom.SecretRef.Name, refFromRunnerReplicaSet(rs)) -} - -func (c *MultiGitHubClient) deinitClientForRunnerDeployment(rd *v1alpha1.RunnerDeployment) { - c.derefClient(rd.Namespace, rd.Spec.Template.Spec.GitHubAPICredentialsFrom.SecretRef.Name, refFromRunnerDeployment(rd)) -} - func (c *MultiGitHubClient) DeinitForHRA(hra *v1alpha1.HorizontalRunnerAutoscaler) { var secretName string if hra.Spec.GitHubAPICredentialsFrom != nil { @@ -310,22 +302,6 @@ func secretDataToGitHubClientConfig(data map[string][]byte) (*github.Config, err return &conf, nil } -func refFromRunnerDeployment(rd *v1alpha1.RunnerDeployment) *runnerOwnerRef { - return &runnerOwnerRef{ - kind: rd.Kind, - ns: rd.Namespace, - name: rd.Name, - } -} - -func refFromRunnerReplicaSet(rs *v1alpha1.RunnerReplicaSet) *runnerOwnerRef { - return &runnerOwnerRef{ - kind: rs.Kind, - ns: rs.Namespace, - name: rs.Name, - } -} - func refFromRunner(r *v1alpha1.Runner) *runnerOwnerRef { return &runnerOwnerRef{ kind: r.Kind, diff --git a/controllers/runner_controller.go b/controllers/runner_controller.go index 4a2f798d..51052c1c 100644 --- a/controllers/runner_controller.go +++ b/controllers/runner_controller.go @@ -715,7 +715,7 @@ func runnerHookEnvs(pod *corev1.Pod) ([]corev1.EnvVar, error) { }, }, }, - corev1.EnvVar{ + { Name: "ACTIONS_RUNNER_REQUIRE_SAME_NODE", Value: strconv.FormatBool(isRequireSameNode), }, @@ -834,7 +834,7 @@ func newRunnerPodWithContainerMode(containerMode string, template corev1.Pod, ru if dockerdContainer != nil { template.Spec.Containers = append(template.Spec.Containers[:dockerdContainerIndex], template.Spec.Containers[dockerdContainerIndex+1:]...) } - if runnerContainerIndex < runnerContainerIndex { + if dockerdContainerIndex < runnerContainerIndex { runnerContainerIndex-- } dockerdContainer = nil @@ -1226,13 +1226,3 @@ func isRequireSameNode(pod *corev1.Pod) (bool, error) { } return false, nil } - -func overwriteRunnerEnv(runner *v1alpha1.Runner, key string, value string) { - for i := range runner.Spec.Env { - if runner.Spec.Env[i].Name == key { - runner.Spec.Env[i].Value = value - return - } - } - runner.Spec.Env = append(runner.Spec.Env, corev1.EnvVar{Name: key, Value: value}) -} diff --git a/controllers/runner_pod_owner.go b/controllers/runner_pod_owner.go index abc5020a..ebafe22b 100644 --- a/controllers/runner_pod_owner.go +++ b/controllers/runner_pod_owner.go @@ -315,7 +315,7 @@ func syncRunnerPodsOwners(ctx context.Context, c client.Client, log logr.Logger, numOwners := len(owners) var hashes []string - for h, _ := range state.podsForOwners { + for h := range state.podsForOwners { hashes = append(hashes, h) } diff --git a/github/fake/fake.go b/github/fake/fake.go index 1416f597..3606792c 100644 --- a/github/fake/fake.go +++ b/github/fake/fake.go @@ -47,7 +47,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) { status := req.URL.Query().Get("status") if h.Statuses != nil { if body, ok := h.Statuses[status]; ok { - fmt.Fprintf(w, body) + fmt.Fprint(w, body) return } } @@ -69,7 +69,7 @@ func (h *MapHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { w.WriteHeader(404) } else { w.WriteHeader(h.Status) - fmt.Fprintf(w, body) + fmt.Fprint(w, body) } } diff --git a/simulator/runnergroup_visibility.go b/simulator/runnergroup_visibility.go index ee62a821..5a655f6a 100644 --- a/simulator/runnergroup_visibility.go +++ b/simulator/runnergroup_visibility.go @@ -41,18 +41,3 @@ func (c *Simulator) GetRunnerGroupsVisibleToRepository(ctx context.Context, org, return visible, nil } - -func (c *Simulator) hasRepoAccessToOrganizationRunnerGroup(ctx context.Context, org string, runnerGroupId int64, repo string) (bool, error) { - repos, err := c.Client.ListRunnerGroupRepositoryAccesses(ctx, org, runnerGroupId) - if err != nil { - return false, err - } - - for _, githubRepo := range repos { - if githubRepo.GetFullName() == repo { - return true, nil - } - } - - return false, nil -} diff --git a/test/e2e/cmd/main.go b/test/e2e/cmd/main.go index 2c8bb9f7..4a487aa7 100644 --- a/test/e2e/cmd/main.go +++ b/test/e2e/cmd/main.go @@ -56,8 +56,3 @@ func delete(cmName string) ([]byte, error) { cmd := exec.Command("kubectl", "delete", "cm", cmName) return cmd.CombinedOutput() } - -func deleteControllerManagerSecret() ([]byte, error) { - cmd := exec.Command("kubectl", "-n", "actions-runner-system", "delete", "secret", "controller-manager") - return cmd.CombinedOutput() -} diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 4e21edb9..1160947c 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -616,9 +616,6 @@ func (e *env) checkGitHubToken(t *testing.T, tok string) error { return nil } -func (e *env) f() { -} - func (e *env) buildAndLoadImages(t *testing.T) { t.Helper() @@ -1070,7 +1067,7 @@ func verifyActionsWorkflowRun(t *testing.T, env *testing.Env, testJobs []job, ti var expected []string - for _ = range testJobs { + for range testJobs { expected = append(expected, "ok") } diff --git a/testing/kubectl.go b/testing/kubectl.go index fbbc75c6..97c90fab 100644 --- a/testing/kubectl.go +++ b/testing/kubectl.go @@ -124,7 +124,7 @@ func (k *Kubectl) kubectlCmd(ctx context.Context, c string, args []string, cfg K } if cfg.Timeout > 0 { - args = append(args, "--timeout="+fmt.Sprintf("%s", cfg.Timeout)) + args = append(args, fmt.Sprintf("--timeout=%v", cfg.Timeout.String())) } cmd := exec.CommandContext(ctx, "kubectl", args...)