From 823be55e7aeec43f88702b0b5d299c599985dd6b Mon Sep 17 00:00:00 2001 From: Sascha Schwarze Date: Thu, 22 Aug 2024 09:50:17 +0200 Subject: [PATCH] Address G115 findings --- deploy/crds/shipwright.io_buildruns.yaml | 4 ++++ deploy/crds/shipwright.io_builds.yaml | 2 ++ pkg/apis/build/v1beta1/build_types.go | 2 ++ pkg/bundle/bundle.go | 15 +++++++++++++-- .../buildlimitcleanup/build_limit_cleanup.go | 12 ++++++++---- pkg/reconciler/buildlimitcleanup/controller.go | 2 ++ 6 files changed, 31 insertions(+), 6 deletions(-) diff --git a/deploy/crds/shipwright.io_buildruns.yaml b/deploy/crds/shipwright.io_buildruns.yaml index 80376a6b73..523873ecde 100644 --- 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: 9223372036854776000 minimum: 1 type: integer succeededLimit: description: SucceededLimit defines the maximum number of succeeded buildruns that should exist. + maximum: 9223372036854776000 minimum: 1 type: integer ttlAfterFailed: @@ -10874,11 +10876,13 @@ spec: failedLimit: description: FailedLimit defines the maximum number of failed buildruns that should exist. + maximum: 9223372036854776000 minimum: 1 type: integer succeededLimit: description: SucceededLimit defines the maximum number of succeeded buildruns that should exist. + maximum: 9223372036854776000 minimum: 1 type: integer ttlAfterFailed: diff --git a/deploy/crds/shipwright.io_builds.yaml b/deploy/crds/shipwright.io_builds.yaml index 72212d30ce..58177da81c 100644 --- 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: 9223372036854776000 minimum: 1 type: integer succeededLimit: description: SucceededLimit defines the maximum number of succeeded buildruns that should exist. + maximum: 9223372036854776000 minimum: 1 type: integer ttlAfterFailed: diff --git a/pkg/apis/build/v1beta1/build_types.go b/pkg/apis/build/v1beta1/build_types.go index 2293dee0e1..6721636f38 100644 --- 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=9223372036854775807 FailedLimit *uint `json:"failedLimit,omitempty"` // SucceededLimit defines the maximum number of succeeded buildruns that should exist. // // +optional // +kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:Maximum=9223372036854775807 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 index 384ba37225..1d14b3f9e4 100644 --- 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 index 1d36c57189..7dbe6f5ceb 100644 --- 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 index eb23e139a3..805b96dd2d 100644 --- 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 }