feat: new linting system.

This commit is contained in:
Ludovic Fernandez 2019-03-04 16:40:05 +01:00 committed by Traefiker Bot
parent fb617044e0
commit ebded2cbc0
24 changed files with 129 additions and 256 deletions

View file

@ -1,35 +1,19 @@
[run]
deadline = "5m"
deadline = "10m"
skip-files = [
"^old/.*",
"^acme/.*",
"^cluster/.*",
"^cmd/convert/.*",
"^h2c/.*",
"^integration/.*",
# "^cmd/traefik/.*",
# "^anonymize/.*",
# "^provider/.*",
# "^tracing/.*",
# "^safe/.*",
# "^h2c/.*",
# "^healthcheck/.*",
# "^middlewares/.*",
# "^server/.*",
]
[linters-settings]
[linters-settings.govet]
check-shadowing = true
check-shadowing = false
[linters-settings.golint]
min-confidence = 0.0
[linters-settings.gocyclo]
min-complexity = 22.0
min-complexity = 14.0
[linters-settings.maligned]
suggest-new = true
@ -44,25 +28,63 @@
[linters]
enable-all = true
disable = [
"gocyclo", # FIXME must be fixed
"gosec",
"dupl",
"maligned",
"lll",
"gas",
"dupl",
"unparam",
"prealloc",
"scopelint",
"gochecknoinits",
"gochecknoglobals",
"unparam",
"scopelint",
"goimports",
]
[issues]
exclude-use-default = false
max-per-linter = 0
max-same = 0
max-same-issues = 0
exclude = [
"field `(foo|fuu)` is unused",
"(.+) is deprecated:",
"cyclomatic complexity (\\d+) of func `\\(\\*Builder\\)\\.buildConstructor` is high", #alt/server/middleware/middlewares.go
"`logger` can be `github.com/containous/traefik/vendor/github.com/stretchr/testify/assert.TestingT`", # alt/middlewares/recovery/recovery.go:
"`fn` can be `net/http.Handler`", # alt/server/alice/chain.go
"SA1019: http.CloseNotifier is deprecated: the CloseNotifier interface predates Go's context package. New code should use Request.Context instead.", # FIXME must be fixed
"Error return value of .((os\\.)?std(out|err)\\..*|.*Close|.*Flush|os\\.Remove(All)?|.*printf?|os\\.(Un)?Setenv). is not checked",
"should have a package comment, unless it's in another file for this package",
]
[[issues.exclude-rules]]
path = ".+_test.go"
linters = ["goconst"]
[[issues.exclude-rules]]
path = "provider/label/internal/.+_test.go"
text = "U1000: field `(foo|fuu)` is unused"
[[issues.exclude-rules]]
path = "middlewares/recovery/recovery.go"
text = "`logger` can be `github.com/containous/traefik/vendor/github.com/stretchr/testify/assert.TestingT`"
[[issues.exclude-rules]]
path = "integration/.+_test.go"
text = "Error return value of `cmd\\.Process\\.Kill` is not checked"
[[issues.exclude-rules]]
path = "integration/(consul_catalog_test|constraint_test).go"
text = "Error return value of `(s.deregisterService|s.deregisterAgentService)` is not checked"
[[issues.exclude-rules]]
path = "integration/grpc_test.go"
text = "Error return value of `closer` is not checked"
[[issues.exclude-rules]]
path = "provider/kubernetes/builder_(endpoint|service)_test.go"
text = "(U1000: func )?`(.+)` is unused"
[[issues.exclude-rules]]
path = "provider/docker/builder_test.go"
text = "(U1000: func )?`(.+)` is unused"
[[issues.exclude-rules]]
path = "cmd/configuration.go"
text = "string `traefik` has (\\d) occurrences, make it a constant"
[[issues.exclude-rules]]
path = "h2c/h2c.go"
text = "Error return value of `rw.Write` is not checked"
[[issues.exclude-rules]]
path = "server/service/bufferpool.go"
text = "SA6002: argument should be pointer-like to avoid allocations"
[[issues.exclude-rules]] # FIXME must be fixed
path = "acme/.+.go"
text = "(assignment copies lock value to domainsCerts|literal copies lock value from)"
[[issues.exclude-rules]] # FIXME must be fixed
path = "cmd/context.go"
text = "S1000: should use a simple channel send/receive instead of `select` with a single case"

View file

