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

Add make verify target for running static analysis checks #34

Merged
merged 3 commits into from
Apr 2, 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
19 changes: 19 additions & 0 deletions .github/workflows/static-analysis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,22 @@ jobs:
uses: golangci/golangci-lint-action@v4
with:
version: v1.57.1
args: --config tools/.golangci.yaml
- run: |
set -euo pipefail

make verify
- run: |
set -euo pipefail

make fmt

DIFF=$(git status --porcelain)

if [ -n "$DIFF" ]; then
echo "These files were modified:"
echo
echo "$DIFF"
echo
exit 1
fi
13 changes: 13 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,19 @@ GO_VERSION ?= 1.21.8
GOOS ?= linux
GOARCH ?= amd64
TEMP_DIR := $(shell mktemp -d)
GOFILES = $(shell find . -name \*.go)

.PHONY: fmt
fmt:
@echo "Verifying gofmt, failures can be fixed with ./scripts/fix.sh"
@!(gofmt -l -s -d ${GOFILES} | grep '[a-z]')

@echo "Verifying goimports, failures can be fixed with ./scripts/fix.sh"
@!(go run golang.org/x/tools/cmd/goimports@latest -l -d ${GOFILES} | grep '[a-z]')

.PHONY: verify
verify:
golangci-lint run --config tools/.golangci.yaml ./...

# Local development build
build:
Expand Down
4 changes: 2 additions & 2 deletions cmd/analyze.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type analyzeOptions struct {
filename string
}

var analyzeOpts *analyzeOptions = &analyzeOptions{}
var analyzeOpts = &analyzeOptions{}

