Skip to content

Commit

Permalink
Introduce make targets for sast and address security issues.
Browse files Browse the repository at this point in the history
  • Loading branch information
thiyyakat committed Nov 27, 2024
1 parent 009f3ef commit 762981a
Show file tree
Hide file tree
Showing 9 changed files with 69 additions and 7 deletions.
3 changes: 3 additions & 0 deletions .ci/check
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,6 @@ LINT_FOLDERS="$(echo ${PACKAGES} | sed "s|github.com/gardener/machine-controller
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ main

# Output of the go coverage tool
*coverprofile.out*

# gosec
gosec-report.sarif
13 changes: 12 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# 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)
include $(MCM_DIR)/hack/tools.mk
-include .env
export

Expand Down Expand Up @@ -126,3 +127,13 @@ clean:
.PHONY: add-license-headers
add-license-headers: $(GO_ADD_LICENSE)
@./hack/add_license_headers.sh ${YEAR}

.PHONY: sast
sast:
@cd $(MCM_DIR) && chmod u+w hack/tools/bin/ && $(MAKE) $(GOSEC)
@MCM_DIR=$(MCM_DIR) bash ./hack/sast.sh

.PHONY: sast-report
sast-report:
@cd $(MCM_DIR) && chmod u+w hack/tools/bin/ && $(MAKE) $(GOSEC)
@MCM_DIR=$(MCM_DIR) bash ./hack/sast.sh --gosec-report true
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,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.0
github.com/onsi/ginkgo/v2 v2.19.0
github.com/onsi/gomega v1.33.1
github.com/prometheus/client_golang v1.19.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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.0 h1:99wYhSMLKS5s0cYjKG6jEGIGq12QWdHYQLjaWsggDyM=
github.com/gardener/machine-controller-manager v0.55.0/go.mod h1:RPpnU8gmTrhDAd79+iKqKlbANiXCRkXoJW+z+5zSTME=
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=
Expand Down
44 changes: 44 additions & 0 deletions hack/sast.sh
Original file line number Diff line number Diff line change
@@ -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"
$MCM_DIR/hack/tools/bin/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.
$MCM_DIR/hack/tools/bin/gosec -exclude-generated -exclude-dir=hack $gosec_report_parse_flags ./...
2 changes: 1 addition & 1 deletion pkg/azure/provider/helpers/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/azure/testhelp/fakes/machineresources.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions pkg/azure/testhelp/providerspec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down

0 comments on commit 762981a

Please sign in to comment.