From f1dffda6b4ef004793c7cf8b47111c2d42492eb1 Mon Sep 17 00:00:00 2001 From: Ankita Thomas Date: Wed, 17 Nov 2021 14:16:29 -0500 Subject: [PATCH] Block cross-package skips/replaces on add (#826) * Block cross-package skips/replaces on add Signed-off-by: Ankita Thomas * bump blang/semver to v4 Signed-off-by: Ankita Thomas Upstream-repository: operator-registry Upstream-commit 0c3c5ca31a6da4011dd2536e686b37802ac40631 --- .../operator-registry/alpha/action/diff.go | 1 + .../alpha/action/diff_test.go | 1 + .../alpha/declcfg/declcfg_to_model.go | 2 +- .../operator-registry/alpha/declcfg/diff.go | 2 +- .../alpha/declcfg/diff_include.go | 7 + .../alpha/declcfg/diff_test.go | 1 + .../operator-registry/alpha/model/model.go | 2 +- staging/operator-registry/go.mod | 2 +- .../pkg/lib/semver/semver.go | 2 +- .../pkg/lib/semver/semver_test.go | 2 +- .../pkg/registry/bundlegraphloader.go | 2 +- .../pkg/registry/bundlegraphloader_test.go | 2 +- staging/operator-registry/pkg/registry/csv.go | 2 +- .../pkg/registry/directoryGraphLoader.go | 2 +- .../pkg/registry/helper_test.go | 2 +- .../pkg/registry/populator.go | 57 ++++++- .../pkg/registry/populator_test.go | 142 ++++++++++++++++++ .../operator-registry/pkg/registry/types.go | 2 +- staging/operator-registry/pkg/sqlite/load.go | 2 +- 19 files changed, 221 insertions(+), 14 deletions(-) diff --git a/staging/operator-registry/alpha/action/diff.go b/staging/operator-registry/alpha/action/diff.go index 4f7427f95a..49e503b867 100644 --- a/staging/operator-registry/alpha/action/diff.go +++ b/staging/operator-registry/alpha/action/diff.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" + "github.com/blang/semver/v4" "github.com/sirupsen/logrus" "github.com/operator-framework/operator-registry/alpha/declcfg" diff --git a/staging/operator-registry/alpha/action/diff_test.go b/staging/operator-registry/alpha/action/diff_test.go index 4ab09caf2b..295eecde23 100644 --- a/staging/operator-registry/alpha/action/diff_test.go +++ b/staging/operator-registry/alpha/action/diff_test.go @@ -10,6 +10,7 @@ import ( "strings" "testing" + "github.com/blang/semver/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/staging/operator-registry/alpha/declcfg/declcfg_to_model.go b/staging/operator-registry/alpha/declcfg/declcfg_to_model.go index d539c165f6..13554ff83c 100644 --- a/staging/operator-registry/alpha/declcfg/declcfg_to_model.go +++ b/staging/operator-registry/alpha/declcfg/declcfg_to_model.go @@ -3,7 +3,7 @@ package declcfg import ( "fmt" - "github.com/blang/semver" + "github.com/blang/semver/v4" "k8s.io/apimachinery/pkg/util/sets" "github.com/operator-framework/operator-registry/alpha/model" diff --git a/staging/operator-registry/alpha/declcfg/diff.go b/staging/operator-registry/alpha/declcfg/diff.go index c4f78d660f..28d83c65ed 100644 --- a/staging/operator-registry/alpha/declcfg/diff.go +++ b/staging/operator-registry/alpha/declcfg/diff.go @@ -6,7 +6,7 @@ import ( "sort" "sync" - "github.com/blang/semver" + "github.com/blang/semver/v4" "github.com/mitchellh/hashstructure/v2" "github.com/sirupsen/logrus" diff --git a/staging/operator-registry/alpha/declcfg/diff_include.go b/staging/operator-registry/alpha/declcfg/diff_include.go index 14a4d5c890..33d587dc69 100644 --- a/staging/operator-registry/alpha/declcfg/diff_include.go +++ b/staging/operator-registry/alpha/declcfg/diff_include.go @@ -1,6 +1,13 @@ package declcfg import ( + "fmt" + "strings" + + "github.com/blang/semver/v4" + "github.com/sirupsen/logrus" + utilerrors "k8s.io/apimachinery/pkg/util/errors" + "github.com/operator-framework/operator-registry/alpha/model" ) diff --git a/staging/operator-registry/alpha/declcfg/diff_test.go b/staging/operator-registry/alpha/declcfg/diff_test.go index 51f92a9083..25dbc6318b 100644 --- a/staging/operator-registry/alpha/declcfg/diff_test.go +++ b/staging/operator-registry/alpha/declcfg/diff_test.go @@ -3,6 +3,7 @@ package declcfg import ( "testing" + "github.com/blang/semver/v4" "github.com/stretchr/testify/require" "github.com/operator-framework/operator-registry/alpha/model" diff --git a/staging/operator-registry/alpha/model/model.go b/staging/operator-registry/alpha/model/model.go index babcd08cc7..75042469a2 100644 --- a/staging/operator-registry/alpha/model/model.go +++ b/staging/operator-registry/alpha/model/model.go @@ -6,7 +6,7 @@ import ( "sort" "strings" - "github.com/blang/semver" + "github.com/blang/semver/v4" "github.com/h2non/filetype" "github.com/h2non/filetype/matchers" "github.com/h2non/filetype/types" diff --git a/staging/operator-registry/go.mod b/staging/operator-registry/go.mod index 94d1d1bbc6..889ebc2d14 100644 --- a/staging/operator-registry/go.mod +++ b/staging/operator-registry/go.mod @@ -4,7 +4,7 @@ go 1.16 require ( github.com/Microsoft/hcsshim v0.8.9 // indirect - github.com/blang/semver v3.5.1+incompatible + github.com/blang/semver/v4 v4.0.0 github.com/bugsnag/bugsnag-go v1.5.3 // indirect github.com/bugsnag/panicwrap v1.2.0 // indirect github.com/containerd/containerd v1.4.8 diff --git a/staging/operator-registry/pkg/lib/semver/semver.go b/staging/operator-registry/pkg/lib/semver/semver.go index 033bb5419e..6875566d08 100644 --- a/staging/operator-registry/pkg/lib/semver/semver.go +++ b/staging/operator-registry/pkg/lib/semver/semver.go @@ -3,7 +3,7 @@ package semver import ( "fmt" - "github.com/blang/semver" + "github.com/blang/semver/v4" ) // BuildIdCompare compares two versions and returns negative one if the first arg is less than the second arg, positive one if it is larger, and zero if they are equal. diff --git a/staging/operator-registry/pkg/lib/semver/semver_test.go b/staging/operator-registry/pkg/lib/semver/semver_test.go index 2291146783..2ee4a5385f 100644 --- a/staging/operator-registry/pkg/lib/semver/semver_test.go +++ b/staging/operator-registry/pkg/lib/semver/semver_test.go @@ -3,7 +3,7 @@ package semver import ( "testing" - "github.com/blang/semver" + "github.com/blang/semver/v4" "github.com/stretchr/testify/require" ) diff --git a/staging/operator-registry/pkg/registry/bundlegraphloader.go b/staging/operator-registry/pkg/registry/bundlegraphloader.go index 05d8abf064..e8664c4e84 100644 --- a/staging/operator-registry/pkg/registry/bundlegraphloader.go +++ b/staging/operator-registry/pkg/registry/bundlegraphloader.go @@ -3,7 +3,7 @@ package registry import ( "fmt" - "github.com/blang/semver" + "github.com/blang/semver/v4" ) // BundleGraphLoader generates updated graphs by adding bundles to them, updating diff --git a/staging/operator-registry/pkg/registry/bundlegraphloader_test.go b/staging/operator-registry/pkg/registry/bundlegraphloader_test.go index 8bc5d03c1a..5d6e3c2a14 100644 --- a/staging/operator-registry/pkg/registry/bundlegraphloader_test.go +++ b/staging/operator-registry/pkg/registry/bundlegraphloader_test.go @@ -4,7 +4,7 @@ import ( "encoding/json" "testing" - "github.com/blang/semver" + "github.com/blang/semver/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/staging/operator-registry/pkg/registry/csv.go b/staging/operator-registry/pkg/registry/csv.go index a3b5ebc191..ec6f3e3239 100644 --- a/staging/operator-registry/pkg/registry/csv.go +++ b/staging/operator-registry/pkg/registry/csv.go @@ -50,7 +50,7 @@ const ( description = "description" // The yaml attribute that specifies the version of the ClusterServiceVersion - // expected to be semver and parseable by blang/semver + // expected to be semver and parseable by blang/semver/v4 version = "version" // The yaml attribute that specifies the related images of the ClusterServiceVersion diff --git a/staging/operator-registry/pkg/registry/directoryGraphLoader.go b/staging/operator-registry/pkg/registry/directoryGraphLoader.go index 03efa4d63f..b639c08b5e 100644 --- a/staging/operator-registry/pkg/registry/directoryGraphLoader.go +++ b/staging/operator-registry/pkg/registry/directoryGraphLoader.go @@ -8,7 +8,7 @@ import ( "path/filepath" "sort" - "github.com/blang/semver" + "github.com/blang/semver/v4" "github.com/onsi/gomega/gstruct/errors" ) diff --git a/staging/operator-registry/pkg/registry/helper_test.go b/staging/operator-registry/pkg/registry/helper_test.go index 3cd940089f..9121fa4506 100644 --- a/staging/operator-registry/pkg/registry/helper_test.go +++ b/staging/operator-registry/pkg/registry/helper_test.go @@ -3,7 +3,7 @@ package registry import ( "testing" - "github.com/blang/semver" + "github.com/blang/semver/v4" "github.com/stretchr/testify/require" ) diff --git a/staging/operator-registry/pkg/registry/populator.go b/staging/operator-registry/pkg/registry/populator.go index ad65e78b2d..b1919159e6 100644 --- a/staging/operator-registry/pkg/registry/populator.go +++ b/staging/operator-registry/pkg/registry/populator.go @@ -5,8 +5,9 @@ import ( "errors" "fmt" "os" + "sort" - "github.com/blang/semver" + "github.com/blang/semver/v4" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/yaml" @@ -142,6 +143,9 @@ func (i *DirectoryPopulator) globalSanityCheck(imagesToAdd []*ImageInput) error } } + if err := i.ValidateEdgeBundlePackage(imagesToAdd); err != nil { + errs = append(errs, err) + } return utilerrors.NewAggregate(errs) } @@ -408,3 +412,54 @@ func DecodeFile(path string, into interface{}) error { return decoder.Decode(into) } + +// ValidateEdgeBundlePackage ensures that all bundles in the input will only skip or replace bundles in the same package. +func (i *DirectoryPopulator) ValidateEdgeBundlePackage(images []*ImageInput) error { + // track packages for encountered bundles + expectedBundlePackages := map[string]string{} + for _, b := range images { + r, err := b.Bundle.Replaces() + if err != nil { + return fmt.Errorf("failed to validate replaces for bundle %s(%s): %v", b.Bundle.Name, b.Bundle.BundleImage, err) + } + + skipped, err := b.Bundle.Skips() + if err != nil { + return fmt.Errorf("failed to validate skipped entries for bundle %s(%s): %v", b.Bundle.Name, b.Bundle.BundleImage, err) + } + + for _, bndl := range append(skipped, r, b.Bundle.Name) { + if len(bndl) == 0 { + continue + } + + if pkg, ok := expectedBundlePackages[bndl]; ok && pkg != b.Bundle.Package { + pkgs := []string{pkg, b.Bundle.Package} + sort.Strings(pkgs) + return fmt.Errorf("bundle %s must belong to exactly one package, found on: %v", bndl, pkgs) + } + expectedBundlePackages[bndl] = b.Bundle.Package + } + } + if len(expectedBundlePackages) == 0 { + return nil + } + + pkgs, err := i.querier.ListPackages(context.TODO()) + if err != nil { + return fmt.Errorf("unable to verify bundle packages: %v", err) + } + for _, pkg := range pkgs { + entries, err := i.querier.GetChannelEntriesFromPackage(context.TODO(), pkg) + if err != nil { + return fmt.Errorf("unable to verify bundles for package %v", err) + } + for _, b := range entries { + if bundlePkg, ok := expectedBundlePackages[b.BundleName]; ok && bundlePkg != b.PackageName { + return fmt.Errorf("bundle %s belongs to package %s on index, cannot be added as an edge for package %s", b.BundleName, b.PackageName, bundlePkg) + } + } + } + + return nil +} diff --git a/staging/operator-registry/pkg/registry/populator_test.go b/staging/operator-registry/pkg/registry/populator_test.go index 280907964c..a7e861f10e 100644 --- a/staging/operator-registry/pkg/registry/populator_test.go +++ b/staging/operator-registry/pkg/registry/populator_test.go @@ -12,11 +12,14 @@ import ( "testing" "time" + "github.com/blang/semver/v4" "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/util/errors" utilerrors "k8s.io/apimachinery/pkg/util/errors" + "github.com/operator-framework/api/pkg/lib/version" + "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-registry/pkg/api" "github.com/operator-framework/operator-registry/pkg/image" "github.com/operator-framework/operator-registry/pkg/registry" @@ -2621,3 +2624,142 @@ func TestEnableAlpha(t *testing.T) { }) } } + +func newUnpackedTestBundle(root, dir, name string, csvSpec json.RawMessage, annotations registry.Annotations) (string, func(), error) { + bundleDir := filepath.Join(root, dir) + cleanup := func() { + os.RemoveAll(bundleDir) + } + if err := os.Mkdir(bundleDir, 0755); err != nil { + return bundleDir, cleanup, err + } + if err := os.Mkdir(filepath.Join(bundleDir, bundle.ManifestsDir), 0755); err != nil { + return bundleDir, cleanup, err + } + if err := os.Mkdir(filepath.Join(bundleDir, bundle.MetadataDir), 0755); err != nil { + return bundleDir, cleanup, err + } + if len(csvSpec) == 0 { + csvSpec = json.RawMessage(`{}`) + } + + rawCSV, err := json.Marshal(registry.ClusterServiceVersion{ + TypeMeta: v1.TypeMeta{ + Kind: sqlite.ClusterServiceVersionKind, + }, + ObjectMeta: v1.ObjectMeta{ + Name: name, + }, + Spec: csvSpec, + }) + if err != nil { + return bundleDir, cleanup, err + } + + rawObj := unstructured.Unstructured{} + if err := json.Unmarshal(rawCSV, &rawObj); err != nil { + return bundleDir, cleanup, err + } + rawObj.SetCreationTimestamp(v1.Time{}) + + jsonout, err := rawObj.MarshalJSON() + out, err := yaml.JSONToYAML(jsonout) + if err != nil { + return bundleDir, cleanup, err + } + if err := ioutil.WriteFile(filepath.Join(bundleDir, bundle.ManifestsDir, "csv.yaml"), out, 0666); err != nil { + return bundleDir, cleanup, err + } + + out, err = yaml.Marshal(registry.AnnotationsFile{Annotations: annotations}) + if err != nil { + return bundleDir, cleanup, err + } + if err := ioutil.WriteFile(filepath.Join(bundleDir, bundle.MetadataDir, "annotations.yaml"), out, 0666); err != nil { + return bundleDir, cleanup, err + } + return bundleDir, cleanup, nil +} + +func TestValidateEdgeBundlePackage(t *testing.T) { + newInput := func(name, versionString, pkg, defaultChannel, channels, replaces string, skips []string) *registry.ImageInput { + v, err := semver.Parse(versionString) + require.NoError(t, err) + + spec := v1alpha1.ClusterServiceVersionSpec{ + Replaces: replaces, + Skips: skips, + Version: version.OperatorVersion{v}, + } + specJson, err := json.Marshal(&spec) + require.NoError(t, err) + + rawCSV, err := json.Marshal(registry.ClusterServiceVersion{ + TypeMeta: v1.TypeMeta{ + Kind: sqlite.ClusterServiceVersionKind, + }, + ObjectMeta: v1.ObjectMeta{ + Name: name, + }, + Spec: specJson, + }) + + rawObj := unstructured.Unstructured{} + require.NoError(t, json.Unmarshal(rawCSV, &rawObj)) + rawObj.SetCreationTimestamp(v1.Time{}) + + jsonout, err := rawObj.MarshalJSON() + require.NoError(t, err) + + b, err := registry.NewBundleFromStrings(name, versionString, pkg, defaultChannel, channels, string(jsonout)) + require.NoError(t, err) + return ®istry.ImageInput{Bundle: b} + } + + logrus.SetLevel(logrus.DebugLevel) + db, cleanup := CreateTestDb(t) + defer cleanup() + + store, err := createAndPopulateDB(db) + require.NoError(t, err) + + r := registry.NewDirectoryPopulator(nil, nil, store, nil, nil) + + tests := []struct { + name string + args []*registry.ImageInput + wantErr error + }{ + { + name: "conflictInAdded", + args: []*registry.ImageInput{ + newInput("b1", "0.0.1", "p1", "a", "a", "", []string{}), + newInput("b2", "0.0.2", "p2", "a", "a", "", []string{"b1"}), + }, + wantErr: fmt.Errorf("bundle b1 must belong to exactly one package, found on: [p1 p2]"), + }, + { + name: "conflictWithExisting", + args: []*registry.ImageInput{ + newInput("b1", "0.0.1", "p1", "a", "a", "", []string{"etcdoperator.v0.9.0"}), + }, + wantErr: fmt.Errorf("bundle etcdoperator.v0.9.0 belongs to package etcd on index, cannot be added as an edge for package p1"), + }, + { + name: "passForNonConflicting", + args: []*registry.ImageInput{ + newInput("b1", "0.0.1", "etcd", "a", "a", "", []string{"etcdoperator.v0.9.0"}), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := r.ValidateEdgeBundlePackage(tt.args) + if tt.wantErr == nil { + require.NoError(t, err) + return + } + require.EqualError(t, err, tt.wantErr.Error()) + }) + } +} diff --git a/staging/operator-registry/pkg/registry/types.go b/staging/operator-registry/pkg/registry/types.go index 719127cfba..d7d2585520 100644 --- a/staging/operator-registry/pkg/registry/types.go +++ b/staging/operator-registry/pkg/registry/types.go @@ -7,7 +7,7 @@ import ( "sort" "strings" - "github.com/blang/semver" + "github.com/blang/semver/v4" ) var ( diff --git a/staging/operator-registry/pkg/sqlite/load.go b/staging/operator-registry/pkg/sqlite/load.go index 65185ee656..95bb079c8f 100644 --- a/staging/operator-registry/pkg/sqlite/load.go +++ b/staging/operator-registry/pkg/sqlite/load.go @@ -7,7 +7,7 @@ import ( "fmt" "strings" - "github.com/blang/semver" + "github.com/blang/semver/v4" _ "github.com/mattn/go-sqlite3" utilerrors "k8s.io/apimachinery/pkg/util/errors"