From e914f578c4b94b565b7fbbd4e6e82f302a20eb1c Mon Sep 17 00:00:00 2001 From: thiyyakat Date: Tue, 3 Dec 2024 14:04:25 +0530 Subject: [PATCH] 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 --- .ci/check | 7 +++- .ci/pipeline_definitions | 6 +-- .gitignore | 3 ++ Dockerfile | 2 +- MCM_VERSION | 2 +- Makefile | 14 ++++++- go.mod | 6 +-- go.sum | 4 +- hack/sast.sh | 44 ++++++++++++++++++++ hack/tools.mk | 23 ---------- hack/tools/bin/.gitkeep | 0 pkg/azure/provider/helpers/driver.go | 2 +- pkg/azure/testhelp/fakes/machineresources.go | 4 +- pkg/azure/testhelp/providerspec.go | 1 + pkg/azure/utils/images.go | 4 ++ pkg/azure/utils/images_test.go | 4 ++ 16 files changed, 87 insertions(+), 39 deletions(-) create mode 100755 hack/sast.sh delete mode 100644 hack/tools.mk create mode 100644 hack/tools/bin/.gitkeep diff --git a/.ci/check b/.ci/check index a3374e846..90ef66ad9 100755 --- a/.ci/check +++ b/.ci/check @@ -26,7 +26,7 @@ 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}" @@ -34,9 +34,14 @@ 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 \ No newline at end of file diff --git a/.ci/pipeline_definitions b/.ci/pipeline_definitions index 6ea27ee70..0df5a6712 100644 --- 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 index edc2772b3..aaeb90aac 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,6 @@ main # Output of the go coverage tool *coverprofile.out* + +# gosec +gosec-report.sarif \ No newline at end of file diff --git a/Dockerfile b/Dockerfile index 94ec32ae0..52df540c0 100644 --- 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 index 60110ff95..8b31ea514 100644 --- a/MCM_VERSION +++ b/MCM_VERSION @@ -1 +1 @@ -v0.54.0 \ No newline at end of file +v0.55.1 \ No newline at end of file diff --git a/Makefile b/Makefile index 3b46b975a..19f78847f 100644 --- 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 \ No newline at end of file diff --git a/go.mod b/go.mod index bf06e6f1b..1a4d5d0fe 100644 --- 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 index d892f2920..64697b913 100644 --- 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 new file mode 100755 index 000000000..50e4a9966 --- /dev/null +++ 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 ./... \ No newline at end of file diff --git a/hack/tools.mk b/hack/tools.mk deleted file mode 100644 index 4ab5b2b33..000000000 --- a/hack/tools.mk +++ /dev/null @@ -1,23 +0,0 @@ -# SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors -# -# SPDX-License-Identifier: Apache-2.0 - -TOOLS_DIR := hack/tools -TOOLS_BIN_DIR := $(TOOLS_DIR)/bin -GO_ADD_LICENSE := $(TOOLS_BIN_DIR)/addlicense - -# default tool versions -GO_ADD_LICENSE_VERSION ?= latest - -export TOOLS_BIN_DIR := $(TOOLS_BIN_DIR) -export PATH := $(abspath $(TOOLS_BIN_DIR)):$(PATH) -$(info "TOOLS_BIN_DIR from tools.mk", $(TOOLS_BIN_DIR)) -$(info "TOOLS_DIR from tools.mk", $(TOOLS_DIR)) -$(info "PATH from tools.mk", $(PATH)) - -######################################### -# Tools # -######################################### - -$(GO_ADD_LICENSE): - GOBIN=$(abspath $(TOOLS_BIN_DIR)) go install github.com/google/addlicense@$(GO_ADD_LICENSE_VERSION) \ No newline at end of file diff --git a/hack/tools/bin/.gitkeep b/hack/tools/bin/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/pkg/azure/provider/helpers/driver.go b/pkg/azure/provider/helpers/driver.go index 78c01aee8..771662fb0 100644 --- 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 index d3cc2b2b0..97df4f970 100644 --- 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 index 46620d525..7a4c4a81c 100644 --- 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 index f2be977a1..43da4ca27 100644 --- 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 index f4c9cb642..4abb96f3c 100644 --- 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 (