Re-enable golangci-lint (projectcalico#2130)

Re-enable golangci-lint
kgtw · Sep 6, 2019 · dc02bc8 · dc02bc8
1 parent dbe74a9
commit dc02bc8
Show file tree

Hide file tree

Showing 72 changed files with 665 additions and 607 deletions.
diff --git a/Makefile b/Makefile
@@ -520,12 +520,14 @@ sub-tag-images-%:
 static-checks:
 	$(MAKE) check-typha-pins golangci-lint check-packr
 
-# TODO: re-enable these linters !
-LINT_ARGS := --disable staticcheck,ineffassign,gosimple,govet,deadcode,errcheck,unused,varcheck,structcheck
+# gosimple uses too much memory for Semaphore
+ifneq ($(CI),)
+LINT_ARGS := --disable govet
+endif
 
 .PHONY: golangci-lint
 golangci-lint: $(GENERATED_FILES)
-	$(DOCKER_RUN) $(CALICO_BUILD) golangci-lint run --deadline 5m $(LINT_ARGS)
+	$(DOCKER_RUN) $(CALICO_BUILD) golangci-lint run --deadline 5m --max-issues-per-linter 0 --max-same-issues 0 $(LINT_ARGS)
 
 .PHONY: check-packr
 check-packr: bpf/packrd/packed-packr.go

diff --git a/bpf/bpf.go b/bpf/bpf.go
@@ -533,24 +533,6 @@ type progInfo struct {
 	Err    string `json:"error"`
 }
 
-type ifaceXdpProg struct {
-	Id  int    `json:"id"`
-	Tag string `json:"tag"`
-}
-
-type ifaceXdp struct {
-	Mode int          `json:"mode"`
-	Prog ifaceXdpProg `json:"prog"`
-}
-
-type ifaceInfo []struct {
-	IfIndex  int      `json:"ifindex"`
-	IfName   string   `json:"ifname"`
-	Link     string   `json:"link"` // other side of the veth pair
-	LinkType string   `json:"link_type"`
-	Xdp      ifaceXdp `json:"xdp"`
-}
-
 type cgroupProgEntry struct {
 	ID          int    `json:"id"`
 	AttachType  string `json:"attach_type"`
@@ -1801,8 +1783,6 @@ func clearSockmap(mapArgs []string) error {
 			return fmt.Errorf("failed to delete item (%v) from map (%v): %s\n%s", e.NextKey, mapArgs, err, output)
 		}
 	}
-
-	return nil
 }
 
 func (b *BPFLib) RemoveSockmap(mode FindObjectMode) error {

diff --git a/bpf/bpf_test.go b/bpf/bpf_test.go
@@ -267,7 +267,8 @@ func TestCIDRMapContent(t *testing.T) {
 			t.Fatalf("Invalid value in the map: %v", value)
 		}
 		netip := ipmask.ToIPNet()
-		if bytes.Compare(netip.IP.To16(), wantedIP.To16()) != 0 {
+		comp := bytes.Compare(netip.IP.To16(), wantedIP.To16())
+		if comp != 0 {
 			t.Fatalf("Invalid ip %v, expected %v", netip.IP, wantedIP)
 		}
 		ones, _ := netip.Mask.Size()
@@ -276,7 +277,6 @@ func TestCIDRMapContent(t *testing.T) {
 		}
 		visited[value] = struct{}{}
 	}
-	visited = nil
 
 	t.Log("Removing a non-existent element of a CIDR map should fail")
 	err = bpfDP.RemoveItemCIDRMap("foo1", IPFamilyV4, ipWrong, mask2)

diff --git a/bpf/mock_bpf_lib.go b/bpf/mock_bpf_lib.go
@@ -24,6 +24,7 @@ import (
 	"strconv"
 
 	"github.com/projectcalico/felix/labelindex"
+	log "github.com/sirupsen/logrus"
 )
 
 var id = 0
@@ -170,7 +171,7 @@ func (b *MockBPFLib) DumpFailsafeMap() ([]ProtoPort, error) {
 		return nil, fmt.Errorf("failsafe map not found")
 	}
 
-	for k, _ := range b.FailsafeMap.M {
+	for k := range b.FailsafeMap.M {
 		ret = append(ret, k)
 	}
 
@@ -225,7 +226,7 @@ func (b *MockBPFLib) GetXDPMode(ifName string) (XDPMode, error) {
 
 func (b *MockBPFLib) GetXDPIfaces() ([]string, error) {
 	var ret []string
-	for ifName, _ := range b.XDPProgs {
+	for ifName := range b.XDPProgs {
 		ret = append(ret, ifName)
 	}
 	return ret, nil
@@ -289,7 +290,7 @@ func (b *MockBPFLib) IsValidMap(ifName string, family IPFamily) (bool, error) {
 func (b *MockBPFLib) ListCIDRMaps(family IPFamily) ([]string, error) {
 	var ret []string
 
-	for k, _ := range b.CIDRMaps {
+	for k := range b.CIDRMaps {
 		ret = append(ret, k.IfName)
 	}
 
@@ -572,7 +573,10 @@ func NewMockSockMap(mapID int) SockMap {
 
 func GetMockXDPTag(bytes []byte) string {
 	h := sha1.New()
-	h.Write(bytes)
+	_, err := h.Write(bytes)
+	if err != nil {
+		log.WithError(err).Panic("failed to write rule hash")
+	}
 	checksum := hex.EncodeToString(h.Sum(nil))
 
 	return string(checksum[:16])
@@ -607,14 +611,14 @@ func (b *MockBPFLib) RemoveSockmap(mode FindObjectMode) error {
 	return nil
 }
 
-func (b *MockBPFLib) loadBPF(objPath, progPath, progType string, mapArgs []string) error {
-	// this is just a refactoring with no real functionality for the mock BPF
-	// library, just succeed
+func (b *MockBPFLib) LoadSockops(objPath string) error {
+	// we don't do anything with the sockops program so just succeed
 	return nil
 }
 
-func (b *MockBPFLib) LoadSockops(objPath string) error {
-	// we don't do anything with the sockops program so just succeed
+func (b *MockBPFLib) loadBPF(objPath, progPath, progType string, mapArgs []string) error {
+	// this is just a refactoring with no real functionality for the mock BPF
+	// library, just succeed
 	return nil
 }
 
@@ -732,7 +736,7 @@ func (b *MockBPFLib) DumpSockmapEndpointsMap(family IPFamily) ([]CIDRMapKey, err
 
 	var ret []CIDRMapKey
 
-	for k, _ := range b.SockmapEndpointsMap.M {
+	for k := range b.SockmapEndpointsMap.M {
 		ip := net.IPv4(k.Ip[0], k.Ip[1], k.Ip[2], k.Ip[3])
 		ipnet := net.IPNet{
 			IP:   ip,

diff --git a/calc/calc_graph_test.go b/calc/calc_graph_test.go
@@ -35,8 +35,8 @@ import (
 
 var testIP = mustParseIP("10.0.0.1")
 var testIP2 = mustParseIP("10.0.0.2")
-var testIPAs6 = net.IP{testIP.To16()}
-var testIPAs4 = net.IP{testIP.To4()}
+var testIPAs6 = net.IP{IP: testIP.To16()}
+var testIPAs4 = net.IP{IP: testIP.To4()}
 
 var _ = DescribeTable("Calculation graph pass-through tests",
 	func(key model.Key, input interface{}, expUpdate interface{}, expRemove interface{}) {

diff --git a/calc/policy_sorter_test.go b/calc/policy_sorter_test.go
@@ -32,10 +32,10 @@ var _ = DescribeTable("PolKV should stringify correctly",
 		Expect(kv.String()).To(Equal(expected))
 	},
 	Entry("zero", PolKV{}, "(nil policy)"),
-	Entry("nil policy", PolKV{Key: model.PolicyKey{"name"}}, "name(nil policy)"),
+	Entry("nil policy", PolKV{Key: model.PolicyKey{Name: "name"}}, "name(nil policy)"),
 	Entry("nil order",
-		PolKV{Key: model.PolicyKey{"name"}, Value: &model.Policy{}}, "name(default)"),
+		PolKV{Key: model.PolicyKey{Name: "name"}, Value: &model.Policy{}}, "name(default)"),
 	Entry("order set",
-		PolKV{Key: model.PolicyKey{"name"}, Value: &model.Policy{Order: &tenPointFive}},
+		PolKV{Key: model.PolicyKey{Name: "name"}, Value: &model.Policy{Order: &tenPointFive}},
 		"name(10.5)"),
 )
diff --git a/calc/resources_for_test.go b/calc/resources_for_test.go
@@ -56,10 +56,10 @@ var (
 
 // Canned workload endpoints.
 
-var localWlEpKey1 = WorkloadEndpointKey{localHostname, "orch", "wl1", "ep1"}
-var remoteWlEpKey1 = WorkloadEndpointKey{remoteHostname, "orch", "wl1", "ep1"}
+var localWlEpKey1 = WorkloadEndpointKey{Hostname: localHostname, OrchestratorID: "orch", WorkloadID: "wl1", EndpointID: "ep1"}
+var remoteWlEpKey1 = WorkloadEndpointKey{Hostname: remoteHostname, OrchestratorID: "orch", WorkloadID: "wl1", EndpointID: "ep1"}
 var localWlEp1Id = "orch/wl1/ep1"
-var localWlEpKey2 = WorkloadEndpointKey{localHostname, "orch", "wl2", "ep2"}
+var localWlEpKey2 = WorkloadEndpointKey{Hostname: localHostname, OrchestratorID: "orch", WorkloadID: "wl2", EndpointID: "ep2"}
 var localWlEp2Id = "orch/wl2/ep2"
 
 var localWlEp1 = WorkloadEndpoint{
@@ -150,13 +150,6 @@ var localWlEp1DifferentIPs = WorkloadEndpoint{
 	},
 }
 
-var ep1IPs = []string{
-	"10.0.0.1", // ep1
-	"fc00:fe11::1",
-	"10.0.0.2", // shared with ep2
-	"fc00:fe11::2",
-}
-
 var localWlEp2 = WorkloadEndpoint{
 	State:      "active",
 	Name:       "cali2",
@@ -598,10 +591,6 @@ var ipPoolKey = IPPoolKey{
 	CIDR: mustParseNet("10.0.0.0/16"),
 }
 
-var ipPoolNoEncap = IPPool{
-	CIDR: mustParseNet("10.0.0.0/16"),
-}
-
 var ipPoolWithIPIP = IPPool{
 	CIDR:     mustParseNet("10.0.0.0/16"),
 	IPIPMode: encap.Always,

diff --git a/calc/rule_convert.go b/calc/rule_convert.go
@@ -43,13 +43,19 @@ func parsedRulesToProtoRules(in []*ParsedRule, ruleIDSeed string) (out []*proto.
 
 func fillInRuleIDs(rules []*proto.Rule, ruleIDSeed string) {
 	s := sha256.New224()
-	s.Write([]byte(ruleIDSeed))
+	_, err := s.Write([]byte(ruleIDSeed))
+	if err != nil {
+		log.WithError(err).Panic("failed to write rule hash")
+	}
 	hash := s.Sum(nil)
 	for ii, rule := range rules {
 		// Each hash chains in the previous hash, so that its position in the chain and
 		// the rules before it affect its hash.
 		s.Reset()
-		s.Write(hash)
+		_, err = s.Write(hash)
+		if err != nil {
+			log.WithError(err).WithField("rule", rule).Panic("Failed to write hash for rule")
+		}
 
 		// We need a form of the rule that we can hash.  Convert it to the protobuf
 		// binary representation, which is deterministic, at least for a given rev of the
@@ -60,7 +66,10 @@ func fillInRuleIDs(rules []*proto.Rule, ruleIDSeed string) {
 		if err != nil {
 			log.WithError(err).WithField("rule", rule).Panic("Failed to marshal rule")
 		}
-		s.Write(data)
+		_, err = s.Write(data)
+		if err != nil {
+			log.WithError(err).WithField("rule", rule).Panic("Failed to write marshalled rule")
+		}
 		hash = s.Sum(hash[0:0])
 		// Encode the hash using a compact character set.  We use the URL-safe base64
 		// variant because it uses '-' and '_', which are more shell-friendly.

diff --git a/calc/rule_convert_test.go b/calc/rule_convert_test.go
@@ -97,15 +97,15 @@ var fullyLoadedProtoRule = proto.Rule{
 	IpVersion: proto.IPVersion_IPV4,
 
 	Protocol: &proto.Protocol{
-		NumberOrName: &proto.Protocol_Number{123},
+		NumberOrName: &proto.Protocol_Number{Number: 123},
 	},
 
 	SrcNet:   []string{"10.0.0.0/8"},
 	SrcPorts: []*proto.PortRange{{First: 10, Last: 10}},
 	DstNet:   []string{"11.0.0.0/16"},
 	DstPorts: []*proto.PortRange{{First: 123, Last: 456}},
 
-	Icmp: &proto.Rule_IcmpTypeCode{&proto.IcmpTypeAndCode{
+	Icmp: &proto.Rule_IcmpTypeCode{IcmpTypeCode: &proto.IcmpTypeAndCode{
 		Type: 10,
 		Code: 12,
 	}},
@@ -114,7 +114,7 @@ var fullyLoadedProtoRule = proto.Rule{
 	DstIpSetIds: []string{"dstID1", "dstID2"},
 
 	NotProtocol: &proto.Protocol{
-		NumberOrName: &proto.Protocol_Name{"tcp"},
+		NumberOrName: &proto.Protocol_Name{Name: "tcp"},
 	},
 
 	NotSrcNet:   []string{"12.0.0.0/8"},
@@ -127,7 +127,7 @@ var fullyLoadedProtoRule = proto.Rule{
 	NotSrcNamedPortIpSetIds: []string{"notSrcNP"},
 	NotDstNamedPortIpSetIds: []string{"notDstNP"},
 
-	NotIcmp: &proto.Rule_NotIcmpTypeCode{&proto.IcmpTypeAndCode{
+	NotIcmp: &proto.Rule_NotIcmpTypeCode{NotIcmpTypeCode: &proto.IcmpTypeAndCode{
 		Type: 11,
 		Code: 13,
 	}},
@@ -152,9 +152,8 @@ var fullyLoadedProtoRule = proto.Rule{
 	},
 
 	HttpMatch: &proto.HTTPMatch{Methods: []string{"GET", "POST"},
-		Paths: []*proto.HTTPMatch_PathMatch{
-			{&proto.HTTPMatch_PathMatch_Exact{Exact: "/foo"}},
-			{&proto.HTTPMatch_PathMatch_Prefix{Prefix: "/bar"}},
+		Paths: []*proto.HTTPMatch_PathMatch{{PathMatch: &proto.HTTPMatch_PathMatch_Exact{Exact: "/foo"}},
+			{PathMatch: &proto.HTTPMatch_PathMatch_Prefix{Prefix: "/bar"}},
 		}},
 }
 
@@ -274,5 +273,5 @@ func mustParseCalicoIPNet(s string) *net.IPNet {
 	if err != nil {
 		panic(err)
 	}
-	return &net.IPNet{*ipNet}
+	return &net.IPNet{IPNet: *ipNet}
 }
diff --git a/calc/state_test.go b/calc/state_test.go
@@ -272,7 +272,7 @@ func (s State) KVDeltas(prev State) []api.Update {
 		if !currentKeys.Contains(kvToPath(kv)) {
 			deltas = append(
 				deltas,
-				api.Update{model.KVPair{Key: kv.Key}, api.UpdateTypeKVDeleted},
+				api.Update{KVPair: model.KVPair{Key: kv.Key}, UpdateType: api.UpdateTypeKVDeleted},
 			)
 		}
 	}
@@ -282,7 +282,7 @@ func (s State) KVDeltas(prev State) []api.Update {
 			if updatedKVs[kvToPath(kv)] {
 				updateType = api.UpdateTypeKVUpdated
 			}
-			deltas = append(deltas, api.Update{kv, updateType})
+			deltas = append(deltas, api.Update{KVPair: kv, UpdateType: updateType})
 		}
 	}
 	return deltas