func init() {
RootCmd.AddCommand(analyzeCmd)
Expand All @@ -52,7 +52,7 @@ func analyzeValidateAndRun() error {
objectCounts := map[string]uint{}
for _, s := range summaries {
if s.TypeMeta != nil {
objectCounts[fmt.Sprintf("%s/%s", s.TypeMeta.APIVersion, s.TypeMeta.Kind)] += 1
objectCounts[fmt.Sprintf("%s/%s", s.TypeMeta.APIVersion, s.TypeMeta.Kind)]++
}
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/checksum.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type checksumOptions struct {
revision int64
}

var checksumOpts *checksumOptions = &checksumOptions{}
var checksumOpts = &checksumOptions{}

func init() {
RootCmd.AddCommand(checksumCmd)
Expand Down
7 changes: 3 additions & 4 deletions cmd/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ type decodeOptions struct {
batchProcess bool // special flag to handle incoming etcd-dump-logs output
}

var options *decodeOptions = &decodeOptions{}
var options = &decodeOptions{}

func init() {
RootCmd.AddCommand(decodeCmd)
Expand Down Expand Up @@ -124,7 +124,7 @@ func runInBatchMode(metaOnly bool, outMediaType string, out io.Writer) (err erro
return nil
}
if err != nil {
return fmt.Errorf("error reading --batch-process input: %v\n", err)
return fmt.Errorf("error reading --batch-process input: %v", err)
}

input = stripNewline(input)
Expand Down Expand Up @@ -210,7 +210,6 @@ func readInput(inputFilename string) ([]byte, error) {
func stripNewline(d []byte) []byte {
if len(d) > 0 && d[len(d)-1] == '\n' {
return d[:len(d)-1]
} else {
return d
}
return d
}
2 changes: 1 addition & 1 deletion cmd/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type encodeOptions struct {
inputFilename string
}

var encodeOpts *encodeOptions = &encodeOptions{}
var encodeOpts = &encodeOptions{}

func init() {
RootCmd.AddCommand(encodeCmd)
Expand Down
6 changes: 3 additions & 3 deletions cmd/extract.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ type extractOptions struct {
filter string
}

var opts *extractOptions = &extractOptions{}
var opts = &extractOptions{}

func init() {
RootCmd.AddCommand(extractCmd)
Expand Down Expand Up @@ -250,7 +250,7 @@ func printLeafItemValue(kv *mvccpb.KeyValue, outMediaType string, out io.Writer)
// printKeySummaries prints all keys in the db file with the given key prefix.
func printKeySummaries(filename string, keyPrefix string, revision int64, fields []string, out io.Writer) error {
if len(fields) == 0 {
return fmt.Errorf("no fields provided, nothing to output.")
return fmt.Errorf("no fields provided, nothing to output")
}

var hasKey bool
Expand Down Expand Up @@ -288,7 +288,7 @@ func printTemplateSummaries(filename string, keyPrefix string, revision int64, t
}

if len(templatestr) == 0 {
return fmt.Errorf("no template provided, nothing to output.")
return fmt.Errorf("no template provided, nothing to output")
}

filters := []data.Filter{}
Expand Down
4 changes: 2 additions & 2 deletions pkg/data/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ var (
keyBucket = []byte("key")
metaBucket = []byte("meta")

finishedCompactKeyName = []byte("finishedCompactRev")
finishedCompactKeyName = []byte("finishedCompactRev")
)

// KeySummary represents a kubernetes object stored in etcd.
Expand Down Expand Up @@ -272,7 +272,7 @@ func ListKeySummaries(filename string, filters []Filter, proj *KeySummaryProject
ks.Version = kv.ModRevision
ks.Stats.ValueSize = len(kv.Value)
}
ks.Stats.VersionCount += 1
ks.Stats.VersionCount++
ks.Stats.AllVersionsKeySize += len(kv.Key)
ks.Stats.AllVersionsValueSize += len(kv.Value)
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/data/data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,22 +114,22 @@ func TestParseFilters(t *testing.T) {
{
name: "namespace-equals",
rawFilter: ".Value.metadata.namespace=default",
expected: []*FieldConstraint{&FieldConstraint{lhs: ".Value.metadata.namespace", op: Equals, rhs: "default"}},
expected: []*FieldConstraint{{lhs: ".Value.metadata.namespace", op: Equals, rhs: "default"}},
},
{
name: "2-filters",
rawFilter: ".Value.metadata.namespace=default,.Value.metadata.name=example",
expected: []*FieldConstraint{
&FieldConstraint{lhs: ".Value.metadata.namespace", op: Equals, rhs: "default"},
&FieldConstraint{lhs: ".Value.metadata.name", op: Equals, rhs: "example"},
{lhs: ".Value.metadata.namespace", op: Equals, rhs: "default"},
{lhs: ".Value.metadata.name", op: Equals, rhs: "example"},
},
},
{
name: "whitespace",
rawFilter: " .Value.metadata.namespace=default\t, .Value.metadata.name=example\n",
expected: []*FieldConstraint{
&FieldConstraint{lhs: ".Value.metadata.namespace", op: Equals, rhs: "default"},
&FieldConstraint{lhs: ".Value.metadata.name", op: Equals, rhs: "example"},
{lhs: ".Value.metadata.namespace", op: Equals, rhs: "default"},
{lhs: ".Value.metadata.name", op: Equals, rhs: "example"},
},
},
}
Expand Down
15 changes: 8 additions & 7 deletions pkg/encoding/scheme.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,18 @@ func init() {
// AddToScheme adds all types of this clientset into the given scheme. This allows composition
// of clientsets, like in:
//
// import (
// "k8s.io/client-go/kubernetes"
// clientsetscheme "k8s.io/client-go/kuberentes/scheme"
// aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme"
// )
// import (
// "k8s.io/client-go/kubernetes"
// clientsetscheme "k8s.io/client-go/kuberentes/scheme"
// aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme"
// )
//
// kclientset, _ := kubernetes.NewForConfig(c)
// aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme)
// kclientset, _ := kubernetes.NewForConfig(c)
// aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme)
//
// After this, RawExtensions in Kubernetes types will serialize kube-aggregator types
// correctly.
//
//nolint:errcheck
func AddToScheme(scheme *runtime.Scheme) {
admissionv1beta1.AddToScheme(scheme)
Expand Down
10 changes: 10 additions & 0 deletions scripts/fix.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
GO_CMD="go"

GOFILES=$(${GO_CMD} list --f "{{with \$d:=.}}{{range .GoFiles}}{{\$d.Dir}}/{{.}}{{\"\n\"}}{{end}}{{end}}" ./...)
TESTGOFILES=$(${GO_CMD} list --f "{{with \$d:=.}}{{range .TestGoFiles}}{{\$d.Dir}}/{{.}}{{\"\n\"}}{{end}}{{end}}" ./...)
XTESTGOFILES=$(${GO_CMD} list --f "{{with \$d:=.}}{{range .XTestGoFiles}}{{\$d.Dir}}/{{.}}{{\"\n\"}}{{end}}{{end}}" ./...)

echo "${GOFILES}" "${TESTGOFILES}" "${XTESTGOFILES}"| xargs -n 100 go run golang.org/x/tools/cmd/goimports@latest -w -local go.etcd.io

go fmt ./...
go mod tidy
103 changes: 103 additions & 0 deletions tools/.golangci.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
---
run:
timeout: 30m
issues.exclude-files: [^zz_generated.*]
issues:
max-same-issues: 0
# Excluding configuration per-path, per-linter, per-text and per-source
exclude-rules:
# exclude ineffassing linter for generated files for conversion
- path: conversion\.go
linters: [ineffassign]
linters:
disable-all: true
enable: # please keep this alphabetized
# Don't use soon to deprecated[1] linters that lead to false
# https://github.com/golangci/golangci-lint/issues/1841
# - deadcode
# - structcheck
# - varcheck
- goimports
- ineffassign
- nakedret
- revive
- staticcheck
- stylecheck
- unconvert # Remove unnecessary type conversions
- unparam
- unused
linters-settings: # please keep this alphabetized
goimports:
local-prefixes: go.etcd.io # Put imports beginning with prefix after 3rd-party packages.
nakedret:
# Align with https://github.com/alexkohler/nakedret/blob/v1.0.2/cmd/nakedret/main.go#L10
max-func-lines: 5
revive:
ignore-generated-header: false
severity: error
confidence: 0.8
enable-all-rules: false
rules:
- name: blank-imports
severity: error
disabled: false
- name: context-as-argument
severity: error
disabled: false
- name: dot-imports
severity: error
disabled: false
- name: error-return
severity: error
disabled: false
- name: error-naming
severity: error
disabled: false
- name: if-return
severity: error
disabled: false
- name: increment-decrement
severity: error
disabled: false
- name: var-declaration
severity: error
disabled: false
- name: package-comments
severity: error
disabled: false
- name: range
severity: error
disabled: false
- name: receiver-naming
severity: error
disabled: false
- name: time-naming
severity: error
disabled: false
- name: indent-error-flow
severity: error
disabled: false
- name: errorf
severity: error
disabled: false
- name: context-keys-type
severity: error
disabled: false
- name: error-strings
severity: error
disabled: false
# TODO: enable the following rules
- name: var-naming
disabled: true
- name: exported
disabled: true
- name: unexported-return
disabled: true
staticcheck:
checks:
- all
- -SA1019 # TODO(fix) Using a deprecated function, variable, constant or field
- -SA2002 # TODO(fix) Called testing.T.FailNow or SkipNow in a goroutine, which isn’t allowed
stylecheck:
checks:
- ST1019 # Importing the same package multiple times.
Loading