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.

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

chore: Add golangci-lint and fix most issues #505

Merged
merged 1 commit into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 13 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,25 @@ ANALYTICS_WRITE_KEY ?=
LDFLAGS := -ldflags='-X "main.Version=$(VERSION)" -X "main.AnalyticsWriteKey=$(ANALYTICS_WRITE_KEY)"'
MOQ := $(shell command -v moq 2> /dev/null)
SRC := $(shell find . -name '*.go')
GOLANGCI_LINT := $(shell command -v golangci-lint 2> /dev/null)

vet:
go vet ./...

test: store/awsapi_mock.go
go test -v ./...

.PHONY: coverage
coverage:
go test -coverpkg ./... -coverprofile coverage.out ./...

lint: vet
ifdef GOLANGCI_LINT
@golangci-lint run --max-same-issues 0 --max-issues-per-linter 0
else
@echo "Please install golangci-lint: brew install golangci-lint"
@false
endif

store/awsapi_mock.go: store/awsapi.go
ifdef MOQ
rm -f $@
Expand Down Expand Up @@ -71,4 +82,4 @@ dist/chamber-$(VERSION)-linux-arm64 dist/chamber-$(VERSION)-linux-aarch64: | dis
dist/chamber-$(VERSION)-windows-amd64.exe: | dist/
GOOS=windows GOARCH=amd64 CGO_ENABLED=0 go build -trimpath $(LDFLAGS) -o $@

.PHONY: clean all fmt build linux
.PHONY: vet test coverage lint clean all fmt build linux
2 changes: 1 addition & 1 deletion cmd/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
}

