From cd14bd6b7421fd411f0b1f526b36cf6980da5a63 Mon Sep 17 00:00:00 2001 From: fydrah Date: Wed, 18 Jul 2018 00:28:20 +0200 Subject: [PATCH] Add code checks, and resolve check errors * gofmt * errcheck: check if there is unchecked errors check if we correctly catch every errors * gocyclo: check if cyclomatic complexity is over 15 Help keeping functions simple * gosimple: check if code can be simplified --- Makefile | 30 ++++++++++++++++++++++++++++++ config.go | 48 ++++++++++++++++++++++++------------------------ main.go | 50 ++++++++++++++++++++++++++------------------------ 3 files changed, 80 insertions(+), 48 deletions(-) diff --git a/Makefile b/Makefile index 6e92e07..5ef24ca 100644 --- a/Makefile +++ b/Makefile @@ -5,6 +5,8 @@ GIT_REPOSITORY := github.com/fydrah/loginapp GIT_COMMIT_ID := $(shell git log -n 1 --pretty=format:%h) GIT_TAG := $(shell git describe --tags) DOCKERFILES := dockerfiles +CYCLO_MAX := 15 +SRC_FILES := $(shell find . -name "*.go" -not -path "./vendor*") LDFLAGS = -w -s -X main.GitVersion=$(GIT_TAG) -X main.GitHash=$(GIT_COMMIT_ID) @@ -23,3 +25,31 @@ build-static: .PHONY: docker-tmp docker-tmp: docker build -t $(DOCKER_REPOSITORY):$(GIT_COMMIT_ID) . + +.PHONY: checks +checks: errcheck gocyclo gosimple + +.PHONY: gofmt +gofmt: + gofmt -w -s $(SRC_FILES) + +.PHONY: errcheck +errcheck: + @echo + @echo "############ Run unchecked errors check" + which errcheck || go get github.com/kisielk/errcheck + errcheck $(GIT_REPOSITORY) + +.PHONY: gocyclo +gocyclo: + @echo + @echo "############ Run cyclomatic complexity check" + which gocyclo || go get github.com/fzipp/gocyclo + gocyclo -over $(CYCLO_MAX) $(SRC_FILES) + +.PHONY: gosimple +gosimple: + @echo + @echo "############ Run simplifying code check (codes reference at https://staticcheck.io/docs/gosimple)" + which gosimple || go get honnef.co/go/tools/cmd/gosimple + gosimple $(GIT_REPOSITORY) diff --git a/config.go b/config.go index 57b562e..547a39b 100644 --- a/config.go +++ b/config.go @@ -17,10 +17,10 @@ package main import ( "fmt" + "github.com/sirupsen/logrus" "gopkg.in/yaml.v2" "io/ioutil" "strings" - "github.com/sirupsen/logrus" ) type AppConfig struct { @@ -41,13 +41,13 @@ type AppConfig struct { CrossClients []string `yaml:"cross_clients"` } `yaml:"oidc"` Tls struct { - Enabled bool `yaml:"enabled"` - Cert string `yaml:"cert"` - Key string `yaml:"key"` + Enabled bool `yaml:"enabled"` + Cert string `yaml:"cert"` + Key string `yaml:"key"` } `yaml:"tls"` Log struct { - Level string `yaml:"level"` - Format string `yaml:"format"` + Level string `yaml:"level"` + Format string `yaml:"format"` } `yaml:"log"` } @@ -60,27 +60,27 @@ func (a *AppConfig) Init(config string) error { return fmt.Errorf("error parse config file %s: %v", config, err) } switch f := strings.ToLower(a.Log.Format); f { - case "json": - logger.Formatter = &logrus.JSONFormatter{} - case "text": - logger.Formatter = &logrus.TextFormatter{} - default: - logger.Formatter = &logrus.JSONFormatter{} - logger.Warningf("Format %q not available, use json|text. Using json format", f) + case "json": + logger.Formatter = &logrus.JSONFormatter{} + case "text": + logger.Formatter = &logrus.TextFormatter{} + default: + logger.Formatter = &logrus.JSONFormatter{} + logger.Warningf("Format %q not available, use json|text. Using json format", f) } logger.Debugf("Using %s log format", a.Log.Format) switch f := strings.ToLower(a.Log.Level); f { - case "debug": - logger.Level = logrus.DebugLevel - case "info": - logger.Level = logrus.InfoLevel - case "warning": - logger.Level = logrus.WarnLevel - case "error": - logger.Level = logrus.ErrorLevel - default: - logger.Level = logrus.InfoLevel - logger.Warningf("Log level %q not available, use debug|info|warning|error. Using Info log level", f) + case "debug": + logger.Level = logrus.DebugLevel + case "info": + logger.Level = logrus.InfoLevel + case "warning": + logger.Level = logrus.WarnLevel + case "error": + logger.Level = logrus.ErrorLevel + default: + logger.Level = logrus.InfoLevel + logger.Warningf("Log level %q not available, use debug|info|warning|error. Using Info log level", f) } logger.Debugf("Using %s log level", a.Log.Format) logger.Debugf("Configuration loaded: %+v", a) diff --git a/main.go b/main.go index 23c8c64..b74077b 100644 --- a/main.go +++ b/main.go @@ -21,22 +21,22 @@ import ( "context" "encoding/json" "fmt" + "github.com/cenkalti/backoff" "github.com/coreos/go-oidc" "github.com/julienschmidt/httprouter" + "github.com/sirupsen/logrus" + "github.com/urfave/cli" "golang.org/x/oauth2" "net/http" "os" - "github.com/urfave/cli" - "github.com/sirupsen/logrus" - "time" "strings" - "github.com/cenkalti/backoff" + "time" ) var ( GitVersion = "X.X.X" - GitHash = "XXXXXXX" - logger = logrus.New() + GitHash = "XXXXXXX" + logger = logrus.New() ) /** @@ -79,9 +79,7 @@ func (s *Server) PrepareCallbackUrl(endpoint string) string { authCodeURL string ) - for _, extra_scope := range s.config.OIDC.ExtraScopes { - scopes = append(scopes, extra_scope) - } + scopes = append(scopes, s.config.OIDC.ExtraScopes...) // Prepare cross client auth // see https://github.com/coreos/dex/blob/master/Documentation/custom-scopes-claims-clients.md for _, cross_client := range s.config.OIDC.CrossClients { @@ -190,7 +188,9 @@ func (s *Server) ProcessCallback(w http.ResponseWriter, r *http.Request, c strin return KubeUserInfo{}, fmt.Errorf("Failed to unmarshal claims from idToken: %v", err) } buff := new(bytes.Buffer) - json.Indent(buff, []byte(claims), "", " ") + if err := json.Indent(buff, []byte(claims), "", " "); err != nil { + return KubeUserInfo{}, fmt.Errorf("Failed to format claims output: %v", err) + } if err := json.Unmarshal(claims, &json_claims); err != nil { panic(err) } @@ -213,13 +213,13 @@ func loggingHandler(next http.Handler) http.Handler { t1 := time.Now() next.ServeHTTP(w, r) t2 := time.Now() - logger.WithFields(logrus.Fields{ - "method": r.Method, - "path": r.URL.String(), - "request_duration": fmt.Sprintf("%s",t2.Sub(t1)), - "protocol": r.Proto, - "remote_address": r.RemoteAddr, - }).Info() + logger.WithFields(logrus.Fields{ + "method": r.Method, + "path": r.URL.String(), + "request_duration": t2.Sub(t1).String(), + "protocol": r.Proto, + "remote_address": r.RemoteAddr, + }).Info() } return http.HandlerFunc(fn) } @@ -229,8 +229,8 @@ func loggingHandler(next http.Handler) http.Handler { */ func (s *Server) Run() error { var ( - provider *oidc.Provider - backoffErr error + provider *oidc.Provider + backoffErr error ) // router setup s.router = httprouter.New() @@ -323,23 +323,25 @@ GLOBAL OPTIONS: app.UsageText = "Simple application for Kubernetes CLI configuration with OIDC" app.Version = fmt.Sprintf("%v build %v", GitVersion, GitHash) app.Authors = []cli.Author{ - cli.Author{ + { Name: "fydrah", Email: "flav.hardy@gmail.com", }, } app.Commands = []cli.Command{ { - Name: "serve", - Usage: "Run loginapp application", + Name: "serve", + Usage: "Run loginapp application", SkipFlagParsing: true, - ArgsUsage: "[configuration file]", + ArgsUsage: "[configuration file]", Before: func(c *cli.Context) error { return nil }, Action: func(c *cli.Context) error { if len(c.Args()) == 0 { - cli.ShowCommandHelp(c, c.Command.Name) + if err := cli.ShowCommandHelp(c, c.Command.Name); err != nil { + return fmt.Errorf("Error while rendering command help: %v", err) + } return fmt.Errorf("Missing argument") } s := &Server{}