From 29692b2d3d593b0972fdbdc4bffdfe83ee3710b9 Mon Sep 17 00:00:00 2001 From: aaronfern Date: Mon, 17 Feb 2025 16:17:22 +0530 Subject: [PATCH 1/3] Adapt pipeline_definition to include SAST linting logs in OCM descriptor --- .ci/check | 3 ++- .ci/pipeline_definitions | 24 +++++++++++-------- cluster-autoscaler/.gitignore | 3 +++ .../cloudprovider/mcm/mcm_manager.go | 17 +++++++++---- cluster-autoscaler/cloudprovider/utils.go | 2 +- cluster-autoscaler/hack/sast.sh | 20 ++++++++++++++++ cluster-autoscaler/integration/framework.go | 12 +++++----- cluster-autoscaler/main.go | 2 +- 8 files changed, 59 insertions(+), 24 deletions(-) diff --git a/.ci/check b/.ci/check index 5194df512d2a..fec0a4fe90e4 100755 --- a/.ci/check +++ b/.ci/check @@ -47,4 +47,5 @@ go get -u golang.org/x/lint/golint PACKAGES="$(go list -e ./... | grep -vE '/tmp/|/vendor/')" LINT_FOLDERS="$(echo ${PACKAGES} | sed "s|k8s.io/autoscaler/cluster-autoscaler|.|g")" -#TODO: To add lint checking, after fixing issues +# Run Static Application Security Testing (SAST) using gosec +make sast-report diff --git a/.ci/pipeline_definitions b/.ci/pipeline_definitions index a7122919bd9d..14c7bd6240c8 100644 --- a/.ci/pipeline_definitions +++ b/.ci/pipeline_definitions @@ -2,17 +2,11 @@ autoscaler: base_definition: repo: source_labels: - - name: 'cloud.gardener.cnudie/dso/scanning-hints/source_analysis/v1' + - name: cloud.gardener.cnudie/dso/scanning-hints/source_analysis/v1 value: - policy: 'scan' - path_config: - exclude_paths: - - '.*/aws-sdk-go/.*' - - '^vendor/.*' - - '.*/vendor/.*' - - '.*/cloudprovider/((?!mcm/).)*/.*' - - '^addon-resizer/.*' - - '^vertical-pod-autoscaler/.*' + policy: skip + comment: | + we use gosec for sast scanning. See attached log. traits: version: preprocess: @@ -62,6 +56,16 @@ autoscaler: image: europe-docker.pkg.dev/gardener-project/releases/gardener/autoscaler/cluster-autoscaler release: nextversion: 'bump_minor' + assets: + - type: build-step-log + step_name: check + purposes: + - lint + - sast + - gosec + comment: | + we use gosec (linter) for SAST scans + see: https://github.com/securego/gosec slack: default_channel: 'internal_scp_workspace' channel_cfgs: diff --git a/cluster-autoscaler/.gitignore b/cluster-autoscaler/.gitignore index 69c0a6474e57..c978dec2b498 100644 --- a/cluster-autoscaler/.gitignore +++ b/cluster-autoscaler/.gitignore @@ -12,3 +12,6 @@ Session.vim .netrwhist .vscode ./integration/logs + +# gosec +gosec-report.sarif \ No newline at end of file diff --git a/cluster-autoscaler/cloudprovider/mcm/mcm_manager.go b/cluster-autoscaler/cloudprovider/mcm/mcm_manager.go index 00b021352e12..0887cf5e00a6 100644 --- a/cluster-autoscaler/cloudprovider/mcm/mcm_manager.go +++ b/cluster-autoscaler/cloudprovider/mcm/mcm_manager.go @@ -23,6 +23,7 @@ package mcm import ( "context" + "crypto/rand" "encoding/json" "errors" "flag" @@ -36,7 +37,8 @@ import ( v1appslister "k8s.io/client-go/listers/apps/v1" "k8s.io/utils/pointer" "maps" - "math/rand" + "math" + "math/big" "net/http" "os" "slices" @@ -471,9 +473,9 @@ func (m *McmManager) SetMachineDeploymentSize(ctx context.Context, nodeGroup *no // don't scale down during rolling update, as that could remove ready node with workload if md.Spec.Replicas >= int32(size) && !isRollingUpdateFinished(md) { return false, fmt.Errorf("MachineDeployment %s is under rolling update , cannot reduce replica count", md.Name) - } + } // #nosec G115 (CWE-190) -- replicas will not overflow the range of int32 clone := md.DeepCopy() - clone.Spec.Replicas = int32(size) + clone.Spec.Replicas = int32(size) // #nosec G115 (CWE-190) -- replicas will not overflow the range of int32 _, err = m.machineClient.MachineDeployments(nodeGroup.Namespace).Update(ctx, clone, metav1.UpdateOptions{}) return true, err @@ -922,7 +924,12 @@ func getZoneValueFromMCLabels(labels map[string]string) string { func (m *McmManager) buildNodeFromTemplate(name string, template *nodeTemplate) (*apiv1.Node, error) { node := apiv1.Node{} - nodeName := fmt.Sprintf("%s-%d", name, rand.Int63()) + n, err := rand.Int(rand.Reader, big.NewInt(math.MaxInt64)) + if err != nil { + fmt.Println("error:", err) + return &node, err + } + nodeName := fmt.Sprintf("%s-%d", name, n.Int64()) node.ObjectMeta = metav1.ObjectMeta{ Name: nodeName, @@ -1118,7 +1125,7 @@ func computeScaleDownData(md *v1alpha1.MachineDeployment, machineNamesForDeletio data.RevisedScaledownAmount = uniqueForDeletionSet.Len() data.RevisedMachineDeployment = nil - expectedReplicas := md.Spec.Replicas - int32(data.RevisedScaledownAmount) + expectedReplicas := md.Spec.Replicas - int32(data.RevisedScaledownAmount) // #nosec G115 (CWE-190) -- RevisedScaledownAmount will not overflow the range of int32 if expectedReplicas == md.Spec.Replicas { klog.Infof("MachineDeployment %q is already set to %d, no need to scale-down", md.Name, expectedReplicas) } else if expectedReplicas < 0 { diff --git a/cluster-autoscaler/cloudprovider/utils.go b/cluster-autoscaler/cloudprovider/utils.go index 8242fd19c66b..02705b83d95c 100644 --- a/cluster-autoscaler/cloudprovider/utils.go +++ b/cluster-autoscaler/cloudprovider/utils.go @@ -77,7 +77,7 @@ func BuildKubeProxy(name string) *apiv1.Pod { priority := scheduling.SystemCriticalPriority return &apiv1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("kube-proxy-%s-%d", name, rand.Int63()), + Name: fmt.Sprintf("kube-proxy-%s-%d", name, rand.Int63()), // #nosec G404 (CWE-338) -- code inherited from upstream Namespace: "kube-system", Annotations: map[string]string{ kubetypes.ConfigSourceAnnotationKey: kubetypes.FileSource, diff --git a/cluster-autoscaler/hack/sast.sh b/cluster-autoscaler/hack/sast.sh index ad7bee0f068f..b7a74768ac01 100755 --- a/cluster-autoscaler/hack/sast.sh +++ b/cluster-autoscaler/hack/sast.sh @@ -43,6 +43,7 @@ dir_to_exclude="-exclude-dir=cloudprovider/alicloud -exclude-dir=cloudprovider/baiducloud -exclude-dir=cloudprovider/bizflycloud -exclude-dir=cloudprovider/brightbox +-exclude-dir=cloudprovider/builder -exclude-dir=cloudprovider/cherryservers -exclude-dir=cloudprovider/civo -exclude-dir=cloudprovider/cloudstack @@ -50,6 +51,7 @@ dir_to_exclude="-exclude-dir=cloudprovider/alicloud -exclude-dir=cloudprovider/digitalocean -exclude-dir=cloudprovider/equinixmetal -exclude-dir=cloudprovider/exoscale +-exclude-dir=cloudprovider/externalgrpc -exclude-dir=cloudprovider/gce -exclude-dir=cloudprovider/hetzner -exclude-dir=cloudprovider/huaweicloud @@ -66,6 +68,24 @@ dir_to_exclude="-exclude-dir=cloudprovider/alicloud -exclude-dir=cloudprovider/tencentcloud -exclude-dir=cloudprovider/volcengine -exclude-dir=cloudprovider/vultr +-exclude-dir=apis +-exclude-dir=cluster-state +-exclude-dir=config +-exclude-dir=context +-exclude-dir=core +-exclude-dir=debuggingsnapshot +-exclude-dir=estimator +-exclude-dir=expander +-exclude-dir=hack +-exclude-dir=loop +-exclude-dir=metrics +-exclude-dir=observers +-exclude-dir=processors +-exclude-dir=proposals +-exclude-dir=provisioningrequest +-exclude-dir=simulator +-exclude-dir=util +-exclude-dir=version " ${TOOLS_BIN_DIR}/gosec -exclude-generated $dir_to_exclude $gosec_report_parse_flags ./... diff --git a/cluster-autoscaler/integration/framework.go b/cluster-autoscaler/integration/framework.go index 7d90a1a68352..ad56e7b3ab31 100644 --- a/cluster-autoscaler/integration/framework.go +++ b/cluster-autoscaler/integration/framework.go @@ -64,12 +64,12 @@ func rotateLogFile(fileName string) (*os.File, error) { if _, err := os.Stat(fileName); err == nil { // !strings.Contains(err.Error(), "no such file or directory") { for i := 9; i > 0; i-- { - os.Rename(fmt.Sprintf("%s.%d", fileName, i), fmt.Sprintf("%s.%d", fileName, i+1)) + _ = os.Rename(fmt.Sprintf("%s.%d", fileName, i), fmt.Sprintf("%s.%d", fileName, i+1)) } - os.Rename(fileName, fmt.Sprintf("%s.%d", fileName, 1)) + _ = os.Rename(fileName, fmt.Sprintf("%s.%d", fileName, 1)) } - return os.Create(fileName) + return os.Create(fileName) //#nosec G304 (CWE-22) -- this is used only for tests. Cannot be exploited } func (driver *Driver) addTaintsToInitialNodes() error { @@ -135,7 +135,7 @@ func (driver *Driver) adjustNodeGroups() error { } // getNumberOfReadyNodes tries to retrieve the list of node objects in the cluster. -func (c *Cluster) getNumberOfReadyNodes() int16 { +func (c *Cluster) getNumberOfReadyNodes() int { nodes, _ := c.Clientset.CoreV1().Nodes().List(context.Background(), metav1.ListOptions{}) count := 0 for _, n := range nodes.Items { @@ -146,7 +146,7 @@ func (c *Cluster) getNumberOfReadyNodes() int16 { } } } - return int16(count) + return count } func (driver *Driver) scaleAutoscaler(replicas int32) error { @@ -206,7 +206,7 @@ func (driver *Driver) runAutoscaler() { outputFile, err := rotateLogFile(CALogFile) gom.Expect(err).ShouldNot(gom.HaveOccurred()) - autoscalerSession, err = gexec.Start(exec.Command(args[0], args[1:]...), outputFile, outputFile) + autoscalerSession, err = gexec.Start(exec.Command(args[0], args[1:]...), outputFile, outputFile) //#nosec G204 (CWE-78) -- this is used only for tests. Cannot be exploited gom.Expect(err).ShouldNot(gom.HaveOccurred()) gom.Expect(autoscalerSession.ExitCode()).Should(gom.Equal(-1)) } diff --git a/cluster-autoscaler/main.go b/cluster-autoscaler/main.go index 626d393d07ae..0a08ab2789e9 100644 --- a/cluster-autoscaler/main.go +++ b/cluster-autoscaler/main.go @@ -669,7 +669,7 @@ func main() { if *enableProfiling { routes.Profiling{}.Install(pathRecorderMux) } - err := http.ListenAndServe(*address, pathRecorderMux) + err := http.ListenAndServe(*address, pathRecorderMux) // #nosec G114 (CWE-676) -- code inherited from upstream klog.Fatalf("Failed to start metrics: %v", err) }() From 6faac2339b3c46d4d99932c327efa29b6c176fe9 Mon Sep 17 00:00:00 2001 From: aaronfern Date: Sun, 2 Mar 2025 17:42:07 +0530 Subject: [PATCH 2/3] Replace depricated golint with golangci-lint and run sast with the check script --- .ci/check | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.ci/check b/.ci/check index fec0a4fe90e4..f313910d490d 100755 --- a/.ci/check +++ b/.ci/check @@ -38,9 +38,9 @@ if [[ "${SOURCE_PATH}" != *"src/k8s.io/autoscaler" ]]; then export PATH="${GOBIN}:${PATH}" fi -# Install Golint (linting tool). -go get -u github.com/golang/lint/golint -go get -u golang.org/x/lint/golint +# Install golangci-lint (linting tool). +GOLANGCI_LINT_VERSION=v1.60.3 +go install github.com/golangci/golangci-lint/cmd/golangci-lint@"${GOLANGCI_LINT_VERSION}" ############################################################################### @@ -48,4 +48,4 @@ PACKAGES="$(go list -e ./... | grep -vE '/tmp/|/vendor/')" LINT_FOLDERS="$(echo ${PACKAGES} | sed "s|k8s.io/autoscaler/cluster-autoscaler|.|g")" # Run Static Application Security Testing (SAST) using gosec -make sast-report +make sast-report -C cluster-autoscaler From 0d1200b177e001b1b770e0d20b24986d1ae384e6 Mon Sep 17 00:00:00 2001 From: aaronfern Date: Sun, 2 Mar 2025 17:42:50 +0530 Subject: [PATCH 3/3] Add a check step to pipeline definitions --- .ci/pipeline_definitions | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.ci/pipeline_definitions b/.ci/pipeline_definitions index 14c7bd6240c8..a0d17b5b0d29 100644 --- a/.ci/pipeline_definitions +++ b/.ci/pipeline_definitions @@ -31,6 +31,8 @@ autoscaler: steps: test: image: 'golang:1.22.2' + check: + image: 'golang:1.22.2' build: image: 'golang:1.22.2' output_dir: 'binary'