Introduce make targets for sast and address security issues. (#169)

* Introduce make targets for sast and address security issues. * Add license-headers, update go lang version to 1.23.3
gardener · Dec 3, 2024 · e914f57 · e914f57
1 parent 009f3ef
commit e914f57
Show file tree

Hide file tree

Showing 16 changed files with 87 additions and 39 deletions.
diff --git a/.ci/check b/.ci/check
@@ -26,17 +26,22 @@ export PATH="${GOBIN}:${PATH}"
 
 # Install golangci-lint (linting tool).
 if [[ -z "${GOLANGCI_LINT_VERSION}" ]]; then
-  export GOLANGCI_LINT_VERSION=v1.57.1
+  export GOLANGCI_LINT_VERSION=v1.60.3
 fi
 echo "Fetching golangci-lint tool"
 go install github.com/golangci/golangci-lint/cmd/golangci-lint@"${GOLANGCI_LINT_VERSION}"
 echo "Successfully fetched golangci-lint"
 golangci-lint version
 
 ###############################################################################
+cd ${SOURCE_PATH}
+
 PACKAGES="$(go list -e ./... | grep -vE '/tmp/')"
 LINT_FOLDERS="$(echo ${PACKAGES} | sed "s|github.com/gardener/machine-controller-manager-provider-azure|.|g")"
 
 echo "Executing golangci-lint"
 # golangci-lint can't be run from outside the directory
 (cd ${SOURCE_PATH} && golangci-lint run -c .golangci.yaml --timeout 10m)
+
+# Run Static Application Security Testing (SAST) using gosec
+make sast-report
diff --git a/.ci/pipeline_definitions b/.ci/pipeline_definitions
@@ -8,9 +8,9 @@ machine-controller-manager-provider-azure:
     steps_template: &steps_anchor
       steps:
         check:
-          image: 'golang:1.22.5'
+          image: 'golang:1.23.3'
         build:
-          image: 'golang:1.22.5'
+          image: 'golang:1.23.3'
           output_dir: 'binary'
         test:
           image: 'europe-docker.pkg.dev/gardener-project/releases/testmachinery/base-step:stable'
@@ -61,7 +61,7 @@ machine-controller-manager-provider-azure:
           interval: '24h'
         update_component_deps:
           set_dependency_version_script_container_image:
-            image_reference: 'golang:1.22.5'
+            image_reference: 'golang:1.23.3'
     release:
       <<: *steps_anchor
       traits:

diff --git a/.gitignore b/.gitignore
@@ -15,3 +15,6 @@ main
 
 # Output of the go coverage tool
 *coverprofile.out*
+
+# gosec
+gosec-report.sarif
diff --git a/Dockerfile b/Dockerfile
@@ -3,7 +3,7 @@
 # SPDX-License-Identifier: Apache-2.0
 
 #############      builder                          #############
-FROM golang:1.22.5 AS builder
+FROM golang:1.23.3 AS builder
 
 WORKDIR /go/src/github.com/gardener/machine-controller-manager-provider-azure
 COPY . .

diff --git a/MCM_VERSION b/MCM_VERSION
@@ -1 +1 @@
-v0.54.0
+v0.55.1
diff --git a/Makefile b/Makefile
@@ -1,8 +1,11 @@
 # SPDX-FileCopyrightText: 2019 SAP SE or an SAP affiliate company and Gardener contributors
 #
 # SPDX-License-Identifier: Apache-2.0
-
+MCM_DIR   	:= $(shell go list -m -f "{{.Dir}}" github.com/gardener/machine-controller-manager)
+TOOLS_DIR := hack/tools
+include $(MCM_DIR)/hack/tools.mk
 -include .env
+
 export
 
 BINARY_PATH         		:= bin/
@@ -13,6 +16,7 @@ PROVIDER_NAME       		:= Azure
 PROJECT_NAME        		:= gardener
 TARGET_CLUSTER_NAME			:= shoot--project--cluster-name
 IS_CONTROL_CLUSTER_SEED 	:= true
+PATH := $(abspath $(TOOLS_BIN_DIR)):$(PATH)
 
 # Below ones are used in tests
 LEADER_ELECT 	    := "true"
@@ -126,3 +130,11 @@ clean:
 .PHONY: add-license-headers
 add-license-headers: $(GO_ADD_LICENSE)
 	@./hack/add_license_headers.sh ${YEAR}
+
+.PHONY: sast
+sast: $(GOSEC)
+	@./hack/sast.sh
+
+.PHONY: sast-report
+sast-report: $(GOSEC)
+	@./hack/sast.sh --gosec-report true
diff --git a/go.mod b/go.mod
@@ -1,8 +1,6 @@
 module github.com/gardener/machine-controller-manager-provider-azure
 
-go 1.22.0
-
-toolchain go1.22.5
+go 1.23.0
 
 require (
 	github.com/Azure/azure-sdk-for-go/sdk/azcore v1.9.0
@@ -12,7 +10,7 @@ require (
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4 v4.3.0
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resourcegraph/armresourcegraph v0.8.2
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources v1.2.0
-	github.com/gardener/machine-controller-manager v0.54.0
+	github.com/gardener/machine-controller-manager v0.55.1
 	github.com/onsi/ginkgo/v2 v2.19.0
 	github.com/onsi/gomega v1.33.1
 	github.com/prometheus/client_golang v1.19.1

diff --git a/go.sum b/go.sum
@@ -40,8 +40,8 @@ github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxER
 github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc=
 github.com/fxamacker/cbor/v2 v2.7.0 h1:iM5WgngdRBanHcxugY4JySA0nk1wZorNOpTgCMedv5E=
 github.com/fxamacker/cbor/v2 v2.7.0/go.mod h1:pxXPTn3joSm21Gbwsv0w9OSA2y1HFR9qXEeXQVeNoDQ=
-github.com/gardener/machine-controller-manager v0.54.0 h1:V7EOODiaBO9VesskdCgxMvo5vgMAmtmUTdb9Y9Nwp50=
-github.com/gardener/machine-controller-manager v0.54.0/go.mod h1:RPpnU8gmTrhDAd79+iKqKlbANiXCRkXoJW+z+5zSTME=
+github.com/gardener/machine-controller-manager v0.55.1 h1:d6mTnuYko+jWeIi7tAFWgWnL1nR5hGcI6pRCDcH0TGY=
+github.com/gardener/machine-controller-manager v0.55.1/go.mod h1:eCng7De6OE15rndmMm6Q1fwMQI39esASCd3WKZ/lLmY=
 github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
 github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
 github.com/go-logr/zapr v1.3.0 h1:XGdV8XW8zdwFiwOA2Dryh1gj2KRQyOOoNmBy4EplIcQ=

diff --git a/hack/sast.sh b/hack/sast.sh
@@ -0,0 +1,44 @@
+#!/usr/bin/env bash
+#
+# SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors
+#
+# SPDX-License-Identifier: Apache-2.0
+
+set -e
+
+root_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )/.." &> /dev/null && pwd )"
+pwd
+gosec_report="false"
+gosec_report_parse_flags=""
+
+parse_flags() {
+  while test $# -gt 1; do
+    case "$1" in
+      --gosec-report)
+        shift; gosec_report="$1"
+        ;;
+      *)
+        echo "Unknown argument: $1"
+        exit 1
+        ;;
+    esac
+    shift
+  done
+}
+
+parse_flags "$@"
+
+echo "> Running gosec"
+gosec --version
+if [[ "$gosec_report" != "false" ]]; then
+  echo "Exporting report to $root_dir/gosec-report.sarif"
+  gosec_report_parse_flags="-track-suppressions -fmt=sarif -out=gosec-report.sarif -stdout"
+fi
+
+# MCM uses code-generators https://github.com/kubernetes/code-generator which create lots of G103 (CWE-242:
+# Use of unsafe calls should be audited) & G104 (CWE-703: Errors unhandled) errors.
+# However, those generators are best-pratice in Kubernetes environment and their results are tested well.
+# Thus, generated code is excluded from gosec scan.
+# Nested go modules are not supported by gosec (see https://github.com/securego/gosec/issues/501), so the ./hack folder
+# is excluded too. It does not contain productive code anyway.
+gosec -exclude-generated -exclude-dir=hack $gosec_report_parse_flags  ./...
diff --git a/hack/tools.mk b/hack/tools.mk
diff --git a/hack/tools/bin/.gitkeep b/hack/tools/bin/.gitkeep
diff --git a/pkg/azure/provider/helpers/driver.go b/pkg/azure/provider/helpers/driver.go
@@ -669,7 +669,7 @@ func LogVMCreation(location, resourceGroup string, vm *armcompute.VirtualMachine
 		}
 		msgBuilder.WriteString(" ]")
 	}
