From b1ae96375fb7df3480d71e1a7a66b1f3f0413d5b Mon Sep 17 00:00:00 2001 From: Yingrong Zhao Date: Thu, 5 Jan 2023 11:27:58 -0500 Subject: [PATCH 1/2] run vet and lint in CI Signed-off-by: Yingrong Zhao --- .github/workflows/build.yml | 2 ++ api/v1/porterconfig_types.go | 4 ++-- controllers/agentaction_controller.go | 21 +++++++-------------- mage/docs/docs.go | 3 +-- mage/docs/docs_test.go | 24 ++++++------------------ magefiles/magefile.go | 16 ++++++++++------ staticcheck.conf | 4 ++++ 7 files changed, 32 insertions(+), 42 deletions(-) create mode 100644 staticcheck.conf diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 19ac22d5..e13a3e6e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -19,6 +19,8 @@ jobs: cache-dependency-path: go.sum - name: Set up Mage run: go run mage.go EnsureMage + -name: VetLint + run: mage -v vet lint - name: Test run: mage -v Test env: diff --git a/api/v1/porterconfig_types.go b/api/v1/porterconfig_types.go index cc2e6072..cf8bbe1b 100644 --- a/api/v1/porterconfig_types.go +++ b/api/v1/porterconfig_types.go @@ -91,12 +91,12 @@ func MergeMap(target, override map[string]interface{}) map[string]interface{} { // SecretsConfig is the plugin stanza for secrets. type SecretsConfig struct { - PluginConfig `json:",squash" yaml:",inline" mapstructure:",squash"` + PluginConfig `yaml:",inline" mapstructure:",squash"` } // StorageConfig is the plugin stanza for storage. type StorageConfig struct { - PluginConfig `json:",squash" yaml:",inline" mapstructure:",squash"` + PluginConfig `yaml:",inline" mapstructure:",squash"` } // PluginConfig is a standardized config stanza that defines which plugin to diff --git a/controllers/agentaction_controller.go b/controllers/agentaction_controller.go index 584f8985..fdc8ed2c 100644 --- a/controllers/agentaction_controller.go +++ b/controllers/agentaction_controller.go @@ -165,16 +165,17 @@ func (r *AgentActionReconciler) applyJobToStatus(log logr.Logger, action *porter setCondition(log, action, porterv1.ConditionStarted, "JobStarted") } +conditions: for _, condition := range job.Status.Conditions { switch condition.Type { case batchv1.JobComplete: action.Status.Phase = porterv1.PhaseSucceeded setCondition(log, action, porterv1.ConditionComplete, "JobCompleted") - break + break conditions case batchv1.JobFailed: action.Status.Phase = porterv1.PhaseFailed setCondition(log, action, porterv1.ConditionFailed, "JobFailed") - break + break conditions } } } @@ -628,9 +629,7 @@ func (r *AgentActionReconciler) getAgentEnv(action *porterv1.AgentAction, agentC }, } - for _, e := range action.Spec.Env { - env = append(env, e) - } + env = append(env, action.Spec.Env...) envFrom := []corev1.EnvFromSource{ // Environment variables for the plugins @@ -644,9 +643,7 @@ func (r *AgentActionReconciler) getAgentEnv(action *porterv1.AgentAction, agentC }, } - for _, e := range action.Spec.EnvFrom { - envFrom = append(envFrom, e) - } + envFrom = append(envFrom, action.Spec.EnvFrom...) return env, envFrom } @@ -714,13 +711,9 @@ func (r *AgentActionReconciler) getAgentVolumes(action *porterv1.AgentAction, pv ) } - for _, volume := range action.Spec.Volumes { - volumes = append(volumes, volume) - } + volumes = append(volumes, action.Spec.Volumes...) - for _, mount := range action.Spec.VolumeMounts { - volumeMounts = append(volumeMounts, mount) - } + volumeMounts = append(volumeMounts, action.Spec.VolumeMounts...) return volumes, volumeMounts } diff --git a/mage/docs/docs.go b/mage/docs/docs.go index 9b9fe897..620be9b6 100644 --- a/mage/docs/docs.go +++ b/mage/docs/docs.go @@ -3,7 +3,6 @@ package docs import ( "fmt" "io" - "io/ioutil" "log" "net/http" "os" @@ -70,7 +69,7 @@ func TriggerNetlifyDeployment(webhook string) error { if r.StatusCode >= 300 { defer r.Body.Close() - msg, _ := ioutil.ReadAll(r.Body) + msg, _ := io.ReadAll(r.Body) return fmt.Errorf("request failed (%d) %s: %s", r.StatusCode, r.Status, msg) } diff --git a/mage/docs/docs_test.go b/mage/docs/docs_test.go index 5479573f..6c3c1182 100644 --- a/mage/docs/docs_test.go +++ b/mage/docs/docs_test.go @@ -1,7 +1,6 @@ package docs import ( - "io/ioutil" "os" "path/filepath" "testing" @@ -12,9 +11,7 @@ import ( func TestEnsurePorterRepository(t *testing.T) { t.Run("has local repo", func(t *testing.T) { - tmp, err := ioutil.TempDir("", "porter-docs-test") - require.NoError(t, err) - defer os.RemoveAll(tmp) + tmp := t.TempDir() resolvedPath, err := ensurePorterRepositoryIn(tmp, "") require.NoError(t, err) @@ -22,9 +19,7 @@ func TestEnsurePorterRepository(t *testing.T) { }) t.Run("missing local repo", func(t *testing.T) { - tmp, err := ioutil.TempDir("", "porter-docs-test") - require.NoError(t, err) - defer os.RemoveAll(tmp) + tmp := t.TempDir() resolvedPath, err := ensurePorterRepositoryIn("missing", tmp) require.NoError(t, err) @@ -32,19 +27,14 @@ func TestEnsurePorterRepository(t *testing.T) { }) t.Run("local repo unset", func(t *testing.T) { - tmp, err := ioutil.TempDir("", "porter-docs-test") - require.NoError(t, err) - defer os.RemoveAll(tmp) - + tmp := t.TempDir() resolvedPath, err := ensurePorterRepositoryIn("", tmp) require.NoError(t, err) require.Equal(t, tmp, resolvedPath) }) t.Run("empty default path clones repo", func(t *testing.T) { - tmp, err := ioutil.TempDir("", "porter-docs-test") - require.NoError(t, err) - defer os.RemoveAll(tmp) + tmp := t.TempDir() resolvedPath, err := ensurePorterRepositoryIn("", tmp) require.NoError(t, err) @@ -55,9 +45,7 @@ func TestEnsurePorterRepository(t *testing.T) { }) t.Run("changes in default path are reset", func(t *testing.T) { - tmp, err := ioutil.TempDir("", "porter-docs-test") - require.NoError(t, err) - defer os.RemoveAll(tmp) + tmp := t.TempDir() repoPath, err := ensurePorterRepositoryIn("", tmp) require.NoError(t, err) @@ -67,7 +55,7 @@ func TestEnsurePorterRepository(t *testing.T) { require.NoError(t, os.Remove(readme)) // Make sure rerunning resets the change - repoPath, err = ensurePorterRepositoryIn("", tmp) + _, err = ensurePorterRepositoryIn("", tmp) require.NoError(t, err) require.FileExists(t, readme) }) diff --git a/magefiles/magefile.go b/magefiles/magefile.go index 4f5d5989..4207c725 100644 --- a/magefiles/magefile.go +++ b/magefiles/magefile.go @@ -8,7 +8,6 @@ import ( "bufio" "bytes" "fmt" - "io/ioutil" "log" "os" "path/filepath" @@ -77,7 +76,7 @@ func addGopathBinOnGithubActions() error { log.Println("Adding GOPATH/bin to the PATH for the GitHub Actions Agent") gopathBin := gopath.GetGopathBin() - return ioutil.WriteFile(githubPath, []byte(gopathBin), 0644) + return os.WriteFile(githubPath, []byte(gopathBin), 0644) } // Ensure EnsureMage is installed and on the PATH. @@ -93,6 +92,11 @@ func Vet() { must.RunV("go", "vet", "./...") } +func Lint() { + tools.EnsureStaticCheck() + must.RunV("staticcheck", "./...") +} + // Build the controller and bundle. func Build() { mg.SerialDeps(BuildController, BuildBundle) @@ -428,7 +432,7 @@ func Bump(sample string) { sampleFile := fmt.Sprintf("config/samples/%s.yaml", sample) - dataB, err := ioutil.ReadFile(sampleFile) + dataB, err := os.ReadFile(sampleFile) mgx.Must(errors.Wrapf(err, "error reading installation definition %s", sampleFile)) updateRetry := fmt.Sprintf(`.metadata.annotations."porter.sh/retry" = "%s"`, time.Now().Format(time.RFC3339)) @@ -570,7 +574,7 @@ func useCluster() bool { } os.Setenv("KUBECONFIG", currentKubeConfig) - err := ioutil.WriteFile(kubeconfig, []byte(contents), 0644) + err := os.WriteFile(kubeconfig, []byte(contents), 0644) mgx.Must(errors.Wrapf(err, "error writing %s", kubeconfig)) setClusterNamespace(operatorNamespace) @@ -663,7 +667,7 @@ func BuildLocalPorterAgent() { // generatedCodeFilter remove generated code files from coverage report func generatedCodeFilter(filename string) error { - fd, err := ioutil.ReadFile(filename) + fd, err := os.ReadFile(filename) if err != nil { return err } @@ -677,7 +681,7 @@ func generatedCodeFilter(filename string) error { } fd = []byte(strings.Join(lines, "\n")) - err = ioutil.WriteFile(filename, fd, 0600) + err = os.WriteFile(filename, fd, 0600) if err != nil { return err } diff --git a/staticcheck.conf b/staticcheck.conf new file mode 100644 index 00000000..6ce9676c --- /dev/null +++ b/staticcheck.conf @@ -0,0 +1,4 @@ +# Default Config +checks = ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-ST1023", "-ST1005"] +initialisms = ["ACL", "API", "ASCII", "CPU", "CSS", "DNS", "EOF", "GUID", "HTML", "HTTP", "HTTPS", "ID", "IP", "JSON", "QPS", "RAM", "RPC", "SLA", "SMTP", "SQL", "SSH", "TCP", "TLS", "TTL", "UDP", "UI", "GID", "UID", "UUID", "URI", "URL", "UTF8", "VM", "XML", "XMPP", "XSRF", "XSS", "SIP", "RTP", "AMQP", "DB", "TS"] +http_status_code_whitelist = ["200", "400", "404", "500"] \ No newline at end of file From 92d7d980cca2f63fee00c285b8ba1254695c513f Mon Sep 17 00:00:00 2001 From: Yingrong Zhao Date: Mon, 9 Jan 2023 17:15:40 -0500 Subject: [PATCH 2/2] address comments Signed-off-by: Yingrong Zhao --- controllers/agentaction_controller.go | 3 --- magefiles/magefile.go | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/controllers/agentaction_controller.go b/controllers/agentaction_controller.go index fdc8ed2c..4739926f 100644 --- a/controllers/agentaction_controller.go +++ b/controllers/agentaction_controller.go @@ -165,17 +165,14 @@ func (r *AgentActionReconciler) applyJobToStatus(log logr.Logger, action *porter setCondition(log, action, porterv1.ConditionStarted, "JobStarted") } -conditions: for _, condition := range job.Status.Conditions { switch condition.Type { case batchv1.JobComplete: action.Status.Phase = porterv1.PhaseSucceeded setCondition(log, action, porterv1.ConditionComplete, "JobCompleted") - break conditions case batchv1.JobFailed: action.Status.Phase = porterv1.PhaseFailed setCondition(log, action, porterv1.ConditionFailed, "JobFailed") - break conditions } } } diff --git a/magefiles/magefile.go b/magefiles/magefile.go index 4207c725..9bae4d3e 100644 --- a/magefiles/magefile.go +++ b/magefiles/magefile.go @@ -93,7 +93,7 @@ func Vet() { } func Lint() { - tools.EnsureStaticCheck() + mg.Deps(tools.EnsureStaticCheck) must.RunV("staticcheck", "./...") }