Skip to content

Commit

Permalink
Introduce ineffassign tooling, fixes
Browse files Browse the repository at this point in the history
`ineffassign` detects ineffectual assignments in Go code. This change
introduces a `bin/ineffassign` into ci, and fixes 4 ineffectual
assignments caught by ineffassign.

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 committed Feb 9, 2019
1 parent 030767d commit 6f95c8d
Show file tree
Hide file tree
Showing 35 changed files with 262 additions and 229 deletions.
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ jobs:
- ./bin/protoc-go.sh
- go test -race -v ./...
- go vet ./...
- ./bin/ineffassign
- ./bin/lint

- language: node_js
Expand Down
9 changes: 9 additions & 0 deletions Gopkg.lock
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,14 @@
pruneopts = ""
revision = "104e2578924bb3b211150c19414d0144b82165bb"

[[projects]]
branch = "master"
digest = "1:0856bb71810be4fa4293ee4ecb6382f602003c928e02ac42dc9b28e4f11fff4c"
name = "github.com/gordonklaus/ineffassign"
packages = ["."]
pruneopts = ""
revision = "1003c8bd00dc2869cb5ca5282e6ce33834fed514"

[[projects]]
digest = "1:64d212c703a2b94054be0ce470303286b177ad260b2f89a307e3d1bb6c073ef6"
name = "github.com/gorilla/websocket"
Expand Down Expand Up @@ -1252,6 +1260,7 @@
"github.com/golang/protobuf/protoc-gen-go",
"github.com/golang/protobuf/ptypes",
"github.com/golang/protobuf/ptypes/duration",
"github.com/gordonklaus/ineffassign",
"github.com/gorilla/websocket",
"github.com/grpc-ecosystem/go-grpc-prometheus",
"github.com/julienschmidt/httprouter",
Expand Down
1 change: 1 addition & 0 deletions Gopkg.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
required = [
"github.com/golang/protobuf/protoc-gen-go",
"github.com/gordonklaus/ineffassign",
"golang.org/x/lint/golint",
"k8s.io/code-generator/cmd/client-gen",
"k8s.io/code-generator/cmd/deepcopy-gen",
Expand Down
6 changes: 6 additions & 0 deletions TEST.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ To analyze the Go code without running tests, run:
go vet ./...
```

To detect ineffectual assignments using ineffassign, run:

```bash
bin/ineffassign
```

To lint the Go code using golint, run:

```bash
Expand Down
15 changes: 15 additions & 0 deletions bin/ineffassign
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/bin/bash

set -eu

cd "$(pwd -P)"

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

cd "$rootdir"

# install ineffassign from vendor
go install ./vendor/github.com/gordonklaus/ineffassign

ineffassign ./
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:ef1a4319 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 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:ef1a4319 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:ef1a4319 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 6f95c8d

Please sign in to comment.