Add new linters

We enable the staticcheck and gosec linters in golangci-lint.
The most important one here is gosec, as it will be useful when
answering questions for the CII Best Practices self-certification
questionnaire (see #989).

Running these linters did not reveal any noticeable issue in the Antrea
codebase.

.golangci.yml

@@ -12,3 +12,5 @@ linters:
     - misspell
     - gofmt
     - deadcode
+    - staticcheck
+    - gosec

pkg/agent/agent_test.go

@@ -55,9 +55,7 @@ func TestInitstore(t *testing.T) {
 	initializer.nodeConfig = &config.NodeConfig{UplinkNetConfig: &uplinkNetConfig}
 
 	err := initializer.initInterfaceStore()
-	if err == nil {
-		t.Errorf("Failed to handle OVS return error")
-	}
+	assert.Error(t, err, "failed to handle OVS return error")
 
 	uuid1 := uuid.New().String()
 	uuid2 := uuid.New().String()
@@ -79,13 +77,13 @@ func TestInitstore(t *testing.T) {
 	initOVSPorts := []ovsconfig.OVSPortData{ovsPort1, ovsPort2}
 
 	mockOVSBridgeClient.EXPECT().GetPortList().Return(initOVSPorts, ovsconfig.NewTransactionError(fmt.Errorf("Failed to list OVS ports"), true))
-	err = initializer.initInterfaceStore()
+	initializer.initInterfaceStore()
 	if store.Len() != 0 {
 		t.Errorf("Failed to load OVS port in store")
 	}
 
 	mockOVSBridgeClient.EXPECT().GetPortList().Return(initOVSPorts, nil)
-	err = initializer.initInterfaceStore()
+	initializer.initInterfaceStore()
 	if store.Len() != 2 {
 		t.Errorf("Failed to load OVS port in store")
 	}

pkg/agent/apiserver/apiserver.go

@@ -49,8 +49,9 @@ import (
 const Name = "antrea-agent-api"
 
 var (
-	scheme    = runtime.NewScheme()
-	codecs    = serializer.NewCodecFactory(scheme)
+	scheme = runtime.NewScheme()
+	codecs = serializer.NewCodecFactory(scheme)
+	// #nosec G101: false positive triggered by variable name which includes "token"
 	TokenPath = "/var/run/antrea/apiserver/loopback-client-token"
 )
 

pkg/agent/client.go

@@ -137,6 +137,7 @@ func (p *antreaClientProvider) updateAntreaClient() error {
 // running inside a pod running on kubernetes. It will return error
 // if called from a process not running in a kubernetes environment.
 func inClusterConfig(caBundle []byte) (*rest.Config, error) {
+	// #nosec G101: false positive triggered by variable name which includes "token"
 	const tokenFile = "/var/run/secrets/kubernetes.io/serviceaccount/token"
 	host, port := os.Getenv("ANTREA_SERVICE_HOST"), os.Getenv("ANTREA_SERVICE_PORT")
 	if len(host) == 0 || len(port) == 0 {

pkg/agent/cniserver/server_test.go

@@ -143,6 +143,7 @@ func TestIPAMService(t *testing.T) {
 		ipamMock.EXPECT().Add(gomock.Any(), gomock.Any()).Times(1)
 		cniConfig, _ := cniServer.checkRequestMessage(&requestMsg)
 		_, err := ipam.ExecIPAMAdd(cniConfig.CniCmdArgs, cniConfig.IPAM.Type, cniConfig.getInfraContainer())
+		require.Nil(t, err, "expected no Add error")
 
 		ipamMock.EXPECT().Del(gomock.Any(), gomock.Any()).Return(fmt.Errorf("IPAM delete error"))
 		response, err := cniServer.CmdDel(cxt, &requestMsg)

pkg/agent/config/traffic_encap_mode.go

@@ -42,7 +42,7 @@ var (
 // Otherwise, false and undefined value is returned
 func GetTrafficEncapModeFromStr(str string) (bool, TrafficEncapModeType) {
 	for idx, ms := range modeStrs {
-		if strings.ToLower(ms) == strings.ToLower(str) {
+		if strings.EqualFold(ms, str) {
 			return true, TrafficEncapModeType(idx)
 		}
 	}

pkg/agent/controller/networkpolicy/cache.go

@@ -15,7 +15,7 @@
 package networkpolicy
 
 import (
-	"crypto/sha1"
+	"crypto/sha1" // #nosec G505: not used for security purposes
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
@@ -80,7 +80,7 @@ type rule struct {
 
 // hashRule calculates a string based on the rule's content.
 func hashRule(r *rule) string {
-	hash := sha1.New()
+	hash := sha1.New() // #nosec G401: not used for security purposes
 	b, _ := json.Marshal(r)
 	hash.Write(b)
 	hashValue := hex.EncodeToString(hash.Sum(nil))

pkg/agent/controller/networkpolicy/reconciler.go

@@ -15,7 +15,7 @@
 package networkpolicy
 
 import (
-	"crypto/md5"
+	"crypto/md5" // #nosec G501: not used for security purposes
 	"encoding/hex"
 	"fmt"
 	"net"
@@ -66,7 +66,7 @@ type servicesHash string
 // actual values of the nested objects to ensure the hash does not change when
 // a pointer changes.
 func hashServices(services []v1beta1.Service) servicesHash {
-	hasher := md5.New()
+	hasher := md5.New() // #nosec G401: not used for security purposes
 	printer.Fprintf(hasher, "%#v", services)
 	return servicesHash(hex.EncodeToString(hasher.Sum(nil)[0:]))
 }

pkg/agent/openflow/pipeline.go

@@ -1184,13 +1184,13 @@ func (c *client) serviceLearnFlow(groupID binding.GroupIDType, svcIP net.IP, svc
 		Action().Learn(sessionAffinityTable, priorityNormal, affinityTimeout, 0, cookieID).
 		DeleteLearned()
 	if protocol == binding.ProtocolTCP {
-		learnFlowBuilder = learnFlowBuilder.MatchTCPDstPort(svcPort)
+		learnFlowBuilder.MatchTCPDstPort(svcPort)
 		learnFlowBuilderLearnAction = learnFlowBuilderLearnAction.MatchLearnedTCPDstPort()
 	} else if protocol == binding.ProtocolUDP {
-		learnFlowBuilder = learnFlowBuilder.MatchUDPDstPort(svcPort)
+		learnFlowBuilder.MatchUDPDstPort(svcPort)
 		learnFlowBuilderLearnAction = learnFlowBuilderLearnAction.MatchLearnedUDPDstPort()
 	} else if protocol == binding.ProtocolSCTP {
-		learnFlowBuilder = learnFlowBuilder.MatchSCTPDstPort(svcPort)
+		learnFlowBuilder.MatchSCTPDstPort(svcPort)
 		learnFlowBuilderLearnAction = learnFlowBuilderLearnAction.MatchLearnedSCTPDstPort()
 	}
 	return learnFlowBuilderLearnAction.

pkg/agent/util/net.go

@@ -15,7 +15,7 @@
 package util
 
 import (
-	"crypto/sha1"
+	"crypto/sha1" // #nosec G505: not used for security purposes
 	"encoding/hex"
 	"fmt"
 	"io"
@@ -29,7 +29,7 @@ const (
 )
 
 func generateInterfaceName(key string, name string, useHead bool) string {
-	hash := sha1.New()
+	hash := sha1.New() // #nosec G401: not used for security purposes
 	io.WriteString(hash, key)
 	interfaceKey := hex.EncodeToString(hash.Sum(nil))
 	prefix := name

pkg/antctl/command_definition.go

@@ -494,7 +494,9 @@ func (cd *commandDefinition) tableOutput(obj interface{}, writer io.Writer) erro
 			for k := range m {
 				args = append(args, k)
 			}
-			break
+			// break after one iteration intentionally (we are just retrieving attribute
+			// names to use as the table header in the output)
+			break // nolint:staticcheck
 		}
 	} else {
 		m, _ := target.(map[string]interface{})

pkg/antctl/raw/supportbundle/command.go

@@ -48,10 +48,11 @@ import (
 )
 
 const (
-	barTmpl      pb.ProgressBarTemplate = `{{string . "prefix"}}{{bar . }} {{percent . }} {{rtime . "ETA %s"}}` // Example: 'Prefix[-->______] 20%'
-	requestRate                         = 50
-	requestBurst                        = 100
-	timeFormat                          = "20060102T150405Z0700"
+	barTmpl pb.ProgressBarTemplate = `{{string . "prefix"}}{{bar . }} {{percent . }} {{rtime . "ETA %s"}}` // Example: 'Prefix[-->______] 20%'
+
+	requestRate  = 50
+	requestBurst = 100
+	timeFormat   = "20060102T150405Z0700"
 )
 
 // Command is the support bundle command implementation.

pkg/antctl/transform/controllerinfo/transform.go

@@ -32,7 +32,7 @@ type Response struct {
 	Version                     string                                  `json:"version,omitempty"`                     // Antrea binary version
 	PodRef                      corev1.ObjectReference                  `json:"podRef,omitempty"`                      // The Pod that Antrea Controller is running in
 	NodeRef                     corev1.ObjectReference                  `json:"nodeRef,omitempty"`                     // The Node that Antrea Controller is running in
-	ServiceRef                  corev1.ObjectReference                  `json:"serviceRef, omitempty"`                 // Antrea Controller Service
+	ServiceRef                  corev1.ObjectReference                  `json:"serviceRef,omitempty"`                  // Antrea Controller Service
 	NetworkPolicyControllerInfo clusterinfo.NetworkPolicyControllerInfo `json:"networkPolicyControllerInfo,omitempty"` // Antrea Controller NetworkPolicy information
 	ConnectedAgentNum           int32                                   `json:"connectedAgentNum,omitempty"`           // Number of agents which are connected to this controller
 	ControllerConditions        []clusterinfo.ControllerCondition       `json:"controllerConditions,omitempty"`        // Controller condition contains types like ControllerHealthy

pkg/antctl/transform/rule/transform.go

@@ -31,7 +31,7 @@ type ipBlock struct {
 
 type peer struct {
 	AddressGroups []string  `json:"addressGroups,omitempty"`
-	IPBlocks      []ipBlock `json:"ipBlocks,omitempty" json:"ipBlocks,omitempty"`
+	IPBlocks      []ipBlock `json:"ipBlocks,omitempty"`
 }
 
 type Response struct {

pkg/apis/clusterinformation/v1beta1/types.go

@@ -78,7 +78,7 @@ type AntreaControllerInfo struct {
 	Version                     string                      `json:"version,omitempty"`                     // Antrea binary version
 	PodRef                      corev1.ObjectReference      `json:"podRef,omitempty"`                      // The Pod that Antrea Controller is running in
 	NodeRef                     corev1.ObjectReference      `json:"nodeRef,omitempty"`                     // The Node that Antrea Controller is running in
-	ServiceRef                  corev1.ObjectReference      `json:"serviceRef, omitempty"`                 // Antrea Controller Service
+	ServiceRef                  corev1.ObjectReference      `json:"serviceRef,omitempty"`                  // Antrea Controller Service
 	NetworkPolicyControllerInfo NetworkPolicyControllerInfo `json:"networkPolicyControllerInfo,omitempty"` // Antrea Controller NetworkPolicy information
 	ConnectedAgentNum           int32                       `json:"connectedAgentNum,omitempty"`           // Number of agents which are connected to this controller
 	ControllerConditions        []ControllerCondition       `json:"controllerConditions,omitempty"`        // Controller condition contains types like ControllerHealthy

pkg/apis/networking/sets.go

@@ -15,7 +15,7 @@
 package networking
 
 import (
-	"crypto/md5"
+	"crypto/md5" // #nosec G501: not used for security purposes
 	"encoding/hex"
 
 	"github.com/davecgh/go-spew/spew"
@@ -41,7 +41,7 @@ type GroupMemberPodSet map[groupMemberPodHash]*GroupMemberPod
 // actual values of the nested objects to ensure the hash does not change when
 // a pointer changes.
 func hashGroupMemberPod(pod *GroupMemberPod) groupMemberPodHash {
-	hasher := md5.New()
+	hasher := md5.New() // #nosec G401: not used for security purposes
 	hashObj := GroupMemberPod{Pod: pod.Pod, IP: pod.IP}
 	printer.Fprintf(hasher, "%#v", hashObj)
 	return groupMemberPodHash(hex.EncodeToString(hasher.Sum(nil)[0:]))

pkg/apis/networking/v1beta1/sets.go

@@ -15,7 +15,7 @@
 package v1beta1
 
 import (
-	"crypto/md5"
+	"crypto/md5" // #nosec G501: not used for security purposes
 	"encoding/hex"
 
 	"github.com/davecgh/go-spew/spew"
@@ -41,7 +41,7 @@ type GroupMemberPodSet map[groupMemberPodHash]*GroupMemberPod
 // actual values of the nested objects to ensure the hash does not change when
 // a pointer changes.
 func hashGroupMemberPod(pod *GroupMemberPod) groupMemberPodHash {
-	hasher := md5.New()
+	hasher := md5.New() // #nosec G401: not used for security purposes
 	hashObj := GroupMemberPod{Pod: pod.Pod, IP: pod.IP}
 	printer.Fprintf(hasher, "%#v", hashObj)
 	return groupMemberPodHash(hex.EncodeToString(hasher.Sum(nil)[0:]))

pkg/apiserver/apiserver.go

@@ -43,7 +43,8 @@ var (
 	Scheme = runtime.NewScheme()
 	// Codecs provides methods for retrieving codecs and serializers for specific
 	// versions and content types.
-	Codecs    = serializer.NewCodecFactory(Scheme)
+	Codecs = serializer.NewCodecFactory(Scheme)
+	// #nosec G101: false positive triggered by variable name which includes "token"
 	TokenPath = "/var/run/antrea/apiserver/loopback-client-token"
 )
 

pkg/apiserver/registry/system/supportbundle/rest.go

@@ -21,7 +21,6 @@ import (
 	"crypto/sha256"
 	"fmt"
 	"io"
-	"math/rand"
 	"os"
 	"path/filepath"
 	"strings"
@@ -214,7 +213,7 @@ func (r *supportBundleREST) collect(ctx context.Context, dumpers ...func(string)
 			return nil, err
 		}
 	}
-	outputFile, err := defaultFS.Create(filepath.Join(afero.GetTempDir(defaultFS, ""), fmt.Sprintf("bundle_%d.tar.gz", rand.Int())))
+	outputFile, err := afero.TempFile(defaultFS, "", "bundle_*.tar.gz")
 	if err != nil {
 		return nil, fmt.Errorf("error when creating output tarfile: %w", err)
 	}

pkg/controller/networkpolicy/networkpolicy_controller.go

@@ -316,6 +316,8 @@ func (n *NetworkPolicyController) createAppliedToGroup(npNsName string, pSel, nS
 	groupSelector := toGroupSelector(npNsName, pSel, nSel)
 	appliedToGroupUID := getNormalizedUID(groupSelector.NormalizedName)
 	// Get or create a AppliedToGroup for the generated UID.
+	// Ignoring returned error (here and elsewhere in this file) as with the
+	// current store implementation, no error is ever returned.
 	_, found, _ := n.appliedToGroupStore.Get(appliedToGroupUID)
 	if found {
 		return appliedToGroupUID
@@ -842,10 +844,9 @@ func (n *NetworkPolicyController) updateNamespace(oldObj, curObj interface{}) {
 	curAddressGroupKeySet := n.filterAddressGroupsForNamespace(curNamespace)
 	// Find groups matching the old Namespace's labels.
 	oldAddressGroupKeySet := n.filterAddressGroupsForNamespace(oldNamespace)
-	addressGroupKeys := sets.String{}
 	// No need to enqueue common AddressGroups as they already have latest
 	// Namespace information.
-	addressGroupKeys = oldAddressGroupKeySet.Difference(curAddressGroupKeySet).Union(curAddressGroupKeySet.Difference(oldAddressGroupKeySet))
+	addressGroupKeys := oldAddressGroupKeySet.Difference(curAddressGroupKeySet).Union(curAddressGroupKeySet.Difference(oldAddressGroupKeySet))
 	for group := range addressGroupKeys {
 		n.enqueueAddressGroup(group)
 	}
@@ -1101,7 +1102,7 @@ func (n *NetworkPolicyController) syncAddressGroup(key string) error {
 	if err != nil {
 		return fmt.Errorf("unable to filter internal NetworkPolicies for AddressGroup %s: %v", key, err)
 	}
-	addressGroupObj, found, err := n.addressGroupStore.Get(key)
+	addressGroupObj, found, _ := n.addressGroupStore.Get(key)
 	if !found {
 		// AddressGroup was already deleted. No need to process further.
 		klog.V(2).Infof("AddressGroup %s not found.", key)
@@ -1208,7 +1209,7 @@ func (n *NetworkPolicyController) syncAppliedToGroup(key string) error {
 	podSetByNode := make(map[string]networking.GroupMemberPodSet)
 	var pods []*v1.Pod
 	appGroupNodeNames := sets.String{}
-	appliedToGroupObj, found, err := n.appliedToGroupStore.Get(key)
+	appliedToGroupObj, found, _ := n.appliedToGroupStore.Get(key)
 	if !found {
 		klog.V(2).Infof("AppliedToGroup %s not found.", key)
 		return nil
@@ -1297,11 +1298,11 @@ func (n *NetworkPolicyController) syncInternalNetworkPolicy(key string) error {
 	// same internal NetworkPolicy is being updated in the NetworkPolicy UPDATE
 	// handler.
 	n.internalNetworkPolicyMutex.Lock()
-	internalNPObj, found, err := n.internalNetworkPolicyStore.Get(key)
+	internalNPObj, found, _ := n.internalNetworkPolicyStore.Get(key)
 	if !found {
 		// Make sure to unlock the store before returning.
 		n.internalNetworkPolicyMutex.Unlock()
-		return fmt.Errorf("internal NetworkPolicy %s not found: %v", key, err)
+		return fmt.Errorf("internal NetworkPolicy %s not found", key)
 	}
 	internalNP := internalNPObj.(*antreatypes.NetworkPolicy)
 	// Maintain a copy of old SpanMeta Nodenames so we can later enqueue Groups

test/e2e/basic_test.go

@@ -84,6 +84,9 @@ func (data *TestData) testDeletePod(t *testing.T, podName string, nodeName strin
 
 	cmds := []string{"antctl", "get", "podinterface", podName, "-n", testNamespace, "-o", "json"}
 	stdout, _, err := runAntctl(antreaPodName, cmds, data)
+	if err != nil {
+		t.Fatalf("Error when running antctl: %v", err)
+	}
 	var podInterfaces []podinterface.Response
 	if err := json.Unmarshal([]byte(stdout), &podInterfaces); err != nil {
 		t.Fatalf("Error when querying the pod interface: %v", err)

test/e2e/framework.go

@@ -1053,6 +1053,9 @@ func (data *TestData) GetEncapMode() (config.TrafficEncapModeType, error) {
 
 func (data *TestData) GetAntreaConfigMap(antreaNamespace string) (*v1.ConfigMap, error) {
 	deployment, err := data.clientset.AppsV1().Deployments(antreaNamespace).Get(context.TODO(), antreaDeployment, metav1.GetOptions{})
+	if err != nil {
+		return nil, fmt.Errorf("failed to retrieve Antrea Controller deployment: %v", err)
+	}
 	var configMapName string
 	for _, volume := range deployment.Spec.Template.Spec.Volumes {
 		if volume.ConfigMap != nil && volume.Name == antreaConfigVolume {
@@ -1061,11 +1064,11 @@ func (data *TestData) GetAntreaConfigMap(antreaNamespace string) (*v1.ConfigMap,
 		}
 	}
 	if len(configMapName) == 0 {
-		return nil, fmt.Errorf("Failed to locate %s ConfigMap volume", antreaConfigVolume)
+		return nil, fmt.Errorf("failed to locate %s ConfigMap volume", antreaConfigVolume)
 	}
 	configMap, err := data.clientset.CoreV1().ConfigMaps(antreaNamespace).Get(context.TODO(), configMapName, metav1.GetOptions{})
 	if err != nil {
-		return nil, fmt.Errorf("Failed to get ConfigMap %s: %v", configMapName, err)
+		return nil, fmt.Errorf("failed to get ConfigMap %s: %v", configMapName, err)
 	}
 	return configMap, nil
 }

test/e2e/networkpolicy_test.go

@@ -333,6 +333,9 @@ func waitForAgentCondition(t *testing.T, data *TestData, podName string, conditi
 	if err := wait.Poll(1*time.Second, defaultTimeout, func() (bool, error) {
 		cmds := []string{"antctl", "get", "agentinfo", "-o", "json"}
 		stdout, _, err := runAntctl(podName, cmds, data)
+		if err != nil {
+			return true, err
+		}
 		var agentInfo agentinfo.AntreaAgentInfoResponse
 		err = json.Unmarshal([]byte(stdout), &agentInfo)
 		if err != nil {

test/e2e/performance_test.go

@@ -32,14 +32,15 @@ import (
 )
 
 const (
-	seed                            uint64 = 0xA1E47 // Use a specific rand seed to make the generated workloads always same
-	perfTestAppLabel                       = "antrea-perf-test"
-	podsConnectionNetworkPolicyName        = "pods.ingress"
-	workloadNetworkPolicyName              = "workloads.ingress"
-	perftoolImage                          = "antrea/perftool"
-	nginxImage                             = "nginx"
-	perftoolContainerName                  = "perftool"
-	nginxContainerName                     = "nginx"
+	seed uint64 = 0xA1E47 // Use a specific rand seed to make the generated workloads always same
+
+	perfTestAppLabel                = "antrea-perf-test"
+	podsConnectionNetworkPolicyName = "pods.ingress"
+	workloadNetworkPolicyName       = "workloads.ingress"
+	perftoolImage                   = "antrea/perftool"
+	nginxImage                      = "nginx"
+	perftoolContainerName           = "perftool"
+	nginxContainerName              = "nginx"
 )
 
 var (
@@ -250,7 +251,11 @@ func networkPolicyRealize(policyRules int, data *TestData, b *testing.B) {
 		go func() {
 			err := populateWorkloadNetworkPolicy(generateWorkloadNetworkPolicy(policyRules), data)
 			if err != nil {
-				b.Fatalf("Error when populating workload network policy: %v", err)
+				// cannot use Fatal in a goroutine
+				// if populating policies fails, waitNetworkPolicyRealize will
+				// eventually time out and the test will fail, although it would be
+				// better to fail early in that case.
+				b.Errorf("Error when populating workload network policy: %v", err)
 			}
 		}()
 

test/e2e/prometheus_test.go

@@ -104,6 +104,7 @@ func getMonitoringAuthToken(t *testing.T, data *TestData) string {
 
 // getMetricsFromApiServer retrieves Antrea metrics from Pod apiserver
 func getMetricsFromApiServer(t *testing.T, url string, token string) string {
+	// #nosec G402: ignore insecure options in test code
 	config := &tls.Config{
 		InsecureSkipVerify: true,
 	}