@ -1,42 +0,0 @@
{
"Vendor": true,
"Sort": [
"path",
"line",
"column",
"severity",
"linter"
],
"Test": true,
"Cyclo": 15,
"Enable": [
"gotypex",
"nakedret",
"vet",
"goimports",
"golint",
"ineffassign",
"gotype",
"misspell",
"structcheck",
"gosimple",
"unconvert",
"varcheck",
"errcheck",
"unused",
"deadcode",
"staticcheck"
],
"Disable": [
"gas",
"maligned",
"interfacer",
"goconst",
"gocyclo",
"vetshadow"
],
"Exclude": [
"autogen/.*"
],
"Deadline": "5m"
}

View file

@ -1,10 +0,0 @@
- repo: git://github.com/pre-commit/pre-commit-hooks
sha: 44e1753f98b0da305332abe26856c3e621c5c439
hooks:
- id: detect-private-key
- repo: git://github.com/containous/pre-commit-hooks
sha: 35e641b5107671e94102b0ce909648559e568d61
hooks:
- id: goFmt
- id: goLint
- id: goErrcheck

View file

@ -69,7 +69,7 @@ test-integration: build ## run the integration tests
TEST_HOST=1 ./script/make.sh test-integration
validate: build ## validate code, vendor and autogen
$(DOCKER_RUN_TRAEFIK) ./script/make.sh validate-gofmt validate-golint validate-misspell validate-vendor validate-autogen
$(DOCKER_RUN_TRAEFIK) ./script/make.sh generate validate-lint validate-misspell validate-vendor validate-autogen
build: dist
docker build $(DOCKER_BUILD_ARGS) -t "$(TRAEFIK_DEV_IMAGE)" -f build.Dockerfile .
@ -114,7 +114,7 @@ generate-webui: build-webui
fi
lint:
script/validate-golint
script/validate-lint
fmt:
gofmt -s -l -w $(SRCS)

View file

