Skip to content

Commit

Permalink
Introduce golangci-lint tooling, fixes (#2239)
Browse files Browse the repository at this point in the history
`golangci-lint` performs numerous checks on Go code, including golint,
ineffassign, govet, and gofmt.

This change modifies `bin/lint` to use `golangci-lint`, and replaces
usage of golint and govet.

Also perform a one-time gofmt cleanup:
- `gofmt -s -w controller/`
- `gofmt -s -w pkg/`

Part of #217

Signed-off-by: Andrew Seigner <siggy@buoyant.io>
  • Loading branch information
siggy authored Feb 13, 2019
1 parent 1cde1f0 commit 2305974
Show file tree
Hide file tree
Showing 37 changed files with 256 additions and 267 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@ web/app/yarn-error.log
.protoc
.gorun
.dep*
.golangci*
**.gogen*
**/*.swp
1 change: 0 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ jobs:
# this or we should error if it dirties the repo.
- ./bin/protoc-go.sh
- go test -race -v ./...
- go vet ./...
- ./bin/lint

- language: node_js
Expand Down
15 changes: 0 additions & 15 deletions Gopkg.lock
Original file line number Diff line number Diff line change
Expand Up @@ -673,17 +673,6 @@
pruneopts = ""
revision = "d9133f5469342136e669e85192a26056b587f503"

[[projects]]
branch = "master"
digest = "1:053024e8981dab276218a180e58f97034d06e8c4dd2e24c9365ad489104e1d90"
name = "golang.org/x/lint"
packages = [
".",
"golint",
]
pruneopts = ""
revision = "8f45f776aaf18cebc8d65861cc70c33c60471952"

[[projects]]
branch = "master"
digest = "1:e8c91565d4707bd93aa70e096884ce533acc5deb2bbb500bb064f49de70acda0"
Expand Down Expand Up @@ -772,9 +761,6 @@
name = "golang.org/x/tools"
packages = [
"go/ast/astutil",
"go/gcexportdata",
"go/internal/gcimporter",
"go/types/typeutil",
"imports",
"internal/fastwalk",
]
Expand Down Expand Up @@ -1274,7 +1260,6 @@
"github.com/sirupsen/logrus",
"github.com/spf13/cobra",
"github.com/wercker/stern/stern",
"golang.org/x/lint/golint",
"golang.org/x/net/context",
"google.golang.org/grpc",
"google.golang.org/grpc/codes",
Expand Down
1 change: 0 additions & 1 deletion Gopkg.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
required = [
"github.com/golang/protobuf/protoc-gen-go",
"golang.org/x/lint/golint",
"k8s.io/code-generator/cmd/client-gen",
"k8s.io/code-generator/cmd/deepcopy-gen",
"k8s.io/code-generator/cmd/defaulter-gen",
Expand Down
8 changes: 1 addition & 7 deletions TEST.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,7 @@ bin/dep ensure
go test -race ./...
```

To analyze the Go code without running tests, run:

```bash
go vet ./...
```

To lint the Go code using golint, run:
To analyze and lint the Go code using golangci-lint, run:

```bash
bin/lint
Expand Down
26 changes: 18 additions & 8 deletions bin/lint
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,32 @@

set -eu

lintversion=1.14.0

cd "$(pwd -P)"

bindir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
rootdir="$( cd $bindir/.. && pwd )"

cd "$rootdir"

# install golint from vendor
go install ./vendor/golang.org/x/lint/golint
os=linux
exe=
if [ "$(uname -s)" = "Darwin" ]; then
os=darwin
elif [ "$(uname -o)" = "Msys" ]; then
os=windows
exe=.exe
fi

# use `go list` to exclude packages in vendor
out=$(go list ./... | xargs golint) || true
lintbin="${rootdir}/.golangci-lint-${lintversion}${exe}"

if [ -n "$out" ]; then
echo "$out"
exit 1
if [ ! -f "$lintbin" ]; then
curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/v$lintversion/install.sh | sh -s -- -b . v$lintversion
mv ./golangci-lint${exe} $lintbin
fi

echo "all clean!"
$lintbin run \
--enable gofmt,golint \
--disable deadcode,errcheck,gosimple,staticcheck,structcheck,unused,varcheck \
"$@"
2 changes: 1 addition & 1 deletion cli/Dockerfile-bin
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
## compile binaries
FROM gcr.io/linkerd-io/go-deps:fb05ef23 as golang
FROM gcr.io/linkerd-io/go-deps:9c6adbc7 as golang
WORKDIR /go/src/github.com/linkerd/linkerd2
COPY cli cli
COPY chart chart
Expand Down
2 changes: 1 addition & 1 deletion cli/cmd/endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ requests.`,
output := renderEndpoints(endpoints, options)
_, err = fmt.Print(output)

return nil
return err
},
}

Expand Down
2 changes: 1 addition & 1 deletion cli/cmd/inject.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ func injectPodSpec(t *v1.PodSpec, identity k8s.TLSIdentity, controlPlaneDNSNameO
Image: options.taggedProxyInitImage(),
ImagePullPolicy: v1.PullPolicy(options.imagePullPolicy),
TerminationMessagePolicy: v1.TerminationMessageFallbackToLogsOnError,
Args: initArgs,
Args: initArgs,
SecurityContext: &v1.SecurityContext{
Capabilities: &v1.Capabilities{
Add: []v1.Capability{v1.Capability("NET_ADMIN")},
Expand Down
2 changes: 1 addition & 1 deletion cni-plugin/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
## compile cni-plugin utility
FROM gcr.io/linkerd-io/go-deps:fb05ef23 as golang
FROM gcr.io/linkerd-io/go-deps:9c6adbc7 as golang
WORKDIR /go/src/github.com/linkerd/linkerd2
COPY proxy-init proxy-init
COPY pkg pkg
Expand Down
2 changes: 1 addition & 1 deletion controller/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
## compile controller services
FROM gcr.io/linkerd-io/go-deps:fb05ef23 as golang
FROM gcr.io/linkerd-io/go-deps:9c6adbc7 as golang
WORKDIR /go/src/github.com/linkerd/linkerd2
COPY controller/gen controller/gen
COPY pkg pkg
Expand Down
12 changes: 6 additions & 6 deletions controller/api/proxy/endpoint_listener_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ var (
}

add = []*updateAddress{
&updateAddress{address: addedAddress1, pod: pod1},
&updateAddress{address: addedAddress2, pod: pod2},
{address: addedAddress1, pod: pod1},
{address: addedAddress2, pod: pod2},
}

remove = []*updateAddress{
&updateAddress{address: removedAddress1},
{address: removedAddress1},
}
)

Expand Down Expand Up @@ -181,7 +181,7 @@ func TestEndpointListener(t *testing.T) {
}

add := []*updateAddress{
&updateAddress{address: addedAddress1, pod: podForAddedAddress1},
{address: addedAddress1, pod: podForAddedAddress1},
}
listener.Update(add, nil)

Expand Down Expand Up @@ -238,7 +238,7 @@ func TestEndpointListener(t *testing.T) {
)

add := []*updateAddress{
&updateAddress{address: addedAddress1, pod: podForAddedAddress1},
{address: addedAddress1, pod: podForAddedAddress1},
}
listener.Update(add, nil)

Expand Down Expand Up @@ -286,7 +286,7 @@ func TestEndpointListener(t *testing.T) {
)

add := []*updateAddress{
&updateAddress{address: addedAddress1, pod: podForAddedAddress1},
{address: addedAddress1, pod: podForAddedAddress1},
}
listener.Update(add, nil)

Expand Down
48 changes: 24 additions & 24 deletions controller/api/proxy/endpoints_watcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ status:
expectedNoEndpointsServiceExists: false,
expectedState: servicePorts{
serviceID{namespace: "ns", name: "name1"}: map[uint32]*servicePort{
8989: &servicePort{
8989: {
addresses: []*updateAddress{
makeUpdateAddress("172.17.0.12", 8989, "ns", "name1-1"),
makeUpdateAddress("172.17.0.19", 8989, "ns", "name1-2"),
Expand All @@ -112,22 +112,22 @@ status:
Namespace: "ns",
},
Subsets: []v1.EndpointSubset{
v1.EndpointSubset{
{
Addresses: []v1.EndpointAddress{
v1.EndpointAddress{
{
IP: "172.17.0.12",
TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "ns", Name: "name1-1"},
},
v1.EndpointAddress{
{
IP: "172.17.0.19",
TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "ns", Name: "name1-2"},
},
v1.EndpointAddress{
{
IP: "172.17.0.20",
TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "ns", Name: "name1-3"},
},
},
Ports: []v1.EndpointPort{v1.EndpointPort{Port: 8989}},
Ports: []v1.EndpointPort{{Port: 8989}},
},
},
},
Expand Down Expand Up @@ -199,7 +199,7 @@ status:
expectedNoEndpointsServiceExists: false,
expectedState: servicePorts{
serviceID{namespace: "ns", name: "name1"}: map[uint32]*servicePort{
8989: &servicePort{
8989: {
addresses: []*updateAddress{
makeUpdateAddress("10.233.66.239", 8990, "ns", "name1-f748fb6b4-hpwpw"),
makeUpdateAddress("10.233.88.244", 8990, "ns", "name1-f748fb6b4-6vcmw"),
Expand All @@ -211,18 +211,18 @@ status:
Namespace: "ns",
},
Subsets: []v1.EndpointSubset{
v1.EndpointSubset{
{
Addresses: []v1.EndpointAddress{
v1.EndpointAddress{
{
IP: "10.233.66.239",
TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "ns", Name: "name1-f748fb6b4-hpwpw"},
},
v1.EndpointAddress{
{
IP: "10.233.88.244",
TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "ns", Name: "name1-f748fb6b4-6vcmw"},
},
},
Ports: []v1.EndpointPort{v1.EndpointPort{Port: 8990, Protocol: "TCP"}},
Ports: []v1.EndpointPort{{Port: 8990, Protocol: "TCP"}},
},
},
},
Expand Down Expand Up @@ -281,7 +281,7 @@ status:
expectedNoEndpointsServiceExists: false,
expectedState: servicePorts{
serviceID{namespace: "ns", name: "world"}: map[uint32]*servicePort{
7778: &servicePort{
7778: {
addresses: []*updateAddress{
makeUpdateAddress("10.1.30.135", 7779, "ns", "world-575bf846b4-tp4hw"),
},
Expand All @@ -292,14 +292,14 @@ status:
Namespace: "ns",
},
Subsets: []v1.EndpointSubset{
v1.EndpointSubset{
{
Addresses: []v1.EndpointAddress{
v1.EndpointAddress{
{
IP: "10.1.30.135",
TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "ns", Name: "world-575bf846b4-tp4hw"},
},
},
Ports: []v1.EndpointPort{v1.EndpointPort{Name: "app", Port: 7779, Protocol: "TCP"}},
Ports: []v1.EndpointPort{{Name: "app", Port: 7779, Protocol: "TCP"}},
},
},
},
Expand Down Expand Up @@ -363,7 +363,7 @@ status:
expectedNoEndpointsServiceExists: false,
expectedState: servicePorts{
serviceID{namespace: "ns", name: "name1"}: map[uint32]*servicePort{
8989: &servicePort{
8989: {
addresses: []*updateAddress{
makeUpdateAddress("172.17.0.25", 8989, "ns", "name1-3"),
},
Expand All @@ -374,22 +374,22 @@ status:
Namespace: "ns",
},
Subsets: []v1.EndpointSubset{
v1.EndpointSubset{
{
Addresses: []v1.EndpointAddress{
v1.EndpointAddress{
{
IP: "172.17.0.23",
TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "ns", Name: "name1-1"},
},
v1.EndpointAddress{
{
IP: "172.17.0.24",
TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "ns", Name: "name1-2"},
},
v1.EndpointAddress{
{
IP: "172.17.0.25",
TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "ns", Name: "name1-3"},
},
},
Ports: []v1.EndpointPort{v1.EndpointPort{Port: 8989}},
Ports: []v1.EndpointPort{{Port: 8989}},
},
},
},
Expand Down Expand Up @@ -417,7 +417,7 @@ spec:
expectedNoEndpointsServiceExists: true,
expectedState: servicePorts{
serviceID{namespace: "ns", name: "name2"}: map[uint32]*servicePort{
7979: &servicePort{
7979: {
targetPort: intstr.IntOrString{Type: intstr.Int, IntVal: 7979},
endpoints: &v1.Endpoints{},
},
Expand All @@ -443,7 +443,7 @@ spec:
expectedNoEndpointsServiceExists: false,
expectedState: servicePorts{
serviceID{namespace: "ns", name: "name3"}: map[uint32]*servicePort{
6969: &servicePort{
6969: {
targetPort: intstr.IntOrString{Type: intstr.Int, IntVal: 6969},
endpoints: &v1.Endpoints{},
},
Expand All @@ -460,7 +460,7 @@ spec:
expectedNoEndpointsServiceExists: false,
expectedState: servicePorts{
serviceID{namespace: "ns", name: "name4"}: map[uint32]*servicePort{
5959: &servicePort{
5959: {
targetPort: intstr.IntOrString{Type: intstr.Int, IntVal: 5959},
endpoints: &v1.Endpoints{},
},
Expand Down
2 changes: 1 addition & 1 deletion controller/api/proxy/k8s_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (k *k8sResolver) localKubernetesServiceIDFromDNSName(host string) (*service
// doesn't require Kubernetes to do this, but some hosting providers like
// GKE do it, and so we need to support it for transparency.
if !matched {
hostLabels, matched = maybeStripSuffixLabels(hostLabels, []string{"cluster", "local"})
hostLabels, _ = maybeStripSuffixLabels(hostLabels, []string{"cluster", "local"})
}
// TODO:
// ```
Expand Down
8 changes: 4 additions & 4 deletions controller/api/proxy/k8s_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestGetState(t *testing.T) {
{
servicePorts: servicePorts{
serviceID{namespace: "ns", name: "name"}: map[uint32]*servicePort{
8888: &servicePort{
8888: {
addresses: []*updateAddress{
makeUpdateAddress("10.1.30.135", 7779, "ns", "world-575bf846b4-tp4hw"),
},
Expand All @@ -80,14 +80,14 @@ func TestGetState(t *testing.T) {
Namespace: "ns",
},
Subsets: []v1.EndpointSubset{
v1.EndpointSubset{
{
Addresses: []v1.EndpointAddress{
v1.EndpointAddress{
{
IP: "10.1.30.135",
TargetRef: &v1.ObjectReference{Kind: "Pod", Namespace: "ns", Name: "world-575bf846b4-tp4hw"},
},
},
Ports: []v1.EndpointPort{v1.EndpointPort{Name: "app", Port: 7779, Protocol: "TCP"}},
Ports: []v1.EndpointPort{{Name: "app", Port: 7779, Protocol: "TCP"}},
},
},
},
Expand Down
Loading

0 comments on commit 2305974

Please sign in to comment.