Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

run vet and lint in CI #136

Merged
merged 3 commits into from
Jan 10, 2023
Merged

run vet and lint in CI #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b1ae963
run vet and lint in CI
VinozzZ Jan 5, 2023
8c1ac4f
Merge branch 'main' into add-staticcheck
VinozzZ Jan 5, 2023
92d7d98
address comments
VinozzZ Jan 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions api/v1/porterconfig_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 `json:",inline" yaml:",inline" mapstructure:",squash"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is always confusing! 😅

I am pretty sure that the json tag does not support squash or inline. Embedded fields are always inlined and do not require a json entry at all.

I'm pretty sure that the json tag just ignores the ,inline anyway but it may be more clear (so that people aren't tricked into thinking it is doing anything) if we remove that json tag entirely.

Suggested change
PluginConfig `json:",inline" yaml:",inline" mapstructure:",squash"`
PluginConfig `yaml:",inline" mapstructure:",squash"` # json is inline automatically because it's an embedded field

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are right on the embedded fields are always inlined and do not require a json entry when used with encoding/json, the standard library.
However, it looks like the controller-gen requires it to be specified. I found the same `json:,inline; in their doc as well. https://book-v1.book.kubebuilder.io/beyond_basics/generating_crd.html

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh I wonder what they are doing that it's used? 🤔

Thanks for checking, I'll try to remember this the next time I'm tempted to remove it!

}

// StorageConfig is the plugin stanza for storage.
type StorageConfig struct {
PluginConfig `json:",squash" yaml:",inline" mapstructure:",squash"`
PluginConfig `json:",inline" yaml:",inline" mapstructure:",squash"`
}

// PluginConfig is a standardized config stanza that defines which plugin to
Expand Down
18 changes: 4 additions & 14 deletions controllers/agentaction_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,9 @@ func (r *AgentActionReconciler) applyJobToStatus(log logr.Logger, action *porter
case batchv1.JobComplete:
action.Status.Phase = porterv1.PhaseSucceeded
setCondition(log, action, porterv1.ConditionComplete, "JobCompleted")
break
case batchv1.JobFailed:
action.Status.Phase = porterv1.PhaseFailed
setCondition(log, action, porterv1.ConditionFailed, "JobFailed")
break
}
}
}
Expand Down Expand Up @@ -628,9 +626,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
Expand All @@ -644,9 +640,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
}
Expand Down Expand Up @@ -714,13 +708,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
}
Expand Down
3 changes: 1 addition & 2 deletions mage/docs/docs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package docs
import (
"fmt"
"io"
"io/ioutil"
"log"
"net/http"
"os"
Expand Down Expand Up @@ -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)
}

Expand Down
24 changes: 6 additions & 18 deletions mage/docs/docs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package docs

import (
"io/ioutil"
"os"
"path/filepath"
"testing"
Expand All @@ -12,39 +11,30 @@ 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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So much cleaner! 💯


resolvedPath, err := ensurePorterRepositoryIn(tmp, "")
require.NoError(t, err)
require.Equal(t, tmp, resolvedPath)
})

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)
require.Equal(t, tmp, resolvedPath)
})

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)
Expand All @@ -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)
Expand All @@ -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)
})
Expand Down
16 changes: 10 additions & 6 deletions magefiles/magefile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"bufio"
"bytes"
"fmt"
"io/ioutil"
"log"
"os"
"path/filepath"
Expand Down Expand Up @@ -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.
Expand All @@ -93,6 +92,11 @@ func Vet() {
must.RunV("go", "vet", "./...")
}

func Lint() {
mg.Deps(tools.EnsureStaticCheck)
must.RunV("staticcheck", "./...")
}

// Build the controller and bundle.
func Build() {
mg.SerialDeps(BuildController, BuildBundle)
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
4 changes: 4 additions & 0 deletions staticcheck.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"]