From 8a6d03bbd37546f24aba46a722910e0cd7b56b2f Mon Sep 17 00:00:00 2001 From: Solly Ross Date: Thu, 6 Jun 2019 14:45:10 -0700 Subject: [PATCH 1/2] [WIP] Linters in GH Actions --- .github/main.workflow | 11 ++ .golangci.yml | 30 ++++ go.mod | 3 +- go.sum | 8 +- hack/lint2check/Dockerfile | 8 + hack/lint2check/README.md | 4 + hack/lint2check/cloudbuild.yaml | 11 ++ hack/lint2check/go.mod | 5 + hack/lint2check/go.sum | 12 ++ hack/lint2check/lint2check.go | 261 ++++++++++++++++++++++++++++++++ hack/verify.sh | 20 +-- 11 files changed, 351 insertions(+), 22 deletions(-) create mode 100644 hack/lint2check/Dockerfile create mode 100644 hack/lint2check/README.md create mode 100644 hack/lint2check/cloudbuild.yaml create mode 100644 hack/lint2check/go.mod create mode 100644 hack/lint2check/go.sum create mode 100644 hack/lint2check/lint2check.go diff --git a/.github/main.workflow b/.github/main.workflow index 7439a4f30c..a409aeaa25 100644 --- a/.github/main.workflow +++ b/.github/main.workflow @@ -7,3 +7,14 @@ action "verify-emoji" { uses = "./hack/release" secrets = ["GITHUB_TOKEN"] } + +workflow "Linters and Test" { + on = "push" + resolves = ["lint"] +} + +action "lint" { + uses = "docker://gcr.io/kubebuilder/lint2check" + secrets = ["GITHUB_TOKEN"] + args = ["./pkg/...", "./examples/..."] +} diff --git a/.golangci.yml b/.golangci.yml index a418256a4a..d8081cf2e5 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,5 +1,35 @@ +run: + deadline: 5m linters-settings: lll: line-length: 170 dupl: threshold: 400 +issues: + # don't skip warning about doc comments + exclude-use-default: false + + # restore some of the defaults + # (fill in the rest as needed) + exclude-rules: + - linters: [errcheck] + text: "Error return value of .((os\\.)?std(out|err)\\..*|.*Close|.*Flush|os\\.Remove(All)?|.*printf?|os\\.(Un)?Setenv). is not checked" +linters: + disable-all: true + enable: + - misspell + - structcheck + - golint + - deadcode + - errcheck + - varcheck + - goconst + - unparam + - ineffassign + - nakedret + - interfacer + - gocyclo + - lll + - dupl + - goimports + - golint diff --git a/go.mod b/go.mod index 568ccb60da..55903fa87f 100644 --- a/go.mod +++ b/go.mod @@ -32,12 +32,13 @@ require ( go.uber.org/atomic v1.3.2 // indirect go.uber.org/multierr v1.1.0 // indirect go.uber.org/zap v1.9.1 - golang.org/x/crypto v0.0.0-20180820150726-614d502a4dac // indirect + golang.org/x/net v0.0.0-20190311183353-d8887717615a // indirect golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be // indirect golang.org/x/sync v0.0.0-20190423024810-112230192c58 // indirect golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2 // indirect google.golang.org/appengine v1.1.0 // indirect gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect + gopkg.in/fsnotify.v1 v1.4.7 gopkg.in/inf.v0 v0.9.1 // indirect k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b k8s.io/apiextensions-apiserver v0.0.0-20190409022649-727a075fdec8 diff --git a/go.sum b/go.sum index b5e1bdd0c3..39c61a01ed 100644 --- a/go.sum +++ b/go.sum @@ -77,10 +77,12 @@ go.uber.org/multierr v1.1.0 h1:HoEmRHQPVSqub6w2z2d2EOVs2fjyFRGyofhKuyDq0QI= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= go.uber.org/zap v1.9.1 h1:XCJQEf3W6eZaVwhRBof6ImoYGJSITeKWsyeh3HFu/5o= go.uber.org/zap v1.9.1/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= -golang.org/x/crypto v0.0.0-20180820150726-614d502a4dac h1:7d7lG9fHOLdL6jZPtnV4LpI41SbohIJ1Atq7U991dMg= -golang.org/x/crypto v0.0.0-20180820150726-614d502a4dac/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90PveolxSbWFaJdECFbxSq0Mqo2M= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd h1:nTDtHvHSdCn1m6ITfMRqtOd/9+7a3s8RBNOZ3eYZzJA= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= +golang.org/x/net v0.0.0-20190311183353-d8887717615a h1:oWX7TPOiFAMXLq8o0ikBYfCJVlRHBcsciT5bXOrH628= +golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be h1:vEDujvNQGv4jgYKudGeI/+DAX4Jffq6hpD55MmoEvKs= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -88,6 +90,8 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEha golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e h1:o3PsSEY8E4eXWkXrIP9YJALUkVZqzHJT5DOasTyn8Vs= golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a h1:1BGLXjeY4akVXGgbC9HugT3Jv3hCI0z56oJR5vAMgBU= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2 h1:+DCIGbF/swA92ohVg0//6X2IVY3KZs6p9mix0ziNYJM= diff --git a/hack/lint2check/Dockerfile b/hack/lint2check/Dockerfile new file mode 100644 index 0000000000..28d450709a --- /dev/null +++ b/hack/lint2check/Dockerfile @@ -0,0 +1,8 @@ +FROM golang + +COPY lint2check /usr/bin/lint2check +COPY golangci-lint /usr/bin/golangci-lint +# make sure lint2check can find golangci-lint +ENV GO111MODULE=on + +ENTRYPOINT ["/usr/bin/lint2check"] diff --git a/hack/lint2check/README.md b/hack/lint2check/README.md new file mode 100644 index 0000000000..e419457274 --- /dev/null +++ b/hack/lint2check/README.md @@ -0,0 +1,4 @@ +# lint2check + +lint2check runs golangci-lint, converts the results to github check API +results, and submits them as a separate check run. diff --git a/hack/lint2check/cloudbuild.yaml b/hack/lint2check/cloudbuild.yaml new file mode 100644 index 0000000000..bb9ff3cd10 --- /dev/null +++ b/hack/lint2check/cloudbuild.yaml @@ -0,0 +1,11 @@ +steps: +- name: "golang" + args: ["go", "build", "-o", "lint2check", "."] + env: ["GO111MODULE=on", "CGO_ENABLED=0"] +- name: "golang" + args: ["go", "get", "github.com/golangci/golangci-lint/cmd/golangci-lint@v1.16.0"] + env: ["GO111MODULE=on", "CGO_ENABLED=0", "GOBIN=/workspace"] +- name: "gcr.io/cloud-builders/docker" + args: ["build", "-t", "gcr.io/$PROJECT_ID/lint2check", "."] +images: +- "gcr.io/$PROJECT_ID/lint2check" diff --git a/hack/lint2check/go.mod b/hack/lint2check/go.mod new file mode 100644 index 0000000000..73e842c305 --- /dev/null +++ b/hack/lint2check/go.mod @@ -0,0 +1,5 @@ +module sigs.k8s.io/controller-runtime/hack/lint2check + +go 1.12 + +require github.com/google/go-github/v25 v25.1.1 diff --git a/hack/lint2check/go.sum b/hack/lint2check/go.sum new file mode 100644 index 0000000000..002a3df740 --- /dev/null +++ b/hack/lint2check/go.sum @@ -0,0 +1,12 @@ +github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= +github.com/google/go-github/v25 v25.1.1 h1:6eW++i/CXcR5GKfYaaJT7oJJtHNU+/iiw55noEPNVao= +github.com/google/go-github/v25 v25.1.1/go.mod h1:6z5pC69qHtrPJ0sXPsj4BLnd82b+r6sLB7qcBoRZqpw= +github.com/google/go-querystring v1.0.0 h1:Xkwi/a1rcvNg1PPYe5vI8GbeBY/jrVuDX5ASuANWTrk= +github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= +golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= diff --git a/hack/lint2check/lint2check.go b/hack/lint2check/lint2check.go new file mode 100644 index 0000000000..27cc44538b --- /dev/null +++ b/hack/lint2check/lint2check.go @@ -0,0 +1,261 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "context" + "encoding/json" + "os/exec" + "os" + "log" + "go/token" + "net/http" + "strings" + "time" + "fmt" + + "github.com/google/go-github/v25/github" +) + +type lintResults struct { + Issues []issue + Report reportData +} + +type issue struct { + FromLinter string + Text string + Pos token.Position + + LineRange *lineRange + Replacement *replacement +} + +type replacement struct { + NeedOnlyDelete bool + NewLines []string +} + +type lineRange struct { + From, To int +} + +type reportData struct { + Warnings []warning + Linters []linterData + Error string +} + +type linterData struct { + Name string + Enabled bool +} + +type warning struct { + Tag string + Text string +} + +type bearerTransport struct { + token string +} + +func (b *bearerTransport) RoundTrip(req *http.Request) (*http.Response, error) { + req.Header.Set("Authorization", "Bearer "+b.token) + req.Header.Set("Accept", "application/vnd.github.antiope-preview+json") + + return http.DefaultTransport.RoundTrip(req) +} + +func main() { + if !lintAndSubmit() { + os.Exit(1) + } +} + +func lintAndSubmit() (succeeded bool) { + ctx := context.Background() + token := os.Getenv("GITHUB_TOKEN") + if token == "" { + log.Print("must specify a GitHub token in $GITHUB_TOKEN") + return false + } + client := github.NewClient(&http.Client{Transport: &bearerTransport{ + token: token, + }}) + + headSha := os.Getenv("GITHUB_SHA") + if headSha == "" { + log.Print("must specify a SHA to register the check against in $GITHUB_SHA") + return false + } + + repoRaw := os.Getenv("GITHUB_REPOSITORY") + if repoRaw == "" || !strings.Contains(repoRaw, "/") { + log.Print("must specify a repository in $GITHUB_REPOSITORY as owner/repo") + return false + } + repoParts := strings.SplitN(repoRaw, "/", 2) + repoOwner := repoParts[0] + repoName := repoParts[1] + + defInProgress := "in_progress" + checkRun, _, err := client.Checks.CreateCheckRun(ctx, repoOwner, repoName, github.CreateCheckRunOptions{ + Name: "Linters", + HeadSHA: headSha, + Status: &defInProgress, + StartedAt: &github.Timestamp{Time: time.Now()}, + }) + if err != nil { + log.Print(err) + return false + } + + succeeded = true + + if err := runLints(checkRun); err != nil { + // don't return immediately -- submit things first + log.Print(err) + succeeded = false + } + + defUnknownErr := "**unknown error while linting**" + if checkRun.Output.Title == nil || checkRun.Output.Summary == nil { + checkRun.Output.Title = &defUnknownErr + checkRun.Output.Summary = &defUnknownErr + } + + log.Printf("Sending check run results %+v", checkRun) + _, _, err = client.Checks.UpdateCheckRun(ctx, repoOwner, repoName, *checkRun.ID, github.UpdateCheckRunOptions{ + Name: *checkRun.Name, + Status: checkRun.Status, + Conclusion: checkRun.Conclusion, + CompletedAt: checkRun.CompletedAt, + Output: checkRun.Output, + }) + if err != nil { + log.Print(err) + return false + } + + log.Print("done") + return succeeded +} + +func runLints(checkRun *github.CheckRun) error { + // TODO(directxman12); there's probably a way to run this directly, + // but this is easiest for now + args := append([]string{"run", "--out-format", "json"}, os.Args[1:]...) + lintOutRaw, checkErrRaw := exec.Command("golangci-lint", args...).Output() + // don't return early, since we might get results + if checkErrRaw != nil { + log.Print(checkErrRaw) + if exitErr, isExitErr := checkErrRaw.(*exec.ExitError); isExitErr { + log.Print(string(exitErr.Stderr)) + } + } + + // set the completed time so we have a timestamp in case of error return + checkRun.CompletedAt = &github.Timestamp{Time: time.Now()} + + var lintRes lintResults + if err := json.Unmarshal(lintOutRaw, &lintRes); err != nil { + defFailure := "failure" + checkRun.Conclusion = &defFailure + return err + } + + conclusion := "success" + summary := fmt.Sprintf("%v problems\n\n%v warnings", len(lintRes.Issues), len(lintRes.Report.Warnings)) + switch { + case len(lintRes.Issues) > 0 || checkErrRaw != nil || lintRes.Report.Error != "": + conclusion = "failure" + case len(lintRes.Report.Warnings) > 0: + conclusion = "neutral" + } + checkRun.Conclusion = &conclusion + + if lintRes.Report.Error != "" { + summary += fmt.Sprintf("\n\nError running linters: %s", lintRes.Report.Error) + } + defTitle := "Linter Runs" + checkRun.Output.Title = &defTitle + checkRun.Output.Summary = &summary + + var linterLines []string + for _, linter := range lintRes.Report.Linters { + if !linter.Enabled { + continue + } + linterLines = append(linterLines, "- "+linter.Name) + } + details := fmt.Sprintf("## Enabled Linters\n\n%s\n", strings.Join(linterLines, "\n")) + + if len(lintRes.Report.Warnings) > 0 { + var warningLines []string + for _, warning := range lintRes.Report.Warnings { + warningLines = append(warningLines, fmt.Sprintf("- *%s*: %s", warning.Tag, warning.Text)) + } + details += fmt.Sprintf("## Warnings\n\n%s\n", strings.Join(warningLines, "\n")) + } + + checkRun.Output.Text = &details + + var annotations []*github.CheckRunAnnotation + for i := range lintRes.Issues { + // don't take references to the iteration variable + issue := lintRes.Issues[i] + defFailure := "failure" + issueDetails := "" + + if issue.Replacement != nil { + if issue.Replacement.NeedOnlyDelete { + issueDetails = "\n\n*delete these lines*" + } else { + issueDetails = fmt.Sprintf("\n\n*replace these lines with*:\n\n```go\n%s\n```", strings.Join(issue.Replacement.NewLines, "\n")) + } + } + + msg := issue.Text + msg += issueDetails + + annot := &github.CheckRunAnnotation{ + Path: &issue.Pos.Filename, + AnnotationLevel: &defFailure, + Message: &msg, + Title: &issue.FromLinter, + RawDetails: nil, + } + + if issue.LineRange != nil { + annot.StartLine = &issue.LineRange.From + annot.EndLine = &issue.LineRange.To + } else { + annot.StartLine = &issue.Pos.Line + annot.EndLine = &issue.Pos.Line + // TODO(directxman12): go-github doesn't support columns yet, + // re-add this when they do + // annot.StartColumn = &issue.Pos.Column + // annot.EndColumn = &issue.Pos.Column + } + + annotations = append(annotations, annot) + } + + checkRun.Output.Annotations = annotations + return checkErrRaw +} diff --git a/hack/verify.sh b/hack/verify.sh index 1d8b01a8a8..e85399fcc7 100755 --- a/hack/verify.sh +++ b/hack/verify.sh @@ -24,25 +24,7 @@ go vet ${MOD_OPT} ./... header_text "running golangci-lint" -golangci-lint run --disable-all \ - --deadline 5m \ - --enable=misspell \ - --enable=structcheck \ - --enable=golint \ - --enable=deadcode \ - --enable=errcheck \ - --enable=varcheck \ - --enable=goconst \ - --enable=unparam \ - --enable=ineffassign \ - --enable=nakedret \ - --enable=interfacer \ - --enable=misspell \ - --enable=gocyclo \ - --enable=lll \ - --enable=dupl \ - --enable=goimports \ - ./pkg/... ./examples/... . +golangci-lint run ./pkg/... ./examples/... . # TODO: Enable these as we fix them to make them pass # --enable=gosec \ From 267cdefb475a2dfa3370ae94a70e58fa808396a9 Mon Sep 17 00:00:00 2001 From: Solly Ross Date: Thu, 6 Jun 2019 17:19:01 -0700 Subject: [PATCH 2/2] [tmp] break some lints --- README.md | 1 + hack/lint2check/README.md | 1 + pkg/builder/build.go | 2 +- pkg/cache/multi_namespace_cache.go | 5 ++++- 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0e02f8a9c7..a5d14d632a 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,7 @@ [![Build Status](https://travis-ci.org/kubernetes-sigs/controller-runtime.svg?branch=master)](https://travis-ci.org/kubernetes-sigs/controller-runtime "Travis") [![Go Report Card](https://goreportcard.com/badge/sigs.k8s.io/controller-runtime)](https://goreportcard.com/report/sigs.k8s.io/controller-runtime) + # Kubernetes controller-runtime Project The Kubernetes controller-runtime Project is a set of go libraries for building Controllers. diff --git a/hack/lint2check/README.md b/hack/lint2check/README.md index e419457274..41c32d9d81 100644 --- a/hack/lint2check/README.md +++ b/hack/lint2check/README.md @@ -1,4 +1,5 @@ # lint2check + lint2check runs golangci-lint, converts the results to github check API results, and submits them as a separate check run. diff --git a/pkg/builder/build.go b/pkg/builder/build.go index 487786586b..2d18185898 100644 --- a/pkg/builder/build.go +++ b/pkg/builder/build.go @@ -41,7 +41,7 @@ var newController = controller.New var newManager = manager.New var getGvk = apiutil.GVKForObject -// Builder builds a Controller. +// fiddler builds a Controller. type Builder struct { apiType runtime.Object mgr manager.Manager diff --git a/pkg/cache/multi_namespace_cache.go b/pkg/cache/multi_namespace_cache.go index fedb426536..61ae535034 100644 --- a/pkg/cache/multi_namespace_cache.go +++ b/pkg/cache/multi_namespace_cache.go @@ -52,7 +52,10 @@ func MultiNamespacedCacheBuilder(namespaces []string) NewCacheFunc { } caches[ns] = c } - return &multiNamespaceCache{namespaceToCache: caches, Scheme: opts.Scheme}, nil + return &multiNamespaceCache{ + namespaceToCache: caches, + Scheme: opts.Scheme, + }, nil } }