-	klog.Infof(msgBuilder.String())
+	klog.Infof("%s", msgBuilder.String())
 }
 
 func createVMCreationParams(providerSpec api.AzureProviderSpec, imageRef armcompute.ImageReference, plan *armcompute.Plan, secret *corev1.Secret, nicID, vmName string, imageRefDiskIDs map[DataDiskLun]DiskID) (armcompute.VirtualMachine, error) {

diff --git a/pkg/azure/testhelp/fakes/machineresources.go b/pkg/azure/testhelp/fakes/machineresources.go
@@ -115,7 +115,7 @@ func (m *MachineResources) HandleDataDisksOnVMDelete() {
 // HasResources checks if the MachineResources object has any of VM, NIC, OSDisk, DataDisk resources.
 // This will be used to just delete an instance of MachineResources when it has none of the resources.
 func (m *MachineResources) HasResources() bool {
-	return m.VM != nil || m.NIC != nil || m.OSDisk != nil || (m.DataDisks != nil && len(m.DataDisks) > 0)
+	return m.VM != nil || m.NIC != nil || m.OSDisk != nil || len(m.DataDisks) > 0
 }
 
 // UpdateNICDeleteOpt updates the delete option for NIC.
@@ -156,7 +156,7 @@ func (m *MachineResources) AttachDataDisk(spec api.AzureProviderSpec, diskName s
 	if _, ok := m.DataDisks[diskName]; ok {
 		return fmt.Errorf("disk %s already exists, cannot create a new disk with the same name", diskName)
 	}
-	dataDisk := createDataDisk(int32(len(m.DataDisks)+1), "None", &deleteOption, 20, testhelp.StorageAccountType, diskName)
+	dataDisk := createDataDisk(int32(len(m.DataDisks)+1), "None", &deleteOption, 20, testhelp.StorageAccountType, diskName) // #nosec G115 -- Test only
 	d := createDiskResource(spec, diskName, m.VM.ID, nil)
 	m.DataDisks[diskName] = d
 	m.VM.Properties.StorageProfile.DataDisks = append(m.VM.Properties.StorageProfile.DataDisks, dataDisk)

diff --git a/pkg/azure/testhelp/providerspec.go b/pkg/azure/testhelp/providerspec.go
@@ -125,6 +125,7 @@ func (b *ProviderSpecBuilder) WithDefaultStorageProfile() *ProviderSpecBuilder {
 func (b *ProviderSpecBuilder) WithDataDisks(diskName string, numDisks int) *ProviderSpecBuilder {
 	dataDisks := make([]api.AzureDataDisk, 0, numDisks)
 	for i := 0; i < numDisks; i++ {
+		// #nosec G115 -- Test only
 		d := api.AzureDataDisk{
 			Name:               diskName,
 			Lun:                int32(i),

diff --git a/pkg/azure/utils/images.go b/pkg/azure/utils/images.go
@@ -1,3 +1,7 @@
+// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors
+//
+// SPDX-License-Identifier: Apache-2.0
+
 package utils
 
 import (

diff --git a/pkg/azure/utils/images_test.go b/pkg/azure/utils/images_test.go
@@ -1,3 +1,7 @@
+// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors
+//
+// SPDX-License-Identifier: Apache-2.0
+
 package utils
 
 import (