From 35dfbaadcfcc1c0a0cde4ae56882ee55c03576a5 Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Fri, 8 Nov 2024 10:08:19 +0100 Subject: [PATCH 1/6] Add sast target to Makefile --- Makefile | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Makefile b/Makefile index d201becb6..bc91b7aa2 100644 --- a/Makefile +++ b/Makefile @@ -140,6 +140,14 @@ generate: $(VGOPATH) $(CONTROLLER_GEN) $(GEN_CRD_API_REFERENCE_DOCS) $(HELM) $(M format: $(GOIMPORTS) $(GOIMPORTSREVISER) @bash $(GARDENER_HACK_DIR)/format.sh ./cmd ./pkg ./test +.PHONY: sast +sast: $(GOSEC) + @bash $(GARDENER_HACK_DIR)/sast.sh + +.PHONY: sast-report +sast-report: $(GOSEC) + @bash $(GARDENER_HACK_DIR)/sast.sh --gosec-report true + .PHONY: test test: @bash $(GARDENER_HACK_DIR)/test.sh ./cmd/... ./pkg/... From 9b785f248c8c0187ebeb177272d366fc14ed7ad4 Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Fri, 8 Nov 2024 10:09:15 +0100 Subject: [PATCH 2/6] Address sast: potential file inclusion --- pkg/apis/config/loader/loader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/config/loader/loader.go b/pkg/apis/config/loader/loader.go index 39034b83c..c5f5b1ae3 100644 --- a/pkg/apis/config/loader/loader.go +++ b/pkg/apis/config/loader/loader.go @@ -36,7 +36,7 @@ func init() { // LoadFromFile takes a filename and de-serializes the contents into ControllerConfiguration object. func LoadFromFile(filename string) (*config.ControllerConfiguration, error) { - bytes, err := os.ReadFile(filename) + bytes, err := os.ReadFile(filename) // #nosec: G304 -- In reality files can be read from the Pod's file system only. if err != nil { return nil, err } From 9617b5ee4118b8c182ff4356eec6c45f76b4553c Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Fri, 8 Nov 2024 10:15:39 +0100 Subject: [PATCH 3/6] Address sast: integer overflow conversion --- pkg/controller/worker/machines.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/controller/worker/machines.go b/pkg/controller/worker/machines.go index 9ddb90d3f..cde6a6b45 100644 --- a/pkg/controller/worker/machines.go +++ b/pkg/controller/worker/machines.go @@ -7,6 +7,7 @@ package worker import ( "context" "fmt" + "math" "path/filepath" "regexp" "slices" @@ -112,7 +113,10 @@ func (w *workerDelegate) generateMachineConfig(ctx context.Context) error { } for _, pool := range w.worker.Spec.Pools { - zoneLen := int32(len(pool.Zones)) + if len(pool.Zones) > math.MaxInt32 { + return fmt.Errorf("pool zones is too large") + } + zoneLen := int32(len(pool.Zones)) // #nosec: G115 - We check if pool zones exceeds max_int32. workerConfig := &apisgcp.WorkerConfig{} if pool.ProviderConfig != nil && pool.ProviderConfig.Raw != nil { @@ -181,7 +185,7 @@ func (w *workerDelegate) generateMachineConfig(ctx context.Context) error { } for zoneIndex, zone := range pool.Zones { - zoneIdx := int32(zoneIndex) + zoneIdx := int32(zoneIndex) // #nosec: G115 - We check if pool zones exceeds max_int32. machineClassSpec := map[string]interface{}{ "region": w.worker.Spec.Region, "zone": zone, From 54a2c23b7583c983a8ac77f4548e24d88c164d4a Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Fri, 8 Nov 2024 10:16:06 +0100 Subject: [PATCH 4/6] Add sast to verfiy step --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index bc91b7aa2..7f898328d 100644 --- a/Makefile +++ b/Makefile @@ -161,10 +161,10 @@ test-clean: @bash $(GARDENER_HACK_DIR)/test-cover-clean.sh .PHONY: verify -verify: check format test +verify: check format test sast .PHONY: verify-extended -verify-extended: check-generate check format test-cov test-clean +verify-extended: check-generate check format test-cov test-clean sast-report .PHONY: integration-test-infra integration-test-infra: From af52efc59640ce2a219c44363ed0c2b2f85489db Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Fri, 8 Nov 2024 12:15:15 +0100 Subject: [PATCH 5/6] Move zone validation --- pkg/apis/gcp/validation/shoot.go | 4 ++++ pkg/controller/worker/machines.go | 4 ---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/apis/gcp/validation/shoot.go b/pkg/apis/gcp/validation/shoot.go index d5fad8510..abe315102 100644 --- a/pkg/apis/gcp/validation/shoot.go +++ b/pkg/apis/gcp/validation/shoot.go @@ -6,6 +6,7 @@ package validation import ( "fmt" + "math" "github.com/gardener/gardener/pkg/apis/core" "github.com/gardener/gardener/pkg/apis/core/helper" @@ -53,6 +54,9 @@ func ValidateWorkers(workers []core.Worker, fldPath *field.Path) field.ErrorList continue } + if len(worker.Zones) > math.MaxInt32 { + allErrs = append(allErrs, field.Invalid(workerFldPath.Child("zones"), len(worker.Zones), "too many zones")) + } } return allErrs diff --git a/pkg/controller/worker/machines.go b/pkg/controller/worker/machines.go index cde6a6b45..a0c651fa2 100644 --- a/pkg/controller/worker/machines.go +++ b/pkg/controller/worker/machines.go @@ -7,7 +7,6 @@ package worker import ( "context" "fmt" - "math" "path/filepath" "regexp" "slices" @@ -113,9 +112,6 @@ func (w *workerDelegate) generateMachineConfig(ctx context.Context) error { } for _, pool := range w.worker.Spec.Pools { - if len(pool.Zones) > math.MaxInt32 { - return fmt.Errorf("pool zones is too large") - } zoneLen := int32(len(pool.Zones)) // #nosec: G115 - We check if pool zones exceeds max_int32. workerConfig := &apisgcp.WorkerConfig{} From 22adeb767b4d81f568dd5c797fb25d50a1738b7b Mon Sep 17 00:00:00 2001 From: Alexander Hebel Date: Fri, 8 Nov 2024 12:15:45 +0100 Subject: [PATCH 6/6] Add negative unit test - missing zone --- pkg/apis/gcp/validation/shoot_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/pkg/apis/gcp/validation/shoot_test.go b/pkg/apis/gcp/validation/shoot_test.go index 3a4dac5af..d0a4f6e2a 100644 --- a/pkg/apis/gcp/validation/shoot_test.go +++ b/pkg/apis/gcp/validation/shoot_test.go @@ -68,5 +68,28 @@ var _ = Describe("Shoot validation", func() { Expect(errorList).To(BeEmpty()) }) + + It("should fail without at least one zone", func() { + workers := []core.Worker{ + { + Name: "bar", + Volume: &core.Volume{ + Type: ptr.To("some-type"), + VolumeSize: "40Gi", + }, + Zones: []string{}, + }, + } + workers[0].Kubernetes = &core.WorkerKubernetes{Version: ptr.To("1.28.0")} + + errorList := ValidateWorkers(workers, field.NewPath("workers")) + + Expect(errorList).To(ConsistOf( + PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeRequired), + "Field": Equal("workers[0].zones"), + })), + )) + }) }) })