if analyticsEnabled && analyticsClient != nil {
analyticsClient.Enqueue(analytics.Track{
_ = analyticsClient.Enqueue(analytics.Track{

Check warning on line 43 in cmd/delete.go

View check run for this annotation

Codecov / codecov/patch

cmd/delete.go#L43

Added line #L43 was not covered by tests
UserId: username,
Event: "Ran Command",
Properties: analytics.NewProperties().
Expand Down
2 changes: 1 addition & 1 deletion cmd/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
}

if analyticsEnabled && analyticsClient != nil {
analyticsClient.Enqueue(analytics.Track{
_ = analyticsClient.Enqueue(analytics.Track{

Check warning on line 78 in cmd/env.go

View check run for this annotation

Codecov / codecov/patch

cmd/env.go#L78

Added line #L78 was not covered by tests
UserId: username,
Event: "Ran Command",
Properties: analytics.NewProperties().
Expand Down
3 changes: 1 addition & 2 deletions cmd/env_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package cmd

import (
"fmt"
"testing"
)

Expand Down Expand Up @@ -35,7 +34,7 @@ func Test_sanitizeKey(t *testing.T) {
}{
{given: "invalid strings", expected: "invalid_strings"},
{given: "extremely invalid strings", expected: "extremely__invalid__strings"},
{given: fmt.Sprintf("\nunbelievably\tinvalid\tstrings\n"), expected: "unbelievably_invalid_strings"},
{given: "\nunbelievably\tinvalid\tstrings\n", expected: "unbelievably_invalid_strings"},
{given: "valid_string", expected: "valid_string"},
{given: "validish-string", expected: "validish_string"},
{given: "valid.string", expected: "valid_string"},
Expand Down
5 changes: 2 additions & 3 deletions cmd/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
services, command, commandArgs := args[:dashIx], args[dashIx], args[dashIx+1:]

if analyticsEnabled && analyticsClient != nil {
analyticsClient.Enqueue(analytics.Track{
_ = analyticsClient.Enqueue(analytics.Track{

Check warning on line 78 in cmd/exec.go

View check run for this annotation

Codecov / codecov/patch

cmd/exec.go#L78

Added line #L78 was not covered by tests
UserId: username,
Event: "Ran Command",
Properties: analytics.NewProperties().
Expand Down Expand Up @@ -118,9 +118,8 @@
}
for _, service := range services {
collisions := make([]string, 0)
var err error
// TODO: these interfaces should look the same as Strict*, so move pristine in there
err = env.Load(cmd.Context(), secretStore, service, &collisions)
err := env.Load(cmd.Context(), secretStore, service, &collisions)

Check warning on line 122 in cmd/exec.go

View check run for this annotation

Codecov / codecov/patch

cmd/exec.go#L122

Added line #L122 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to list store contents: %w", err)
}
Expand Down
13 changes: 10 additions & 3 deletions cmd/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
var err error

if analyticsEnabled && analyticsClient != nil {
analyticsClient.Enqueue(analytics.Track{
_ = analyticsClient.Enqueue(analytics.Track{

Check warning on line 45 in cmd/export.go

View check run for this annotation

Codecov / codecov/patch

cmd/export.go#L45

Added line #L45 was not covered by tests
UserId: username,
Event: "Ran Command",
Properties: analytics.NewProperties().
Expand Down Expand Up @@ -82,6 +82,7 @@
if file, err = os.OpenFile(exportOutput, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644); err != nil {
return fmt.Errorf("Failed to open output file for writing: %w", err)
}
// TODO: check for errors flushing, syncing, or closing
defer file.Close()
defer file.Sync()
}
Expand Down Expand Up @@ -144,7 +145,10 @@
for _, k := range sortedKeys(params) {
key := sanitizeKey(strings.TrimPrefix(k, "tf_var_"))

w.Write([]byte(fmt.Sprintf(`%s = "%s"`+"\n", key, doubleQuoteEscape(params[k]))))
_, err := w.Write([]byte(fmt.Sprintf(`%s = "%s"`+"\n", key, doubleQuoteEscape(params[k]))))
if err != nil {
return fmt.Errorf("failed to write variable with key %s: %v", k, err)

Check warning on line 150 in cmd/export.go

View check run for this annotation

Codecov / codecov/patch

cmd/export.go#L148-L150

Added lines #L148 - L150 were not covered by tests
}
}
return nil
}
Expand All @@ -170,7 +174,10 @@
p := properties.NewProperties()
p.DisableExpansion = true
for _, k := range sortedKeys(params) {
p.Set(k, params[k])
_, _, err := p.Set(k, params[k])
if err != nil {
return fmt.Errorf("failed to set property %s: %v", k, err)

Check warning on line 179 in cmd/export.go

View check run for this annotation

Codecov / codecov/patch

cmd/export.go#L177-L179

Added lines #L177 - L179 were not covered by tests
}
}

// Java expects properties in ISO-8859-1 by default
Expand Down
2 changes: 1 addition & 1 deletion cmd/history.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
}

if analyticsEnabled && analyticsClient != nil {
analyticsClient.Enqueue(analytics.Track{
_ = analyticsClient.Enqueue(analytics.Track{

Check warning on line 38 in cmd/history.go

View check run for this annotation

Codecov / codecov/patch

cmd/history.go#L38

Added line #L38 was not covered by tests
UserId: username,
Event: "Ran Command",
Properties: analytics.NewProperties().
Expand Down
2 changes: 1 addition & 1 deletion cmd/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
}

if analyticsEnabled && analyticsClient != nil {
analyticsClient.Enqueue(analytics.Track{
_ = analyticsClient.Enqueue(analytics.Track{

Check warning on line 57 in cmd/import.go

View check run for this annotation

Codecov / codecov/patch

cmd/import.go#L57

Added line #L57 was not covered by tests
UserId: username,
Event: "Ran Command",
Properties: analytics.NewProperties().
Expand Down
2 changes: 1 addition & 1 deletion cmd/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
}

if analyticsEnabled && analyticsClient != nil {
analyticsClient.Enqueue(analytics.Track{
_ = analyticsClient.Enqueue(analytics.Track{

Check warning on line 46 in cmd/list.go

View check run for this annotation

Codecov / codecov/patch

cmd/list.go#L46

Added line #L46 was not covered by tests
UserId: username,
Event: "Ran Command",
Properties: analytics.NewProperties().
Expand Down
2 changes: 1 addition & 1 deletion cmd/read.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
}

if analyticsEnabled && analyticsClient != nil {
analyticsClient.Enqueue(analytics.Track{
_ = analyticsClient.Enqueue(analytics.Track{

Check warning on line 45 in cmd/read.go

View check run for this annotation

Codecov / codecov/patch

cmd/read.go#L45

Added line #L45 was not covered by tests
UserId: username,
Event: "Ran Command",
Properties: analytics.NewProperties().
Expand Down
7 changes: 4 additions & 3 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@

if cmd, err := RootCmd.ExecuteC(); err != nil {
if strings.Contains(err.Error(), "arg(s)") || strings.Contains(err.Error(), "usage") {
cmd.Usage()
_ = cmd.Usage()

Check warning on line 108 in cmd/root.go

View check run for this annotation

Codecov / codecov/patch

cmd/root.go#L108

Added line #L108 was not covered by tests
}
os.Exit(1)
}
Expand Down Expand Up @@ -206,7 +206,8 @@
return nil, errors.New("Unable to use --kms-key-alias with this backend. Use CHAMBER_KMS_KEY_ALIAS instead.")
}

parsedRetryMode, err := aws.ParseRetryMode(retryMode)
var parsedRetryMode aws.RetryMode
parsedRetryMode, err = aws.ParseRetryMode(retryMode)

Check warning on line 210 in cmd/root.go

View check run for this annotation

Codecov / codecov/patch

cmd/root.go#L209-L210

Added lines #L209 - L210 were not covered by tests
if err != nil {
return nil, fmt.Errorf("Invalid retry mode %s", retryMode)
}
Expand All @@ -225,7 +226,7 @@
})

username = os.Getenv("USER")
analyticsClient.Enqueue(analytics.Identify{
_ = analyticsClient.Enqueue(analytics.Identify{

Check warning on line 229 in cmd/root.go

View check run for this annotation

Codecov / codecov/patch

cmd/root.go#L229

Added line #L229 was not covered by tests
UserId: username,
Traits: analytics.NewTraits().
Set("chamber-version", chamberVersion),
Expand Down
2 changes: 1 addition & 1 deletion cmd/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
func versionRun(cmd *cobra.Command, args []string) error {
fmt.Fprintf(os.Stdout, "chamber %s\n", chamberVersion)
if analyticsEnabled && analyticsClient != nil {
analyticsClient.Enqueue(analytics.Track{
_ = analyticsClient.Enqueue(analytics.Track{

Check warning on line 25 in cmd/version.go

View check run for this annotation

Codecov / codecov/patch

cmd/version.go#L25

Added line #L25 was not covered by tests
UserId: username,
Event: "Ran Command",
Properties: analytics.NewProperties().
Expand Down
6 changes: 3 additions & 3 deletions cmd/write.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import (
"bufio"
"fmt"
"io/ioutil"
"io"
"os"
"strings"

Expand Down Expand Up @@ -44,7 +44,7 @@
}

if analyticsEnabled && analyticsClient != nil {
analyticsClient.Enqueue(analytics.Track{
_ = analyticsClient.Enqueue(analytics.Track{

Check warning on line 47 in cmd/write.go

View check run for this annotation

Codecov / codecov/patch

cmd/write.go#L47

Added line #L47 was not covered by tests
UserId: username,
Event: "Ran Command",
Properties: analytics.NewProperties().
Expand All @@ -67,7 +67,7 @@
}
value = strings.TrimSuffix(v, "\n")
} else {
v, err := ioutil.ReadAll(os.Stdin)
v, err := io.ReadAll(os.Stdin)

Check warning on line 70 in cmd/write.go

View check run for this annotation

Codecov / codecov/patch

cmd/write.go#L70

Added line #L70 was not covered by tests
if err != nil {
return err
}
Expand Down
3 changes: 0 additions & 3 deletions environ/environ.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,9 @@ func (e *Environ) load(ctx context.Context, s store.Store, service string, colli
if err != nil {
return err
}
envVarKeys := make([]string, 0)
for _, rawSecret := range rawSecrets {
envVarKey := secretKeyToEnvVarName(rawSecret.Key)

envVarKeys = append(envVarKeys, envVarKey)

if e.IsSet(envVarKey) {
*collisions = append(*collisions, envVarKey)
}
Expand Down
14 changes: 9 additions & 5 deletions store/backendbenchmarks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"sync"
"testing"
"time"

"github.com/stretchr/testify/require"
)

// This file contains some tests which can be used to benchmark
Expand Down Expand Up @@ -49,11 +51,13 @@ func benchmarkStore(t *testing.T, store Store, services []string) {

for i := 0; i < concurrency; i++ {
wg.Add(1)
go emulateExec(t, ctx, &wg, store, services)

go func() {
// TODO: collect errors in a channel
_ = emulateExec(t, ctx, &wg, store, services)
}()
}
wg.Wait()
elapsed := time.Now().Sub(start)
elapsed := time.Since(start)
t.Logf("Concurrently started %d services in %s", concurrency, elapsed)
}
}
Expand Down Expand Up @@ -106,7 +110,7 @@ func setupStore(t *testing.T, ctx context.Context, store Store, services []strin
Key: key,
}

store.Write(ctx, id, RandStringRunes(100))
require.NoError(t, store.Write(ctx, id, RandStringRunes(100)))
}
}
}
Expand All @@ -120,7 +124,7 @@ func cleanupStore(t *testing.T, ctx context.Context, store Store, services []str
Key: key,
}

store.Delete(ctx, id)
require.NoError(t, store.Delete(ctx, id))
}
}
}
19 changes: 4 additions & 15 deletions store/s3store.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"io"
"sort"
"time"

Expand Down Expand Up @@ -258,9 +258,7 @@
return err
}

if _, ok := index.Latest[id.Key]; ok {
delete(index.Latest, id.Key)
}
delete(index.Latest, id.Key)

Check warning on line 261 in store/s3store.go

View check run for this annotation

Codecov / codecov/patch

store/s3store.go#L261

Added line #L261 was not covered by tests

if err := s.deleteObjectById(ctx, id); err != nil {
return err
Expand Down Expand Up @@ -317,7 +315,7 @@
return secretObject{}, false, err
}

raw, err := ioutil.ReadAll(resp.Body)
raw, err := io.ReadAll(resp.Body)

Check warning on line 318 in store/s3store.go

View check run for this annotation

Codecov / codecov/patch

store/s3store.go#L318

Added line #L318 was not covered by tests
if err != nil {
return secretObject{}, false, err
}
Expand Down Expand Up @@ -366,7 +364,7 @@
return latest{}, err
}

raw, err := ioutil.ReadAll(resp.Body)
raw, err := io.ReadAll(resp.Body)

Check warning on line 367 in store/s3store.go

View check run for this annotation

Codecov / codecov/patch

store/s3store.go#L367

Added line #L367 was not covered by tests
if err != nil {
return latest{}, err
}
Expand All @@ -390,15 +388,6 @@
return s.puts3raw(ctx, path, raw)
}

func stringInSlice(val string, sl []string) bool {
for _, v := range sl {
if v == val {
return true
}
}
return false
}

func getObjectPath(id SecretId) string {
return fmt.Sprintf("%s/%s.json", id.Service, id.Key)
}
Expand Down
Loading
Loading