Merge pull request #1670 from SaschaSchwarze0/sascha-fix-g115

Address G115 findings
shipwright-io · Aug 22, 2024 · 88d806e · 88d806e
2 parents 58c1055 + 48df4b9
commit 88d806e
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 6 deletions.
diff --git a/deploy/crds/shipwright.io_buildruns.yaml b/deploy/crds/shipwright.io_buildruns.yaml
@@ -6768,11 +6768,13 @@ spec:
                           failedLimit:
                             description: FailedLimit defines the maximum number of
                               failed buildruns that should exist.
+                            maximum: 10000
                             minimum: 1
                             type: integer
                           succeededLimit:
                             description: SucceededLimit defines the maximum number
                               of succeeded buildruns that should exist.
+                            maximum: 10000
                             minimum: 1
                             type: integer
                           ttlAfterFailed:
@@ -10874,11 +10876,13 @@ spec:
                       failedLimit:
                         description: FailedLimit defines the maximum number of failed
                           buildruns that should exist.
+                        maximum: 10000
                         minimum: 1
                         type: integer
                       succeededLimit:
                         description: SucceededLimit defines the maximum number of
                           succeeded buildruns that should exist.
+                        maximum: 10000
                         minimum: 1
                         type: integer
                       ttlAfterFailed:

diff --git a/deploy/crds/shipwright.io_builds.yaml b/deploy/crds/shipwright.io_builds.yaml
@@ -2507,11 +2507,13 @@ spec:
                   failedLimit:
                     description: FailedLimit defines the maximum number of failed
                       buildruns that should exist.
+                    maximum: 10000
                     minimum: 1
                     type: integer
                   succeededLimit:
                     description: SucceededLimit defines the maximum number of succeeded
                       buildruns that should exist.
+                    maximum: 10000
                     minimum: 1
                     type: integer
                   ttlAfterFailed:

diff --git a/pkg/apis/build/v1beta1/build_types.go b/pkg/apis/build/v1beta1/build_types.go
@@ -343,11 +343,13 @@ type BuildRetention struct {
 	//
 	// +optional
 	// +kubebuilder:validation:Minimum=1
+	// +kubebuilder:validation:Maximum=10000
 	FailedLimit *uint `json:"failedLimit,omitempty"`
 	// SucceededLimit defines the maximum number of succeeded buildruns that should exist.
 	//
 	// +optional
 	// +kubebuilder:validation:Minimum=1
+	// +kubebuilder:validation:Maximum=10000
 	SucceededLimit *uint `json:"succeededLimit,omitempty"`
 	// TTLAfterFailed defines the maximum duration of time the failed buildrun should exist.
 	//

diff --git a/pkg/bundle/bundle.go b/pkg/bundle/bundle.go
@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
+	"math"
 	"os"
 	"path/filepath"
 	"strings"
@@ -252,7 +253,7 @@ func Unpack(in io.Reader, targetPath string) (*UnpackDetails, error) {
 
 		switch header.Typeflag {
 		case tar.TypeDir:
-			if err := os.MkdirAll(target, os.FileMode(header.Mode)); err != nil {
+			if err := os.MkdirAll(target, fileMode(header)); err != nil {
 				return nil, err
 			}
 
@@ -263,7 +264,7 @@ func Unpack(in io.Reader, targetPath string) (*UnpackDetails, error) {
 				return nil, err
 			}
 
-			file, err := os.OpenFile(target, os.O_CREATE|os.O_RDWR, os.FileMode(header.Mode))
+			file, err := os.OpenFile(target, os.O_CREATE|os.O_RDWR, fileMode(header))
 			if err != nil {
 				return nil, err
 			}
@@ -290,3 +291,13 @@ func Unpack(in io.Reader, targetPath string) (*UnpackDetails, error) {
 		}
 	}
 }
+
+func fileMode(tarHeader *tar.Header) os.FileMode {
+	mode := tarHeader.Mode
+	if mode < 0 || mode > math.MaxUint32 {
+		return 0
+	}
+
+	// #nosec G115 was checked above
+	return os.FileMode(mode)
+}
diff --git a/pkg/reconciler/buildlimitcleanup/build_limit_cleanup.go b/pkg/reconciler/buildlimitcleanup/build_limit_cleanup.go
@@ -99,13 +99,15 @@ func (r *ReconcileBuild) Reconcile(ctx context.Context, request reconcile.Reques
 
 	// Check limits and delete oldest buildruns if limit is reached.
 	if b.Spec.Retention.SucceededLimit != nil {
-		if len(buildRunSucceeded) > int(*b.Spec.Retention.SucceededLimit) {
+		// #nosec G115, is validated in the type
+		succeededLimit := int(*b.Spec.Retention.SucceededLimit)
+		if len(buildRunSucceeded) > succeededLimit {
 			// Sort buildruns with oldest one at the beginning
 			sort.Slice(buildRunSucceeded, func(i, j int) bool {
 				return buildRunSucceeded[i].ObjectMeta.CreationTimestamp.Before(&buildRunSucceeded[j].ObjectMeta.CreationTimestamp)
 			})
 			lenOfList := len(buildRunSucceeded)
-			for i := 0; i < lenOfList-int(*b.Spec.Retention.SucceededLimit); i++ {
+			for i := 0; i < lenOfList-succeededLimit; i++ {
 				ctxlog.Info(ctx, "Deleting succeeded buildrun as cleanup limit has been reached.", namespace, request.Namespace, name, buildRunSucceeded[i].Name)
 				err := r.client.Delete(ctx, &buildRunSucceeded[i], &client.DeleteOptions{})
 				if err != nil {
@@ -120,13 +122,15 @@ func (r *ReconcileBuild) Reconcile(ctx context.Context, request reconcile.Reques
 	}
 
 	if b.Spec.Retention.FailedLimit != nil {
-		if len(buildRunFailed) > int(*b.Spec.Retention.FailedLimit) {
+		// #nosec G115, is validated in the type
+		failedLimit := int(*b.Spec.Retention.FailedLimit)
+		if len(buildRunFailed) > failedLimit {
 			// Sort buildruns with oldest one at the beginning
 			sort.Slice(buildRunFailed, func(i, j int) bool {
 				return buildRunFailed[i].ObjectMeta.CreationTimestamp.Before(&buildRunFailed[j].ObjectMeta.CreationTimestamp)
 			})
 			lenOfList := len(buildRunFailed)
-			for i := 0; i < lenOfList-int(*b.Spec.Retention.FailedLimit); i++ {
+			for i := 0; i < lenOfList-failedLimit; i++ {
 				ctxlog.Info(ctx, "Deleting failed buildrun as cleanup limit has been reached.", namespace, request.Namespace, name, buildRunFailed[i].Name)
 				err := r.client.Delete(ctx, &buildRunFailed[i], &client.DeleteOptions{})
 				if err != nil {

diff --git a/pkg/reconciler/buildlimitcleanup/controller.go b/pkg/reconciler/buildlimitcleanup/controller.go
@@ -67,8 +67,10 @@ func add(mgr manager.Manager, r reconcile.Reconciler, maxConcurrentReconciles in
 					return true
 				} else if n.Spec.Retention.SucceededLimit != nil && o.Spec.Retention.SucceededLimit == nil {
 					return true
+					// #nosec G115, is validated in the type
 				} else if n.Spec.Retention.FailedLimit != nil && o.Spec.Retention.FailedLimit != nil && int(*n.Spec.Retention.FailedLimit) < int(*o.Spec.Retention.FailedLimit) {
 					return true
+					// #nosec G115, is validated in the type
 				} else if n.Spec.Retention.SucceededLimit != nil && o.Spec.Retention.SucceededLimit != nil && int(*n.Spec.Retention.SucceededLimit) < int(*o.Spec.Retention.SucceededLimit) {
 					return true
 				}