Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add bundle size validator check #210

Merged
merged 7 commits into from
Feb 8, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
github.com/go-bindata/go-bindata/v3 v3.1.3
github.com/google/cel-go v0.9.0
github.com/mikefarah/yq/v3 v3.0.0-20201202084205-8846255d1c37
github.com/operator-framework/operator-registry v1.19.5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a circular dependency. the api project shouldn't depend on the operator-registry

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, it shows for me that the code to do the zip ought to be shipped here and we do a follow up for opm get it from here. Tks
/hold

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HI @kevinrizza,

That is changed.

/hold cancel

github.com/sirupsen/logrus v1.8.1
github.com/spf13/cobra v1.2.1
github.com/stretchr/testify v1.7.0
Expand Down Expand Up @@ -86,7 +87,7 @@ require (
golang.org/x/tools v0.1.6-0.20210820212750-d4cc65f0b2ff // indirect
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/grpc v1.40.0 // indirect
google.golang.org/grpc v1.41.0 // indirect
google.golang.org/protobuf v1.27.1 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/op/go-logging.v1 v1.0.0-20160211212156-b2cb9fa56473 // indirect
Expand Down
345 changes: 342 additions & 3 deletions go.sum

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions pkg/manifests/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ type Bundle struct {
V1beta1CRDs []*apiextensionsv1beta1.CustomResourceDefinition
V1CRDs []*apiextensionsv1.CustomResourceDefinition
Dependencies []*Dependency
// CompressedSize stores the gzip size of the bundle
CompressedSize *int64
}

func (b *Bundle) ObjectsToValidate() []interface{} {
Expand Down
9 changes: 9 additions & 0 deletions pkg/manifests/bundleloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"path/filepath"
"strings"

"github.com/operator-framework/operator-registry/pkg/lib/encoding"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -35,6 +36,14 @@ func (b *bundleLoader) LoadBundle() error {
errs = append(errs, err)
}

// Compress the bundle to check its size
if data, err := os.ReadFile(b.dir); err == nil {
if content, err := encoding.GzipBase64Encode(data); err != nil {
total := int64(len(content))
b.bundle.CompressedSize = &total
}
}

if !b.foundCSV {
errs = append(errs, fmt.Errorf("unable to find a csv in bundle directory %s", b.dir))
} else if b.bundle == nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/validation/errors/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func ErrInvalidBundle(detail string, value interface{}) Error {
}

func WarnInvalidBundle(detail string, value interface{}) Error {
return invalidBundle(LevelError, detail, value)
return invalidBundle(LevelWarn, detail, value)
}

func invalidBundle(lvl Level, detail string, value interface{}) Error {
Expand Down
35 changes: 34 additions & 1 deletion pkg/validation/internal/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ import (

var BundleValidator interfaces.Validator = interfaces.ValidatorFunc(validateBundles)

// 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)

func validateBundles(objs ...interface{}) (results []errors.ManifestResult) {
for _, obj := range objs {
switch v := obj.(type) {
Expand All @@ -32,6 +37,10 @@ func validateBundle(bundle *manifests.Bundle) (result errors.ManifestResult) {
if saErrors != nil {
result.Add(saErrors...)
}
sizeErrors := validateBundleSize(bundle)
if sizeErrors != nil {
result.Add(sizeErrors...)
}
return result
}

Expand Down Expand Up @@ -99,7 +108,7 @@ func validateOwnedCRDs(bundle *manifests.Bundle, csv *operatorsv1alpha1.ClusterS

// All CRDs present in a CSV must be present in the bundle.
for key := range keySet {
result.Add(errors.WarnInvalidBundle(fmt.Sprintf("CRD %q is present in bundle %q but not defined in CSV", key, bundle.Name), key))
result.Add(errors.ErrInvalidBundle(fmt.Sprintf("CRD %q is present in bundle %q but not defined in CSV", key, bundle.Name), key))
}

return result
Expand All @@ -117,6 +126,30 @@ func getOwnedCustomResourceDefintionKeys(csv *operatorsv1alpha1.ClusterServiceVe
return keys
}

// validateBundleSize will check the bundle size according to its limits
// note that this check will raise an error if the size is bigger than the max allowed
// and warnings when:
// - we are unable to check the bundle size because we are running a check without load the bundle
// - 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)
var errs []errors.Error

if bundle.CompressedSize == nil || *bundle.CompressedSize == 0 {
errs = append(errs, errors.WarnFailedValidation("unable to check the bundle size", nil))
njhale marked this conversation as resolved.
Show resolved Hide resolved
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))
}

return errs
}

// 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.
Expand Down
83 changes: 80 additions & 3 deletions pkg/validation/internal/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ package internal
import (
"testing"

"github.com/operator-framework/api/pkg/manifests"
"github.com/operator-framework/api/pkg/operators/v1alpha1"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"

"github.com/stretchr/testify/require"
"github.com/operator-framework/api/pkg/manifests"
"github.com/operator-framework/api/pkg/operators/v1alpha1"
"github.com/operator-framework/api/pkg/validation/errors"
)

func TestValidateBundle(t *testing.T) {
Expand Down Expand Up @@ -160,3 +161,79 @@ func TestValidateServiceAccount(t *testing.T) {
})
}
}

func TestBundleSize(t *testing.T) {
type args struct {
size int64
}
tests := []struct {
name string
args args
wantError bool
wantWarning bool
errStrings []string
warnStrings []string
}{
{
name: "should pass when the size is not bigger or closer of the limit",
args: args{
size: int64(max_bundle_size / 2),
},
},
{
name: "should warn when the size is closer of the limit",
args: args{
size: int64(max_bundle_size - 10),
},
wantWarning: true,
warnStrings: []string{"Warning: : nearing maximum bundle compressed size with gzip: size=~3 MegaByte, max=4 MegaByte"},
},
{
name: "should warn when is not possible to check the size",
wantWarning: true,
warnStrings: []string{"Warning: : 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),
},
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,
}
result := validateBundleSize(bundle)

var warns, errs []errors.Error
for _, r := range result {
if r.Level == errors.LevelWarn {
warns = append(warns, r)
} else if r.Level == errors.LevelError {
errs = append(errs, r)
}
}
require.Equal(t, tt.wantWarning, len(warns) > 0)
if tt.wantWarning {
require.Equal(t, len(tt.warnStrings), len(warns))
for _, w := range warns {
wString := w.Error()
require.Contains(t, tt.warnStrings, wString)
}
}

require.Equal(t, tt.wantError, len(errs) > 0)
if tt.wantError {
require.Equal(t, len(tt.errStrings), len(errs))
for _, err := range errs {
errString := err.Error()
require.Contains(t, tt.errStrings, errString)
}
}
})
}
}
Loading