From 976b0abec5db484978c534d151decce80948813b Mon Sep 17 00:00:00 2001 From: Luca CORRIERI Date: Fri, 4 Aug 2023 11:02:41 +0200 Subject: [PATCH] fix(lints): fix code smell from CI --- internal/burrito/config/config.go | 25 +++++++++++++++---- internal/controllers/manager.go | 2 -- .../terraformpullrequest/comment/default.go | 5 +++- .../terraformpullrequest/github/provider.go | 8 +----- .../terraformpullrequest/gitlab/provider.go | 8 +----- internal/testing/loading.go | 14 ++++++++--- internal/webhook/github/provider_test.go | 6 ++--- internal/webhook/gitlab/provider_test.go | 6 ++--- 8 files changed, 42 insertions(+), 32 deletions(-) diff --git a/internal/burrito/config/config.go b/internal/burrito/config/config.go index 9d68a7f8..c62eb457 100644 --- a/internal/burrito/config/config.go +++ b/internal/burrito/config/config.go @@ -123,12 +123,20 @@ func (c *Config) Load(flags *pflag.FlagSet) error { name = strings.ReplaceAll(name, "-", "_") return pflag.NormalizedName(name) }) - v.BindPFlags(flags) + err := v.BindPFlags(flags) + if err != nil { + fmt.Fprintf(os.Stderr, "Error binding flags: %s\n", err) + return err + } // Nested configuration options set with environment variables use an // underscore as a separator. v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) - bindEnvironmentVariables(v, *c) + err = bindEnvironmentVariables(v, *c) + if err != nil { + fmt.Fprintf(os.Stderr, "Error binding environment variables: %s\n", err) + return err + } return v.Unmarshal(c) } @@ -137,7 +145,7 @@ func (c *Config) Load(flags *pflag.FlagSet) error { // fields to environment variables. This is a workaround to a limitation of // Viper, found here: // https://github.com/spf13/viper/issues/188#issuecomment-399884438 -func bindEnvironmentVariables(v *viper.Viper, iface interface{}, parts ...string) { +func bindEnvironmentVariables(v *viper.Viper, iface interface{}, parts ...string) error { ifv := reflect.ValueOf(iface) ift := reflect.TypeOf(iface) for i := 0; i < ift.NumField(); i++ { @@ -149,11 +157,18 @@ func bindEnvironmentVariables(v *viper.Viper, iface interface{}, parts ...string } switch val.Kind() { case reflect.Struct: - bindEnvironmentVariables(v, val.Interface(), append(parts, tv)...) + err := bindEnvironmentVariables(v, val.Interface(), append(parts, tv)...) + if err != nil { + return err + } default: - v.BindEnv(strings.Join(append(parts, tv), ".")) + err := v.BindEnv(strings.Join(append(parts, tv), ".")) + if err != nil { + return err + } } } + return nil } func TestConfig() *Config { diff --git a/internal/controllers/manager.go b/internal/controllers/manager.go index 6fb99efe..dafd487f 100644 --- a/internal/controllers/manager.go +++ b/internal/controllers/manager.go @@ -44,8 +44,6 @@ var ( scheme = runtime.NewScheme() ) -const () - type Controllers struct { config *config.Config } diff --git a/internal/controllers/terraformpullrequest/comment/default.go b/internal/controllers/terraformpullrequest/comment/default.go index c8123ecb..e54e1267 100644 --- a/internal/controllers/terraformpullrequest/comment/default.go +++ b/internal/controllers/terraformpullrequest/comment/default.go @@ -66,6 +66,9 @@ func (c *DefaultComment) Generate(commit string) (string, error) { Layers: reportedLayers, } comment := bytes.NewBufferString("") - defaultTemplate.Execute(comment, data) + err := defaultTemplate.Execute(comment, data) + if err != nil { + return "", err + } return comment.String(), nil } diff --git a/internal/controllers/terraformpullrequest/github/provider.go b/internal/controllers/terraformpullrequest/github/provider.go index f4e4cfe4..4394a579 100644 --- a/internal/controllers/terraformpullrequest/github/provider.go +++ b/internal/controllers/terraformpullrequest/github/provider.go @@ -21,13 +21,7 @@ type Github struct { } func (g *Github) IsConfigPresent(c *config.Config) bool { - if &c.Controller.GithubConfig == nil { - return false - } - if c.Controller.GithubConfig.APIToken == "" { - return false - } - return true + return c.Controller.GithubConfig.APIToken != "" } func (g *Github) Init(c *config.Config) error { diff --git a/internal/controllers/terraformpullrequest/gitlab/provider.go b/internal/controllers/terraformpullrequest/gitlab/provider.go index 01f1d8df..1ebf59e3 100644 --- a/internal/controllers/terraformpullrequest/gitlab/provider.go +++ b/internal/controllers/terraformpullrequest/gitlab/provider.go @@ -18,13 +18,7 @@ type Gitlab struct { } func (g *Gitlab) IsConfigPresent(c *config.Config) bool { - if &c.Controller.GitlabConfig == nil { - return false - } - if c.Controller.GitlabConfig.APIToken == "" { - return false - } - return true + return c.Controller.GitlabConfig.APIToken != "" } func (g *Gitlab) Init(c *config.Config) error { diff --git a/internal/testing/loading.go b/internal/testing/loading.go index ca520ab2..48bb6206 100644 --- a/internal/testing/loading.go +++ b/internal/testing/loading.go @@ -20,7 +20,10 @@ import ( func LoadResources(client client.Client, path string) { log := logf.FromContext(context.TODO()) - resources := parseResources(path) + resources, err := parseResources(path) + if err != nil { + panic(err) + } for _, r := range resources { log.Info(fmt.Sprintf("Creating %s, %s/%s", r.GetObjectKind().GroupVersionKind().Kind, r.GetNamespace(), r.GetName())) err := client.Create(context.TODO(), r) @@ -30,14 +33,14 @@ func LoadResources(client client.Client, path string) { } } -func parseResources(path string) []client.Object { +func parseResources(path string) ([]client.Object, error) { log := logf.FromContext(context.TODO()) _ = configv1alpha1.AddToScheme(scheme.Scheme) decoder := scheme.Codecs.UniversalDeserializer() list := []client.Object{} r := []byte{} - filepath.WalkDir(path, func(path string, d fs.DirEntry, err error) error { + err := filepath.WalkDir(path, func(path string, d fs.DirEntry, walkErr error) error { if d.IsDir() { return nil } @@ -55,6 +58,9 @@ func parseResources(path string) []client.Object { r = append(r, data...) return nil }) + if err != nil { + return nil, err + } for _, doc := range strings.Split(string(r), "---") { if doc == "" || doc == "\n" { @@ -68,5 +74,5 @@ func parseResources(path string) []client.Object { list = append(list, obj.(client.Object)) } - return list + return list, nil } diff --git a/internal/webhook/github/provider_test.go b/internal/webhook/github/provider_test.go index 5f8a690b..544f6640 100644 --- a/internal/webhook/github/provider_test.go +++ b/internal/webhook/github/provider_test.go @@ -7,7 +7,7 @@ import ( "encoding/hex" "encoding/json" "fmt" - "io/ioutil" + "io" "os" "net/http" @@ -42,7 +42,7 @@ func TestGithub_GetEvent_PushEvent(t *testing.T) { } defer payloadFile.Close() - payloadBytes, err := ioutil.ReadAll(payloadFile) + payloadBytes, err := io.ReadAll(payloadFile) if err != nil { t.Fatalf("failed to read payload file: %v", err) } @@ -99,7 +99,7 @@ func TestGithub_GetEvent_PullRequestEvent(t *testing.T) { } defer payloadFile.Close() - payloadBytes, err := ioutil.ReadAll(payloadFile) + payloadBytes, err := io.ReadAll(payloadFile) if err != nil { t.Fatalf("failed to read payload file: %v", err) } diff --git a/internal/webhook/gitlab/provider_test.go b/internal/webhook/gitlab/provider_test.go index 1ba3c678..9b0f92ed 100644 --- a/internal/webhook/gitlab/provider_test.go +++ b/internal/webhook/gitlab/provider_test.go @@ -3,7 +3,7 @@ package gitlab_test import ( "bytes" "encoding/json" - "io/ioutil" + "io" "os" "net/http" @@ -38,7 +38,7 @@ func TestGitlab_GetEvent_PushEvent(t *testing.T) { } defer payloadFile.Close() - payloadBytes, err := ioutil.ReadAll(payloadFile) + payloadBytes, err := io.ReadAll(payloadFile) if err != nil { t.Fatalf("failed to read payload file: %v", err) } @@ -90,7 +90,7 @@ func TestGitlab_GetEvent_MergeRequestEvent(t *testing.T) { } defer payloadFile.Close() - payloadBytes, err := ioutil.ReadAll(payloadFile) + payloadBytes, err := io.ReadAll(payloadFile) if err != nil { t.Fatalf("failed to read payload file: %v", err) }