Skip to content

Commit f1dffda

Browse files
committed
Block cross-package skips/replaces on add (#826)
* Block cross-package skips/replaces on add Signed-off-by: Ankita Thomas <[email protected]> * bump blang/semver to v4 Signed-off-by: Ankita Thomas <[email protected]> Upstream-repository: operator-registry Upstream-commit 0c3c5ca31a6da4011dd2536e686b37802ac40631
1 parent 8970fc0 commit f1dffda

File tree

19 files changed

+221
-14
lines changed

19 files changed

+221
-14
lines changed

staging/operator-registry/alpha/action/diff.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"fmt"
77

8+
"github.com/blang/semver/v4"
89
"github.com/sirupsen/logrus"
910

1011
"github.com/operator-framework/operator-registry/alpha/declcfg"

staging/operator-registry/alpha/action/diff_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"strings"
1111
"testing"
1212

13+
"github.com/blang/semver/v4"
1314
"github.com/stretchr/testify/assert"
1415
"github.com/stretchr/testify/require"
1516

staging/operator-registry/alpha/declcfg/declcfg_to_model.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package declcfg
33
import (
44
"fmt"
55

6-
"github.com/blang/semver"
6+
"github.com/blang/semver/v4"
77
"k8s.io/apimachinery/pkg/util/sets"
88

99
"github.com/operator-framework/operator-registry/alpha/model"

staging/operator-registry/alpha/declcfg/diff.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"sort"
77
"sync"
88

9-
"github.com/blang/semver"
9+
"github.com/blang/semver/v4"
1010
"github.com/mitchellh/hashstructure/v2"
1111
"github.com/sirupsen/logrus"
1212

staging/operator-registry/alpha/declcfg/diff_include.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
package declcfg
22

33
import (
4+
"fmt"
5+
"strings"
6+
7+
"github.com/blang/semver/v4"
8+
"github.com/sirupsen/logrus"
9+
utilerrors "k8s.io/apimachinery/pkg/util/errors"
10+
411
"github.com/operator-framework/operator-registry/alpha/model"
512
)
613

staging/operator-registry/alpha/declcfg/diff_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package declcfg
33
import (
44
"testing"
55

6+
"github.com/blang/semver/v4"
67
"github.com/stretchr/testify/require"
78

89
"github.com/operator-framework/operator-registry/alpha/model"

staging/operator-registry/alpha/model/model.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"sort"
77
"strings"
88

9-
"github.com/blang/semver"
9+
"github.com/blang/semver/v4"
1010
"github.com/h2non/filetype"
1111
"github.com/h2non/filetype/matchers"
1212
"github.com/h2non/filetype/types"

staging/operator-registry/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ go 1.16
44

55
require (
66
github.com/Microsoft/hcsshim v0.8.9 // indirect
7-
github.com/blang/semver v3.5.1+incompatible
7+
github.com/blang/semver/v4 v4.0.0
88
github.com/bugsnag/bugsnag-go v1.5.3 // indirect
99
github.com/bugsnag/panicwrap v1.2.0 // indirect
1010
github.com/containerd/containerd v1.4.8

staging/operator-registry/pkg/lib/semver/semver.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package semver
33
import (
44
"fmt"
55

6-
"github.com/blang/semver"
6+
"github.com/blang/semver/v4"
77
)
88

99
// 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.

staging/operator-registry/pkg/lib/semver/semver_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package semver
33
import (
44
"testing"
55

6-
"github.com/blang/semver"
6+
"github.com/blang/semver/v4"
77
"github.com/stretchr/testify/require"
88
)
99

staging/operator-registry/pkg/registry/bundlegraphloader.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package registry
33
import (
44
"fmt"
55

6-
"github.com/blang/semver"
6+
"github.com/blang/semver/v4"
77
)
88

99
// BundleGraphLoader generates updated graphs by adding bundles to them, updating

staging/operator-registry/pkg/registry/bundlegraphloader_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import (
44
"encoding/json"
55
"testing"
66

7-
"github.com/blang/semver"
7+
"github.com/blang/semver/v4"
88

99
"github.com/stretchr/testify/assert"
1010
"github.com/stretchr/testify/require"

staging/operator-registry/pkg/registry/csv.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ const (
5050
description = "description"
5151

5252
// The yaml attribute that specifies the version of the ClusterServiceVersion
53-
// expected to be semver and parseable by blang/semver
53+
// expected to be semver and parseable by blang/semver/v4
5454
version = "version"
5555

5656
// The yaml attribute that specifies the related images of the ClusterServiceVersion

staging/operator-registry/pkg/registry/directoryGraphLoader.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
"path/filepath"
99
"sort"
1010

11-
"github.com/blang/semver"
11+
"github.com/blang/semver/v4"
1212
"github.com/onsi/gomega/gstruct/errors"
1313
)
1414

staging/operator-registry/pkg/registry/helper_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package registry
33
import (
44
"testing"
55

6-
"github.com/blang/semver"
6+
"github.com/blang/semver/v4"
77
"github.com/stretchr/testify/require"
88
)
99

staging/operator-registry/pkg/registry/populator.go

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ import (
55
"errors"
66
"fmt"
77
"os"
8+
"sort"
89

9-
"github.com/blang/semver"
10+
"github.com/blang/semver/v4"
1011
utilerrors "k8s.io/apimachinery/pkg/util/errors"
1112
"k8s.io/apimachinery/pkg/util/yaml"
1213

@@ -142,6 +143,9 @@ func (i *DirectoryPopulator) globalSanityCheck(imagesToAdd []*ImageInput) error
142143
}
143144
}
144145

146+
if err := i.ValidateEdgeBundlePackage(imagesToAdd); err != nil {
147+
errs = append(errs, err)
148+
}
145149
return utilerrors.NewAggregate(errs)
146150
}
147151

@@ -408,3 +412,54 @@ func DecodeFile(path string, into interface{}) error {
408412

409413
return decoder.Decode(into)
410414
}
415+
416+
// ValidateEdgeBundlePackage ensures that all bundles in the input will only skip or replace bundles in the same package.
417+
func (i *DirectoryPopulator) ValidateEdgeBundlePackage(images []*ImageInput) error {
418+
// track packages for encountered bundles
419+
expectedBundlePackages := map[string]string{}
420+
for _, b := range images {
421+
r, err := b.Bundle.Replaces()
422+
if err != nil {
423+
return fmt.Errorf("failed to validate replaces for bundle %s(%s): %v", b.Bundle.Name, b.Bundle.BundleImage, err)
424+
}
425+
426+
skipped, err := b.Bundle.Skips()
427+
if err != nil {
428+
return fmt.Errorf("failed to validate skipped entries for bundle %s(%s): %v", b.Bundle.Name, b.Bundle.BundleImage, err)
429+
}
430+
431+
for _, bndl := range append(skipped, r, b.Bundle.Name) {
432+
if len(bndl) == 0 {
433+
continue
434+
}
435+
436+
if pkg, ok := expectedBundlePackages[bndl]; ok && pkg != b.Bundle.Package {
437+
pkgs := []string{pkg, b.Bundle.Package}
438+
sort.Strings(pkgs)
439+
return fmt.Errorf("bundle %s must belong to exactly one package, found on: %v", bndl, pkgs)
440+
}
441+
expectedBundlePackages[bndl] = b.Bundle.Package
442+
}
443+
}
444+
if len(expectedBundlePackages) == 0 {
445+
return nil
446+
}
447+
448+
pkgs, err := i.querier.ListPackages(context.TODO())
449+
if err != nil {
450+
return fmt.Errorf("unable to verify bundle packages: %v", err)
451+
}
452+
for _, pkg := range pkgs {
453+
entries, err := i.querier.GetChannelEntriesFromPackage(context.TODO(), pkg)
454+
if err != nil {
455+
return fmt.Errorf("unable to verify bundles for package %v", err)
456+
}
457+
for _, b := range entries {
458+
if bundlePkg, ok := expectedBundlePackages[b.BundleName]; ok && bundlePkg != b.PackageName {
459+
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)
460+
}
461+
}
462+
}
463+
464+
return nil
465+
}

staging/operator-registry/pkg/registry/populator_test.go

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,14 @@ import (
1212
"testing"
1313
"time"
1414

15+
"github.com/blang/semver/v4"
1516
"github.com/sirupsen/logrus"
1617
"github.com/stretchr/testify/require"
1718
"k8s.io/apimachinery/pkg/util/errors"
1819
utilerrors "k8s.io/apimachinery/pkg/util/errors"
1920

21+
"github.com/operator-framework/api/pkg/lib/version"
22+
"github.com/operator-framework/api/pkg/operators/v1alpha1"
2023
"github.com/operator-framework/operator-registry/pkg/api"
2124
"github.com/operator-framework/operator-registry/pkg/image"
2225
"github.com/operator-framework/operator-registry/pkg/registry"
@@ -2621,3 +2624,142 @@ func TestEnableAlpha(t *testing.T) {
26212624
})
26222625
}
26232626
}
2627+
2628+
func newUnpackedTestBundle(root, dir, name string, csvSpec json.RawMessage, annotations registry.Annotations) (string, func(), error) {
2629+
bundleDir := filepath.Join(root, dir)
2630+
cleanup := func() {
2631+
os.RemoveAll(bundleDir)
2632+
}
2633+
if err := os.Mkdir(bundleDir, 0755); err != nil {
2634+
return bundleDir, cleanup, err
2635+
}
2636+
if err := os.Mkdir(filepath.Join(bundleDir, bundle.ManifestsDir), 0755); err != nil {
2637+
return bundleDir, cleanup, err
2638+
}
2639+
if err := os.Mkdir(filepath.Join(bundleDir, bundle.MetadataDir), 0755); err != nil {
2640+
return bundleDir, cleanup, err
2641+
}
2642+
if len(csvSpec) == 0 {
2643+
csvSpec = json.RawMessage(`{}`)
2644+
}
2645+
2646+
rawCSV, err := json.Marshal(registry.ClusterServiceVersion{
2647+
TypeMeta: v1.TypeMeta{
2648+
Kind: sqlite.ClusterServiceVersionKind,
2649+
},
2650+
ObjectMeta: v1.ObjectMeta{
2651+
Name: name,
2652+
},
2653+
Spec: csvSpec,
2654+
})
2655+
if err != nil {
2656+
return bundleDir, cleanup, err
2657+
}
2658+
2659+
rawObj := unstructured.Unstructured{}
2660+
if err := json.Unmarshal(rawCSV, &rawObj); err != nil {
2661+
return bundleDir, cleanup, err
2662+
}
2663+
rawObj.SetCreationTimestamp(v1.Time{})
2664+
2665+
jsonout, err := rawObj.MarshalJSON()
2666+
out, err := yaml.JSONToYAML(jsonout)
2667+
if err != nil {
2668+
return bundleDir, cleanup, err
2669+
}
2670+
if err := ioutil.WriteFile(filepath.Join(bundleDir, bundle.ManifestsDir, "csv.yaml"), out, 0666); err != nil {
2671+
return bundleDir, cleanup, err
2672+
}
2673+
2674+
out, err = yaml.Marshal(registry.AnnotationsFile{Annotations: annotations})
2675+
if err != nil {
2676+
return bundleDir, cleanup, err
2677+
}
2678+
if err := ioutil.WriteFile(filepath.Join(bundleDir, bundle.MetadataDir, "annotations.yaml"), out, 0666); err != nil {
2679+
return bundleDir, cleanup, err
2680+
}
2681+
return bundleDir, cleanup, nil
2682+
}
2683+
2684+
func TestValidateEdgeBundlePackage(t *testing.T) {
2685+
newInput := func(name, versionString, pkg, defaultChannel, channels, replaces string, skips []string) *registry.ImageInput {
2686+
v, err := semver.Parse(versionString)
2687+
require.NoError(t, err)
2688+
2689+
spec := v1alpha1.ClusterServiceVersionSpec{
2690+
Replaces: replaces,
2691+
Skips: skips,
2692+
Version: version.OperatorVersion{v},
2693+
}
2694+
specJson, err := json.Marshal(&spec)
2695+
require.NoError(t, err)
2696+
2697+
rawCSV, err := json.Marshal(registry.ClusterServiceVersion{
2698+
TypeMeta: v1.TypeMeta{
2699+
Kind: sqlite.ClusterServiceVersionKind,
2700+
},
2701+
ObjectMeta: v1.ObjectMeta{
2702+
Name: name,
2703+
},
2704+
Spec: specJson,
2705+
})
2706+
2707+
rawObj := unstructured.Unstructured{}
2708+
require.NoError(t, json.Unmarshal(rawCSV, &rawObj))
2709+
rawObj.SetCreationTimestamp(v1.Time{})
2710+
2711+
jsonout, err := rawObj.MarshalJSON()
2712+
require.NoError(t, err)
2713+
2714+
b, err := registry.NewBundleFromStrings(name, versionString, pkg, defaultChannel, channels, string(jsonout))
2715+
require.NoError(t, err)
2716+
return &registry.ImageInput{Bundle: b}
2717+
}
2718+
2719+
logrus.SetLevel(logrus.DebugLevel)
2720+
db, cleanup := CreateTestDb(t)
2721+
defer cleanup()
2722+
2723+
store, err := createAndPopulateDB(db)
2724+
require.NoError(t, err)
2725+
2726+
r := registry.NewDirectoryPopulator(nil, nil, store, nil, nil)
2727+
2728+
tests := []struct {
2729+
name string
2730+
args []*registry.ImageInput
2731+
wantErr error
2732+
}{
2733+
{
2734+
name: "conflictInAdded",
2735+
args: []*registry.ImageInput{
2736+
newInput("b1", "0.0.1", "p1", "a", "a", "", []string{}),
2737+
newInput("b2", "0.0.2", "p2", "a", "a", "", []string{"b1"}),
2738+
},
2739+
wantErr: fmt.Errorf("bundle b1 must belong to exactly one package, found on: [p1 p2]"),
2740+
},
2741+
{
2742+
name: "conflictWithExisting",
2743+
args: []*registry.ImageInput{
2744+
newInput("b1", "0.0.1", "p1", "a", "a", "", []string{"etcdoperator.v0.9.0"}),
2745+
},
2746+
wantErr: fmt.Errorf("bundle etcdoperator.v0.9.0 belongs to package etcd on index, cannot be added as an edge for package p1"),
2747+
},
2748+
{
2749+
name: "passForNonConflicting",
2750+
args: []*registry.ImageInput{
2751+
newInput("b1", "0.0.1", "etcd", "a", "a", "", []string{"etcdoperator.v0.9.0"}),
2752+
},
2753+
},
2754+
}
2755+
for _, tt := range tests {
2756+
t.Run(tt.name, func(t *testing.T) {
2757+
err := r.ValidateEdgeBundlePackage(tt.args)
2758+
if tt.wantErr == nil {
2759+
require.NoError(t, err)
2760+
return
2761+
}
2762+
require.EqualError(t, err, tt.wantErr.Error())
2763+
})
2764+
}
2765+
}

staging/operator-registry/pkg/registry/types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
"sort"
88
"strings"
99

10-
"github.com/blang/semver"
10+
"github.com/blang/semver/v4"
1111
)
1212

1313
var (

staging/operator-registry/pkg/sqlite/load.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
"fmt"
88
"strings"
99

10-
"github.com/blang/semver"
10+
"github.com/blang/semver/v4"
1111
_ "github.com/mattn/go-sqlite3"
1212
utilerrors "k8s.io/apimachinery/pkg/util/errors"
1313

0 commit comments

Comments
 (0)