From ac4aa0d182f04cd8b5b103bdce9640054fdede9b Mon Sep 17 00:00:00 2001 From: Emile Vauge Date: Tue, 22 Mar 2016 01:32:02 +0100 Subject: [PATCH] add errcheck validation Signed-off-by: Emile Vauge --- .pre-commit-config.yaml | 7 +-- Makefile | 4 +- acme/acme.go | 128 ++++++++++++++++++++------------------- build.Dockerfile | 12 ++-- cmd.go | 17 +++--- configuration.go | 4 +- script/validate-errcheck | 28 +++++++++ server.go | 14 ++++- 8 files changed, 129 insertions(+), 85 deletions(-) create mode 100755 script/validate-errcheck diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8178c66f6..438c30722 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,11 +1,10 @@ - repo: git://github.com/pre-commit/pre-commit-hooks - sha: 97b88d9610bcc03982ddac33caba98bb2b751f5f + sha: 44e1753f98b0da305332abe26856c3e621c5c439 hooks: - id: detect-private-key - - repo: git://github.com/containous/pre-commit-hooks - sha: HEAD + sha: 35e641b5107671e94102b0ce909648559e568d61 hooks: - id: goFmt - id: goLint - \ No newline at end of file + - id: goErrcheck diff --git a/Makefile b/Makefile index 588d40123..39fed86de 100644 --- a/Makefile +++ b/Makefile @@ -41,8 +41,8 @@ test-unit: build test-integration: build $(DOCKER_RUN_TRAEFIK) ./script/make.sh generate test-integration -validate: build - $(DOCKER_RUN_TRAEFIK) ./script/make.sh validate-gofmt validate-govet validate-golint +validate: build + $(DOCKER_RUN_TRAEFIK) ./script/make.sh validate-gofmt validate-govet validate-golint validate-gofmt: build $(DOCKER_RUN_TRAEFIK) ./script/make.sh validate-gofmt diff --git a/acme/acme.go b/acme/acme.go index 677401111..c3498d074 100644 --- a/acme/acme.go +++ b/acme/acme.go @@ -27,6 +27,34 @@ type Account struct { DomainsCertificate DomainsCertificates } +// GetEmail returns email +func (a Account) GetEmail() string { + return a.Email +} + +// GetRegistration returns lets encrypt registration resource +func (a Account) GetRegistration() *acme.RegistrationResource { + return a.Registration +} + +// GetPrivateKey returns private key +func (a Account) GetPrivateKey() crypto.PrivateKey { + if privateKey, err := x509.ParsePKCS1PrivateKey(a.PrivateKey); err == nil { + return privateKey + } + log.Errorf("Cannot unmarshall private key %+v", a.PrivateKey) + return nil +} + +// Certificate is used to store certificate info +type Certificate struct { + Domain string + CertURL string + CertStableURL string + PrivateKey []byte + Certificate []byte +} + // DomainsCertificates stores a certificate for multiple domains type DomainsCertificates struct { Certs []*DomainsCertificate @@ -64,7 +92,7 @@ func (dc *DomainsCertificates) renewCertificates(acmeCert *Certificate, domain D return nil } } - return errors.New("Certificate to renew to found from domain " + domain.Main) + return errors.New("Certificate to renew not found for domain " + domain.Main) } func (dc *DomainsCertificates) addCertificateForDomains(acmeCert *Certificate, domain Domain) (*DomainsCertificate, error) { @@ -84,7 +112,9 @@ func (dc *DomainsCertificates) getCertificateForDomain(domainToFind string) (*Do dc.lock.RLock() defer dc.lock.RUnlock() for _, domainsCertificate := range dc.Certs { - domains := append([]string{domainsCertificate.Domains.Main}, domainsCertificate.Domains.SANs...) + domains := []string{} + domains = append(domains, domainsCertificate.Domains.Main) + domains = append(domains, domainsCertificate.Domains.SANs...) for _, domain := range domains { if domain == domainToFind { return domainsCertificate, true @@ -112,34 +142,6 @@ type DomainsCertificate struct { tlsCert *tls.Certificate } -// GetEmail returns email -func (a Account) GetEmail() string { - return a.Email -} - -// GetRegistration returns lets encrypt registration resource -func (a Account) GetRegistration() *acme.RegistrationResource { - return a.Registration -} - -// GetPrivateKey returns private key -func (a Account) GetPrivateKey() crypto.PrivateKey { - if privateKey, err := x509.ParsePKCS1PrivateKey(a.PrivateKey); err == nil { - return privateKey - } - log.Errorf("Cannot unmarshall private key %+v", a.PrivateKey) - return nil -} - -// Certificate is used to store certificate info -type Certificate struct { - Domain string - CertURL string - CertStableURL string - PrivateKey []byte - Certificate []byte -} - // ACME allows to connect to lets encrypt and retrieve certs type ACME struct { Email string @@ -157,10 +159,9 @@ type Domain struct { SANs []string } -// CreateACMEConfig creates a tls.config from using ACME configuration -func (a *ACME) CreateACMEConfig(tlsConfig *tls.Config, CheckOnDemandDomain func(domain string) bool) error { +// CreateConfig creates a tls.config from using ACME configuration +func (a *ACME) CreateConfig(tlsConfig *tls.Config, CheckOnDemandDomain func(domain string) bool) error { acme.Logger = fmtlog.New(ioutil.Discard, "", 0) - // TODO: generate default cert if empty if len(a.StorageFile) == 0 { return errors.New("Empty StorageFile, please provide a filenmae for certs storage") @@ -225,27 +226,7 @@ func (a *ACME) CreateACMEConfig(tlsConfig *tls.Config, CheckOnDemandDomain func( return err } - go func() { - log.Infof("Retrieving ACME certificates...") - for _, domain := range a.Domains { - // check if cert isn't already loaded - if _, exists := account.DomainsCertificate.exists(domain); !exists { - domains := append([]string{domain.Main}, domain.SANs...) - certificateResource, err := a.getDomainsCertificates(client, domains) - if err != nil { - log.Errorf("Error getting ACME certificate for domain %s: %s", domains, err.Error()) - } - _, err = account.DomainsCertificate.addCertificateForDomains(certificateResource, domain) - if err != nil { - log.Errorf("Error adding ACME certificate for domain %s: %s", domains, err.Error()) - } - if err = a.saveAccount(account); err != nil { - log.Errorf("Error Saving ACME account %+v: %s", account, err.Error()) - } - } - } - log.Infof("Retrieved ACME certificates") - }() + go a.retrieveCertificates(client, account) tlsConfig.GetCertificate = func(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { if challengeCert, ok := wrapperChallengeProvider.getCertificate(clientHello.ServerName); ok { @@ -265,7 +246,6 @@ func (a *ACME) CreateACMEConfig(tlsConfig *tls.Config, CheckOnDemandDomain func( ticker := time.NewTicker(24 * time.Hour) go func() { - time.Sleep(24 * time.Hour) for { select { case <-ticker.C: @@ -280,8 +260,35 @@ func (a *ACME) CreateACMEConfig(tlsConfig *tls.Config, CheckOnDemandDomain func( return nil } -func (a *ACME) renewCertificates(client *acme.Client, Account *Account) error { - for _, certificateResource := range Account.DomainsCertificate.Certs { +func (a *ACME) retrieveCertificates(client *acme.Client, account *Account) { + log.Infof("Retrieving ACME certificates...") + for _, domain := range a.Domains { + // check if cert isn't already loaded + if _, exists := account.DomainsCertificate.exists(domain); !exists { + domains := []string{} + domains = append(domains, domain.Main) + domains = append(domains, domain.SANs...) + certificateResource, err := a.getDomainsCertificates(client, domains) + if err != nil { + log.Errorf("Error getting ACME certificate for domain %s: %s", domains, err.Error()) + continue + } + _, err = account.DomainsCertificate.addCertificateForDomains(certificateResource, domain) + if err != nil { + log.Errorf("Error adding ACME certificate for domain %s: %s", domains, err.Error()) + continue + } + if err = a.saveAccount(account); err != nil { + log.Errorf("Error Saving ACME account %+v: %s", account, err.Error()) + continue + } + } + } + log.Infof("Retrieved ACME certificates") +} + +func (a *ACME) renewCertificates(client *acme.Client, account *Account) error { + for _, certificateResource := range account.DomainsCertificate.Certs { // <= 7 days left, renew certificate if certificateResource.tlsCert.Leaf.NotAfter.Before(time.Now().Add(time.Duration(24 * 7 * time.Hour))) { log.Debugf("Renewing certificate %+v", certificateResource.Domains) @@ -303,11 +310,11 @@ func (a *ACME) renewCertificates(client *acme.Client, Account *Account) error { PrivateKey: renewedCert.PrivateKey, Certificate: renewedCert.Certificate, } - err = Account.DomainsCertificate.renewCertificates(renewedACMECert, certificateResource.Domains) + err = account.DomainsCertificate.renewCertificates(renewedACMECert, certificateResource.Domains) if err != nil { return err } - if err = a.saveAccount(Account); err != nil { + if err = a.saveAccount(account); err != nil { return err } } @@ -316,9 +323,6 @@ func (a *ACME) renewCertificates(client *acme.Client, Account *Account) error { } func (a *ACME) buildACMEClient(Account *Account) (*acme.Client, error) { - - // A client facilitates communication with the CA server. This CA URL is - // configured for a local dev instance of Boulder running in Docker in a VM. caServer := "https://acme-v01.api.letsencrypt.org/directory" if len(a.CAServer) > 0 { caServer = a.CAServer diff --git a/build.Dockerfile b/build.Dockerfile index 5711a8778..ad2145376 100644 --- a/build.Dockerfile +++ b/build.Dockerfile @@ -1,11 +1,11 @@ FROM golang:1.6.0-alpine -RUN apk update && apk add git bash gcc - -RUN go get github.com/Masterminds/glide -RUN go get github.com/mitchellh/gox -RUN go get github.com/jteeuwen/go-bindata/... -RUN go get github.com/golang/lint/golint +RUN apk update && apk add git bash gcc musl-dev \ +&& go get github.com/Masterminds/glide \ +&& go get github.com/mitchellh/gox \ +&& go get github.com/jteeuwen/go-bindata/... \ +&& go get github.com/golang/lint/golint \ +&& go get github.com/kisielk/errcheck # Which docker version to test on ENV DOCKER_VERSION 1.10.1 diff --git a/cmd.go b/cmd.go index f42303512..3cbab053a 100644 --- a/cmd.go +++ b/cmd.go @@ -166,13 +166,12 @@ func init() { traefikCmd.PersistentFlags().StringVar(&arguments.Boltdb.Endpoint, "boltdb.endpoint", "127.0.0.1:4001", "Boltdb server endpoint") traefikCmd.PersistentFlags().StringVar(&arguments.Boltdb.Prefix, "boltdb.prefix", "/traefik", "Prefix used for KV store") - viper.BindPFlag("configFile", traefikCmd.PersistentFlags().Lookup("configFile")) - viper.BindPFlag("graceTimeOut", traefikCmd.PersistentFlags().Lookup("graceTimeOut")) - //viper.BindPFlag("defaultEntryPoints", traefikCmd.PersistentFlags().Lookup("defaultEntryPoints")) - viper.BindPFlag("logLevel", traefikCmd.PersistentFlags().Lookup("logLevel")) + _ = viper.BindPFlag("configFile", traefikCmd.PersistentFlags().Lookup("configFile")) + _ = viper.BindPFlag("graceTimeOut", traefikCmd.PersistentFlags().Lookup("graceTimeOut")) + _ = viper.BindPFlag("logLevel", traefikCmd.PersistentFlags().Lookup("logLevel")) // TODO: wait for this issue to be corrected: https://github.com/spf13/viper/issues/105 - viper.BindPFlag("providersThrottleDuration", traefikCmd.PersistentFlags().Lookup("providersThrottleDuration")) - viper.BindPFlag("maxIdleConnsPerHost", traefikCmd.PersistentFlags().Lookup("maxIdleConnsPerHost")) + _ = viper.BindPFlag("providersThrottleDuration", traefikCmd.PersistentFlags().Lookup("providersThrottleDuration")) + _ = viper.BindPFlag("maxIdleConnsPerHost", traefikCmd.PersistentFlags().Lookup("maxIdleConnsPerHost")) viper.SetDefault("providersThrottleDuration", time.Duration(2*time.Second)) viper.SetDefault("logLevel", "ERROR") viper.SetDefault("MaxIdleConnsPerHost", 200) @@ -197,7 +196,11 @@ func run() { if len(globalConfiguration.TraefikLogsFile) > 0 { fi, err := os.OpenFile(globalConfiguration.TraefikLogsFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666) - defer fi.Close() + defer func() { + if err := fi.Close(); err != nil { + log.Error("Error closinf file", err) + } + }() if err != nil { log.Fatal("Error opening file", err) } else { diff --git a/configuration.go b/configuration.go index 16cb42157..597c3d725 100644 --- a/configuration.go +++ b/configuration.go @@ -94,7 +94,9 @@ func (ep *EntryPoints) Set(value string) error { var tls *TLS if len(result["TLS"]) > 0 { certs := Certificates{} - certs.Set(result["TLS"]) + if err := certs.Set(result["TLS"]); err != nil { + return err + } tls = &TLS{ Certificates: certs, } diff --git a/script/validate-errcheck b/script/validate-errcheck new file mode 100755 index 000000000..cfdad38f2 --- /dev/null +++ b/script/validate-errcheck @@ -0,0 +1,28 @@ +#!/bin/bash + +source "$(dirname "$BASH_SOURCE")/.validate" + +IFS=$'\n' +files=( $(validate_diff --diff-filter=ACMR --name-only -- '*.go' | grep -v '^vendor/' || true) ) +unset IFS + +errors=() +failedErrcheck=$(errcheck .) +if [ "$failedErrcheck" ]; then + errors+=( "$failedErrcheck" ) +fi + +if [ ${#errors[@]} -eq 0 ]; then + echo 'Congratulations! All Go source files have been errchecked.' +else + { + echo "Errors from errcheck:" + for err in "${errors[@]}"; do + echo "$err" + done + echo + echo 'Please fix the above errors. You can test via "errcheck" and commit the result.' + echo + } >&2 + false +fi diff --git a/server.go b/server.go index 7abd40a57..47398cfdb 100644 --- a/server.go +++ b/server.go @@ -248,7 +248,7 @@ func (server *Server) createTLSConfig(entryPointName string, tlsOption *TLS, rou } return false } - err := server.globalConfiguration.ACME.CreateACMEConfig(config, checkOnDemandDomain) + err := server.globalConfiguration.ACME.CreateConfig(config, checkOnDemandDomain) if err != nil { return nil, err } @@ -343,6 +343,10 @@ func (server *Server) loadConfig(configurations configs, globalConfiguration Glo if len(frontend.EntryPoints) == 0 { frontend.EntryPoints = globalConfiguration.DefaultEntryPoints } + if len(frontend.EntryPoints) == 0 { + log.Errorf("No entrypoint defined for frontend %s, defaultEntryPoints:%s. Skipping it", frontendName, globalConfiguration.DefaultEntryPoints) + continue + } for _, entryPointName := range frontend.EntryPoints { log.Debugf("Wiring frontend %s to entryPoint %s", frontendName, entryPointName) if _, ok := serverEntryPoints[entryPointName]; !ok { @@ -390,7 +394,9 @@ func (server *Server) loadConfig(configurations configs, globalConfiguration Glo return nil, err } log.Debugf("Creating server %s at %s with weight %d", serverName, url.String(), server.Weight) - rebalancer.UpsertServer(url, roundrobin.Weight(server.Weight)) + if err := rebalancer.UpsertServer(url, roundrobin.Weight(server.Weight)); err != nil { + return nil, err + } } case types.Wrr: log.Debugf("Creating load-balancer wrr") @@ -401,7 +407,9 @@ func (server *Server) loadConfig(configurations configs, globalConfiguration Glo return nil, err } log.Debugf("Creating server %s at %s with weight %d", serverName, url.String(), server.Weight) - rr.UpsertServer(url, roundrobin.Weight(server.Weight)) + if err := rr.UpsertServer(url, roundrobin.Weight(server.Weight)); err != nil { + return nil, err + } } } var negroni = negroni.New()