Merge pull request #229 from camilamacedo86/fix-ammout

bundle size validator: fix and improvements

Author: kevinrizza

.github/workflows/go.yaml

@@ -48,4 +48,4 @@ jobs:
       with:
         fetch-depth: 0
     - name: Run go-apidiff
-      uses: joelanford/go-apidiff@master
+      uses: joelanford/go-apidiff@main

pkg/manifests/bundle.go

@@ -20,7 +20,9 @@ type Bundle struct {
 	V1CRDs         []*apiextensionsv1.CustomResourceDefinition
 	Dependencies   []*Dependency
 	// CompressedSize stores the gzip size of the bundle
-	CompressedSize *int64
+	CompressedSize int64
+	// Size stores the size of the bundle
+	Size int64
 }
 
 func (b *Bundle) ObjectsToValidate() []interface{} {

pkg/manifests/bundleloader.go

@@ -49,26 +49,30 @@ func (b *bundleLoader) LoadBundle() error {
 
 // Compress the bundle to check its size
 func (b *bundleLoader) calculateCompressedBundleSize() error {
+	if b.bundle == nil {
+		return nil
+	}
 	err := filepath.Walk(b.dir,
 		func(path string, info os.FileInfo, err error) error {
 			if err != nil {
 				return err
 			}
+			if info.IsDir() {
+				return nil
+			}
+			data, err := os.ReadFile(path)
+			if err == nil {
+				// Sum the bundle amount
+				b.bundle.Size += info.Size()
 
-			if !info.IsDir() {
-				if data, err := os.ReadFile(path); err == nil {
-					content, err := encoding.GzipBase64Encode(data)
-					if err != nil {
-						return err
-					}
-					total := int64(len(content))
-					if b.bundle.CompressedSize != nil {
-						total += *b.bundle.CompressedSize
-					}
-					b.bundle.CompressedSize = &total
+				// Sum the compressed amount
+				contentGzip, err := encoding.GzipBase64Encode(data)
+				if err != nil {
+					return err
 				}
+				b.bundle.CompressedSize += int64(len(contentGzip))
 			}
-			return nil
+			return err
 		})
 	if err != nil {
 		return err

pkg/validation/internal/bundle.go

@@ -17,8 +17,9 @@ var BundleValidator interfaces.Validator = interfaces.ValidatorFunc(validateBund
 
 // max_bundle_size is the maximum size of a bundle in bytes.
 // This ensures the bundle can be staged in a single ConfigMap by OLM during installation.
-// The value is derived from the standard upper bound for k8s resources (~4MB).
-const max_bundle_size = 4 << (10 * 2)
+// The value is derived from the standard upper bound for k8s resources (~1MB).
+// We will use this value to check the bundle compressed is < ~1MB
+const max_bundle_size = int64(1 << (10 * 2))
 
 func validateBundles(objs ...interface{}) (results []errors.ManifestResult) {
 	for _, obj := range objs {
@@ -133,23 +134,55 @@ func getOwnedCustomResourceDefintionKeys(csv *operatorsv1alpha1.ClusterServiceVe
 // - we could identify that the bundle size is close to the limit (bigger than 85%)
 func validateBundleSize(bundle *manifests.Bundle) []errors.Error {
 	warnPercent := 0.85
-	warnSize := int64(max_bundle_size * warnPercent)
+	warnSize := float64(max_bundle_size) * warnPercent
 	var errs []errors.Error
 
-	if bundle.CompressedSize == nil || *bundle.CompressedSize == 0 {
-		errs = append(errs, errors.WarnFailedValidation("unable to check the bundle size", nil))
+	if bundle.CompressedSize == 0 {
+		errs = append(errs, errors.WarnFailedValidation("unable to check the bundle compressed size", bundle.Name))
 		return errs
 	}
 
-	if *bundle.CompressedSize > max_bundle_size {
-		errs = append(errs, errors.ErrInvalidBundle(fmt.Sprintf("maximum bundle compressed size with gzip size exceeded: size=~%d MegaByte, max=%d MegaByte", *bundle.CompressedSize/(1<<(10*2)), max_bundle_size/(1<<(10*2))), nil))
-	} else if *bundle.CompressedSize > warnSize {
-		errs = append(errs, errors.WarnInvalidBundle(fmt.Sprintf("nearing maximum bundle compressed size with gzip: size=~%d MegaByte, max=%d MegaByte", *bundle.CompressedSize/(1<<(10*2)), max_bundle_size/(1<<(10*2))), nil))
+	if bundle.Size == 0 {
+		errs = append(errs, errors.WarnFailedValidation("unable to check the bundle size", bundle.Name))
+		return errs
+	}
+
+	// From OPM (https://github.com/operator-framework/operator-registry) 1.17.5
+	// and OLM (https://github.com/operator-framework/operator-lifecycle-manager) : v0.19.0
+	// the total size checked is compressed
+	if bundle.CompressedSize > max_bundle_size {
+		errs = append(errs, errors.ErrInvalidBundle(
+			fmt.Sprintf("maximum bundle compressed size with gzip size exceeded: size=~%s , max=%s. Bundle uncompressed size is %s",
+				formatBytesInUnit(bundle.CompressedSize),
+				formatBytesInUnit(max_bundle_size),
+				formatBytesInUnit(bundle.Size)),
+			bundle.Name))
+	} else if float64(bundle.CompressedSize) > warnSize {
+		errs = append(errs, errors.WarnInvalidBundle(
+			fmt.Sprintf("nearing maximum bundle compressed size with gzip: size=~%s , max=%s. Bundle uncompressed size is %s",
+				formatBytesInUnit(bundle.CompressedSize),
+				formatBytesInUnit(max_bundle_size),
+				formatBytesInUnit(bundle.Size)),
+			bundle.Name))
 	}
 
 	return errs
 }
 
+func formatBytesInUnit(b int64) string {
+	const unit = 1000
+	if b < unit {
+		return fmt.Sprintf("%d B", b)
+	}
+	div, exp := int64(unit), 0
+	for n := b / unit; n >= unit; n /= unit {
+		div *= unit
+		exp++
+	}
+	return fmt.Sprintf("%.1f %cB",
+		float64(b)/float64(div), "kMGTPE"[exp])
+}
+
 // getBundleCRDKeys returns a set of definition keys for all CRDs in bundle.
 func getBundleCRDKeys(bundle *manifests.Bundle) (keys []schema.GroupVersionKind) {
 	// Collect all v1 and v1beta1 CRD keys, skipping group which CSVs do not support.

pkg/validation/internal/bundle_test.go

@@ -1,6 +1,7 @@
 package internal
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -164,7 +165,8 @@ func TestValidateServiceAccount(t *testing.T) {
 
 func TestBundleSize(t *testing.T) {
 	type args struct {
-		size int64
+		sizeCompressed int64
+		size           int64
 	}
 	tests := []struct {
 		name        string
@@ -177,35 +179,54 @@ func TestBundleSize(t *testing.T) {
 		{
 			name: "should pass when the size is not bigger or closer of the limit",
 			args: args{
-				size: int64(max_bundle_size / 2),
+				sizeCompressed: max_bundle_size / 2,
+				size:           max_bundle_size / 2,
 			},
 		},
 		{
 			name: "should warn when the size is closer of the limit",
 			args: args{
-				size: int64(max_bundle_size - 10),
+				sizeCompressed: max_bundle_size - 100000,
+				size:           (max_bundle_size - 100000) * 10,
 			},
 			wantWarning: true,
-			warnStrings: []string{"Warning: : nearing maximum bundle compressed size with gzip: size=~3 MegaByte, max=4 MegaByte"},
+			warnStrings: []string{
+				"Warning: Value : nearing maximum bundle compressed size with gzip: size=~948.6 kB , max=1.0 MB. Bundle uncompressed size is 9.5 MB",
+			},
+		},
+		{
+			name: "should warn when is not possible to check the size compressed",
+			args: args{
+				size: max_bundle_size * 1024,
+			},
+			wantWarning: true,
+			warnStrings: []string{"Warning: Value : unable to check the bundle compressed size"},
 		},
 		{
-			name:        "should warn when is not possible to check the size",
+			name: "should warn when is not possible to check the size",
+			args: args{
+				sizeCompressed: max_bundle_size / 2,
+			},
 			wantWarning: true,
-			warnStrings: []string{"Warning: : unable to check the bundle size"},
+			warnStrings: []string{"Warning: Value : unable to check the bundle size"},
 		},
 		{
 			name: "should raise an error when the size is bigger than the limit",
 			args: args{
-				size: int64(2 * max_bundle_size),
+				sizeCompressed: 2 * max_bundle_size,
+				size:           (2 * max_bundle_size) * 10,
+			},
+			wantError: true,
+			errStrings: []string{
+				"Error: Value : maximum bundle compressed size with gzip size exceeded: size=~2.1 MB , max=1.0 MB. Bundle uncompressed size is 21.0 MB",
 			},
-			wantError:  true,
-			errStrings: []string{"Error: : maximum bundle compressed size with gzip size exceeded: size=~8 MegaByte, max=4 MegaByte"},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			bundle := &manifests.Bundle{
-				CompressedSize: &tt.args.size,
+				CompressedSize: tt.args.sizeCompressed,
+				Size:           tt.args.size,
 			}
 			result := validateBundleSize(bundle)
 
@@ -265,6 +286,10 @@ func Test_EnsureGetBundleSizeValue(t *testing.T) {
 			bundle, err := manifests.GetBundleFromDir(tt.args.bundleDir)
 			require.NoError(t, err)
 
+			// Should have the values calculated
+			require.Greater(t, bundle.Size, int64(0), fmt.Sprintf("the bundle size is %d when should be > 0", bundle.Size))
+			require.Greater(t, bundle.CompressedSize, int64(0), fmt.Sprintf("the bundle compressed size is %d when should be > 0", bundle.CompressedSize))
+
 			results := validateBundle(bundle)
 			require.Equal(t, tt.wantWarning, len(results.Warnings) > 0)
 			if tt.wantWarning {