Skip to content

Commit

Permalink
Adapt pipeline_definition to include SAST linting logs in OCM descrip…
Browse files Browse the repository at this point in the history
…tor (#347)

* Adapt pipeline_definition to include SAST linting logs in OCM descriptor

* Replace depricated golint with golangci-lint and run sast with the check script

* Add a check step to pipeline definitions
  • Loading branch information
aaronfern authored Mar 3, 2025
1 parent 8556b61 commit 6f0899e
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 27 deletions.
9 changes: 5 additions & 4 deletions .ci/check
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,14 @@ 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}"

###############################################################################

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 -C cluster-autoscaler
26 changes: 16 additions & 10 deletions .ci/pipeline_definitions
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -37,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'
Expand All @@ -62,6 +58,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:
Expand Down
3 changes: 3 additions & 0 deletions cluster-autoscaler/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,6 @@ Session.vim
.netrwhist
.vscode
./integration/logs

# gosec
gosec-report.sarif
17 changes: 12 additions & 5 deletions cluster-autoscaler/cloudprovider/mcm/mcm_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package mcm

import (
"context"
"crypto/rand"
"encoding/json"
"errors"
"flag"
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion cluster-autoscaler/cloudprovider/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 20 additions & 0 deletions cluster-autoscaler/hack/sast.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,15 @@ 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
-exclude-dir=cloudprovider/clusterapi
-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
Expand All @@ -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 ./...
12 changes: 6 additions & 6 deletions cluster-autoscaler/integration/framework.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -146,7 +146,7 @@ func (c *Cluster) getNumberOfReadyNodes() int16 {
}
}
}
return int16(count)
return count
}

func (driver *Driver) scaleAutoscaler(replicas int32) error {
Expand Down Expand Up @@ -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))
}
Expand Down
2 changes: 1 addition & 1 deletion cluster-autoscaler/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}()

Expand Down

0 comments on commit 6f0899e

Please sign in to comment.