diff --git a/pkg/controller/registry/resolver/installabletypes.go b/pkg/controller/registry/resolver/installabletypes.go index 1fd00375f3..0c017c7cc5 100644 --- a/pkg/controller/registry/resolver/installabletypes.go +++ b/pkg/controller/registry/resolver/installabletypes.go @@ -60,24 +60,30 @@ func NewBundleInstallableFromOperator(o *Operator) (BundleInstallable, error) { if src == nil { return BundleInstallable{}, fmt.Errorf("unable to resolve the source of bundle %s", o.Identifier()) } + id := bundleId(o.Identifier(), o.Channel(), src.Catalog) var constraints []solver.Constraint + if src.Catalog.Virtual() && src.Subscription == nil { + // CSVs already associated with a Subscription + // may be replaced, but freestanding CSVs must + // appear in any solution. + constraints = append(constraints, PrettyConstraint( + solver.Mandatory(), + fmt.Sprintf("clusterserviceversion %s exists and is not referenced by a subscription", o.Identifier()), + )) + } for _, p := range o.bundle.GetProperties() { if p.GetType() == operatorregistry.DeprecatedType { constraints = append(constraints, PrettyConstraint( solver.Prohibited(), - fmt.Sprintf("bundle %s is deprecated", bundleId(o.Identifier(), o.Channel(), src.Catalog)), + fmt.Sprintf("bundle %s is deprecated", id), )) break } } - return NewBundleInstallable(o.Identifier(), o.Channel(), src.Catalog, constraints...), nil -} - -func NewBundleInstallable(bundle, channel string, catalog registry.CatalogKey, constraints ...solver.Constraint) BundleInstallable { return BundleInstallable{ - identifier: bundleId(bundle, channel, catalog), + identifier: id, constraints: constraints, - } + }, nil } type GenericInstallable struct { diff --git a/pkg/controller/registry/resolver/operators.go b/pkg/controller/registry/resolver/operators.go index 19a0b933ed..1c21425bf2 100644 --- a/pkg/controller/registry/resolver/operators.go +++ b/pkg/controller/registry/resolver/operators.go @@ -11,10 +11,9 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "github.com/operator-framework/api/pkg/operators/v1alpha1" + "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry" "github.com/operator-framework/operator-registry/pkg/api" opregistry "github.com/operator-framework/operator-registry/pkg/registry" - - "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry" ) type APISet map[opregistry.APIKey]struct{} @@ -197,6 +196,7 @@ type OperatorSourceInfo struct { StartingCSV string Catalog registry.CatalogKey DefaultChannel bool + Subscription *v1alpha1.Subscription } func (i *OperatorSourceInfo) String() string { @@ -216,7 +216,6 @@ type OperatorSurface interface { SourceInfo() *OperatorSourceInfo Bundle() *api.Bundle Inline() bool - Dependencies() []*api.Dependency Properties() []*api.Property Skips() []string } @@ -229,7 +228,6 @@ type Operator struct { version *semver.Version bundle *api.Bundle sourceInfo *OperatorSourceInfo - dependencies []*api.Dependency properties []*api.Property skips []string } @@ -261,21 +259,27 @@ func NewOperatorFromBundle(bundle *api.Bundle, startingCSV string, sourceKey reg // legacy support - if the api doesn't contain properties/dependencies, build them from required/provided apis properties := bundle.Properties if len(properties) == 0 { - properties, err = apisToProperties(provided) + properties, err = providedAPIsToProperties(provided) if err != nil { return nil, err } } - dependencies := bundle.Dependencies - if len(dependencies) == 0 { - dependencies, err = apisToDependencies(required) + if len(bundle.Dependencies) > 0 { + ps, err := legacyDependenciesToProperties(bundle.Dependencies) + if err != nil { + return nil, fmt.Errorf("failed to translate legacy dependencies to properties: %w", err) + } + properties = append(properties, ps...) + } else { + ps, err := requiredAPIsToProperties(required) if err != nil { return nil, err } + properties = append(properties, ps...) } // legacy support - if the grpc api doesn't contain required/provided apis, fallback to csv parsing - if len(required) == 0 && len(provided) == 0 && len(properties) == 0 && len(dependencies) == 0 { + if len(required) == 0 && len(provided) == 0 && len(properties) == 0 { // fallback to csv parsing if bundle.CsvJson == "" { if bundle.GetBundlePath() != "" { @@ -307,7 +311,6 @@ func NewOperatorFromBundle(bundle *api.Bundle, startingCSV string, sourceKey reg requiredAPIs: required, bundle: bundle, sourceInfo: sourceInfo, - dependencies: dependencies, properties: properties, skips: bundle.Skips, }, nil @@ -338,15 +341,15 @@ func NewOperatorFromV1Alpha1CSV(csv *v1alpha1.ClusterServiceVersion) (*Operator, requiredAPIs[opregistry.APIKey{Group: api.Group, Version: api.Version, Kind: api.Kind, Plural: api.Name}] = struct{}{} } - dependencies, err := apisToDependencies(requiredAPIs) + properties, err := providedAPIsToProperties(providedAPIs) if err != nil { return nil, err } - - properties, err := apisToProperties(providedAPIs) + dependencies, err := requiredAPIsToProperties(requiredAPIs) if err != nil { return nil, err } + properties = append(properties, dependencies...) return &Operator{ name: csv.GetName(), @@ -354,7 +357,6 @@ func NewOperatorFromV1Alpha1CSV(csv *v1alpha1.ClusterServiceVersion) (*Operator, providedAPIs: providedAPIs, requiredAPIs: requiredAPIs, sourceInfo: &ExistingOperator, - dependencies: dependencies, properties: properties, }, nil } @@ -413,48 +415,48 @@ func (o *Operator) Inline() bool { return o.bundle != nil && o.bundle.GetBundlePath() == "" } -func (o *Operator) Dependencies() []*api.Dependency { - return o.dependencies -} - func (o *Operator) Properties() []*api.Property { return o.properties } func (o *Operator) DependencyPredicates() (predicates []OperatorPredicate, err error) { predicates = make([]OperatorPredicate, 0) - for _, d := range o.Dependencies() { - var p OperatorPredicate - if d == nil || d.Type == "" { - continue - } - p, err = PredicateForDependency(d) + for _, property := range o.Properties() { + predicate, err := PredicateForProperty(property) if err != nil { - return + return nil, err + } + if predicate == nil { + continue } - predicates = append(predicates, p) + predicates = append(predicates, predicate) } return } -// TODO: this should go in its own dependency/predicate builder package -// TODO: can we make this more extensible, i.e. via cue -func PredicateForDependency(dependency *api.Dependency) (OperatorPredicate, error) { - p, ok := predicates[dependency.Type] +func PredicateForProperty(property *api.Property) (OperatorPredicate, error) { + if property == nil { + return nil, nil + } + p, ok := predicates[property.Type] if !ok { - return nil, fmt.Errorf("no predicate for dependency type %s", dependency.Type) + return nil, nil } - return p(dependency.Value) + return p(property.Value) } var predicates = map[string]func(string) (OperatorPredicate, error){ - opregistry.GVKType: predicateForGVKDependency, - opregistry.PackageType: predicateForPackageDependency, - opregistry.LabelType: predicateForLabelDependency, + "olm.gvk.required": predicateForRequiredGVKProperty, + "olm.package.required": predicateForRequiredPackageProperty, + "olm.label.required": predicateForRequiredLabelProperty, } -func predicateForGVKDependency(value string) (OperatorPredicate, error) { - var gvk opregistry.GVKDependency +func predicateForRequiredGVKProperty(value string) (OperatorPredicate, error) { + var gvk struct { + Group string `json:"group"` + Version string `json:"version"` + Kind string `json:"kind"` + } if err := json.Unmarshal([]byte(value), &gvk); err != nil { return nil, err } @@ -465,44 +467,51 @@ func predicateForGVKDependency(value string) (OperatorPredicate, error) { }), nil } -func predicateForPackageDependency(value string) (OperatorPredicate, error) { - var pkg opregistry.PackageDependency +func predicateForRequiredPackageProperty(value string) (OperatorPredicate, error) { + var pkg struct { + PackageName string `json:"packageName"` + VersionRange string `json:"versionRange"` + } if err := json.Unmarshal([]byte(value), &pkg); err != nil { return nil, err } - ver, err := semver.ParseRange(pkg.Version) + ver, err := semver.ParseRange(pkg.VersionRange) if err != nil { return nil, err } - return And(WithPackage(pkg.PackageName), WithVersionInRange(ver)), nil } -func predicateForLabelDependency(value string) (OperatorPredicate, error) { - var label opregistry.LabelDependency +func predicateForRequiredLabelProperty(value string) (OperatorPredicate, error) { + var label struct { + Label string `json:"label"` + } if err := json.Unmarshal([]byte(value), &label); err != nil { return nil, err } - return WithLabel(label.Label), nil } -func apisToDependencies(apis APISet) (out []*api.Dependency, err error) { +func requiredAPIsToProperties(apis APISet) (out []*api.Property, err error) { if len(apis) == 0 { return } - out = make([]*api.Dependency, 0) + out = make([]*api.Property, 0) for a := range apis { - val, err := json.Marshal(opregistry.GVKDependency{ + val, err := json.Marshal(struct { + Group string `json:"group"` + Version string `json:"version"` + Kind string `json:"kind"` + }{ Group: a.Group, - Kind: a.Kind, Version: a.Version, + Kind: a.Kind, }) if err != nil { return nil, err } - out = append(out, &api.Dependency{ - Type: opregistry.GVKType, + out = append(out, &api.Property{ + Type: "olm.gvk.required", Value: string(val), }) } @@ -512,13 +521,13 @@ func apisToDependencies(apis APISet) (out []*api.Dependency, err error) { return nil, nil } -func apisToProperties(apis APISet) (out []*api.Property, err error) { +func providedAPIsToProperties(apis APISet) (out []*api.Property, err error) { out = make([]*api.Property, 0) for a := range apis { val, err := json.Marshal(opregistry.GVKProperty{ Group: a.Group, - Kind: a.Kind, Version: a.Version, + Kind: a.Kind, }) if err != nil { panic(err) @@ -533,3 +542,63 @@ func apisToProperties(apis APISet) (out []*api.Property, err error) { } return nil, nil } + +func legacyDependenciesToProperties(dependencies []*api.Dependency) ([]*api.Property, error) { + var result []*api.Property + for _, dependency := range dependencies { + switch dependency.Type { + case "olm.gvk": + type gvk struct { + Group string `json:"group"` + Version string `json:"version"` + Kind string `json:"kind"` + } + var vfrom gvk + if err := json.Unmarshal([]byte(dependency.Value), &vfrom); err != nil { + return nil, fmt.Errorf("failed to unmarshal legacy 'olm.gvk' dependency: %w", err) + } + vto := gvk{ + Group: vfrom.Group, + Version: vfrom.Version, + Kind: vfrom.Kind, + } + vb, err := json.Marshal(&vto) + if err != nil { + return nil, fmt.Errorf("unexpected error marshaling generated 'olm.package.required' property: %w", err) + } + result = append(result, &api.Property{ + Type: "olm.gvk.required", + Value: string(vb), + }) + case "olm.package": + var vfrom struct { + PackageName string `json:"packageName"` + VersionRange string `json:"version"` + } + if err := json.Unmarshal([]byte(dependency.Value), &vfrom); err != nil { + return nil, fmt.Errorf("failed to unmarshal legacy 'olm.package' dependency: %w", err) + } + vto := struct { + PackageName string `json:"packageName"` + VersionRange string `json:"versionRange"` + }{ + PackageName: vfrom.PackageName, + VersionRange: vfrom.VersionRange, + } + vb, err := json.Marshal(&vto) + if err != nil { + return nil, fmt.Errorf("unexpected error marshaling generated 'olm.package.required' property: %w", err) + } + result = append(result, &api.Property{ + Type: "olm.package.required", + Value: string(vb), + }) + case "olm.label": + result = append(result, &api.Property{ + Type: "olm.label.required", + Value: dependency.Value, + }) + } + } + return result, nil +} diff --git a/pkg/controller/registry/resolver/operators_test.go b/pkg/controller/registry/resolver/operators_test.go index 5cbbdeabcd..4f3897fb94 100644 --- a/pkg/controller/registry/resolver/operators_test.go +++ b/pkg/controller/registry/resolver/operators_test.go @@ -737,7 +737,7 @@ func TestAPIMultiOwnerSet_PopAPIKey(t *testing.T) { { name: "OneApi/OneOwner", s: map[opregistry.APIKey]OperatorSet{ - opregistry.APIKey{"g", "v", "k", "p"}: map[string]OperatorSurface{ + {Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]OperatorSurface{ "owner1": &Operator{name: "op1"}, }, }, @@ -745,7 +745,7 @@ func TestAPIMultiOwnerSet_PopAPIKey(t *testing.T) { { name: "OneApi/MultiOwner", s: map[opregistry.APIKey]OperatorSet{ - opregistry.APIKey{"g", "v", "k", "p"}: map[string]OperatorSurface{ + {Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]OperatorSurface{ "owner1": &Operator{name: "op1"}, "owner2": &Operator{name: "op2"}, }, @@ -754,11 +754,11 @@ func TestAPIMultiOwnerSet_PopAPIKey(t *testing.T) { { name: "MultipleApi/MultiOwner", s: map[opregistry.APIKey]OperatorSet{ - opregistry.APIKey{"g", "v", "k", "p"}: map[string]OperatorSurface{ + {Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]OperatorSurface{ "owner1": &Operator{name: "op1"}, "owner2": &Operator{name: "op2"}, }, - opregistry.APIKey{"g2", "v2", "k2", "p2"}: map[string]OperatorSurface{ + {Group: "g2", Version: "v2", Kind: "k2", Plural: "p2"}: map[string]OperatorSurface{ "owner1": &Operator{name: "op1"}, "owner2": &Operator{name: "op2"}, }, @@ -797,7 +797,7 @@ func TestAPIMultiOwnerSet_PopAPIRequirers(t *testing.T) { { name: "OneApi/OneOwner", s: map[opregistry.APIKey]OperatorSet{ - opregistry.APIKey{"g", "v", "k", "p"}: map[string]OperatorSurface{ + {Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]OperatorSurface{ "owner1": &Operator{name: "op1"}, }, }, @@ -808,7 +808,7 @@ func TestAPIMultiOwnerSet_PopAPIRequirers(t *testing.T) { { name: "OneApi/MultiOwner", s: map[opregistry.APIKey]OperatorSet{ - opregistry.APIKey{"g", "v", "k", "p"}: map[string]OperatorSurface{ + {Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]OperatorSurface{ "owner1": &Operator{name: "op1"}, "owner2": &Operator{name: "op2"}, }, @@ -821,11 +821,11 @@ func TestAPIMultiOwnerSet_PopAPIRequirers(t *testing.T) { { name: "MultipleApi/MultiOwner", s: map[opregistry.APIKey]OperatorSet{ - opregistry.APIKey{"g", "v", "k", "p"}: map[string]OperatorSurface{ + {Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]OperatorSurface{ "owner1": &Operator{name: "op1"}, "owner2": &Operator{name: "op2"}, }, - opregistry.APIKey{"g2", "v2", "k2", "p2"}: map[string]OperatorSurface{ + {Group: "g2", Version: "v2", Kind: "k2", Plural: "p2"}: map[string]OperatorSurface{ "owner1": &Operator{name: "op1"}, "owner2": &Operator{name: "op2"}, }, @@ -879,7 +879,7 @@ func TestOperatorSourceInfo_String(t *testing.T) { i := &OperatorSourceInfo{ Package: tt.fields.Package, Channel: tt.fields.Channel, - Catalog: registry.CatalogKey{tt.fields.CatalogSource, tt.fields.CatalogSourceNamespace}, + Catalog: registry.CatalogKey{Name: tt.fields.CatalogSource, Namespace: tt.fields.CatalogSourceNamespace}, } if got := i.String(); got != tt.want { t.Errorf("OperatorSourceInfo.String() = %v, want %v", got, tt.want) @@ -889,7 +889,7 @@ func TestOperatorSourceInfo_String(t *testing.T) { } func TestNewOperatorFromBundle(t *testing.T) { - version := opver.OperatorVersion{semver.MustParse("0.1.0-abc")} + version := opver.OperatorVersion{Version: semver.MustParse("0.1.0-abc")} csv := v1alpha1.ClusterServiceVersion{ TypeMeta: metav1.TypeMeta{ Kind: v1alpha1.ClusterServiceVersionKind, @@ -1087,7 +1087,7 @@ func TestNewOperatorFromBundle(t *testing.T) { sourceInfo: &OperatorSourceInfo{ Package: "testPackage", Channel: "testChannel", - Catalog: registry.CatalogKey{"source", "testNamespace"}, + Catalog: registry.CatalogKey{Name: "source", Namespace: "testNamespace"}, }, }, }, @@ -1137,14 +1137,12 @@ func TestNewOperatorFromBundle(t *testing.T) { Type: "olm.gvk", Value: "{\"group\":\"apis.group.com\",\"kind\":\"OwnedAPI\",\"version\":\"v1\"}", }, - }, - dependencies: []*api.Dependency{ { - Type: "olm.gvk", + Type: "olm.gvk.required", Value: "{\"group\":\"crd.group.com\",\"kind\":\"RequiredCRD\",\"version\":\"v1\"}", }, { - Type: "olm.gvk", + Type: "olm.gvk.required", Value: "{\"group\":\"apis.group.com\",\"kind\":\"RequiredAPI\",\"version\":\"v1\"}", }, }, @@ -1152,7 +1150,7 @@ func TestNewOperatorFromBundle(t *testing.T) { sourceInfo: &OperatorSourceInfo{ Package: "testPackage", Channel: "testChannel", - Catalog: registry.CatalogKey{"source", "testNamespace"}, + Catalog: registry.CatalogKey{Name: "source", Namespace: "testNamespace"}, }, }, }, @@ -1172,7 +1170,7 @@ func TestNewOperatorFromBundle(t *testing.T) { sourceInfo: &OperatorSourceInfo{ Package: "testPackage", Channel: "testChannel", - Catalog: registry.CatalogKey{"source", "testNamespace"}, + Catalog: registry.CatalogKey{Name: "source", Namespace: "testNamespace"}, }, }, }, @@ -1221,14 +1219,12 @@ func TestNewOperatorFromBundle(t *testing.T) { Type: "olm.gvk", Value: "{\"group\":\"apis.group.com\",\"kind\":\"OwnedAPI\",\"version\":\"v1\"}", }, - }, - dependencies: []*api.Dependency{ { - Type: "olm.gvk", + Type: "olm.gvk.required", Value: "{\"group\":\"apis.group.com\",\"kind\":\"RequiredAPI\",\"version\":\"v1\"}", }, { - Type: "olm.gvk", + Type: "olm.gvk.required", Value: "{\"group\":\"crd.group.com\",\"kind\":\"RequiredCRD\",\"version\":\"v1\"}", }, }, @@ -1237,7 +1233,7 @@ func TestNewOperatorFromBundle(t *testing.T) { sourceInfo: &OperatorSourceInfo{ Package: "testPackage", Channel: "testChannel", - Catalog: registry.CatalogKey{"source", "testNamespace"}, + Catalog: registry.CatalogKey{Name: "source", Namespace: "testNamespace"}, }, }, }, @@ -1257,7 +1253,7 @@ func TestNewOperatorFromBundle(t *testing.T) { sourceInfo: &OperatorSourceInfo{ Package: "testPackage", Channel: "testChannel", - Catalog: registry.CatalogKey{"source", "testNamespace"}, + Catalog: registry.CatalogKey{Name: "source", Namespace: "testNamespace"}, DefaultChannel: true, }, }, @@ -1278,7 +1274,7 @@ func TestNewOperatorFromBundle(t *testing.T) { sourceInfo: &OperatorSourceInfo{ Package: "testPackage", Channel: "testChannel", - Catalog: registry.CatalogKey{"source", "testNamespace"}, + Catalog: registry.CatalogKey{Name: "source", Namespace: "testNamespace"}, }, }, }, @@ -1302,14 +1298,12 @@ func TestNewOperatorFromBundle(t *testing.T) { Type: "olm.gvk", Value: "{\"group\":\"apis.group.com\",\"kind\":\"OwnedAPI\",\"version\":\"v1\"}", }, - }, - dependencies: []*api.Dependency{ { - Type: "olm.gvk", + Type: "olm.gvk.required", Value: "{\"group\":\"crd.group.com\",\"kind\":\"RequiredCRD\",\"version\":\"v1\"}", }, { - Type: "olm.gvk", + Type: "olm.gvk.required", Value: "{\"group\":\"apis.group.com\",\"kind\":\"RequiredAPI\",\"version\":\"v1\"}", }, }, @@ -1317,7 +1311,7 @@ func TestNewOperatorFromBundle(t *testing.T) { sourceInfo: &OperatorSourceInfo{ Package: "testPackage", Channel: "testChannel", - Catalog: registry.CatalogKey{"source", "testNamespace"}, + Catalog: registry.CatalogKey{Name: "source", Namespace: "testNamespace"}, }, }, }, @@ -1326,16 +1320,15 @@ func TestNewOperatorFromBundle(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := NewOperatorFromBundle(tt.args.bundle, "", tt.args.sourceKey, tt.args.defaultChannel) require.Equal(t, tt.wantErr, err) - require.ElementsMatch(t, tt.want.dependencies, got.dependencies) - require.ElementsMatch(t, tt.want.properties, got.properties) - tt.want.properties, tt.want.dependencies, got.dependencies, got.properties = nil, nil, nil, nil + requirePropertiesEqual(t, tt.want.properties, got.properties) + tt.want.properties, got.properties = nil, nil require.Equal(t, tt.want, got) }) } } func TestNewOperatorFromCSV(t *testing.T) { - version := opver.OperatorVersion{semver.MustParse("0.1.0-abc")} + version := opver.OperatorVersion{Version: semver.MustParse("0.1.0-abc")} type args struct { csv *v1alpha1.ClusterServiceVersion } @@ -1455,13 +1448,13 @@ func TestNewOperatorFromCSV(t *testing.T) { {Group: "g", Version: "v1", Kind: "APIKind", Plural: "apikinds"}: {}, {Group: "g", Version: "v1", Kind: "CRDKind", Plural: "crdkinds"}: {}, }, - dependencies: []*api.Dependency{ + properties: []*api.Property{ { - Type: "olm.gvk", + Type: "olm.gvk.required", Value: "{\"group\":\"g\",\"kind\":\"APIKind\",\"version\":\"v1\"}", }, { - Type: "olm.gvk", + Type: "olm.gvk.required", Value: "{\"group\":\"g\",\"kind\":\"CRDKind\",\"version\":\"v1\"}", }, }, @@ -1534,14 +1527,12 @@ func TestNewOperatorFromCSV(t *testing.T) { Type: "olm.gvk", Value: "{\"group\":\"g\",\"kind\":\"CRDOwnedKind\",\"version\":\"v1\"}", }, - }, - dependencies: []*api.Dependency{ { - Type: "olm.gvk", + Type: "olm.gvk.required", Value: "{\"group\":\"g2\",\"kind\":\"APIReqKind\",\"version\":\"v1\"}", }, { - Type: "olm.gvk", + Type: "olm.gvk.required", Value: "{\"group\":\"g2\",\"kind\":\"CRDReqKind\",\"version\":\"v1\"}", }, }, @@ -1554,10 +1545,34 @@ func TestNewOperatorFromCSV(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := NewOperatorFromV1Alpha1CSV(tt.args.csv) require.Equal(t, tt.wantErr, err) - require.ElementsMatch(t, tt.want.dependencies, got.dependencies) - require.ElementsMatch(t, tt.want.properties, got.properties) - tt.want.properties, tt.want.dependencies, got.dependencies, got.properties = nil, nil, nil, nil + requirePropertiesEqual(t, tt.want.properties, got.properties) + tt.want.properties, got.properties = nil, nil require.Equal(t, tt.want, got) }) } } + +func requirePropertiesEqual(t *testing.T, a, b []*api.Property) { + type Property struct { + Type string + Value interface{} + } + nice := func(in *api.Property) Property { + var i interface{} + if err := json.Unmarshal([]byte(in.Value), &i); err != nil { + t.Fatalf("property value %q could not be unmarshaled as json: %s", in.Value, err) + } + return Property{ + Type: in.Type, + Value: i, + } + } + var l, r []Property + for _, p := range a { + l = append(l, nice(p)) + } + for _, p := range b { + r = append(r, nice(p)) + } + require.ElementsMatch(t, l, r) +} diff --git a/pkg/controller/registry/resolver/resolver.go b/pkg/controller/registry/resolver/resolver.go index e2ee4a66e0..a4473f36bf 100644 --- a/pkg/controller/registry/resolver/resolver.go +++ b/pkg/controller/registry/resolver/resolver.go @@ -55,12 +55,16 @@ func (r *SatResolver) SolveOperators(namespaces []string, csvs []*v1alpha1.Clust startingCSVs := make(map[string]struct{}) // build a virtual catalog of all currently installed CSVs - existingSnapshot, existingInstallables, err := r.newSnapshotForNamespace(namespaces[0], subs, csvs) + existingSnapshot, err := r.newSnapshotForNamespace(namespaces[0], subs, csvs) if err != nil { return nil, err } namespacedCache := r.cache.Namespaced(namespaces...).WithExistingOperators(existingSnapshot) + _, existingInstallables, err := r.getBundleInstallables(registry.NewVirtualCatalogKey(namespaces[0]), existingSnapshot.Find(), namespacedCache, visited) + if err != nil { + return nil, err + } for _, i := range existingInstallables { installables[i.Identifier()] = i } @@ -248,7 +252,8 @@ func (r *SatResolver) getSubscriptionInstallables(sub *v1alpha1.Subscription, cu candidates := make([]*BundleInstallable, 0) for _, o := range Filter(sortedBundles, channelPredicates...) { predicates := append(cachePredicates, WithCSVName(o.Identifier())) - id, installable, err := r.getBundleInstallables(catalog, predicates, namespacedCache, visited) + stack := namespacedCache.Catalog(catalog).Find(predicates...) + id, installable, err := r.getBundleInstallables(catalog, stack, namespacedCache, visited) if err != nil { return nil, err } @@ -292,17 +297,10 @@ func (r *SatResolver) getSubscriptionInstallables(sub *v1alpha1.Subscription, cu return installables, nil } -func (r *SatResolver) getBundleInstallables(catalog registry.CatalogKey, predicates []OperatorPredicate, namespacedCache MultiCatalogOperatorFinder, visited map[OperatorSurface]*BundleInstallable) (map[solver.Identifier]struct{}, map[solver.Identifier]*BundleInstallable, error) { +func (r *SatResolver) getBundleInstallables(catalog registry.CatalogKey, bundleStack []*Operator, namespacedCache MultiCatalogOperatorFinder, visited map[OperatorSurface]*BundleInstallable) (map[solver.Identifier]struct{}, map[solver.Identifier]*BundleInstallable, error) { errs := make([]error, 0) installables := make(map[solver.Identifier]*BundleInstallable, 0) // all installables, including dependencies - var finder OperatorFinder = namespacedCache - if !catalog.Empty() { - finder = namespacedCache.Catalog(catalog) - } - - bundleStack := finder.Find(predicates...) - // track the first layer of installable ids var initial = make(map[*Operator]struct{}) for _, o := range bundleStack { @@ -353,11 +351,18 @@ func (r *SatResolver) getBundleInstallables(catalog registry.CatalogKey, predica } sources[*si] = struct{}{} - sourcePredicate = Or(sourcePredicate, And( - WithPackage(si.Package), - WithChannel(si.Channel), - WithCatalog(si.Catalog), - )) + if si.Catalog.Virtual() { + sourcePredicate = Or(sourcePredicate, And( + WithCSVName(b.Identifier()), + WithCatalog(si.Catalog), + )) + } else { + sourcePredicate = Or(sourcePredicate, And( + WithPackage(si.Package), + WithChannel(si.Channel), + WithCatalog(si.Catalog), + )) + } } sortedBundles, err := r.sortBundles(namespacedCache.FindPreferred(&bundle.sourceInfo.Catalog, sourcePredicate)) if err != nil { @@ -441,15 +446,14 @@ func (r *SatResolver) inferProperties(csv *v1alpha1.ClusterServiceVersion, subs return properties, nil } -func (r *SatResolver) newSnapshotForNamespace(namespace string, subs []*v1alpha1.Subscription, csvs []*v1alpha1.ClusterServiceVersion) (*CatalogSnapshot, []solver.Installable, error) { - installables := make([]solver.Installable, 0) +func (r *SatResolver) newSnapshotForNamespace(namespace string, subs []*v1alpha1.Subscription, csvs []*v1alpha1.ClusterServiceVersion) (*CatalogSnapshot, error) { existingOperatorCatalog := registry.NewVirtualCatalogKey(namespace) // build a catalog snapshot of CSVs without subscriptions - csvsWithSubscriptions := make(map[*v1alpha1.ClusterServiceVersion]struct{}) + csvSubscriptions := make(map[*v1alpha1.ClusterServiceVersion]*v1alpha1.Subscription) for _, sub := range subs { for _, csv := range csvs { if csv.Name == sub.Status.InstalledCSV { - csvsWithSubscriptions[csv] = struct{}{} + csvSubscriptions[csv] = sub break } } @@ -457,17 +461,9 @@ func (r *SatResolver) newSnapshotForNamespace(namespace string, subs []*v1alpha1 var csvsMissingProperties []*v1alpha1.ClusterServiceVersion standaloneOperators := make([]*Operator, 0) for _, csv := range csvs { - var constraints []solver.Constraint - if _, ok := csvsWithSubscriptions[csv]; !ok { - // CSVs already associated with a Subscription - // may be replaced, but freestanding CSVs must - // appear in any solution. - constraints = append(constraints, solver.Mandatory()) - } - op, err := NewOperatorFromV1Alpha1CSV(csv) if err != nil { - return nil, nil, err + return nil, err } if anno, ok := csv.GetAnnotations()[projection.PropertiesAnnotationKey]; !ok { @@ -478,13 +474,14 @@ func (r *SatResolver) newSnapshotForNamespace(namespace string, subs []*v1alpha1 op.properties = append(op.properties, inferred...) } } else if props, err := projection.PropertyListFromPropertiesAnnotation(anno); err != nil { - return nil, nil, fmt.Errorf("failed to retrieve properties of csv %q: %w", csv.GetName(), err) + return nil, fmt.Errorf("failed to retrieve properties of csv %q: %w", csv.GetName(), err) } else { op.properties = props } op.sourceInfo = &OperatorSourceInfo{ - Catalog: existingOperatorCatalog, + Catalog: existingOperatorCatalog, + Subscription: csvSubscriptions[csv], } // Try to determine source package name from properties and add to SourceInfo. for _, p := range op.properties { @@ -501,10 +498,6 @@ func (r *SatResolver) newSnapshotForNamespace(namespace string, subs []*v1alpha1 } standaloneOperators = append(standaloneOperators, op) - - // all standalone operators are mandatory - i := NewBundleInstallable(op.Identifier(), "", existingOperatorCatalog, constraints...) - installables = append(installables, &i) } if len(csvsMissingProperties) > 0 { @@ -515,7 +508,7 @@ func (r *SatResolver) newSnapshotForNamespace(namespace string, subs []*v1alpha1 r.log.Infof("considered csvs without properties annotation during resolution: %v", names) } - return NewRunningOperatorSnapshot(r.log, existingOperatorCatalog, standaloneOperators), installables, nil + return NewRunningOperatorSnapshot(r.log, existingOperatorCatalog, standaloneOperators), nil } func (r *SatResolver) addInvariants(namespacedCache MultiCatalogOperatorFinder, installables map[solver.Identifier]solver.Installable) { diff --git a/pkg/controller/registry/resolver/resolver_test.go b/pkg/controller/registry/resolver/resolver_test.go index 0472ef65fe..22106e1558 100644 --- a/pkg/controller/registry/resolver/resolver_test.go +++ b/pkg/controller/registry/resolver/resolver_test.go @@ -1298,8 +1298,19 @@ func getFakeOperatorCache(fakedNamespacedOperatorCache NamespacedOperatorCache) func genOperator(name, version, replaces, pkg, channel, catalogName, catalogNamespace string, requiredAPIs, providedAPIs APISet, dependencies []*api.Dependency, defaultChannel string, deprecated bool) *Operator { semversion, _ := semver.Make(version) + properties := apiSetToProperties(providedAPIs, nil, deprecated) if len(dependencies) == 0 { - dependencies = apiSetToDependencies(requiredAPIs, nil) + ps, err := requiredAPIsToProperties(requiredAPIs) + if err != nil { + panic(err) + } + properties = append(properties, ps...) + } else { + ps, err := legacyDependenciesToProperties(dependencies) + if err != nil { + panic(err) + } + properties = append(properties, ps...) } o := &Operator{ name: name, @@ -1309,10 +1320,9 @@ func genOperator(name, version, replaces, pkg, channel, catalogName, catalogName PackageName: pkg, ChannelName: channel, Dependencies: dependencies, - Properties: apiSetToProperties(providedAPIs, nil, deprecated), + Properties: properties, }, - dependencies: dependencies, - properties: apiSetToProperties(providedAPIs, nil, deprecated), + properties: properties, sourceInfo: &OperatorSourceInfo{ Catalog: registry.CatalogKey{ Name: catalogName, @@ -1490,6 +1500,46 @@ func TestSolveOperators_WithSkips(t *testing.T) { require.EqualValues(t, expected, operators) } +func TestSolveOperatorsWithClusterServiceVersionHavingDependency(t *testing.T) { + const namespace = "test-namespace" + catalog := registry.CatalogKey{Name: "test-catalog", Namespace: namespace} + + a1 := existingOperator(namespace, "a-1", "a", "default", "", nil, nil, nil, nil) + a1.Annotations = map[string]string{ + "operatorframework.io/properties": `{"properties":[{"type":"olm.package.required","value":{"packageName":"b","versionRange":"1.0.0"}}]}`, + } + + b1 := existingOperator(namespace, "b-1", "b", "default", "", nil, nil, nil, nil) + b1.Annotations = map[string]string{ + "operatorframework.io/properties": `{"properties":[{"type":"olm.package","value":{"packageName":"b","version":"1.0.0"}}]}`, + } + + csvs := []*v1alpha1.ClusterServiceVersion{a1, b1} + subs := []*v1alpha1.Subscription{ + existingSub(namespace, "b-1", "b", "default", catalog), + } + + log, _ := test.NewNullLogger() + r := SatResolver{ + cache: getFakeOperatorCache(NamespacedOperatorCache{ + snapshots: map[registry.CatalogKey]*CatalogSnapshot{ + catalog: { + key: catalog, + operators: []*Operator{ + genOperator("b-2", "2.0.0", "b-1", "b", "default", catalog.Name, catalog.Namespace, nil, nil, nil, "", false), + }, + }, + }, + }), + log: log, + } + + operators, err := r.SolveOperators([]string{namespace}, csvs, subs) + assert.NoError(t, err) + //expected := OperatorSet{} + require.Empty(t, operators) +} + func TestInferProperties(t *testing.T) { catalog := registry.CatalogKey{Namespace: "namespace", Name: "name"}