@ -103,8 +103,10 @@ func (a *ACME) AddRoutes(router *mux.Router) {
}
tokenValue := a.challengeHTTPProvider.getTokenValue(token, domain)
if len(tokenValue) > 0 {
rw.WriteHeader(http.StatusOK)
rw.Write(tokenValue)
_, err := rw.Write(tokenValue)
if err != nil {
http.Error(rw, err.Error(), http.StatusInternalServerError)
}
return
}
}
@ -131,7 +133,11 @@ func (a *ACME) CreateClusterConfig(leadership *cluster.Leadership, tlsConfig *tl
listener := func(object cluster.Object) error {
account := object.(*Account)
account.Init()
err := account.Init()
if err != nil {
return err
}
if !leadership.IsLeader() {
a.client, err = a.buildACMEClient(account)
if err != nil {
@ -187,7 +193,11 @@ func (a *ACME) leadershipListener(elected bool) error {
}
account := object.(*Account)
account.Init()
err = account.Init()
if err != nil {
return err
}
// Reset Account values if caServer changed, thus registration URI can be updated
if account != nil && account.Registration != nil && !isAccountMatchingCaServer(account.Registration.URI, a.CAServer) {
log.Info("Account URI does not match the current CAServer. The account will be reset")

View file

@ -15,6 +15,7 @@ import (
"github.com/containous/traefik/tls/generate"
"github.com/containous/traefik/types"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestDomainsSet(t *testing.T) {
@ -58,7 +59,7 @@ func TestDomainsSet(t *testing.T) {
t.Parallel()
domains := types.Domains{}
domains.Set(test.input)
_ = domains.Set(test.input)
assert.Exactly(t, test.expected, domains)
})
}
@ -110,7 +111,7 @@ func TestDomainsSetAppend(t *testing.T) {
for _, test := range testCases {
t.Run(test.input, func(t *testing.T) {
domains.Set(test.input)
_ = domains.Set(test.input)
assert.Exactly(t, test.expected, domains)
})
}
@ -237,7 +238,9 @@ func TestRemoveDuplicates(t *testing.T) {
},
},
}
domainsCertificates.Init()
err := domainsCertificates.Init()
require.NoError(t, err)
if len(domainsCertificates.Certs) != 2 {
t.Errorf("Expected domainsCertificates length %d %+v\nGot %+v", 2, domainsCertificates.Certs, len(domainsCertificates.Certs))
@ -790,7 +793,8 @@ func TestRemoveEmptyCertificates(t *testing.T) {
t.Parallel()
a := &Account{DomainsCertificate: *test.dc}
a.Init()
err := a.Init()
require.NoError(t, err)
assert.Equal(t, len(test.expectedDc.Certs), len(a.DomainsCertificate.Certs))
sort.Sort(&a.DomainsCertificate)

View file

@ -88,9 +88,7 @@ func (c *challengeHTTPProvider) CleanUp(domain, token, keyAuth string) error {
account := object.(*Account)
if _, ok := account.HTTPChallenge[token]; ok {
if _, domainOk := account.HTTPChallenge[token][domain]; domainOk {
delete(account.HTTPChallenge[token], domain)
}
delete(account.HTTPChallenge[token], domain)
if len(account.HTTPChallenge[token]) == 0 {
delete(account.HTTPChallenge, token)
}

View file

@ -37,7 +37,11 @@ func (c *challengeTLSProvider) getCertificate(domain string) (cert *tls.Certific
return nil, false
}
account.Init()
err := account.Init()
if err != nil {
log.Errorf("Unable to init ACME Account: %v", err)
return nil, false
}
var result *tls.Certificate
operation := func() error {
@ -58,7 +62,7 @@ func (c *challengeTLSProvider) getCertificate(domain string) (cert *tls.Certific
ebo := backoff.NewExponentialBackOff()
ebo.MaxElapsedTime = 60 * time.Second
err := backoff.RetryNotify(safe.OperationWithRecover(operation), ebo, notify)
err = backoff.RetryNotify(safe.OperationWithRecover(operation), ebo, notify)
if err != nil {
log.Errorf("Error getting cert: %v", err)
return nil, false

View file

@ -21,7 +21,8 @@ func TestGet(t *testing.T) {
fileContent, err := ioutil.ReadFile(acmeFile)
assert.NoError(t, err)
tmpFile.Write(fileContent)
_, err = tmpFile.Write(fileContent)
assert.NoError(t, err)
localStore := NewLocalStore(tmpFile.Name())
account, err := localStore.Get()

View file

@ -4,8 +4,8 @@ RUN apk --update upgrade \
&& apk --no-cache --no-progress add git mercurial bash gcc musl-dev curl tar \
&& rm -rf /var/cache/apk/*
RUN go get golang.org/x/lint/golint \
&& go get github.com/client9/misspell/cmd/misspell
RUN curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | bash -s -- -b $GOPATH/bin v1.15.0 \
&& go get github.com/client9/misspell/cmd/misspell
# Which docker version to test on
ARG DOCKER_VERSION=17.03.2

View file

@ -39,28 +39,34 @@ func main() {
log.SetOutput(os.Stdout)
log.SetLevel(logrus.DebugLevel)
oldconfig := &types.Configuration{}
toml.Decode(oldvalue, oldconfig)
oldConfig := &types.Configuration{}
_, err := toml.Decode(oldvalue, oldConfig)
if err != nil {
log.Fatal(err)
}
newconfig := config.Configuration{
newConfig := config.Configuration{
Routers: make(map[string]*config.Router),
Middlewares: make(map[string]*config.Middleware),
Services: make(map[string]*config.Service),
}
for frontendName, frontend := range oldconfig.Frontends {
newconfig.Routers[replaceFrontend(frontendName)] = convertFrontend(frontend)
for frontendName, frontend := range oldConfig.Frontends {
newConfig.Routers[replaceFrontend(frontendName)] = convertFrontend(frontend)
if frontend.PassHostHeader {
log.Warn("ignore PassHostHeader")
}
}
for backendName, backend := range oldconfig.Backends {
newconfig.Services[replaceBackend(backendName)] = convertBackend(backend)
for backendName, backend := range oldConfig.Backends {
newConfig.Services[replaceBackend(backendName)] = convertBackend(backend)
}
encoder := toml.NewEncoder(os.Stdout)
encoder.Encode(newconfig)
err = encoder.Encode(newConfig)
if err != nil {
log.Fatal(err)
}
}
func replaceBackend(name string) string {
@ -139,7 +145,6 @@ func convertBackend(backend *types.Backend) *config.Service {
Headers: backend.HealthCheck.Headers,
}
}
}
return service

View file

@ -16,7 +16,6 @@ import (
"github.com/containous/staert"
"github.com/containous/traefik/cluster"
"github.com/containous/traefik/integration/try"
"github.com/containous/traefik/old/types"
"github.com/go-check/check"
checker "github.com/vdemeester/shakers"
)
@ -49,39 +48,6 @@ func (s *ConsulSuite) setupConsul(c *check.C) {
c.Assert(err, checker.IsNil)
}
func (s *ConsulSuite) setupConsulTLS(c *check.C) {
s.createComposeProject(c, "consul_tls")
s.composeProject.Start(c)
consul.Register()
clientTLS := &types.ClientTLS{
CA: "resources/tls/ca.cert",
Cert: "resources/tls/consul.cert",
Key: "resources/tls/consul.key",
InsecureSkipVerify: true,
}
TLSConfig, err := clientTLS.CreateTLSConfig()
c.Assert(err, checker.IsNil)
kv, err := valkeyrie.NewStore(
store.CONSUL,
[]string{s.composeProject.Container(c, "consul").NetworkSettings.IPAddress + ":8585"},
&store.Config{
ConnectionTimeout: 10 * time.Second,
TLS: TLSConfig,
},
)
if err != nil {
c.Fatal("Cannot create store consul")
}
s.kv = kv
// wait for consul
err = try.Do(60*time.Second, try.KVExists(kv, "test"))
c.Assert(err, checker.IsNil)
}
func (s *ConsulSuite) TearDownTest(c *check.C) {
// shutdown and delete compose project
if s.composeProject != nil {

View file

@ -85,7 +85,7 @@ func (s *Etcd3Suite) SetUpSuite(c *check.C) {
func (s *Etcd3Suite) TearDownTest(c *check.C) {
// Delete all Traefik keys from ETCD
s.kv.DeleteTree("/traefik")
_ = s.kv.DeleteTree("/traefik")
}
func (s *Etcd3Suite) TearDownSuite(c *check.C) {

View file

@ -126,7 +126,7 @@ func Do(timeout time.Duration, operation DoCondition) error {
}
}
func doTryGet(url string, timeout time.Duration, transport *http.Transport, conditions ...ResponseCondition) (*http.Response, error) {
func doTryGet(url string, timeout time.Duration, transport http.RoundTripper, conditions ...ResponseCondition) (*http.Response, error) {
req, err := http.NewRequest(http.MethodGet, url, nil)
if err != nil {
return nil, err
@ -135,11 +135,11 @@ func doTryGet(url string, timeout time.Duration, transport *http.Transport, cond
return doTryRequest(req, timeout, transport, conditions...)
}
func doTryRequest(request *http.Request, timeout time.Duration, transport *http.Transport, conditions ...ResponseCondition) (*http.Response, error) {
func doTryRequest(request *http.Request, timeout time.Duration, transport http.RoundTripper, conditions ...ResponseCondition) (*http.Response, error) {
return doRequest(Do, timeout, request, transport, conditions...)
}
func doRequest(action timedAction, timeout time.Duration, request *http.Request, transport *http.Transport, conditions ...ResponseCondition) (*http.Response, error) {
func doRequest(action timedAction, timeout time.Duration, request *http.Request, transport http.RoundTripper, conditions ...ResponseCondition) (*http.Response, error) {
var resp *http.Response
return resp, action(timeout, func() error {
var err error

View file

@ -6,11 +6,13 @@ import (
"crypto/x509/pkix"
"encoding/pem"
"fmt"
"io"
"net/http"
"net/url"
"strings"
"github.com/containous/traefik/config"
"github.com/containous/traefik/log"
"github.com/containous/traefik/middlewares"
"github.com/containous/traefik/tracing"
"github.com/opentracing/opentracing-go/ext"
@ -152,15 +154,18 @@ func getDNInfo(prefix string, options *DistinguishedNameOptions, cs *pkix.Name)
return ""
}
func writeParts(content *strings.Builder, entries []string, prefix string) {
func writeParts(content io.StringWriter, entries []string, prefix string) {
for _, entry := range entries {
writePart(content, entry, prefix)
}
}
func writePart(content *strings.Builder, entry string, prefix string) {
func writePart(content io.StringWriter, entry string, prefix string) {
if len(entry) > 0 {
content.WriteString(fmt.Sprintf("%s=%s,", prefix, entry))
_, err := content.WriteString(fmt.Sprintf("%s=%s,", prefix, entry))
if err != nil {
log.Error(err)
}
}
}

View file

@ -25,7 +25,7 @@ type redirect struct {
}
// New creates a Redirect middleware.
func newRedirect(ctx context.Context, next http.Handler, regex string, replacement string, permanent bool, name string) (http.Handler, error) {
func newRedirect(_ context.Context, next http.Handler, regex string, replacement string, permanent bool, name string) (http.Handler, error) {
re, err := regexp.Compile(regex)
if err != nil {
return nil, err

View file

@ -309,18 +309,19 @@ func (p *Provider) listContainers(ctx context.Context, dockerClient client.Conta
}
func inspectContainers(ctx context.Context, dockerClient client.ContainerAPIClient, containerID string) dockerData {
dData := dockerData{}
containerInspected, err := dockerClient.ContainerInspect(ctx, containerID)
if err != nil {
log.FromContext(ctx).Warnf("Failed to inspect container %s, error: %s", containerID, err)
} else {
// This condition is here to avoid to have empty IP https://github.com/containous/traefik/issues/2459
// We register only container which are running
if containerInspected.ContainerJSONBase != nil && containerInspected.ContainerJSONBase.State != nil && containerInspected.ContainerJSONBase.State.Running {
dData = parseContainer(containerInspected)
}
return dockerData{}
}
return dData
// This condition is here to avoid to have empty IP https://github.com/containous/traefik/issues/2459
// We register only container which are running
if containerInspected.ContainerJSONBase != nil && containerInspected.ContainerJSONBase.State != nil && containerInspected.ContainerJSONBase.State.Running {
return parseContainer(containerInspected)
}
return dockerData{}
}
func parseContainer(container dockertypes.ContainerJSON) dockerData {

View file

@ -4,7 +4,7 @@ set -e
# List of bundles to create when no argument is passed
DEFAULT_BUNDLES=(
generate
validate-gofmt
validate-lint
binary
test-unit

View file

@ -1,30 +0,0 @@
#!/usr/bin/env bash
source "$(dirname "$BASH_SOURCE")/.validate"
IFS=$'\n'
files=( $(validate_diff --diff-filter=ACMR --name-only -- '*.go' | grep -v '^vendor/\|autogen' || true) )
unset IFS
badFiles=()
for f in "${files[@]}"; do
# we use "git show" here to validate that what's committed is formatted
if [ "$(git show "$VALIDATE_HEAD:$f" | gofmt -s -l)" ]; then
badFiles+=( "$f" )
fi
done
if [ ${#badFiles[@]} -eq 0 ]; then
echo 'Congratulations! All Go source files are properly formatted.'
else
{
echo "These files are not properly gofmt'd:"
for f in "${badFiles[@]}"; do
echo " - $f"
done
echo
echo 'Please reformat the above files using "gofmt -s -w" and commit the result.'
echo
} >&2
false
fi

View file

@ -1,31 +0,0 @@
#!/usr/bin/env bash
source "$(dirname "$BASH_SOURCE")/.validate"
IFS=$'\n'
files=( $(validate_diff --diff-filter=ACMR --name-only -- '*.go' | grep -v '^vendor/\|autogen' || true) )
unset IFS
errors=()
for f in "${files[@]}"; do
# we use "git show" here to validate that what's committed passes go vet
failedLint=$(golint "$f")
if [ "$failedLint" ]; then
errors+=( "$failedLint" )
fi
done
if [ ${#errors[@]} -eq 0 ]; then
echo 'Congratulations! All Go source files have been linted.'
else
{
echo "Errors from golint:"
for err in "${errors[@]}"; do
echo "$err"
done
echo
echo 'Please fix the above errors. You can test via "golint" and commit the result.'
echo
} >&2
false
fi

View file

@ -1,31 +0,0 @@
#!/usr/bin/env bash
source "$(dirname "$BASH_SOURCE")/.validate"
IFS=$'\n'
files=( $(validate_diff --diff-filter=ACMR --name-only -- '*.go' | grep -v '^vendor/' || true) )
unset IFS
errors=()
for f in "${files[@]}"; do
# we use "git show" here to validate that what's committed passes go vet
failedVet=$(go vet -printf=false "$f")
if [ "$failedVet" ]; then
errors+=( "$failedVet" )
fi
done
if [ ${#errors[@]} -eq 0 ]; then
echo 'Congratulations! All Go source files have been vetted.'
else
{
echo "Errors from govet:"
for err in "${errors[@]}"; do
echo "$err"
done
echo
echo 'Please fix the above errors. You can test via "go vet" and commit the result.'
echo
} >&2
false
fi

3
script/validate-lint Executable file
View file

@ -0,0 +1,3 @@
#!/usr/bin/env bash
golangci-lint run

View file

@ -3,10 +3,8 @@
source "$(dirname "$BASH_SOURCE")/.validate"
IFS=$'\n'
src=( $(validate_diff --diff-filter=ACMR --name-only -- '*.go' | grep -v '^vendor/\|autogen' || true) )
docs=( $(validate_diff --diff-filter=ACMR --name-only -- 'docs/*.md') )
files=( $(validate_diff --diff-filter=ACMR --name-only -- 'docs/*.md') )
unset IFS
files=("${src[@]}" "${docs[@]}")
errors=()
for f in "${files[@]}"; do

View file

@ -4,7 +4,7 @@ import (
"io"
"github.com/containous/traefik/log"
"github.com/instana/go-sensor"
instana "github.com/instana/go-sensor"
"github.com/opentracing/opentracing-go"
)