From d8ae4fbda11998dc226c1342c021433bcdfff1ab Mon Sep 17 00:00:00 2001 From: Mikalai Radchuk Date: Mon, 11 Sep 2023 16:59:50 +0100 Subject: [PATCH 1/6] Updates codebase to use `catalogmetadata` package Signed-off-by: Mikalai Radchuk Signed-off-by: Daniel Franz --- cmd/manager/main.go | 11 ++- cmd/resolutioncli/main.go | 26 +++-- cmd/resolutioncli/variable_source.go | 4 +- internal/controllers/operator_controller.go | 27 ++---- internal/controllers/variable_source.go | 9 +- internal/resolution/variables/bundle.go | 42 ++++++--- .../resolution/variables/installed_package.go | 20 ++-- .../resolution/variables/required_package.go | 20 ++-- .../variablesources/bundle_deployment.go | 11 ++- .../bundles_and_dependencies.go | 94 ++++++++++--------- .../resolution/variablesources/composite.go | 8 +- .../variablesources/crd_constraints.go | 29 +++--- .../variablesources/installed_package.go | 58 +++++------- .../resolution/variablesources/operator.go | 10 +- .../variablesources/required_package.go | 38 ++++---- 15 files changed, 207 insertions(+), 200 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 769442ad3..b54614576 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -35,8 +35,8 @@ import ( rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" "github.com/operator-framework/operator-controller/internal/controllers" - "github.com/operator-framework/operator-controller/internal/resolution/entitysources" "github.com/operator-framework/operator-controller/pkg/features" ) @@ -99,12 +99,15 @@ func main() { os.Exit(1) } + cl := mgr.GetClient() + catalogClient := catalogclient.New(cl) + if err = (&controllers.OperatorReconciler{ - Client: mgr.GetClient(), + Client: cl, Scheme: mgr.GetScheme(), Resolver: solver.NewDeppySolver( - entitysources.NewCatalogdEntitySource(mgr.GetClient()), - controllers.NewVariableSource(mgr.GetClient()), + nil, + controllers.NewVariableSource(cl, catalogClient), ), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Operator") diff --git a/cmd/resolutioncli/main.go b/cmd/resolutioncli/main.go index d9ad80b13..d15153753 100644 --- a/cmd/resolutioncli/main.go +++ b/cmd/resolutioncli/main.go @@ -34,8 +34,9 @@ import ( catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" + catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" "github.com/operator-framework/operator-controller/internal/controllers" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) @@ -121,11 +122,14 @@ func run(ctx context.Context, packageName, packageVersion, packageChannel, catal } cl := clientBuilder.Build() + catalogClient := catalogclient.New(cl) + resolver := solver.NewDeppySolver( - newIndexRefEntitySourceEntitySource(catalogRef), + // TODO: Add a variable source to replace `indexRefEntitySource` which will read from catalogRef + nil, append( - variablesources.NestedVariableSource{newPackageVariableSource(packageName, packageVersion, packageChannel)}, - controllers.NewVariableSource(cl)..., + variablesources.NestedVariableSource{newPackageVariableSource(catalogClient, packageName, packageVersion, packageChannel)}, + controllers.NewVariableSource(cl, catalogClient)..., ), ) @@ -150,22 +154,14 @@ func resolve(ctx context.Context, resolver *solver.DeppySolver, packageName stri } // Get the bundle image reference for the bundle - bundleImage, err := bundleEntity.BundlePath() - if err != nil { - return "", err - } - - return bundleImage, nil + return bundleEntity.Image, nil } -func getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*olmentity.BundleEntity, error) { +func getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) { for _, variable := range solution.SelectedVariables() { switch v := variable.(type) { case *olmvariables.BundleVariable: - entityPkgName, err := v.BundleEntity().PackageName() - if err != nil { - return nil, err - } + entityPkgName := v.BundleEntity().Package if packageName == entityPkgName { return v.BundleEntity(), nil } diff --git a/cmd/resolutioncli/variable_source.go b/cmd/resolutioncli/variable_source.go index d100fa622..5d4069d2f 100644 --- a/cmd/resolutioncli/variable_source.go +++ b/cmd/resolutioncli/variable_source.go @@ -19,12 +19,14 @@ package main import ( "github.com/operator-framework/deppy/pkg/deppy/input" + catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -func newPackageVariableSource(packageName, packageVersion, packageChannel string) func(inputVariableSource input.VariableSource) (input.VariableSource, error) { +func newPackageVariableSource(catalog *catalogclient.Client, packageName, packageVersion, packageChannel string) func(inputVariableSource input.VariableSource) (input.VariableSource, error) { return func(inputVariableSource input.VariableSource) (input.VariableSource, error) { pkgSource, err := variablesources.NewRequiredPackageVariableSource( + catalog, packageName, variablesources.InVersionRange(packageVersion), variablesources.InChannel(packageChannel), diff --git a/internal/controllers/operator_controller.go b/internal/controllers/operator_controller.go index b07663424..c260fb7f1 100644 --- a/internal/controllers/operator_controller.go +++ b/internal/controllers/operator_controller.go @@ -44,6 +44,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" "github.com/operator-framework/operator-controller/internal/controllers/validators" "github.com/operator-framework/operator-controller/internal/resolution/entities" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" @@ -165,20 +166,9 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha return ctrl.Result{}, err } - // Get the bundle image reference for the bundle - bundleImage, err := bundleEntity.BundlePath() - if err != nil { - op.Status.InstalledBundleResource = "" - setInstalledStatusConditionUnknown(&op.Status.Conditions, "installation has not been attempted as resolution failed", op.GetGeneration()) - - op.Status.ResolvedBundleResource = "" - setResolvedStatusConditionFailed(&op.Status.Conditions, err.Error(), op.GetGeneration()) - return ctrl.Result{}, err - } - - // Now we can set the Resolved Condition, and the resolvedBundleSource field to the bundleImage value. - op.Status.ResolvedBundleResource = bundleImage - setResolvedStatusConditionSuccess(&op.Status.Conditions, fmt.Sprintf("resolved to %q", bundleImage), op.GetGeneration()) + // Now we can set the Resolved Condition, and the resolvedBundleSource field to the bundleEntity.Image value. + op.Status.ResolvedBundleResource = bundleEntity.Image + setResolvedStatusConditionSuccess(&op.Status.Conditions, fmt.Sprintf("resolved to %q", bundleEntity.Image), op.GetGeneration()) mediaType, err := bundleEntity.MediaType() if err != nil { @@ -192,7 +182,7 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha } // Ensure a BundleDeployment exists with its bundle source from the bundle // image we just looked up in the solution. - dep := r.generateExpectedBundleDeployment(*op, bundleImage, bundleProvisioner) + dep := r.generateExpectedBundleDeployment(*op, bundleEntity.Image, bundleProvisioner) if err := r.ensureBundleDeployment(ctx, dep); err != nil { // originally Reason: operatorsv1alpha1.ReasonInstallationFailed op.Status.InstalledBundleResource = "" @@ -262,14 +252,11 @@ func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alph } } -func (r *OperatorReconciler) getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*entities.BundleEntity, error) { +func (r *OperatorReconciler) getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) { for _, variable := range solution.SelectedVariables() { switch v := variable.(type) { case *olmvariables.BundleVariable: - entityPkgName, err := v.BundleEntity().PackageName() - if err != nil { - return nil, err - } + entityPkgName := v.BundleEntity().Package if packageName == entityPkgName { return v.BundleEntity(), nil } diff --git a/internal/controllers/variable_source.go b/internal/controllers/variable_source.go index 21fa0b219..c3fa7bacd 100644 --- a/internal/controllers/variable_source.go +++ b/internal/controllers/variable_source.go @@ -21,19 +21,20 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/input" + catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -func NewVariableSource(cl client.Client) variablesources.NestedVariableSource { +func NewVariableSource(cl client.Client, catalog *catalogclient.Client) variablesources.NestedVariableSource { return variablesources.NestedVariableSource{ func(inputVariableSource input.VariableSource) (input.VariableSource, error) { - return variablesources.NewOperatorVariableSource(cl, inputVariableSource), nil + return variablesources.NewOperatorVariableSource(cl, catalog, inputVariableSource), nil }, func(inputVariableSource input.VariableSource) (input.VariableSource, error) { - return variablesources.NewBundleDeploymentVariableSource(cl, inputVariableSource), nil + return variablesources.NewBundleDeploymentVariableSource(cl, catalog, inputVariableSource), nil }, func(inputVariableSource input.VariableSource) (input.VariableSource, error) { - return variablesources.NewBundlesAndDepsVariableSource(inputVariableSource), nil + return variablesources.NewBundlesAndDepsVariableSource(catalog, inputVariableSource), nil }, func(inputVariableSource input.VariableSource) (input.VariableSource, error) { return variablesources.NewCRDUniquenessConstraintsVariableSource(inputVariableSource), nil diff --git a/internal/resolution/variables/bundle.go b/internal/resolution/variables/bundle.go index dce8bed50..4be11a634 100644 --- a/internal/resolution/variables/bundle.go +++ b/internal/resolution/variables/bundle.go @@ -1,42 +1,44 @@ package variables import ( + "fmt" + "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/constraint" "github.com/operator-framework/deppy/pkg/deppy/input" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" ) var _ deppy.Variable = &BundleVariable{} type BundleVariable struct { *input.SimpleVariable - bundleEntity *olmentity.BundleEntity - dependencies []*olmentity.BundleEntity + bundle *catalogmetadata.Bundle + dependencies []*catalogmetadata.Bundle } -func (b *BundleVariable) BundleEntity() *olmentity.BundleEntity { - return b.bundleEntity +func (b *BundleVariable) BundleEntity() *catalogmetadata.Bundle { + return b.bundle } -func (b *BundleVariable) Dependencies() []*olmentity.BundleEntity { +func (b *BundleVariable) Dependencies() []*catalogmetadata.Bundle { return b.dependencies } -func NewBundleVariable(bundleEntity *olmentity.BundleEntity, dependencyBundleEntities []*olmentity.BundleEntity) *BundleVariable { - dependencyIDs := make([]deppy.Identifier, 0, len(dependencyBundleEntities)) - for _, bundle := range dependencyBundleEntities { - dependencyIDs = append(dependencyIDs, bundle.ID) +func NewBundleVariable(id deppy.Identifier, bundle *catalogmetadata.Bundle, dependencies []*catalogmetadata.Bundle) *BundleVariable { + var dependencyIDs []deppy.Identifier + for _, bundle := range dependencies { + dependencyIDs = append(dependencyIDs, BundleToBundleVariableIDs(bundle)...) } var constraints []deppy.Constraint if len(dependencyIDs) > 0 { constraints = append(constraints, constraint.Dependency(dependencyIDs...)) } return &BundleVariable{ - SimpleVariable: input.NewSimpleVariable(bundleEntity.ID, constraints...), - bundleEntity: bundleEntity, - dependencies: dependencyBundleEntities, + SimpleVariable: input.NewSimpleVariable(id, constraints...), + bundle: bundle, + dependencies: dependencies, } } @@ -56,3 +58,17 @@ func NewBundleUniquenessVariable(id deppy.Identifier, atMostIDs ...deppy.Identif SimpleVariable: input.NewSimpleVariable(id, constraint.AtMost(1, atMostIDs...)), } } + +// BundleToBundleVariableIDs returns a list of all possible IDs for a bundle. +// A bundle can be present in multiple channels and we need a separate variable +// with a unique ID for each occurrence. +// Result must be deterministic. +func BundleToBundleVariableIDs(bundle *catalogmetadata.Bundle) []deppy.Identifier { + out := make([]deppy.Identifier, 0, len(bundle.InChannels)) + for _, ch := range bundle.InChannels { + out = append(out, deppy.Identifier( + fmt.Sprintf("%s-%s-%s-%s", bundle.CatalogName, bundle.Package, ch.Name, bundle.Name), + )) + } + return out +} diff --git a/internal/resolution/variables/installed_package.go b/internal/resolution/variables/installed_package.go index 0d176b81a..6ab86126c 100644 --- a/internal/resolution/variables/installed_package.go +++ b/internal/resolution/variables/installed_package.go @@ -7,28 +7,28 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/constraint" "github.com/operator-framework/deppy/pkg/deppy/input" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" ) var _ deppy.Variable = &InstalledPackageVariable{} type InstalledPackageVariable struct { *input.SimpleVariable - bundleEntities []*olmentity.BundleEntity + bundles []*catalogmetadata.Bundle } -func (r *InstalledPackageVariable) BundleEntities() []*olmentity.BundleEntity { - return r.bundleEntities +func (r *InstalledPackageVariable) BundleEntities() []*catalogmetadata.Bundle { + return r.bundles } -func NewInstalledPackageVariable(packageName string, bundleEntities []*olmentity.BundleEntity) *InstalledPackageVariable { +func NewInstalledPackageVariable(packageName string, bundles []*catalogmetadata.Bundle) *InstalledPackageVariable { id := deppy.IdentifierFromString(fmt.Sprintf("installed package %s", packageName)) - entityIDs := make([]deppy.Identifier, 0, len(bundleEntities)) - for _, bundle := range bundleEntities { - entityIDs = append(entityIDs, bundle.ID) + var variableIDs []deppy.Identifier + for _, bundle := range bundles { + variableIDs = append(variableIDs, BundleToBundleVariableIDs(bundle)...) } return &InstalledPackageVariable{ - SimpleVariable: input.NewSimpleVariable(id, constraint.Mandatory(), constraint.Dependency(entityIDs...)), - bundleEntities: bundleEntities, + SimpleVariable: input.NewSimpleVariable(id, constraint.Mandatory(), constraint.Dependency(variableIDs...)), + bundles: bundles, } } diff --git a/internal/resolution/variables/required_package.go b/internal/resolution/variables/required_package.go index d2916f5f3..3aff97743 100644 --- a/internal/resolution/variables/required_package.go +++ b/internal/resolution/variables/required_package.go @@ -7,28 +7,28 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/constraint" "github.com/operator-framework/deppy/pkg/deppy/input" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" ) var _ deppy.Variable = &RequiredPackageVariable{} type RequiredPackageVariable struct { *input.SimpleVariable - bundleEntities []*olmentity.BundleEntity + bundles []*catalogmetadata.Bundle } -func (r *RequiredPackageVariable) BundleEntities() []*olmentity.BundleEntity { - return r.bundleEntities +func (r *RequiredPackageVariable) BundleEntities() []*catalogmetadata.Bundle { + return r.bundles } -func NewRequiredPackageVariable(packageName string, bundleEntities []*olmentity.BundleEntity) *RequiredPackageVariable { +func NewRequiredPackageVariable(packageName string, bundles []*catalogmetadata.Bundle) *RequiredPackageVariable { id := deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName)) - entityIDs := make([]deppy.Identifier, 0, len(bundleEntities)) - for _, bundle := range bundleEntities { - entityIDs = append(entityIDs, bundle.ID) + var variableIDs []deppy.Identifier + for _, bundle := range bundles { + variableIDs = append(variableIDs, BundleToBundleVariableIDs(bundle)...) } return &RequiredPackageVariable{ - SimpleVariable: input.NewSimpleVariable(id, constraint.Mandatory(), constraint.Dependency(entityIDs...)), - bundleEntities: bundleEntities, + SimpleVariable: input.NewSimpleVariable(id, constraint.Mandatory(), constraint.Dependency(variableIDs...)), + bundles: bundles, } } diff --git a/internal/resolution/variablesources/bundle_deployment.go b/internal/resolution/variablesources/bundle_deployment.go index 4363d6bc2..fd1ea283e 100644 --- a/internal/resolution/variablesources/bundle_deployment.go +++ b/internal/resolution/variablesources/bundle_deployment.go @@ -5,6 +5,7 @@ import ( "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" + catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -13,17 +14,19 @@ var _ input.VariableSource = &BundleDeploymentVariableSource{} type BundleDeploymentVariableSource struct { client client.Client + catalog *catalogclient.Client inputVariableSource input.VariableSource } -func NewBundleDeploymentVariableSource(cl client.Client, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource { +func NewBundleDeploymentVariableSource(cl client.Client, catalog *catalogclient.Client, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource { return &BundleDeploymentVariableSource{ client: cl, + catalog: catalog, inputVariableSource: inputVariableSource, } } -func (o *BundleDeploymentVariableSource) GetVariables(ctx context.Context, entitySource input.EntitySource) ([]deppy.Variable, error) { +func (o *BundleDeploymentVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { variableSources := SliceVariableSource{} if o.inputVariableSource != nil { variableSources = append(variableSources, o.inputVariableSource) @@ -42,7 +45,7 @@ func (o *BundleDeploymentVariableSource) GetVariables(ctx context.Context, entit continue } processed[sourceImage.Ref] = struct{}{} - ips, err := NewInstalledPackageVariableSource(bundleDeployment.Spec.Template.Spec.Source.Image.Ref) + ips, err := NewInstalledPackageVariableSource(o.catalog, bundleDeployment.Spec.Template.Spec.Source.Image.Ref) if err != nil { return nil, err } @@ -50,5 +53,5 @@ func (o *BundleDeploymentVariableSource) GetVariables(ctx context.Context, entit } } - return variableSources.GetVariables(ctx, entitySource) + return variableSources.GetVariables(ctx, nil) } diff --git a/internal/resolution/variablesources/bundles_and_dependencies.go b/internal/resolution/variablesources/bundles_and_dependencies.go index da3a12683..283c28f58 100644 --- a/internal/resolution/variablesources/bundles_and_dependencies.go +++ b/internal/resolution/variablesources/bundles_and_dependencies.go @@ -5,34 +5,36 @@ import ( "fmt" "sort" - bsemver "github.com/blang/semver/v4" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" - "github.com/operator-framework/operator-controller/internal/resolution/util/predicates" - entitysort "github.com/operator-framework/operator-controller/internal/resolution/util/sort" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" + catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" + catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" + catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) var _ input.VariableSource = &BundlesAndDepsVariableSource{} type BundlesAndDepsVariableSource struct { + catalog *catalogclient.Client variableSources []input.VariableSource } -func NewBundlesAndDepsVariableSource(inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource { +func NewBundlesAndDepsVariableSource(catalog *catalogclient.Client, inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource { return &BundlesAndDepsVariableSource{ + catalog: catalog, variableSources: inputVariableSources, } } -func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context, entitySource input.EntitySource) ([]deppy.Variable, error) { +func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { var variables []deppy.Variable // extract required package variables for _, variableSource := range b.variableSources { - inputVariables, err := variableSource.GetVariables(ctx, entitySource) + inputVariables, err := variableSource.GetVariables(ctx, nil) if err != nil { return nil, err } @@ -40,76 +42,80 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context, entityS } // create bundle queue for dependency resolution - var bundleEntityQueue []*olmentity.BundleEntity + var bundleQueue []*catalogmetadata.Bundle for _, variable := range variables { switch v := variable.(type) { case *olmvariables.RequiredPackageVariable: - bundleEntityQueue = append(bundleEntityQueue, v.BundleEntities()...) + bundleQueue = append(bundleQueue, v.BundleEntities()...) case *olmvariables.InstalledPackageVariable: - bundleEntityQueue = append(bundleEntityQueue, v.BundleEntities()...) + bundleQueue = append(bundleQueue, v.BundleEntities()...) } } + allBundles, err := b.catalog.Bundles(ctx) + if err != nil { + return nil, err + } + // build bundle and dependency variables visited := map[deppy.Identifier]struct{}{} - for len(bundleEntityQueue) > 0 { + for len(bundleQueue) > 0 { // pop head of queue - var head *olmentity.BundleEntity - head, bundleEntityQueue = bundleEntityQueue[0], bundleEntityQueue[1:] + var head *catalogmetadata.Bundle + head, bundleQueue = bundleQueue[0], bundleQueue[1:] - // ignore bundles that have already been processed - if _, ok := visited[head.ID]; ok { - continue - } - visited[head.ID] = struct{}{} + for _, id := range olmvariables.BundleToBundleVariableIDs(head) { + // ignore bundles that have already been processed + if _, ok := visited[id]; ok { + continue + } + visited[id] = struct{}{} - // get bundle dependencies - dependencyEntityBundles, err := b.getEntityDependencies(ctx, head, entitySource) - if err != nil { - return nil, fmt.Errorf("could not determine dependencies for entity with id '%s': %w", head.ID, err) - } + // get bundle dependencies + dependencies, err := b.filterBundleDependencies(allBundles, head) + if err != nil { + return nil, fmt.Errorf("could not determine dependencies for entity with id '%s': %w", id, err) + } + + // add bundle dependencies to queue for processing + bundleQueue = append(bundleQueue, dependencies...) - // add bundle dependencies to queue for processing - bundleEntityQueue = append(bundleEntityQueue, dependencyEntityBundles...) + // create variable + variables = append(variables, olmvariables.NewBundleVariable(id, head, dependencies)) + } - // create variable - variables = append(variables, olmvariables.NewBundleVariable(head, dependencyEntityBundles)) } return variables, nil } -func (b *BundlesAndDepsVariableSource) getEntityDependencies(ctx context.Context, bundleEntity *olmentity.BundleEntity, entitySource input.EntitySource) ([]*olmentity.BundleEntity, error) { - var dependencies []*olmentity.BundleEntity +func (b *BundlesAndDepsVariableSource) filterBundleDependencies(allBundles []*catalogmetadata.Bundle, bundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) { + var dependencies []*catalogmetadata.Bundle added := map[deppy.Identifier]struct{}{} // gather required package dependencies // todo(perdasilva): disambiguate between not found and actual errors - requiredPackages, _ := bundleEntity.RequiredPackages() + requiredPackages, _ := bundle.RequiredPackages() for _, requiredPackage := range requiredPackages { - semverRange, err := bsemver.ParseRange(requiredPackage.VersionRange) - if err != nil { - return nil, err - } - packageDependencyBundles, err := entitySource.Filter(ctx, input.And(predicates.WithPackageName(requiredPackage.PackageName), predicates.InBlangSemverRange(semverRange))) - if err != nil { - return nil, err - } + packageDependencyBundles := catalogfilter.Filter(allBundles, catalogfilter.And(catalogfilter.WithPackageName(requiredPackage.PackageName), catalogfilter.InBlangSemverRange(*requiredPackage.SemverRange))) if len(packageDependencyBundles) == 0 { - return nil, fmt.Errorf("could not find package dependencies for bundle '%s'", bundleEntity.ID) + return nil, fmt.Errorf("could not find package dependencies for bundle %q", bundle.Name) } for i := 0; i < len(packageDependencyBundles); i++ { - entity := packageDependencyBundles[i] - if _, ok := added[entity.ID]; !ok { - dependencies = append(dependencies, olmentity.NewBundleEntity(&entity)) - added[entity.ID] = struct{}{} + bundle := packageDependencyBundles[i] + for _, id := range olmvariables.BundleToBundleVariableIDs(bundle) { + if _, ok := added[id]; !ok { + dependencies = append(dependencies, bundle) + added[id] = struct{}{} + } } + } } // sort bundles in version order sort.SliceStable(dependencies, func(i, j int) bool { - return entitysort.ByChannelAndVersion(dependencies[i].Entity, dependencies[j].Entity) + return catalogsort.ByVersion(dependencies[i], dependencies[j]) }) return dependencies, nil diff --git a/internal/resolution/variablesources/composite.go b/internal/resolution/variablesources/composite.go index b66bf75ef..48b690d4e 100644 --- a/internal/resolution/variablesources/composite.go +++ b/internal/resolution/variablesources/composite.go @@ -29,7 +29,7 @@ var _ input.VariableSource = &NestedVariableSource{} type NestedVariableSource []func(inputVariableSource input.VariableSource) (input.VariableSource, error) -func (s NestedVariableSource) GetVariables(ctx context.Context, entitySource input.EntitySource) ([]deppy.Variable, error) { +func (s NestedVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { if len(s) == 0 { return nil, errors.New("empty nested variable sources") } @@ -43,15 +43,15 @@ func (s NestedVariableSource) GetVariables(ctx context.Context, entitySource inp } } - return variableSource.GetVariables(ctx, entitySource) + return variableSource.GetVariables(ctx, nil) } type SliceVariableSource []input.VariableSource -func (s SliceVariableSource) GetVariables(ctx context.Context, entitySource input.EntitySource) ([]deppy.Variable, error) { +func (s SliceVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { var variables []deppy.Variable for _, variableSource := range s { - inputVariables, err := variableSource.GetVariables(ctx, entitySource) + inputVariables, err := variableSource.GetVariables(ctx, nil) if err != nil { return nil, err } diff --git a/internal/resolution/variablesources/crd_constraints.go b/internal/resolution/variablesources/crd_constraints.go index 43e274ab2..f422f8ce6 100644 --- a/internal/resolution/variablesources/crd_constraints.go +++ b/internal/resolution/variablesources/crd_constraints.go @@ -7,7 +7,7 @@ import ( "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) @@ -18,7 +18,7 @@ var _ input.VariableSource = &CRDUniquenessConstraintsVariableSource{} // 2. at most 1 bundle per gvk (provided by the bundle) // these variables guarantee that no two operators provide the same gvk and no two version of // the same operator are running at the same time. -// This variable source does not itself reach out to its entitySource. It produces its variables +// This variable source does not itself reach out to catalog metadata. It produces its variables // by searching for BundleVariables that are produced by its 'inputVariableSource' and working out // which bundles correspond to which package and which gvks are provided by which bundle type CRDUniquenessConstraintsVariableSource struct { @@ -34,8 +34,8 @@ func NewCRDUniquenessConstraintsVariableSource(inputVariableSource input.Variabl } } -func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Context, entitySource input.EntitySource) ([]deppy.Variable, error) { - variables, err := g.inputVariableSource.GetVariables(ctx, entitySource) +func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { + variables, err := g.inputVariableSource.GetVariables(ctx, nil) if err != nil { return nil, err } @@ -47,19 +47,18 @@ func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Contex for _, variable := range variables { switch v := variable.(type) { case *olmvariables.BundleVariable: - bundleEntities := []*olmentity.BundleEntity{v.BundleEntity()} - bundleEntities = append(bundleEntities, v.Dependencies()...) - for _, bundleEntity := range bundleEntities { - // get bundleID package and update map - packageName, err := bundleEntity.PackageName() - if err != nil { - return nil, fmt.Errorf("error creating global constraints: %w", err) - } + bundles := []*catalogmetadata.Bundle{v.BundleEntity()} + bundles = append(bundles, v.Dependencies()...) + for _, bundle := range bundles { + for _, id := range olmvariables.BundleToBundleVariableIDs(bundle) { + // get bundleID package and update map + packageName := bundle.Package - if _, ok := pkgToBundleMap[packageName]; !ok { - pkgToBundleMap[packageName] = map[deppy.Identifier]struct{}{} + if _, ok := pkgToBundleMap[packageName]; !ok { + pkgToBundleMap[packageName] = map[deppy.Identifier]struct{}{} + } + pkgToBundleMap[packageName][id] = struct{}{} } - pkgToBundleMap[packageName][bundleEntity.ID] = struct{}{} } } } diff --git a/internal/resolution/variablesources/installed_package.go b/internal/resolution/variablesources/installed_package.go index a36256f1d..2a91584c5 100644 --- a/internal/resolution/variablesources/installed_package.go +++ b/internal/resolution/variablesources/installed_package.go @@ -3,73 +3,56 @@ package variablesources import ( "context" "fmt" + "sort" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" - "github.com/operator-framework/operator-controller/internal/resolution/util/predicates" - "github.com/operator-framework/operator-controller/internal/resolution/util/sort" + catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" + catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" + catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort" "github.com/operator-framework/operator-controller/internal/resolution/variables" ) var _ input.VariableSource = &InstalledPackageVariableSource{} type InstalledPackageVariableSource struct { + catalog *catalogclient.Client bundleImage string } -func (r *InstalledPackageVariableSource) GetVariables(ctx context.Context, entitySource input.EntitySource) ([]deppy.Variable, error) { - // find corresponding bundle entity for the installed content - resultSet, err := entitySource.Filter(ctx, predicates.WithBundleImage(r.bundleImage)) +func (r *InstalledPackageVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { + allBundles, err := r.catalog.Bundles(ctx) if err != nil { return nil, err } + + // find corresponding bundle entity for the installed content + resultSet := catalogfilter.Filter(allBundles, catalogfilter.WithBundleImage(r.bundleImage)) if len(resultSet) == 0 { return nil, r.notFoundError() } - // sort by channel and version - // TODO: this is a bit of a hack and it assumes a well formed catalog. - // we currently have one entity per bundle/channel, i.e. if a bundle - // appears in multiple channels, we have multiple entities for it. - // this means that for a well formed catalog, we could get multiple entities - // back as a response to the filter above. For now, we sort by channel and version - // and take the top most element. Soon, we will add package and channel variables making - // this unnecessary. // TODO: fast follow - we should check whether we are already supporting the channel attribute in the operator spec. // if so, we should take the value from spec of the operator CR in the owner ref of the bundle deployment. // If that channel is set, we need to update the filter above to filter by channel as well. - resultSet = resultSet.Sort(sort.ByChannelAndVersion) - installedBundle := olmentity.NewBundleEntity(&resultSet[0]) + sort.SliceStable(resultSet, func(i, j int) bool { + return catalogsort.ByVersion(resultSet[i], resultSet[j]) + }) + installedBundle := resultSet[0] // now find the bundles that replace the installed bundle // TODO: this algorithm does not yet consider skips and skipRange // we simplify the process here by just searching for the bundle that replaces the installed bundle - packageName, err := installedBundle.PackageName() - if err != nil { - return nil, err - } - - channelEntry, err := installedBundle.BundleChannelEntry() - if err != nil { - return nil, err - } - - resultSet, err = entitySource.Filter(ctx, predicates.Replaces(channelEntry.Name)) - if err != nil { - return nil, err - } - resultSet = resultSet.Sort(sort.ByChannelAndVersion) - upgradeEdges := make([]*olmentity.BundleEntity, 0, len(resultSet)) - for i := range resultSet { - upgradeEdges = append(upgradeEdges, olmentity.NewBundleEntity(&resultSet[i])) - } + upgradeEdges := catalogfilter.Filter(allBundles, catalogfilter.Replaces(installedBundle.Name)) + sort.SliceStable(upgradeEdges, func(i, j int) bool { + return catalogsort.ByVersion(upgradeEdges[i], upgradeEdges[j]) + }) // you can always upgrade to yourself, i.e. not upgrade upgradeEdges = append(upgradeEdges, installedBundle) return []deppy.Variable{ - variables.NewInstalledPackageVariable(packageName, upgradeEdges), + variables.NewInstalledPackageVariable(installedBundle.Package, upgradeEdges), }, nil } @@ -77,8 +60,9 @@ func (r *InstalledPackageVariableSource) notFoundError() error { return fmt.Errorf("bundleImage %q not found", r.bundleImage) } -func NewInstalledPackageVariableSource(bundleImage string) (*InstalledPackageVariableSource, error) { +func NewInstalledPackageVariableSource(catalog *catalogclient.Client, bundleImage string) (*InstalledPackageVariableSource, error) { return &InstalledPackageVariableSource{ + catalog: catalog, bundleImage: bundleImage, }, nil } diff --git a/internal/resolution/variablesources/operator.go b/internal/resolution/variablesources/operator.go index a827cd273..8c666a9f0 100644 --- a/internal/resolution/variablesources/operator.go +++ b/internal/resolution/variablesources/operator.go @@ -4,6 +4,7 @@ import ( "context" operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" @@ -14,17 +15,19 @@ var _ input.VariableSource = &OperatorVariableSource{} type OperatorVariableSource struct { client client.Client + catalog *catalogclient.Client inputVariableSource input.VariableSource } -func NewOperatorVariableSource(cl client.Client, inputVariableSource input.VariableSource) *OperatorVariableSource { +func NewOperatorVariableSource(cl client.Client, catalog *catalogclient.Client, inputVariableSource input.VariableSource) *OperatorVariableSource { return &OperatorVariableSource{ client: cl, + catalog: catalog, inputVariableSource: inputVariableSource, } } -func (o *OperatorVariableSource) GetVariables(ctx context.Context, entitySource input.EntitySource) ([]deppy.Variable, error) { +func (o *OperatorVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { variableSources := SliceVariableSource{} if o.inputVariableSource != nil { variableSources = append(variableSources, o.inputVariableSource) @@ -38,6 +41,7 @@ func (o *OperatorVariableSource) GetVariables(ctx context.Context, entitySource // build required package variable sources for _, operator := range operatorList.Items { rps, err := NewRequiredPackageVariableSource( + o.catalog, operator.Spec.PackageName, InVersionRange(operator.Spec.Version), InChannel(operator.Spec.Channel), @@ -48,5 +52,5 @@ func (o *OperatorVariableSource) GetVariables(ctx context.Context, entitySource variableSources = append(variableSources, rps) } - return variableSources.GetVariables(ctx, entitySource) + return variableSources.GetVariables(ctx, nil) } diff --git a/internal/resolution/variablesources/required_package.go b/internal/resolution/variablesources/required_package.go index a580c4618..ada3d4473 100644 --- a/internal/resolution/variablesources/required_package.go +++ b/internal/resolution/variablesources/required_package.go @@ -3,14 +3,16 @@ package variablesources import ( "context" "fmt" + "sort" mmsemver "github.com/Masterminds/semver/v3" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" - "github.com/operator-framework/operator-controller/internal/resolution/util/predicates" - "github.com/operator-framework/operator-controller/internal/resolution/util/sort" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" + catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" + catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" + catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) @@ -24,7 +26,7 @@ func InVersionRange(versionRange string) RequiredPackageVariableSourceOption { vr, err := mmsemver.NewConstraint(versionRange) if err == nil { r.versionRange = versionRange - r.predicates = append(r.predicates, predicates.InMastermindsSemverRange(vr)) + r.predicates = append(r.predicates, catalogfilter.InMastermindsSemverRange(vr)) return nil } @@ -38,26 +40,30 @@ func InChannel(channelName string) RequiredPackageVariableSourceOption { return func(r *RequiredPackageVariableSource) error { if channelName != "" { r.channelName = channelName - r.predicates = append(r.predicates, predicates.InChannel(channelName)) + r.predicates = append(r.predicates, catalogfilter.InChannel(channelName)) } return nil } } type RequiredPackageVariableSource struct { + catalog *catalogclient.Client + packageName string versionRange string channelName string - predicates []input.Predicate + predicates []catalogfilter.Predicate[catalogmetadata.Bundle] } -func NewRequiredPackageVariableSource(packageName string, options ...RequiredPackageVariableSourceOption) (*RequiredPackageVariableSource, error) { +func NewRequiredPackageVariableSource(catalog *catalogclient.Client, packageName string, options ...RequiredPackageVariableSourceOption) (*RequiredPackageVariableSource, error) { if packageName == "" { return nil, fmt.Errorf("package name must not be empty") } r := &RequiredPackageVariableSource{ + catalog: catalog, + packageName: packageName, - predicates: []input.Predicate{predicates.WithPackageName(packageName)}, + predicates: []catalogfilter.Predicate[catalogmetadata.Bundle]{catalogfilter.WithPackageName(packageName)}, } for _, option := range options { if err := option(r); err != nil { @@ -67,21 +73,21 @@ func NewRequiredPackageVariableSource(packageName string, options ...RequiredPac return r, nil } -func (r *RequiredPackageVariableSource) GetVariables(ctx context.Context, entitySource input.EntitySource) ([]deppy.Variable, error) { - resultSet, err := entitySource.Filter(ctx, input.And(r.predicates...)) +func (r *RequiredPackageVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { + resultSet, err := r.catalog.Bundles(ctx) if err != nil { return nil, err } + + resultSet = catalogfilter.Filter(resultSet, catalogfilter.And(r.predicates...)) if len(resultSet) == 0 { return nil, r.notFoundError() } - resultSet = resultSet.Sort(sort.ByChannelAndVersion) - var bundleEntities []*olmentity.BundleEntity - for i := 0; i < len(resultSet); i++ { - bundleEntities = append(bundleEntities, olmentity.NewBundleEntity(&resultSet[i])) - } + sort.SliceStable(resultSet, func(i, j int) bool { + return catalogsort.ByVersion(resultSet[i], resultSet[j]) + }) return []deppy.Variable{ - olmvariables.NewRequiredPackageVariable(r.packageName, bundleEntities), + olmvariables.NewRequiredPackageVariable(r.packageName, resultSet), }, nil } From fc5dace3b8b6c51b1f02a8ea0f6afd44d03465a8 Mon Sep 17 00:00:00 2001 From: Mikalai Radchuk Date: Tue, 12 Sep 2023 14:44:43 +0100 Subject: [PATCH 2/6] Updates resolutioncli to remove entity source Signed-off-by: Mikalai Radchuk --- cmd/resolutioncli/client.go | 78 ++++++++++++++++ cmd/resolutioncli/entity_source.go | 109 ---------------------- cmd/resolutioncli/main.go | 6 +- internal/catalogmetadata/client/client.go | 4 +- 4 files changed, 82 insertions(+), 115 deletions(-) create mode 100644 cmd/resolutioncli/client.go delete mode 100644 cmd/resolutioncli/entity_source.go diff --git a/cmd/resolutioncli/client.go b/cmd/resolutioncli/client.go new file mode 100644 index 000000000..b9b931cf7 --- /dev/null +++ b/cmd/resolutioncli/client.go @@ -0,0 +1,78 @@ +/* +Copyright 2023. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "context" + + "github.com/operator-framework/operator-registry/alpha/action" + + "github.com/operator-framework/operator-controller/internal/catalogmetadata" + "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" +) + +type indexRefClient struct { + renderer action.Render + bundlesCache []*catalogmetadata.Bundle +} + +func newIndexRefClient(indexRef string) *indexRefClient { + return &indexRefClient{ + renderer: action.Render{ + Refs: []string{indexRef}, + AllowedRefMask: action.RefDCImage | action.RefDCDir, + }, + } +} + +func (c *indexRefClient) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) { + if c.bundlesCache == nil { + cfg, err := c.renderer.Run(ctx) + if err != nil { + return nil, err + } + + var ( + channels []*catalogmetadata.Channel + bundles []*catalogmetadata.Bundle + ) + + for i := range cfg.Channels { + channels = append(channels, &catalogmetadata.Channel{ + Channel: cfg.Channels[i], + }) + } + + for i := range cfg.Bundles { + bundles = append(bundles, &catalogmetadata.Bundle{ + Bundle: cfg.Bundles[i], + }) + } + + // TODO: update fake catalog name string to be catalog name once we support multiple catalogs in CLI + catalogName := "offline-catalog" + + bundles, err = client.PopulateExtraFields(catalogName, channels, bundles) + if err != nil { + return nil, err + } + + c.bundlesCache = bundles + } + + return c.bundlesCache, nil +} diff --git a/cmd/resolutioncli/entity_source.go b/cmd/resolutioncli/entity_source.go deleted file mode 100644 index 664cf9943..000000000 --- a/cmd/resolutioncli/entity_source.go +++ /dev/null @@ -1,109 +0,0 @@ -/* -Copyright 2023. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package main - -import ( - "context" - - "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" - "github.com/operator-framework/operator-registry/alpha/action" - - "github.com/operator-framework/operator-controller/internal/resolution/entitysources" -) - -type indexRefEntitySource struct { - renderer action.Render - entitiesCache input.EntityList -} - -func newIndexRefEntitySourceEntitySource(indexRef string) *indexRefEntitySource { - return &indexRefEntitySource{ - renderer: action.Render{ - Refs: []string{indexRef}, - AllowedRefMask: action.RefDCImage | action.RefDCDir, - }, - } -} - -func (es *indexRefEntitySource) Get(_ context.Context, _ deppy.Identifier) (*input.Entity, error) { - panic("not implemented") -} - -func (es *indexRefEntitySource) Filter(ctx context.Context, filter input.Predicate) (input.EntityList, error) { - entities, err := es.entities(ctx) - if err != nil { - return nil, err - } - - resultSet := input.EntityList{} - for i := range entities { - if filter(&entities[i]) { - resultSet = append(resultSet, entities[i]) - } - } - return resultSet, nil -} - -func (es *indexRefEntitySource) GroupBy(ctx context.Context, fn input.GroupByFunction) (input.EntityListMap, error) { - entities, err := es.entities(ctx) - if err != nil { - return nil, err - } - - resultSet := input.EntityListMap{} - for i := range entities { - keys := fn(&entities[i]) - for _, key := range keys { - resultSet[key] = append(resultSet[key], entities[i]) - } - } - return resultSet, nil -} - -func (es *indexRefEntitySource) Iterate(ctx context.Context, fn input.IteratorFunction) error { - entities, err := es.entities(ctx) - if err != nil { - return err - } - - for i := range entities { - if err := fn(&entities[i]); err != nil { - return err - } - } - return nil -} - -func (es *indexRefEntitySource) entities(ctx context.Context) (input.EntityList, error) { - if es.entitiesCache == nil { - cfg, err := es.renderer.Run(ctx) - if err != nil { - return nil, err - } - - // TODO: update empty catalog name string to be catalog name once we support multiple catalogs in CLI - entities, err := entitysources.MetadataToEntities("", cfg.Channels, cfg.Bundles) - if err != nil { - return nil, err - } - - es.entitiesCache = entities - } - - return es.entitiesCache, nil -} diff --git a/cmd/resolutioncli/main.go b/cmd/resolutioncli/main.go index d15153753..3b8221033 100644 --- a/cmd/resolutioncli/main.go +++ b/cmd/resolutioncli/main.go @@ -35,7 +35,6 @@ import ( operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/catalogmetadata" - catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" "github.com/operator-framework/operator-controller/internal/controllers" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" @@ -109,7 +108,7 @@ func validateFlags(packageName, indexRef string) error { return nil } -func run(ctx context.Context, packageName, packageVersion, packageChannel, catalogRef, inputDir string) error { +func run(ctx context.Context, packageName, packageVersion, packageChannel, indexRef, inputDir string) error { clientBuilder := fake.NewClientBuilder().WithScheme(scheme) if inputDir != "" { @@ -122,10 +121,9 @@ func run(ctx context.Context, packageName, packageVersion, packageChannel, catal } cl := clientBuilder.Build() - catalogClient := catalogclient.New(cl) + catalogClient := newIndexRefClient(indexRef) resolver := solver.NewDeppySolver( - // TODO: Add a variable source to replace `indexRefEntitySource` which will read from catalogRef nil, append( variablesources.NestedVariableSource{newPackageVariableSource(catalogClient, packageName, packageVersion, packageChannel)}, diff --git a/internal/catalogmetadata/client/client.go b/internal/catalogmetadata/client/client.go index bf743a282..bb48955f8 100644 --- a/internal/catalogmetadata/client/client.go +++ b/internal/catalogmetadata/client/client.go @@ -41,7 +41,7 @@ func (c *Client) Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) return nil, err } - bundles, err = populateExtraFields(catalog.Name, channels, bundles) + bundles, err = PopulateExtraFields(catalog.Name, channels, bundles) if err != nil { return nil, err } @@ -67,7 +67,7 @@ func fetchCatalogMetadata[T catalogmetadata.Schemas](ctx context.Context, cl cli return content, nil } -func populateExtraFields(catalogName string, channels []*catalogmetadata.Channel, bundles []*catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) { +func PopulateExtraFields(catalogName string, channels []*catalogmetadata.Channel, bundles []*catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) { bundlesMap := map[string]*catalogmetadata.Bundle{} for i := range bundles { bundleKey := fmt.Sprintf("%s-%s", bundles[i].Package, bundles[i].Name) From d1802073c9c92a08275a6d981998a30035503793 Mon Sep 17 00:00:00 2001 From: dtfranz Date: Tue, 12 Sep 2023 10:40:22 -0700 Subject: [PATCH 3/6] Removes entity sources and GVK related code * Removes provides+required GVK and associated tests. GVK related code was a dead weight since were never set GVK properties on bundle entity. * Remove entity + entitysource and associated util pkgs Signed-off-by: dtfranz --- internal/controllers/operator_controller.go | 5 +- internal/resolution/entities/bundle_entity.go | 285 -------------- .../resolution/entities/bundle_entity_test.go | 370 ------------------ .../entitysources/catalogdsource.go | 190 --------- .../resolution/util/predicates/predicates.go | 100 ----- .../util/predicates/predicates_test.go | 222 ----------- internal/resolution/util/sort/sort.go | 80 ---- internal/resolution/util/sort/sort_test.go | 176 --------- .../bundles_and_dependencies.go | 1 - 9 files changed, 2 insertions(+), 1427 deletions(-) delete mode 100644 internal/resolution/entities/bundle_entity.go delete mode 100644 internal/resolution/entities/bundle_entity_test.go delete mode 100644 internal/resolution/entitysources/catalogdsource.go delete mode 100644 internal/resolution/util/predicates/predicates.go delete mode 100644 internal/resolution/util/predicates/predicates_test.go delete mode 100644 internal/resolution/util/sort/sort.go delete mode 100644 internal/resolution/util/sort/sort_test.go diff --git a/internal/controllers/operator_controller.go b/internal/controllers/operator_controller.go index c260fb7f1..1136e9467 100644 --- a/internal/controllers/operator_controller.go +++ b/internal/controllers/operator_controller.go @@ -46,7 +46,6 @@ import ( operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/catalogmetadata" "github.com/operator-framework/operator-controller/internal/controllers/validators" - "github.com/operator-framework/operator-controller/internal/resolution/entities" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) @@ -363,13 +362,13 @@ func (r *OperatorReconciler) existingBundleDeploymentUnstructured(ctx context.Co // rukpak bundle provisioner class name that is capable of unpacking the bundle type func mapBundleMediaTypeToBundleProvisioner(mediaType string) (string, error) { switch mediaType { - case entities.MediaTypePlain: + case catalogmetadata.MediaTypePlain: return "core-rukpak-io-plain", nil // To ensure compatibility with bundles created with OLMv0 where the // olm.bundle.mediatype property doesn't exist, we assume that if the // property is empty (i.e doesn't exist) that the bundle is one created // with OLMv0 and therefore should use the registry provisioner - case entities.MediaTypeRegistry, "": + case catalogmetadata.MediaTypeRegistry, "": return "core-rukpak-io-registry", nil default: return "", fmt.Errorf("unknown bundle mediatype: %s", mediaType) diff --git a/internal/resolution/entities/bundle_entity.go b/internal/resolution/entities/bundle_entity.go deleted file mode 100644 index 6f28d4a32..000000000 --- a/internal/resolution/entities/bundle_entity.go +++ /dev/null @@ -1,285 +0,0 @@ -package entities - -import ( - "encoding/json" - "fmt" - "sync" - - bsemver "github.com/blang/semver/v4" - "github.com/operator-framework/deppy/pkg/deppy/input" - "github.com/operator-framework/operator-registry/alpha/property" -) - -const PropertyBundlePath = "olm.bundle.path" -const PropertyBundleChannelEntry = "olm.bundle.channelEntry" -const PropertyBundleMediaType = "olm.bundle.mediatype" - -type MediaType string - -const ( - MediaTypePlain = "plain+v0" - MediaTypeRegistry = "registry+v1" -) - -// ---- - -type propertyRequirement bool - -const ( - required propertyRequirement = true - optional propertyRequirement = false -) - -type PackageRequired struct { - property.PackageRequired - SemverRange *bsemver.Range `json:"-"` -} - -type ChannelEntry struct { - Name string `json:"name"` - Replaces string `json:"replaces"` - // Skips and skipRange will probably go here as well -} - -type GVK property.GVK - -func (g GVK) String() string { - return fmt.Sprintf(`group:"%s" version:"%s" kind:"%s"`, g.Group, g.Version, g.Kind) -} - -type GVKRequired property.GVKRequired - -func (g GVKRequired) String() string { - return fmt.Sprintf(`group:"%s" version:"%s" kind:"%s"`, g.Group, g.Version, g.Kind) -} - -func (g GVKRequired) AsGVK() GVK { - return GVK(g) -} - -type BundleEntity struct { - *input.Entity - - // these properties are lazy loaded as they are requested - bundlePackage *property.Package - providedGVKs []GVK - requiredGVKs []GVKRequired - requiredPackages []PackageRequired - channel *property.Channel - channelEntry *ChannelEntry - semVersion *bsemver.Version - bundlePath string - mediaType string - mu sync.RWMutex -} - -func NewBundleEntity(entity *input.Entity) *BundleEntity { - return &BundleEntity{ - Entity: entity, - mu: sync.RWMutex{}, - } -} - -func (b *BundleEntity) PackageName() (string, error) { - if err := b.loadPackage(); err != nil { - return "", err - } - return b.bundlePackage.PackageName, nil -} - -func (b *BundleEntity) Version() (*bsemver.Version, error) { - if err := b.loadPackage(); err != nil { - return nil, err - } - return b.semVersion, nil -} - -func (b *BundleEntity) ProvidedGVKs() ([]GVK, error) { - if err := b.loadProvidedGVKs(); err != nil { - return nil, err - } - return b.providedGVKs, nil -} - -func (b *BundleEntity) RequiredGVKs() ([]GVKRequired, error) { - if err := b.loadRequiredGVKs(); err != nil { - return nil, err - } - return b.requiredGVKs, nil -} - -func (b *BundleEntity) RequiredPackages() ([]PackageRequired, error) { - if err := b.loadRequiredPackages(); err != nil { - return nil, err - } - return b.requiredPackages, nil -} - -func (b *BundleEntity) ChannelName() (string, error) { - if err := b.loadChannelProperties(); err != nil { - return "", err - } - return b.channel.ChannelName, nil -} - -func (b *BundleEntity) Channel() (*property.Channel, error) { - if err := b.loadChannelProperties(); err != nil { - return nil, err - } - return b.channel, nil -} - -func (b *BundleEntity) BundleChannelEntry() (*ChannelEntry, error) { - if err := b.loadBundleChannelEntry(); err != nil { - return nil, err - } - return b.channelEntry, nil -} - -func (b *BundleEntity) loadBundleChannelEntry() error { - b.mu.Lock() - defer b.mu.Unlock() - if b.channelEntry == nil { - channelEntry, err := loadFromEntity[*ChannelEntry](b.Entity, PropertyBundleChannelEntry, optional) - if err != nil || channelEntry == nil { - return fmt.Errorf("error determining replaces for entity '%s': %w", b.ID, err) - } - b.channelEntry = channelEntry - } - return nil -} - -func (b *BundleEntity) BundlePath() (string, error) { - if err := b.loadBundlePath(); err != nil { - return "", err - } - return b.bundlePath, nil -} - -func (b *BundleEntity) MediaType() (string, error) { - if err := b.loadMediaType(); err != nil { - return "", err - } - - return b.mediaType, nil -} - -func (b *BundleEntity) loadMediaType() error { - b.mu.Lock() - defer b.mu.Unlock() - if b.mediaType == "" { - mediaType, err := loadFromEntity[string](b.Entity, PropertyBundleMediaType, optional) - if err != nil { - return fmt.Errorf("error determining bundle mediatype for entity '%s': %w", b.ID, err) - } - b.mediaType = mediaType - } - return nil -} - -func (b *BundleEntity) loadPackage() error { - b.mu.Lock() - defer b.mu.Unlock() - if b.bundlePackage == nil { - bundlePackage, err := loadFromEntity[property.Package](b.Entity, property.TypePackage, required) - if err != nil { - return fmt.Errorf("error determining package for entity '%s': %w", b.ID, err) - } - b.bundlePackage = &bundlePackage - if b.semVersion == nil { - semVer, err := bsemver.Parse(b.bundlePackage.Version) - if err != nil { - return fmt.Errorf("could not parse semver (%s) for entity '%s': %w", b.bundlePackage.Version, b.ID, err) - } - b.semVersion = &semVer - } - } - return nil -} - -func (b *BundleEntity) loadProvidedGVKs() error { - b.mu.Lock() - defer b.mu.Unlock() - if b.providedGVKs == nil { - providedGVKs, err := loadFromEntity[[]GVK](b.Entity, property.TypeGVK, optional) - if err != nil { - return fmt.Errorf("error determining bundle provided gvks for entity '%s': %w", b.ID, err) - } - b.providedGVKs = providedGVKs - } - return nil -} - -func (b *BundleEntity) loadRequiredGVKs() error { - b.mu.Lock() - defer b.mu.Unlock() - if b.requiredGVKs == nil { - requiredGVKs, err := loadFromEntity[[]GVKRequired](b.Entity, property.TypeGVKRequired, optional) - if err != nil { - return fmt.Errorf("error determining bundle required gvks for entity '%s': %w", b.ID, err) - } - b.requiredGVKs = requiredGVKs - } - return nil -} - -func (b *BundleEntity) loadRequiredPackages() error { - b.mu.Lock() - defer b.mu.Unlock() - if b.requiredPackages == nil { - requiredPackages, err := loadFromEntity[[]PackageRequired](b.Entity, property.TypePackageRequired, optional) - if err != nil { - return fmt.Errorf("error determining bundle required packages for entity '%s': %w", b.ID, err) - } - for _, requiredPackage := range requiredPackages { - semverRange, err := bsemver.ParseRange(requiredPackage.VersionRange) - if err != nil { - return fmt.Errorf("error determining bundle required package semver range for entity '%s': '%w'", b.ID, err) - } - requiredPackage.SemverRange = &semverRange - } - b.requiredPackages = requiredPackages - } - return nil -} - -func (b *BundleEntity) loadChannelProperties() error { - b.mu.Lock() - defer b.mu.Unlock() - if b.channel == nil { - channel, err := loadFromEntity[property.Channel](b.Entity, property.TypeChannel, required) - if err != nil { - return fmt.Errorf("error determining bundle channel properties for entity '%s': %w", b.ID, err) - } - b.channel = &channel - } - return nil -} - -func (b *BundleEntity) loadBundlePath() error { - b.mu.Lock() - defer b.mu.Unlock() - if b.bundlePath == "" { - bundlePath, err := loadFromEntity[string](b.Entity, PropertyBundlePath, required) - if err != nil { - return fmt.Errorf("error determining bundle path for entity '%s': %w", b.ID, err) - } - b.bundlePath = bundlePath - } - return nil -} - -func loadFromEntity[T interface{}](entity *input.Entity, propertyName string, required propertyRequirement) (T, error) { - deserializedProperty := *new(T) - propertyValue, ok := entity.Properties[propertyName] - if ok { - // TODO: In order to avoid invalid properties we should use a decoder that only allows the properties we expect. - // ie. decoder.DisallowUnknownFields() - if err := json.Unmarshal([]byte(propertyValue), &deserializedProperty); err != nil { - return deserializedProperty, fmt.Errorf("property '%s' ('%s') could not be parsed: %w", propertyName, propertyValue, err) - } - } else if required { - return deserializedProperty, fmt.Errorf("required property '%s' not found", propertyName) - } - return deserializedProperty, nil -} diff --git a/internal/resolution/entities/bundle_entity_test.go b/internal/resolution/entities/bundle_entity_test.go deleted file mode 100644 index b7b11052b..000000000 --- a/internal/resolution/entities/bundle_entity_test.go +++ /dev/null @@ -1,370 +0,0 @@ -package entities_test - -import ( - "fmt" - "testing" - - mmsemver "github.com/Masterminds/semver/v3" - bsemver "github.com/blang/semver/v4" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/operator-framework/deppy/pkg/deppy/input" - "github.com/operator-framework/operator-registry/alpha/property" - - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" -) - -func TestBundleEntity(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "BundleEntity Suite") -} - -var _ = Describe("BundleEntity", func() { - Describe("PackageName", func() { - It("should return the package name if present", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.package": "{\"packageName\":\"prometheus\",\"version\":\"0.14.0\"}", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - packageName, err := bundleEntity.PackageName() - Expect(err).ToNot(HaveOccurred()) - Expect(packageName).To(Equal("prometheus")) - }) - It("should return an error if the property is not found", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{}) - bundleEntity := olmentity.NewBundleEntity(entity) - packageName, err := bundleEntity.PackageName() - Expect(packageName).To(Equal("")) - Expect(err.Error()).To(Equal("error determining package for entity 'operatorhub/prometheus/0.14.0': required property 'olm.package' not found")) - }) - It("should return error if the property is malformed", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.package": "badPackageNameStructure", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - packageName, err := bundleEntity.PackageName() - Expect(packageName).To(Equal("")) - Expect(err.Error()).To(Equal("error determining package for entity 'operatorhub/prometheus/0.14.0': property 'olm.package' ('badPackageNameStructure') could not be parsed: invalid character 'b' looking for beginning of value")) - }) - }) - - Describe("Version", func() { - It("should return the bundle blang version if present", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.package": "{\"packageName\":\"prometheus\",\"version\":\"0.14.0\"}", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - version, err := bundleEntity.Version() - Expect(err).ToNot(HaveOccurred()) - Expect(*version).To(Equal(bsemver.MustParse("0.14.0"))) - }) - It("should return the bundle Masterminds version if present", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.package": "{\"packageName\":\"prometheus\",\"version\":\"0.14.0\"}", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - bVersion, err := bundleEntity.Version() - Expect(err).ToNot(HaveOccurred()) - mVersion, err := mmsemver.NewVersion(bVersion.String()) - Expect(err).ToNot(HaveOccurred()) - Expect(*mVersion).To(Equal(*mmsemver.MustParse("0.14.0"))) - }) - It("should return an error if the property is not found", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{}) - bundleEntity := olmentity.NewBundleEntity(entity) - version, err := bundleEntity.Version() - Expect(version).To(BeNil()) - Expect(err.Error()).To(Equal("error determining package for entity 'operatorhub/prometheus/0.14.0': required property 'olm.package' not found")) - }) - It("should return error if the property is malformed", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.package": "badPackageStructure", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - version, err := bundleEntity.Version() - Expect(version).To(BeNil()) - Expect(err.Error()).To(Equal("error determining package for entity 'operatorhub/prometheus/0.14.0': property 'olm.package' ('badPackageStructure') could not be parsed: invalid character 'b' looking for beginning of value")) - }) - It("should return error if the version is malformed", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.package": "{\"packageName\":\"prometheus\",\"version\":\"badversion\"}", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - version, err := bundleEntity.Version() - Expect(version).To(BeNil()) - Expect(err.Error()).To(Equal("could not parse semver (badversion) for entity 'operatorhub/prometheus/0.14.0': No Major.Minor.Patch elements found")) - }) - }) - - Describe("ProvidedGVKs", func() { - It("should return the bundle provided gvks if present", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.gvk": "[{\"group\":\"foo.io\",\"kind\":\"Foo\",\"version\":\"v1\"},{\"group\":\"bar.io\",\"kind\":\"Bar\",\"version\":\"v1alpha1\"}]", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - providedGvks, err := bundleEntity.ProvidedGVKs() - Expect(err).ToNot(HaveOccurred()) - Expect(providedGvks).To(Equal([]olmentity.GVK{ - {Group: "foo.io", Kind: "Foo", Version: "v1"}, - {Group: "bar.io", Kind: "Bar", Version: "v1alpha1"}, - })) - }) - It("should return an error if the property is not found", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{}) - bundleEntity := olmentity.NewBundleEntity(entity) - providedGvks, err := bundleEntity.ProvidedGVKs() - Expect(providedGvks).To(BeNil()) - Expect(err).ToNot(HaveOccurred()) - }) - It("should return error if the property is malformed", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.gvk": "badGvkStructure", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - providedGvks, err := bundleEntity.ProvidedGVKs() - Expect(providedGvks).To(BeNil()) - Expect(err.Error()).To(Equal("error determining bundle provided gvks for entity 'operatorhub/prometheus/0.14.0': property 'olm.gvk' ('badGvkStructure') could not be parsed: invalid character 'b' looking for beginning of value")) - }) - }) - - Describe("RequiredGVKs", func() { - It("should return the bundle required gvks if present", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.gvk.required": "[{\"group\":\"foo.io\",\"kind\":\"Foo\",\"version\":\"v1\"},{\"group\":\"bar.io\",\"kind\":\"Bar\",\"version\":\"v1alpha1\"}]", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - requiredGvks, err := bundleEntity.RequiredGVKs() - Expect(err).ToNot(HaveOccurred()) - Expect(requiredGvks).To(Equal([]olmentity.GVKRequired{ - {Group: "foo.io", Kind: "Foo", Version: "v1"}, - {Group: "bar.io", Kind: "Bar", Version: "v1alpha1"}, - })) - }) - It("should return an error if the property is not found", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{}) - bundleEntity := olmentity.NewBundleEntity(entity) - requiredGvks, err := bundleEntity.RequiredGVKs() - Expect(requiredGvks).To(BeNil()) - Expect(err).ToNot(HaveOccurred()) - }) - It("should return error if the property is malformed", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.gvk.required": "badGvkStructure", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - requiredGvks, err := bundleEntity.RequiredGVKs() - Expect(requiredGvks).To(BeNil()) - Expect(err.Error()).To(Equal("error determining bundle required gvks for entity 'operatorhub/prometheus/0.14.0': property 'olm.gvk.required' ('badGvkStructure') could not be parsed: invalid character 'b' looking for beginning of value")) - }) - }) - - Describe("RequiredPackages", func() { - It("should return the bundle required packages if present", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.package.required": `[{"packageName": "packageA", "versionRange": ">1.0.0"}, {"packageName": "packageB", "versionRange": ">0.5.0 <0.8.6"}]`, - }) - bundleEntity := olmentity.NewBundleEntity(entity) - requiredPackages, err := bundleEntity.RequiredPackages() - Expect(err).ToNot(HaveOccurred()) - Expect(requiredPackages).To(Equal([]olmentity.PackageRequired{ - {PackageRequired: property.PackageRequired{PackageName: "packageA", VersionRange: ">1.0.0"}}, - {PackageRequired: property.PackageRequired{PackageName: "packageB", VersionRange: ">0.5.0 <0.8.6"}}, - })) - }) - It("should return an error if the property is not found", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{}) - bundleEntity := olmentity.NewBundleEntity(entity) - requiredPackages, err := bundleEntity.RequiredPackages() - Expect(requiredPackages).To(BeNil()) - Expect(err).ToNot(HaveOccurred()) - }) - It("should return error if the property is malformed", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.package.required": "badRequiredPackageStructure", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - requiredPackages, err := bundleEntity.RequiredPackages() - Expect(requiredPackages).To(BeNil()) - Expect(err.Error()).To(Equal("error determining bundle required packages for entity 'operatorhub/prometheus/0.14.0': property 'olm.package.required' ('badRequiredPackageStructure') could not be parsed: invalid character 'b' looking for beginning of value")) - }) - }) - - Describe("ChannelName", func() { - It("should return the bundle channel name if present", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.channel": "{\"channelName\":\"beta\",\"priority\":0}", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - channelName, err := bundleEntity.ChannelName() - Expect(err).ToNot(HaveOccurred()) - Expect(channelName).To(Equal("beta")) - }) - It("should return an error if the property is not found", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{}) - bundleEntity := olmentity.NewBundleEntity(entity) - channelName, err := bundleEntity.ChannelName() - Expect(channelName).To(BeEmpty()) - Expect(err.Error()).To(Equal("error determining bundle channel properties for entity 'operatorhub/prometheus/0.14.0': required property 'olm.channel' not found")) - }) - It("should return error if the property is malformed", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.channel": "badChannelPropertiesStructure", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - channelName, err := bundleEntity.ChannelName() - Expect(channelName).To(BeEmpty()) - Expect(err.Error()).To(Equal("error determining bundle channel properties for entity 'operatorhub/prometheus/0.14.0': property 'olm.channel' ('badChannelPropertiesStructure') could not be parsed: invalid character 'b' looking for beginning of value")) - }) - }) - - Describe("Channel", func() { - It("should return the bundle channel properties if present", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.channel": `{"channelName":"beta","priority":0, "replaces": "bundle.v1.0.0", "skips": ["bundle.v0.9.0", "bundle.v0.9.6"], "skipRange": ">=0.9.0 <=0.9.6"}`, - }) - bundleEntity := olmentity.NewBundleEntity(entity) - channelProperties, err := bundleEntity.Channel() - Expect(err).ToNot(HaveOccurred()) - Expect(*channelProperties).To(Equal(property.Channel{ - ChannelName: "beta", - Priority: 0, - }, - )) - }) - It("should return an error if the property is not found", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{}) - bundleEntity := olmentity.NewBundleEntity(entity) - channelProperties, err := bundleEntity.Channel() - Expect(channelProperties).To(BeNil()) - Expect(err.Error()).To(Equal("error determining bundle channel properties for entity 'operatorhub/prometheus/0.14.0': required property 'olm.channel' not found")) - }) - It("should return error if the property is malformed", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.channel": "badChannelPropertiesStructure", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - channelProperties, err := bundleEntity.Channel() - Expect(channelProperties).To(BeNil()) - Expect(err.Error()).To(Equal("error determining bundle channel properties for entity 'operatorhub/prometheus/0.14.0': property 'olm.channel' ('badChannelPropertiesStructure') could not be parsed: invalid character 'b' looking for beginning of value")) - }) - }) - - Describe("BundlePath", func() { - It("should return the bundle channel properties if present", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.bundle.path": `"bundle.io/path/to/bundle"`, - }) - bundleEntity := olmentity.NewBundleEntity(entity) - bundlePath, err := bundleEntity.BundlePath() - Expect(err).ToNot(HaveOccurred()) - Expect(bundlePath).To(Equal("bundle.io/path/to/bundle")) - }) - It("should return an error if the property is not found", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{}) - bundleEntity := olmentity.NewBundleEntity(entity) - bundlePath, err := bundleEntity.BundlePath() - Expect(bundlePath).To(BeEmpty()) - Expect(err.Error()).To(Equal("error determining bundle path for entity 'operatorhub/prometheus/0.14.0': required property 'olm.bundle.path' not found")) - }) - It("should return error if the property is malformed", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - "olm.bundle.path": "badBundlePath", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - bundlePath, err := bundleEntity.BundlePath() - Expect(bundlePath).To(BeEmpty()) - Expect(err.Error()).To(Equal("error determining bundle path for entity 'operatorhub/prometheus/0.14.0': property 'olm.bundle.path' ('badBundlePath') could not be parsed: invalid character 'b' looking for beginning of value")) - }) - }) - - Describe("ChannelEntry", func() { - It("should return the channel entry property if present", func() { - entity := input.NewEntity("test", map[string]string{ - "olm.bundle.channelEntry": `{"name":"test.v0.3.0","replaces": "test.v0.2.0"}`, - }) - bundleEntity := olmentity.NewBundleEntity(entity) - channelEntry, err := bundleEntity.BundleChannelEntry() - Expect(err).ToNot(HaveOccurred()) - Expect(channelEntry).To(Equal(&olmentity.ChannelEntry{Name: "test.v0.3.0", Replaces: "test.v0.2.0"})) - }) - It("should not return an error if the property is not found", func() { - entity := input.NewEntity("test", map[string]string{ - "olm.thingy": `{"whatever":"this"}`, - }) - bundleEntity := olmentity.NewBundleEntity(entity) - channelEntry, err := bundleEntity.BundleChannelEntry() - Expect(channelEntry).To(BeNil()) - Expect(err).To(HaveOccurred()) - }) - }) - - Describe("MediaType", func() { - It("should return the bundle mediatype property if present", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - olmentity.PropertyBundleMediaType: fmt.Sprintf(`"%s"`, olmentity.MediaTypePlain), - }) - bundleEntity := olmentity.NewBundleEntity(entity) - mediaType, err := bundleEntity.MediaType() - Expect(err).ToNot(HaveOccurred()) - Expect(mediaType).To(Equal(olmentity.MediaTypePlain)) - }) - It("should not return an error if the property is not found", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{}) - bundleEntity := olmentity.NewBundleEntity(entity) - mediaType, err := bundleEntity.MediaType() - Expect(mediaType).To(BeEmpty()) - Expect(err).ToNot(HaveOccurred()) - }) - It("should return error if the property is malformed", func() { - entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ - olmentity.PropertyBundleMediaType: "badtype", - }) - bundleEntity := olmentity.NewBundleEntity(entity) - mediaType, err := bundleEntity.MediaType() - Expect(mediaType).To(BeEmpty()) - Expect(err.Error()).To(Equal("error determining bundle mediatype for entity 'operatorhub/prometheus/0.14.0': property 'olm.bundle.mediatype' ('badtype') could not be parsed: invalid character 'b' looking for beginning of value")) - }) - }) - - // Increase test coverage - Describe("GVKRequired properties", func() { - It("should return the GVKRequired properties", func() { - gvk := olmentity.GVKRequired{ - Group: "foo.io", - Kind: "Foo", - Version: "v1", - } - Expect(gvk.AsGVK().Version).To(Equal("v1")) - Expect(gvk.AsGVK().Group).To(Equal("foo.io")) - Expect(gvk.AsGVK().Kind).To(Equal("Foo")) - }) - It("should return the GVKRequired properties as a string", func() { - gvk := olmentity.GVKRequired{ - Group: "foo.io", - Kind: "Foo", - Version: "v1", - } - Expect(gvk.String()).To(Equal(`group:"foo.io" version:"v1" kind:"Foo"`)) - }) - }) - Describe("GVK properties", func() { - It("should return the gvk properties", func() { - gvk := olmentity.GVK{ - Group: "foo.io", - Kind: "Foo", - Version: "v1", - } - Expect(gvk.Version).To(Equal("v1")) - Expect(gvk.Group).To(Equal("foo.io")) - Expect(gvk.Kind).To(Equal("Foo")) - }) - It("should return the gvk properties as a string", func() { - gvk := olmentity.GVK{ - Group: "foo.io", - Kind: "Foo", - Version: "v1", - } - Expect(gvk.String()).To(Equal(`group:"foo.io" version:"v1" kind:"Foo"`)) - }) - }) -}) diff --git a/internal/resolution/entitysources/catalogdsource.go b/internal/resolution/entitysources/catalogdsource.go deleted file mode 100644 index 106f893f8..000000000 --- a/internal/resolution/entitysources/catalogdsource.go +++ /dev/null @@ -1,190 +0,0 @@ -package entitysources - -import ( - "context" - "encoding/json" - "fmt" - - catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" - "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" - "github.com/operator-framework/operator-registry/alpha/declcfg" - "github.com/operator-framework/operator-registry/alpha/property" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/operator-framework/operator-controller/internal/resolution/entities" -) - -// CatalogdEntitySource is a source for(/collection of) deppy defined input.Entity, built from content -// made accessible on-cluster by https://github.com/operator-framework/catalogd. -// It is an implementation of deppy defined input.EntitySource -type CatalogdEntitySource struct { - client client.Client -} - -func NewCatalogdEntitySource(client client.Client) *CatalogdEntitySource { - return &CatalogdEntitySource{client: client} -} - -func (es *CatalogdEntitySource) Get(_ context.Context, _ deppy.Identifier) (*input.Entity, error) { - panic("not implemented") -} - -func (es *CatalogdEntitySource) Filter(ctx context.Context, filter input.Predicate) (input.EntityList, error) { - resultSet := input.EntityList{} - entities, err := getEntities(ctx, es.client) - if err != nil { - return nil, err - } - for i := range entities { - if filter(&entities[i]) { - resultSet = append(resultSet, entities[i]) - } - } - return resultSet, nil -} - -func (es *CatalogdEntitySource) GroupBy(ctx context.Context, fn input.GroupByFunction) (input.EntityListMap, error) { - entities, err := getEntities(ctx, es.client) - if err != nil { - return nil, err - } - resultSet := input.EntityListMap{} - for i := range entities { - keys := fn(&entities[i]) - for _, key := range keys { - resultSet[key] = append(resultSet[key], entities[i]) - } - } - return resultSet, nil -} - -func (es *CatalogdEntitySource) Iterate(ctx context.Context, fn input.IteratorFunction) error { - entities, err := getEntities(ctx, es.client) - if err != nil { - return err - } - for i := range entities { - if err := fn(&entities[i]); err != nil { - return err - } - } - return nil -} - -func getEntities(ctx context.Context, cl client.Client) (input.EntityList, error) { - allEntitiesList := input.EntityList{} - - var catalogList catalogd.CatalogList - if err := cl.List(ctx, &catalogList); err != nil { - return nil, err - } - for _, catalog := range catalogList.Items { - channels, bundles, err := fetchCatalogMetadata(ctx, cl, catalog.Name) - if err != nil { - return nil, err - } - - catalogEntitiesList, err := MetadataToEntities(catalog.Name, channels, bundles) - if err != nil { - return nil, err - } - - allEntitiesList = append(allEntitiesList, catalogEntitiesList...) - } - - return allEntitiesList, nil -} - -func MetadataToEntities(catalogName string, channels []declcfg.Channel, bundles []declcfg.Bundle) (input.EntityList, error) { - entityList := input.EntityList{} - - bundlesMap := map[string]*declcfg.Bundle{} - for i := range bundles { - bundleKey := fmt.Sprintf("%s-%s", bundles[i].Package, bundles[i].Name) - bundlesMap[bundleKey] = &bundles[i] - } - - for _, ch := range channels { - for _, chEntry := range ch.Entries { - bundleKey := fmt.Sprintf("%s-%s", ch.Package, chEntry.Name) - bundle, ok := bundlesMap[bundleKey] - if !ok { - return nil, fmt.Errorf("bundle %q not found in catalog %q (package %q, channel %q)", chEntry.Name, catalogName, ch.Package, ch.Name) - } - - props := map[string]string{} - - for _, prop := range bundle.Properties { - switch prop.Type { - case property.TypePackage: - // this is already a json marshalled object, so it doesn't need to be marshalled - // like the other ones - props[property.TypePackage] = string(prop.Value) - case entities.PropertyBundleMediaType: - props[entities.PropertyBundleMediaType] = string(prop.Value) - } - } - - imgValue, err := json.Marshal(bundle.Image) - if err != nil { - return nil, err - } - props[entities.PropertyBundlePath] = string(imgValue) - - channelValue, _ := json.Marshal(property.Channel{ChannelName: ch.Name, Priority: 0}) - props[property.TypeChannel] = string(channelValue) - replacesValue, _ := json.Marshal(entities.ChannelEntry{ - Name: bundle.Name, - Replaces: chEntry.Replaces, - }) - props[entities.PropertyBundleChannelEntry] = string(replacesValue) - - catalogScopedEntryName := fmt.Sprintf("%s-%s", catalogName, bundle.Name) - entity := input.Entity{ - ID: deppy.IdentifierFromString(fmt.Sprintf("%s%s%s", catalogScopedEntryName, bundle.Package, ch.Name)), - Properties: props, - } - entityList = append(entityList, entity) - } - } - - return entityList, nil -} - -func fetchCatalogMetadata(ctx context.Context, cl client.Client, catalogName string) ([]declcfg.Channel, []declcfg.Bundle, error) { - channels, err := fetchCatalogMetadataByScheme[declcfg.Channel](ctx, cl, declcfg.SchemaChannel, catalogName) - if err != nil { - return nil, nil, err - } - bundles, err := fetchCatalogMetadataByScheme[declcfg.Bundle](ctx, cl, declcfg.SchemaBundle, catalogName) - if err != nil { - return nil, nil, err - } - - return channels, bundles, nil -} - -type declcfgSchema interface { - declcfg.Package | declcfg.Bundle | declcfg.Channel -} - -// TODO: Cleanup once https://github.com/golang/go/issues/45380 implemented -// We should be able to get rid of the schema arg and switch based on the type passed to this generic -func fetchCatalogMetadataByScheme[T declcfgSchema](ctx context.Context, cl client.Client, schema, catalogName string) ([]T, error) { - cmList := catalogd.CatalogMetadataList{} - if err := cl.List(ctx, &cmList, client.MatchingLabels{"schema": schema, "catalog": catalogName}); err != nil { - return nil, err - } - - contents := []T{} - for _, cm := range cmList.Items { - var content T - if err := json.Unmarshal(cm.Spec.Content, &content); err != nil { - return nil, err - } - contents = append(contents, content) - } - - return contents, nil -} diff --git a/internal/resolution/util/predicates/predicates.go b/internal/resolution/util/predicates/predicates.go deleted file mode 100644 index f374efa2c..000000000 --- a/internal/resolution/util/predicates/predicates.go +++ /dev/null @@ -1,100 +0,0 @@ -package predicates - -import ( - mmsemver "github.com/Masterminds/semver/v3" - bsemver "github.com/blang/semver/v4" - "github.com/operator-framework/deppy/pkg/deppy/input" - - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" -) - -func WithPackageName(packageName string) input.Predicate { - return func(entity *input.Entity) bool { - bundleEntity := olmentity.NewBundleEntity(entity) - name, err := bundleEntity.PackageName() - if err != nil { - return false - } - return name == packageName - } -} - -func InMastermindsSemverRange(semverRange *mmsemver.Constraints) input.Predicate { - return func(entity *input.Entity) bool { - bundleEntity := olmentity.NewBundleEntity(entity) - bVersion, err := bundleEntity.Version() - if err != nil { - return false - } - // No error should occur here because the simple version was successfully parsed by blang - // We are unaware of any tests cases that would cause one to fail but not the other - // This will cause code coverage to drop for this line. We don't ignore the error because - // there might be that one extreme edge case that might cause one to fail but not the other - mVersion, err := mmsemver.NewVersion(bVersion.String()) - if err != nil { - return false - } - return semverRange.Check(mVersion) - } -} - -func InBlangSemverRange(semverRange bsemver.Range) input.Predicate { - return func(entity *input.Entity) bool { - bundleEntity := olmentity.NewBundleEntity(entity) - bundleVersion, err := bundleEntity.Version() - if err != nil { - return false - } - return semverRange(*bundleVersion) - } -} - -func InChannel(channelName string) input.Predicate { - return func(entity *input.Entity) bool { - bundleEntity := olmentity.NewBundleEntity(entity) - bundleChannel, err := bundleEntity.ChannelName() - if err != nil { - return false - } - return channelName == bundleChannel - } -} - -func ProvidesGVK(gvk *olmentity.GVK) input.Predicate { - return func(entity *input.Entity) bool { - bundleEntity := olmentity.NewBundleEntity(entity) - providedGVKs, err := bundleEntity.ProvidedGVKs() - if err != nil { - return false - } - for i := 0; i < len(providedGVKs); i++ { - providedGVK := &providedGVKs[i] - if providedGVK.String() == gvk.String() { - return true - } - } - return false - } -} - -func WithBundleImage(bundleImage string) input.Predicate { - return func(entity *input.Entity) bool { - bundleEntity := olmentity.NewBundleEntity(entity) - bundlePath, err := bundleEntity.BundlePath() - if err != nil { - return false - } - return bundlePath == bundleImage - } -} - -func Replaces(bundleID string) input.Predicate { - return func(entity *input.Entity) bool { - bundleEntity := olmentity.NewBundleEntity(entity) - replaces, err := bundleEntity.BundleChannelEntry() - if err != nil { - return false - } - return replaces.Replaces == bundleID - } -} diff --git a/internal/resolution/util/predicates/predicates_test.go b/internal/resolution/util/predicates/predicates_test.go deleted file mode 100644 index 2c034d1f0..000000000 --- a/internal/resolution/util/predicates/predicates_test.go +++ /dev/null @@ -1,222 +0,0 @@ -package predicates_test - -import ( - "testing" - - mmsemver "github.com/Masterminds/semver/v3" - bsemver "github.com/blang/semver/v4" - "github.com/operator-framework/deppy/pkg/deppy/input" - "github.com/operator-framework/operator-registry/alpha/property" - - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" - "github.com/operator-framework/operator-controller/internal/resolution/util/predicates" -) - -type testData struct { - entity map[string]string - value string - result bool -} - -func TestPredicatesWithPackageName(t *testing.T) { - testData := []testData{ - {map[string]string{property.TypePackage: `{"packageName": "mypackage", "version": "1.0.0"}`}, - "mypackage", - true}, - {map[string]string{property.TypePackage: `{"packageName": "mypackage", "version": "1.0.0"}`}, - "notmypackage", - false}, - {map[string]string{}, - "mypackage", - false}, - } - - for _, d := range testData { - t.Run("InMastermindsSemverRange", func(t *testing.T) { - entity := input.NewEntity("test", d.entity) - if predicates.WithPackageName(d.value)(entity) != d.result { - if d.result { - t.Errorf("package %v should be in entity %v", d.value, entity) - } else { - t.Errorf("package %v should not be in entity %v", d.value, entity) - } - } - }) - } -} - -func TestPredicatesInMastermindsSemverRange(t *testing.T) { - testData := []testData{ - {map[string]string{property.TypePackage: `{"packageName": "mypackage", "version": "1.0.0"}`}, - ">=1.0.0", - true}, - {map[string]string{property.TypePackage: `{"packageName": "mypackage", "version": "1.0.0"}`}, - ">=2.0.0", - false}, - {map[string]string{}, - ">=1.0.0", - false}, - } - - for _, d := range testData { - t.Run("InMastermindsSemverRange", func(t *testing.T) { - entity := input.NewEntity("test", d.entity) - c, err := mmsemver.NewConstraint(d.value) - if err != nil { - t.Fatalf("unable to parse constraint '%v': %v", d.value, err) - } - if predicates.InMastermindsSemverRange(c)(entity) != d.result { - if d.result { - t.Errorf("version %v should be in entity %v", d.value, entity) - } else { - t.Errorf("version %v should not be in entity %v", d.value, entity) - } - } - }) - } -} - -func TestPredicatesInBlangSemverRange(t *testing.T) { - testData := []testData{ - {map[string]string{property.TypePackage: `{"packageName": "mypackage", "version": "1.0.0"}`}, - ">=1.0.0", - true}, - {map[string]string{property.TypePackage: `{"packageName": "mypackage", "version": "1.0.0"}`}, - ">=2.0.0", - false}, - {map[string]string{}, - ">=1.0.0", - false}, - } - - for _, d := range testData { - t.Run("InBlangSemverRange", func(t *testing.T) { - entity := input.NewEntity("test", d.entity) - r := bsemver.MustParseRange(d.value) - if predicates.InBlangSemverRange(r)(entity) != d.result { - if d.result { - t.Errorf("version %v should be in entity %v", d.value, entity) - } else { - t.Errorf("version %v should not be in entity %v", d.value, entity) - } - } - }) - } -} - -func TestPredicatesInChannel(t *testing.T) { - testData := []testData{ - {map[string]string{property.TypeChannel: `{"channelName":"stable","priority":0}`}, - "stable", - true}, - {map[string]string{property.TypeChannel: `{"channelName":"stable","priority":0}`}, - "unstable", - false}, - {map[string]string{}, - "stable", - false}, - } - - for _, d := range testData { - t.Run("InChannel", func(t *testing.T) { - entity := input.NewEntity("test", d.entity) - if predicates.InChannel(d.value)(entity) != d.result { - if d.result { - t.Errorf("channel %v should be in entity %v", d.value, entity) - } else { - t.Errorf("channel %v should not be in entity %v", d.value, entity) - } - } - }) - } -} - -func TestPredicatesWithBundleImage(t *testing.T) { - testData := []testData{ - {map[string]string{olmentity.PropertyBundlePath: `"registry.io/repo/image@sha256:1234567890"`}, - "registry.io/repo/image@sha256:1234567890", - true}, - {map[string]string{olmentity.PropertyBundlePath: `"registry.io/repo/image@sha256:1234567890"`}, - "registry.io/repo/image@sha256:0987654321", - false}, - {map[string]string{}, - "registry.io/repo/image@sha256:1234567890", - false}, - } - - for _, d := range testData { - t.Run("WithBundleImage", func(t *testing.T) { - entity := input.NewEntity("test", d.entity) - if predicates.WithBundleImage(d.value)(entity) != d.result { - if d.result { - t.Errorf("bundle %v should be in entity %v", d.value, entity) - } else { - t.Errorf("bundle %v should not be in entity %v", d.value, entity) - } - } - }) - } -} - -type testGVK struct { - entity map[string]string - value *olmentity.GVK - result bool -} - -func TestProvidesGVK(t *testing.T) { - testData := []testGVK{ - {map[string]string{property.TypeGVK: `[{"group":"foo.io","kind":"Foo","version":"v1"},{"group":"bar.io","kind":"Bar","version":"v1"}]`}, - &olmentity.GVK{Group: "foo.io", Version: "v1", Kind: "Foo"}, - true}, - {map[string]string{property.TypeGVK: `[{"group":"foo.io","kind":"Foo","version":"v1"},{"group":"bar.io","kind":"Bar","version":"v1"}]`}, - &olmentity.GVK{Group: "baz.io", Version: "v1alpha1", Kind: "Baz"}, - false}, - {map[string]string{}, - &olmentity.GVK{Group: "foo.io", Version: "v1", Kind: "Foo"}, - false}, - } - - for _, d := range testData { - t.Run("WithBundleImage", func(t *testing.T) { - entity := input.NewEntity("test", d.entity) - if predicates.ProvidesGVK(d.value)(entity) != d.result { - if d.result { - t.Errorf("replaces %v should be in entity %v", d.value, entity) - } else { - t.Errorf("replaces %v should not be in entity %v", d.value, entity) - } - } - }) - } -} - -func TestPredicatesReplaces(t *testing.T) { - testData := []testData{ - {map[string]string{"olm.bundle.channelEntry": `{"replaces": "test.v0.2.0"}`}, - "test.v0.2.0", - true}, - {map[string]string{"olm.bundle.channelEntry": `{"replaces": "test.v0.2.0"}`}, - "test.v0.1.0", - false}, - {map[string]string{}, - "test.v0.2.0", - false}, - {map[string]string{"olm.bundle.channelEntry": `{"replaces"}`}, - "test.v0.2.0", - false}, - } - - for _, d := range testData { - t.Run("WithBundleImage", func(t *testing.T) { - entity := input.NewEntity("test", d.entity) - if predicates.Replaces(d.value)(entity) != d.result { - if d.result { - t.Errorf("replaces %v should be in entity %v", d.value, entity) - } else { - t.Errorf("replaces %v should not be in entity %v", d.value, entity) - } - } - }) - } -} diff --git a/internal/resolution/util/sort/sort.go b/internal/resolution/util/sort/sort.go deleted file mode 100644 index d013b982f..000000000 --- a/internal/resolution/util/sort/sort.go +++ /dev/null @@ -1,80 +0,0 @@ -package sort - -import ( - "strings" - - "github.com/operator-framework/deppy/pkg/deppy/input" - - "github.com/operator-framework/operator-controller/internal/resolution/entities" -) - -// ByChannelAndVersion is an entity sort function that orders the entities in -// package, channel (default channel at the head), and inverse version (higher versions on top) -// if a property does not exist for one of the entities, the one missing the property is pushed down -// if both entities are missing the same property they are ordered by id -func ByChannelAndVersion(entity1 *input.Entity, entity2 *input.Entity) bool { - e1 := entities.NewBundleEntity(entity1) - e2 := entities.NewBundleEntity(entity2) - - // first sort package lexical order - pkgOrder := packageOrder(e1, e2) - if pkgOrder != 0 { - return pkgOrder < 0 - } - - // todo(perdasilva): handle default channel in ordering once it is being exposed by the entity - channelOrder := channelOrder(e1, e2) - if channelOrder != 0 { - return channelOrder < 0 - } - - // order version from highest to lowest (favor the latest release) - versionOrder := versionOrder(e1, e2) - return versionOrder > 0 -} - -func compareErrors(err1 error, err2 error) int { - if err1 != nil && err2 == nil { - return 1 - } - - if err1 == nil && err2 != nil { - return -1 - } - return 0 -} - -func packageOrder(e1, e2 *entities.BundleEntity) int { - name1, err1 := e1.PackageName() - name2, err2 := e2.PackageName() - errComp := compareErrors(err1, err2) - if errComp != 0 { - return errComp - } - return strings.Compare(name1, name2) -} - -func channelOrder(e1, e2 *entities.BundleEntity) int { - channelProperties1, err1 := e1.Channel() - channelProperties2, err2 := e2.Channel() - errComp := compareErrors(err1, err2) - if errComp != 0 { - return errComp - } - if channelProperties1.Priority != channelProperties2.Priority { - return channelProperties1.Priority - channelProperties2.Priority - } - return strings.Compare(channelProperties1.ChannelName, channelProperties2.ChannelName) -} - -func versionOrder(e1, e2 *entities.BundleEntity) int { - ver1, err1 := e1.Version() - ver2, err2 := e2.Version() - errComp := compareErrors(err1, err2) - if errComp != 0 { - // the sign gets inverted because version is sorted - // from highest to lowest - return -1 * errComp - } - return ver1.Compare(*ver2) -} diff --git a/internal/resolution/util/sort/sort_test.go b/internal/resolution/util/sort/sort_test.go deleted file mode 100644 index f3d5ec003..000000000 --- a/internal/resolution/util/sort/sort_test.go +++ /dev/null @@ -1,176 +0,0 @@ -package sort_test - -import ( - "sort" - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/operator-framework/deppy/pkg/deppy/input" - "github.com/operator-framework/operator-registry/alpha/property" - - entitysort "github.com/operator-framework/operator-controller/internal/resolution/util/sort" -) - -func TestSortByPackageName(t *testing.T) { - e1 := input.NewEntity("test1", map[string]string{ - property.TypePackage: `{"packageName": "package1", "version": "1.0.0"}`, - }) - e2 := input.NewEntity("test2", map[string]string{ - property.TypePackage: `{"packageName": "package2", "version": "1.0.0"}`, - }) - e3 := input.NewEntity("test3", map[string]string{ - property.TypePackage: `{"packageName": "package3", "version": "1.0.0"}`, - }) - entities := []*input.Entity{e2, e3, e1} - - sort.Slice(entities, func(i, j int) bool { - return entitysort.ByChannelAndVersion(entities[i], entities[j]) - }) - - if diff := cmp.Diff(entities[0], e1); diff != "" { - t.Error(diff) - } - if diff := cmp.Diff(entities[1], e2); diff != "" { - t.Error(diff) - } - if diff := cmp.Diff(entities[2], e3); diff != "" { - t.Error(diff) - } -} - -func TestSortByChannelName(t *testing.T) { - e1 := input.NewEntity("test1", map[string]string{ - property.TypePackage: `{"packageName": "package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stableA","priority":0}`, - }) - e2 := input.NewEntity("test2", map[string]string{ - property.TypePackage: `{"packageName": "package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stableB","priority":0}`, - }) - e3 := input.NewEntity("test3", map[string]string{ - property.TypePackage: `{"packageName": "package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stableC","priority":0}`, - }) - entities := []*input.Entity{e2, e3, e1} - - sort.Slice(entities, func(i, j int) bool { - return entitysort.ByChannelAndVersion(entities[i], entities[j]) - }) - - if diff := cmp.Diff(entities[0], e1); diff != "" { - t.Error(diff) - } - if diff := cmp.Diff(entities[1], e2); diff != "" { - t.Error(diff) - } - if diff := cmp.Diff(entities[2], e3); diff != "" { - t.Error(diff) - } -} - -func TestSortByVersionNumber(t *testing.T) { - e1 := input.NewEntity("test1", map[string]string{ - property.TypePackage: `{"packageName": "mypackage", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }) - e2 := input.NewEntity("test2", map[string]string{ - property.TypePackage: `{"packageName": "mypackage", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }) - e3 := input.NewEntity("test3", map[string]string{ - property.TypePackage: `{"packageName": "mypackage", "version": "3.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }) - entities := []*input.Entity{e2, e3, e1} - - sort.Slice(entities, func(i, j int) bool { - return entitysort.ByChannelAndVersion(entities[i], entities[j]) - }) - - if diff := cmp.Diff(entities[0], e3); diff != "" { - t.Error(diff) - } - if diff := cmp.Diff(entities[1], e2); diff != "" { - t.Error(diff) - } - if diff := cmp.Diff(entities[2], e1); diff != "" { - t.Error(diff) - } -} - -func TestSortByVersionNumberAndChannelPriority(t *testing.T) { - e1 := input.NewEntity("test1", map[string]string{ - property.TypePackage: `{"packageName": "mypackage", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"beta","priority":1}`, - }) - e2 := input.NewEntity("test2", map[string]string{ - property.TypePackage: `{"packageName": "mypackage", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }) - e3 := input.NewEntity("test3", map[string]string{ - property.TypePackage: `{"packageName": "mypackage", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"beta","priority":1}`, - }) - e4 := input.NewEntity("test4", map[string]string{ - property.TypePackage: `{"packageName": "mypackage", "version": "3.0.0"}`, - property.TypeChannel: `{"channelName":"beta","priority":1}`, - }) - entities := []*input.Entity{e2, e3, e1, e4} - - sort.Slice(entities, func(i, j int) bool { - return entitysort.ByChannelAndVersion(entities[i], entities[j]) - }) - - if diff := cmp.Diff(entities[0], e2); diff != "" { - t.Error(diff) - } - if diff := cmp.Diff(entities[1], e4); diff != "" { - t.Error(diff) - } - if diff := cmp.Diff(entities[2], e3); diff != "" { - t.Error(diff) - } - if diff := cmp.Diff(entities[3], e1); diff != "" { - t.Error(diff) - } -} - -func TestSortMissingProperty(t *testing.T) { - e1 := input.NewEntity("test1", map[string]string{ - property.TypePackage: `{"packageName": "mypackageA", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }) - e2 := input.NewEntity("test2", map[string]string{ - property.TypePackage: `{"packageName": "mypackageA"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }) - e3 := input.NewEntity("test3", map[string]string{ - property.TypePackage: `{"packageName": "mypackageA", "version": "3.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }) - e4 := input.NewEntity("test4", map[string]string{ - property.TypePackage: `{"packageName": "mypackageB", "version": "3.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }) - e5 := input.NewEntity("test5", map[string]string{}) - entities := []*input.Entity{e2, e3, e1, e4, e5} - - sort.Slice(entities, func(i, j int) bool { - return entitysort.ByChannelAndVersion(entities[i], entities[j]) - }) - if diff := cmp.Diff(entities[0], e3); diff != "" { - t.Error(diff) - } - if diff := cmp.Diff(entities[1], e1); diff != "" { - t.Error(diff) - } - if diff := cmp.Diff(entities[2], e4); diff != "" { - t.Error(diff) - } - if diff := cmp.Diff(entities[3], e2); diff != "" { // no version - t.Error(diff) - } - if diff := cmp.Diff(entities[4], e5); diff != "" { // no package - or anything - t.Error(diff) - } -} diff --git a/internal/resolution/variablesources/bundles_and_dependencies.go b/internal/resolution/variablesources/bundles_and_dependencies.go index 283c28f58..84b90c27b 100644 --- a/internal/resolution/variablesources/bundles_and_dependencies.go +++ b/internal/resolution/variablesources/bundles_and_dependencies.go @@ -109,7 +109,6 @@ func (b *BundlesAndDepsVariableSource) filterBundleDependencies(allBundles []*ca added[id] = struct{}{} } } - } } From 66c7be9a66330e6cdbd51a009e8530862db054b4 Mon Sep 17 00:00:00 2001 From: dtfranz Date: Tue, 12 Sep 2023 10:59:19 -0700 Subject: [PATCH 4/6] Deppy version bump, remove remaining entity/entitysource vestigials Signed-off-by: dtfranz --- cmd/manager/main.go | 1 - cmd/resolutioncli/main.go | 1 - go.mod | 3 ++- go.sum | 8 ++++---- .../resolution/variablesources/bundle_deployment.go | 7 ++++--- .../variablesources/bundles_and_dependencies.go | 5 ++--- internal/resolution/variablesources/composite.go | 8 ++++---- .../resolution/variablesources/composite_test.go | 13 +++++-------- .../resolution/variablesources/crd_constraints.go | 4 ++-- .../resolution/variablesources/installed_package.go | 2 +- internal/resolution/variablesources/operator.go | 4 ++-- .../resolution/variablesources/required_package.go | 2 +- 12 files changed, 27 insertions(+), 31 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index b54614576..27b923833 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -106,7 +106,6 @@ func main() { Client: cl, Scheme: mgr.GetScheme(), Resolver: solver.NewDeppySolver( - nil, controllers.NewVariableSource(cl, catalogClient), ), }).SetupWithManager(mgr); err != nil { diff --git a/cmd/resolutioncli/main.go b/cmd/resolutioncli/main.go index 3b8221033..5b036eb6c 100644 --- a/cmd/resolutioncli/main.go +++ b/cmd/resolutioncli/main.go @@ -124,7 +124,6 @@ func run(ctx context.Context, packageName, packageVersion, packageChannel, index catalogClient := newIndexRefClient(indexRef) resolver := solver.NewDeppySolver( - nil, append( variablesources.NestedVariableSource{newPackageVariableSource(catalogClient, packageName, packageVersion, packageChannel)}, controllers.NewVariableSource(cl, catalogClient)..., diff --git a/go.mod b/go.mod index 9eb47ca34..5b1416f55 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/onsi/gomega v1.27.10 github.com/operator-framework/api v0.17.4-0.20230223191600-0131a6301e42 github.com/operator-framework/catalogd v0.6.0 - github.com/operator-framework/deppy v0.0.0-20230629133131-bb7b6ae7b266 + github.com/operator-framework/deppy v0.0.1 github.com/operator-framework/operator-registry v1.28.0 github.com/operator-framework/rukpak v0.13.0 github.com/spf13/pflag v1.0.5 @@ -73,6 +73,7 @@ require ( github.com/golang/protobuf v1.5.3 // indirect github.com/google/cel-go v0.12.6 // indirect github.com/google/gnostic v0.5.7-v3refs // indirect + github.com/google/go-cmp v0.5.9 // indirect github.com/google/gofuzz v1.2.0 // indirect github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 // indirect github.com/google/uuid v1.3.0 // indirect diff --git a/go.sum b/go.sum index 559142292..c881f14d9 100644 --- a/go.sum +++ b/go.sum @@ -542,7 +542,7 @@ github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH github.com/imdario/mergo v0.3.13 h1:lFzP57bqS/wsqKssCGmtLAb8A0wKjLGrve2q3PPVcBk= github.com/imdario/mergo v0.3.13/go.mod h1:4lJ1jqUDcsbIECGy0RUJAXNIhg+6ocWgb1ALK2O4oXg= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= -github.com/inconshreveable/mousetrap v1.0.1 h1:U3uMjPSQEBMNp1lFxmllqCPM6P5u/Xq7Pgzkat/bFNc= +github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/j-keck/arping v0.0.0-20160618110441-2cf9dc699c56/go.mod h1:ymszkNOg6tORTn+6F6j+Jc8TOr5osrynvN6ivFWZ2GA= github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A= github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo= @@ -704,8 +704,8 @@ github.com/operator-framework/api v0.17.4-0.20230223191600-0131a6301e42 h1:d/Pnr github.com/operator-framework/api v0.17.4-0.20230223191600-0131a6301e42/go.mod h1:l/cuwtPxkVUY7fzYgdust2m9tlmb8I4pOvbsUufRb24= github.com/operator-framework/catalogd v0.6.0 h1:dSZ54MVSHJ8hcoV7OCRxnk3x4O3ramlyPvvz0vsKYdk= github.com/operator-framework/catalogd v0.6.0/go.mod h1:I0n086a4a+nP1YZy742IrPaWvOlWu0Mj6qA6j4K96Vg= -github.com/operator-framework/deppy v0.0.0-20230629133131-bb7b6ae7b266 h1:SQEUaAoRWNhr2poLH6z/RsEWZG7PppDWHsr5vAvJkJc= -github.com/operator-framework/deppy v0.0.0-20230629133131-bb7b6ae7b266/go.mod h1:6kgHMeS5vQt3gqWGgJIig1yT5uflBUsCc1orP+I3nbk= +github.com/operator-framework/deppy v0.0.1 h1:PLTtaFGwktPhKuKZkfUruTimrWpyaO3tghbsjs0uMjc= +github.com/operator-framework/deppy v0.0.1/go.mod h1:EV6vnxRodSFRn2TFztfxFhMPGh5QufOhn3tpIP1Z8cc= github.com/operator-framework/operator-registry v1.28.0 h1:vtmd2WgJxkx7vuuOxW4k5Le/oo0SfonSeJVMU3rKIfk= github.com/operator-framework/operator-registry v1.28.0/go.mod h1:UYw3uaZyHwHgnczLRYmUqMpgRgP2EfkqOsaR+LI+nK8= github.com/operator-framework/rukpak v0.13.0 h1:QP0P9ybwtkFpfVOMY9z5v4+vRyBdoqAotv8RP/SZ0hw= @@ -801,7 +801,7 @@ github.com/spf13/cast v1.3.0/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkU github.com/spf13/cobra v0.0.2-0.20171109065643-2da4a54c5cee/go.mod h1:1l0Ry5zgKvJasoi3XT1TypsSe7PqH0Sj9dhYf7v3XqQ= github.com/spf13/cobra v0.0.3/go.mod h1:1l0Ry5zgKvJasoi3XT1TypsSe7PqH0Sj9dhYf7v3XqQ= github.com/spf13/cobra v1.0.0/go.mod h1:/6GTrnGXV9HjY+aR4k0oJ5tcvakLuG6EuKReYlHNrgE= -github.com/spf13/cobra v1.6.1 h1:o94oiPyS4KD1mPy2fmcYYHHfCxLqYjJOhGsCHFZtEzA= +github.com/spf13/cobra v1.7.0 h1:hyqWnYt1ZQShIddO5kBpj3vu05/++x6tJ6dg8EC572I= github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo= github.com/spf13/pflag v0.0.0-20170130214245-9ff6c6923cff/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/pflag v1.0.1-0.20171106142849-4c012f6dcd95/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= diff --git a/internal/resolution/variablesources/bundle_deployment.go b/internal/resolution/variablesources/bundle_deployment.go index fd1ea283e..7624a28f3 100644 --- a/internal/resolution/variablesources/bundle_deployment.go +++ b/internal/resolution/variablesources/bundle_deployment.go @@ -5,9 +5,10 @@ import ( "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" - catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" "sigs.k8s.io/controller-runtime/pkg/client" + + catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" ) var _ input.VariableSource = &BundleDeploymentVariableSource{} @@ -26,7 +27,7 @@ func NewBundleDeploymentVariableSource(cl client.Client, catalog *catalogclient. } } -func (o *BundleDeploymentVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { +func (o *BundleDeploymentVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { variableSources := SliceVariableSource{} if o.inputVariableSource != nil { variableSources = append(variableSources, o.inputVariableSource) @@ -53,5 +54,5 @@ func (o *BundleDeploymentVariableSource) GetVariables(ctx context.Context, _ inp } } - return variableSources.GetVariables(ctx, nil) + return variableSources.GetVariables(ctx) } diff --git a/internal/resolution/variablesources/bundles_and_dependencies.go b/internal/resolution/variablesources/bundles_and_dependencies.go index 84b90c27b..7d49410c3 100644 --- a/internal/resolution/variablesources/bundles_and_dependencies.go +++ b/internal/resolution/variablesources/bundles_and_dependencies.go @@ -29,12 +29,12 @@ func NewBundlesAndDepsVariableSource(catalog *catalogclient.Client, inputVariabl } } -func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { +func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { var variables []deppy.Variable // extract required package variables for _, variableSource := range b.variableSources { - inputVariables, err := variableSource.GetVariables(ctx, nil) + inputVariables, err := variableSource.GetVariables(ctx) if err != nil { return nil, err } @@ -83,7 +83,6 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context, _ input // create variable variables = append(variables, olmvariables.NewBundleVariable(id, head, dependencies)) } - } return variables, nil diff --git a/internal/resolution/variablesources/composite.go b/internal/resolution/variablesources/composite.go index 48b690d4e..d0e3a20b9 100644 --- a/internal/resolution/variablesources/composite.go +++ b/internal/resolution/variablesources/composite.go @@ -29,7 +29,7 @@ var _ input.VariableSource = &NestedVariableSource{} type NestedVariableSource []func(inputVariableSource input.VariableSource) (input.VariableSource, error) -func (s NestedVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { +func (s NestedVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { if len(s) == 0 { return nil, errors.New("empty nested variable sources") } @@ -43,15 +43,15 @@ func (s NestedVariableSource) GetVariables(ctx context.Context, _ input.EntitySo } } - return variableSource.GetVariables(ctx, nil) + return variableSource.GetVariables(ctx) } type SliceVariableSource []input.VariableSource -func (s SliceVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { +func (s SliceVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { var variables []deppy.Variable for _, variableSource := range s { - inputVariables, err := variableSource.GetVariables(ctx, nil) + inputVariables, err := variableSource.GetVariables(ctx) if err != nil { return nil, err } diff --git a/internal/resolution/variablesources/composite_test.go b/internal/resolution/variablesources/composite_test.go index 18a9acbd0..bfbf859e8 100644 --- a/internal/resolution/variablesources/composite_test.go +++ b/internal/resolution/variablesources/composite_test.go @@ -52,7 +52,6 @@ func TestNestedVariableSource(t *testing.T) { } { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() - mockEntitySource := input.NewCacheQuerier(map[deppy.Identifier]input.Entity{}) nestedSource := variablesources.NestedVariableSource{} for i := range tt.varSources { @@ -70,7 +69,7 @@ func TestNestedVariableSource(t *testing.T) { }) } - variables, err := nestedSource.GetVariables(ctx, mockEntitySource) + variables, err := nestedSource.GetVariables(ctx) if tt.wantErr != "" { assert.EqualError(t, err, tt.wantErr) } else { @@ -82,7 +81,6 @@ func TestNestedVariableSource(t *testing.T) { t.Run("error from a nested constructor", func(t *testing.T) { ctx := context.Background() - mockEntitySource := input.NewCacheQuerier(map[deppy.Identifier]input.Entity{}) nestedSource := variablesources.NestedVariableSource{ func(inputVariableSource input.VariableSource) (input.VariableSource, error) { @@ -90,7 +88,7 @@ func TestNestedVariableSource(t *testing.T) { }, } - variables, err := nestedSource.GetVariables(ctx, mockEntitySource) + variables, err := nestedSource.GetVariables(ctx) assert.EqualError(t, err, "fake error from a constructor") assert.Nil(t, variables) }) @@ -123,10 +121,9 @@ func TestSliceVariableSource(t *testing.T) { } { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() - mockEntitySource := input.NewCacheQuerier(map[deppy.Identifier]input.Entity{}) sliceSource := variablesources.SliceVariableSource(tt.varSources) - variables, err := sliceSource.GetVariables(ctx, mockEntitySource) + variables, err := sliceSource.GetVariables(ctx) if tt.wantErr != "" { assert.EqualError(t, err, tt.wantErr) } else { @@ -145,7 +142,7 @@ type mockVariableSource struct { fakeError error } -func (m *mockVariableSource) GetVariables(ctx context.Context, entitySource input.EntitySource) ([]deppy.Variable, error) { +func (m *mockVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { if m.fakeError != nil { return nil, m.fakeError } @@ -154,7 +151,7 @@ func (m *mockVariableSource) GetVariables(ctx context.Context, entitySource inpu return m.fakeVariables, nil } - nestedVars, err := m.inputVariableSource.GetVariables(ctx, entitySource) + nestedVars, err := m.inputVariableSource.GetVariables(ctx) if err != nil { return nil, err } diff --git a/internal/resolution/variablesources/crd_constraints.go b/internal/resolution/variablesources/crd_constraints.go index f422f8ce6..2108a6317 100644 --- a/internal/resolution/variablesources/crd_constraints.go +++ b/internal/resolution/variablesources/crd_constraints.go @@ -34,8 +34,8 @@ func NewCRDUniquenessConstraintsVariableSource(inputVariableSource input.Variabl } } -func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { - variables, err := g.inputVariableSource.GetVariables(ctx, nil) +func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { + variables, err := g.inputVariableSource.GetVariables(ctx) if err != nil { return nil, err } diff --git a/internal/resolution/variablesources/installed_package.go b/internal/resolution/variablesources/installed_package.go index 2a91584c5..0a550b53a 100644 --- a/internal/resolution/variablesources/installed_package.go +++ b/internal/resolution/variablesources/installed_package.go @@ -21,7 +21,7 @@ type InstalledPackageVariableSource struct { bundleImage string } -func (r *InstalledPackageVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { +func (r *InstalledPackageVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { allBundles, err := r.catalog.Bundles(ctx) if err != nil { return nil, err diff --git a/internal/resolution/variablesources/operator.go b/internal/resolution/variablesources/operator.go index 8c666a9f0..f0664c310 100644 --- a/internal/resolution/variablesources/operator.go +++ b/internal/resolution/variablesources/operator.go @@ -27,7 +27,7 @@ func NewOperatorVariableSource(cl client.Client, catalog *catalogclient.Client, } } -func (o *OperatorVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { +func (o *OperatorVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { variableSources := SliceVariableSource{} if o.inputVariableSource != nil { variableSources = append(variableSources, o.inputVariableSource) @@ -52,5 +52,5 @@ func (o *OperatorVariableSource) GetVariables(ctx context.Context, _ input.Entit variableSources = append(variableSources, rps) } - return variableSources.GetVariables(ctx, nil) + return variableSources.GetVariables(ctx) } diff --git a/internal/resolution/variablesources/required_package.go b/internal/resolution/variablesources/required_package.go index ada3d4473..4a7ca26c5 100644 --- a/internal/resolution/variablesources/required_package.go +++ b/internal/resolution/variablesources/required_package.go @@ -73,7 +73,7 @@ func NewRequiredPackageVariableSource(catalog *catalogclient.Client, packageName return r, nil } -func (r *RequiredPackageVariableSource) GetVariables(ctx context.Context, _ input.EntitySource) ([]deppy.Variable, error) { +func (r *RequiredPackageVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { resultSet, err := r.catalog.Bundles(ctx) if err != nil { return nil, err From c02fe9bf4392bd70f840e4a00b57f48bc2d33e75 Mon Sep 17 00:00:00 2001 From: dtfranz Date: Fri, 15 Sep 2023 16:30:15 -0700 Subject: [PATCH 5/6] Add interface for client mocking Signed-off-by: dtfranz --- cmd/resolutioncli/variable_source.go | 2 +- internal/catalogmetadata/client/client.go | 4 ++++ internal/controllers/variable_source.go | 2 +- internal/resolution/variablesources/bundle_deployment.go | 4 ++-- .../resolution/variablesources/bundles_and_dependencies.go | 6 +++--- internal/resolution/variablesources/installed_package.go | 4 ++-- internal/resolution/variablesources/operator.go | 4 ++-- internal/resolution/variablesources/required_package.go | 4 ++-- 8 files changed, 17 insertions(+), 13 deletions(-) diff --git a/cmd/resolutioncli/variable_source.go b/cmd/resolutioncli/variable_source.go index 5d4069d2f..7aa1b4252 100644 --- a/cmd/resolutioncli/variable_source.go +++ b/cmd/resolutioncli/variable_source.go @@ -23,7 +23,7 @@ import ( "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -func newPackageVariableSource(catalog *catalogclient.Client, packageName, packageVersion, packageChannel string) func(inputVariableSource input.VariableSource) (input.VariableSource, error) { +func newPackageVariableSource(catalog catalogclient.CatalogClient, packageName, packageVersion, packageChannel string) func(inputVariableSource input.VariableSource) (input.VariableSource, error) { return func(inputVariableSource input.VariableSource) (input.VariableSource, error) { pkgSource, err := variablesources.NewRequiredPackageVariableSource( catalog, diff --git a/internal/catalogmetadata/client/client.go b/internal/catalogmetadata/client/client.go index bb48955f8..45e2b864d 100644 --- a/internal/catalogmetadata/client/client.go +++ b/internal/catalogmetadata/client/client.go @@ -12,6 +12,10 @@ import ( "github.com/operator-framework/operator-controller/internal/catalogmetadata" ) +type CatalogClient interface { + Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) +} + func New(cl client.Client) *Client { return &Client{cl: cl} } diff --git a/internal/controllers/variable_source.go b/internal/controllers/variable_source.go index c3fa7bacd..a4a748111 100644 --- a/internal/controllers/variable_source.go +++ b/internal/controllers/variable_source.go @@ -25,7 +25,7 @@ import ( "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -func NewVariableSource(cl client.Client, catalog *catalogclient.Client) variablesources.NestedVariableSource { +func NewVariableSource(cl client.Client, catalog catalogclient.CatalogClient) variablesources.NestedVariableSource { return variablesources.NestedVariableSource{ func(inputVariableSource input.VariableSource) (input.VariableSource, error) { return variablesources.NewOperatorVariableSource(cl, catalog, inputVariableSource), nil diff --git a/internal/resolution/variablesources/bundle_deployment.go b/internal/resolution/variablesources/bundle_deployment.go index 7624a28f3..5ce673298 100644 --- a/internal/resolution/variablesources/bundle_deployment.go +++ b/internal/resolution/variablesources/bundle_deployment.go @@ -15,11 +15,11 @@ var _ input.VariableSource = &BundleDeploymentVariableSource{} type BundleDeploymentVariableSource struct { client client.Client - catalog *catalogclient.Client + catalog catalogclient.CatalogClient inputVariableSource input.VariableSource } -func NewBundleDeploymentVariableSource(cl client.Client, catalog *catalogclient.Client, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource { +func NewBundleDeploymentVariableSource(cl client.Client, catalog catalogclient.CatalogClient, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource { return &BundleDeploymentVariableSource{ client: cl, catalog: catalog, diff --git a/internal/resolution/variablesources/bundles_and_dependencies.go b/internal/resolution/variablesources/bundles_and_dependencies.go index 7d49410c3..5f3d6d136 100644 --- a/internal/resolution/variablesources/bundles_and_dependencies.go +++ b/internal/resolution/variablesources/bundles_and_dependencies.go @@ -18,11 +18,11 @@ import ( var _ input.VariableSource = &BundlesAndDepsVariableSource{} type BundlesAndDepsVariableSource struct { - catalog *catalogclient.Client + catalog catalogclient.CatalogClient variableSources []input.VariableSource } -func NewBundlesAndDepsVariableSource(catalog *catalogclient.Client, inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource { +func NewBundlesAndDepsVariableSource(catalog catalogclient.CatalogClient, inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource { return &BundlesAndDepsVariableSource{ catalog: catalog, variableSources: inputVariableSources, @@ -98,7 +98,7 @@ func (b *BundlesAndDepsVariableSource) filterBundleDependencies(allBundles []*ca for _, requiredPackage := range requiredPackages { packageDependencyBundles := catalogfilter.Filter(allBundles, catalogfilter.And(catalogfilter.WithPackageName(requiredPackage.PackageName), catalogfilter.InBlangSemverRange(*requiredPackage.SemverRange))) if len(packageDependencyBundles) == 0 { - return nil, fmt.Errorf("could not find package dependencies for bundle %q", bundle.Name) + return nil, fmt.Errorf("could not find package dependencies for bundle '%s'", bundle.Name) } for i := 0; i < len(packageDependencyBundles); i++ { bundle := packageDependencyBundles[i] diff --git a/internal/resolution/variablesources/installed_package.go b/internal/resolution/variablesources/installed_package.go index 0a550b53a..0dbee55ad 100644 --- a/internal/resolution/variablesources/installed_package.go +++ b/internal/resolution/variablesources/installed_package.go @@ -17,7 +17,7 @@ import ( var _ input.VariableSource = &InstalledPackageVariableSource{} type InstalledPackageVariableSource struct { - catalog *catalogclient.Client + catalog catalogclient.CatalogClient bundleImage string } @@ -60,7 +60,7 @@ func (r *InstalledPackageVariableSource) notFoundError() error { return fmt.Errorf("bundleImage %q not found", r.bundleImage) } -func NewInstalledPackageVariableSource(catalog *catalogclient.Client, bundleImage string) (*InstalledPackageVariableSource, error) { +func NewInstalledPackageVariableSource(catalog catalogclient.CatalogClient, bundleImage string) (*InstalledPackageVariableSource, error) { return &InstalledPackageVariableSource{ catalog: catalog, bundleImage: bundleImage, diff --git a/internal/resolution/variablesources/operator.go b/internal/resolution/variablesources/operator.go index f0664c310..a57112a4d 100644 --- a/internal/resolution/variablesources/operator.go +++ b/internal/resolution/variablesources/operator.go @@ -15,11 +15,11 @@ var _ input.VariableSource = &OperatorVariableSource{} type OperatorVariableSource struct { client client.Client - catalog *catalogclient.Client + catalog catalogclient.CatalogClient inputVariableSource input.VariableSource } -func NewOperatorVariableSource(cl client.Client, catalog *catalogclient.Client, inputVariableSource input.VariableSource) *OperatorVariableSource { +func NewOperatorVariableSource(cl client.Client, catalog catalogclient.CatalogClient, inputVariableSource input.VariableSource) *OperatorVariableSource { return &OperatorVariableSource{ client: cl, catalog: catalog, diff --git a/internal/resolution/variablesources/required_package.go b/internal/resolution/variablesources/required_package.go index 4a7ca26c5..947e6d1f2 100644 --- a/internal/resolution/variablesources/required_package.go +++ b/internal/resolution/variablesources/required_package.go @@ -47,7 +47,7 @@ func InChannel(channelName string) RequiredPackageVariableSourceOption { } type RequiredPackageVariableSource struct { - catalog *catalogclient.Client + catalog catalogclient.CatalogClient packageName string versionRange string @@ -55,7 +55,7 @@ type RequiredPackageVariableSource struct { predicates []catalogfilter.Predicate[catalogmetadata.Bundle] } -func NewRequiredPackageVariableSource(catalog *catalogclient.Client, packageName string, options ...RequiredPackageVariableSourceOption) (*RequiredPackageVariableSource, error) { +func NewRequiredPackageVariableSource(catalog catalogclient.CatalogClient, packageName string, options ...RequiredPackageVariableSourceOption) (*RequiredPackageVariableSource, error) { if packageName == "" { return nil, fmt.Errorf("package name must not be empty") } From d35618be97953d67d593ed593d30c027bd69fdae Mon Sep 17 00:00:00 2001 From: dtfranz Date: Fri, 15 Sep 2023 16:33:26 -0700 Subject: [PATCH 6/6] Fix tests, bump catalogd, address comments. Signed-off-by: dtfranz --- cmd/resolutioncli/main.go | 14 +- cmd/resolutioncli/variable_source.go | 5 +- go.mod | 1 - internal/catalogmetadata/client/client.go | 4 - internal/controllers/operator_controller.go | 24 +- .../controllers/operator_controller_test.go | 149 +++--- internal/controllers/variable_source.go | 9 +- internal/resolution/variables/bundle.go | 2 +- internal/resolution/variables/bundle_test.go | 58 ++- .../resolution/variables/installed_package.go | 2 +- .../variables/installed_package_test.go | 37 +- .../resolution/variables/required_package.go | 2 +- .../variables/required_package_test.go | 36 +- .../variablesources/bundle_deployment.go | 10 +- .../variablesources/bundle_deployment_test.go | 101 ++-- .../variablesources/bundle_provider.go | 14 + .../bundles_and_dependencies.go | 17 +- .../bundles_and_dependencies_test.go | 471 ++++++++++++------ .../variablesources/crd_constraints.go | 2 +- .../variablesources/crd_constraints_test.go | 386 +++++++------- .../variablesources/installed_package.go | 15 +- .../variablesources/installed_package_test.go | 128 +++-- .../resolution/variablesources/operator.go | 9 +- .../variablesources/operator_test.go | 135 +++-- .../variablesources/required_package.go | 9 +- .../variablesources/required_package_test.go | 140 +++--- test/util/fake_catalog_client.go | 31 ++ 27 files changed, 1097 insertions(+), 714 deletions(-) create mode 100644 internal/resolution/variablesources/bundle_provider.go create mode 100644 test/util/fake_catalog_client.go diff --git a/cmd/resolutioncli/main.go b/cmd/resolutioncli/main.go index 5b036eb6c..9350fbb9b 100644 --- a/cmd/resolutioncli/main.go +++ b/cmd/resolutioncli/main.go @@ -145,24 +145,24 @@ func resolve(ctx context.Context, resolver *solver.DeppySolver, packageName stri return "", err } - bundleEntity, err := getBundleEntityFromSolution(solution, packageName) + bundle, err := bundleFromSolution(solution, packageName) if err != nil { return "", err } // Get the bundle image reference for the bundle - return bundleEntity.Image, nil + return bundle.Image, nil } -func getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) { +func bundleFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) { for _, variable := range solution.SelectedVariables() { switch v := variable.(type) { case *olmvariables.BundleVariable: - entityPkgName := v.BundleEntity().Package - if packageName == entityPkgName { - return v.BundleEntity(), nil + bundlePkgName := v.Bundle().Package + if packageName == bundlePkgName { + return v.Bundle(), nil } } } - return nil, fmt.Errorf("entity for package %q not found in solution", packageName) + return nil, fmt.Errorf("bundle for package %q not found in solution", packageName) } diff --git a/cmd/resolutioncli/variable_source.go b/cmd/resolutioncli/variable_source.go index 7aa1b4252..cad8db182 100644 --- a/cmd/resolutioncli/variable_source.go +++ b/cmd/resolutioncli/variable_source.go @@ -19,14 +19,13 @@ package main import ( "github.com/operator-framework/deppy/pkg/deppy/input" - catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -func newPackageVariableSource(catalog catalogclient.CatalogClient, packageName, packageVersion, packageChannel string) func(inputVariableSource input.VariableSource) (input.VariableSource, error) { +func newPackageVariableSource(catalogClient *indexRefClient, packageName, packageVersion, packageChannel string) func(inputVariableSource input.VariableSource) (input.VariableSource, error) { return func(inputVariableSource input.VariableSource) (input.VariableSource, error) { pkgSource, err := variablesources.NewRequiredPackageVariableSource( - catalog, + catalogClient, packageName, variablesources.InVersionRange(packageVersion), variablesources.InChannel(packageChannel), diff --git a/go.mod b/go.mod index 5b1416f55..795fe5e8a 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,6 @@ require ( github.com/Masterminds/semver/v3 v3.2.1 github.com/blang/semver/v4 v4.0.0 github.com/go-logr/logr v1.2.4 - github.com/google/go-cmp v0.5.9 github.com/onsi/ginkgo/v2 v2.12.1 github.com/onsi/gomega v1.27.10 github.com/operator-framework/api v0.17.4-0.20230223191600-0131a6301e42 diff --git a/internal/catalogmetadata/client/client.go b/internal/catalogmetadata/client/client.go index 45e2b864d..bb48955f8 100644 --- a/internal/catalogmetadata/client/client.go +++ b/internal/catalogmetadata/client/client.go @@ -12,10 +12,6 @@ import ( "github.com/operator-framework/operator-controller/internal/catalogmetadata" ) -type CatalogClient interface { - Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) -} - func New(cl client.Client) *Client { return &Client{cl: cl} } diff --git a/internal/controllers/operator_controller.go b/internal/controllers/operator_controller.go index 1136e9467..3b0ef5067 100644 --- a/internal/controllers/operator_controller.go +++ b/internal/controllers/operator_controller.go @@ -154,9 +154,9 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha return ctrl.Result{}, unsat } - // lookup the bundle entity in the solution that corresponds to the + // lookup the bundle in the solution that corresponds to the // Operator's desired package name. - bundleEntity, err := r.getBundleEntityFromSolution(solution, op.Spec.PackageName) + bundle, err := r.bundleFromSolution(solution, op.Spec.PackageName) if err != nil { op.Status.InstalledBundleResource = "" setInstalledStatusConditionUnknown(&op.Status.Conditions, "installation has not been attempted as resolution failed", op.GetGeneration()) @@ -165,11 +165,11 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha return ctrl.Result{}, err } - // Now we can set the Resolved Condition, and the resolvedBundleSource field to the bundleEntity.Image value. - op.Status.ResolvedBundleResource = bundleEntity.Image - setResolvedStatusConditionSuccess(&op.Status.Conditions, fmt.Sprintf("resolved to %q", bundleEntity.Image), op.GetGeneration()) + // Now we can set the Resolved Condition, and the resolvedBundleSource field to the bundle.Image value. + op.Status.ResolvedBundleResource = bundle.Image + setResolvedStatusConditionSuccess(&op.Status.Conditions, fmt.Sprintf("resolved to %q", bundle.Image), op.GetGeneration()) - mediaType, err := bundleEntity.MediaType() + mediaType, err := bundle.MediaType() if err != nil { setInstalledStatusConditionFailed(&op.Status.Conditions, err.Error(), op.GetGeneration()) return ctrl.Result{}, err @@ -181,7 +181,7 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha } // Ensure a BundleDeployment exists with its bundle source from the bundle // image we just looked up in the solution. - dep := r.generateExpectedBundleDeployment(*op, bundleEntity.Image, bundleProvisioner) + dep := r.generateExpectedBundleDeployment(*op, bundle.Image, bundleProvisioner) if err := r.ensureBundleDeployment(ctx, dep); err != nil { // originally Reason: operatorsv1alpha1.ReasonInstallationFailed op.Status.InstalledBundleResource = "" @@ -251,17 +251,17 @@ func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alph } } -func (r *OperatorReconciler) getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) { +func (r *OperatorReconciler) bundleFromSolution(solution *solver.Solution, packageName string) (*catalogmetadata.Bundle, error) { for _, variable := range solution.SelectedVariables() { switch v := variable.(type) { case *olmvariables.BundleVariable: - entityPkgName := v.BundleEntity().Package - if packageName == entityPkgName { - return v.BundleEntity(), nil + bundlePkgName := v.Bundle().Package + if packageName == bundlePkgName { + return v.Bundle(), nil } } } - return nil, fmt.Errorf("entity for package %q not found in solution", packageName) + return nil, fmt.Errorf("bundle for package %q not found in solution", packageName) } func (r *OperatorReconciler) generateExpectedBundleDeployment(o operatorsv1alpha1.Operator, bundlePath string, bundleProvisioner string) *unstructured.Unstructured { diff --git a/internal/controllers/operator_controller_test.go b/internal/controllers/operator_controller_test.go index bcc65caef..db35d8599 100644 --- a/internal/controllers/operator_controller_test.go +++ b/internal/controllers/operator_controller_test.go @@ -2,13 +2,14 @@ package controllers_test import ( "context" + "encoding/json" "fmt" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" "github.com/operator-framework/deppy/pkg/deppy/solver" + "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/alpha/property" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -20,21 +21,25 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" "github.com/operator-framework/operator-controller/internal/conditionsets" "github.com/operator-framework/operator-controller/internal/controllers" + testutil "github.com/operator-framework/operator-controller/test/util" ) var _ = Describe("Operator Controller Test", func() { var ( - ctx context.Context - reconciler *controllers.OperatorReconciler + ctx context.Context + fakeCatalogClient testutil.FakeCatalogClient + reconciler *controllers.OperatorReconciler ) BeforeEach(func() { ctx = context.Background() + fakeCatalogClient = testutil.NewFakeCatalogClient(testBundleList) reconciler = &controllers.OperatorReconciler{ Client: cl, Scheme: sch, - Resolver: solver.NewDeppySolver(testEntitySource, controllers.NewVariableSource(cl)), + Resolver: solver.NewDeppySolver(controllers.NewVariableSource(cl, &fakeCatalogClient)), } }) When("the operator does not exist", func() { @@ -575,43 +580,6 @@ var _ = Describe("Operator Controller Test", func() { }) }) }) - When("the selected bundle's image ref cannot be parsed", func() { - const pkgName = "badimage" - BeforeEach(func() { - By("initializing cluster state") - operator = &operatorsv1alpha1.Operator{ - ObjectMeta: metav1.ObjectMeta{Name: opKey.Name}, - Spec: operatorsv1alpha1.OperatorSpec{PackageName: pkgName}, - } - err := cl.Create(ctx, operator) - Expect(err).NotTo(HaveOccurred()) - }) - It("sets resolution failure status and returns an error", func() { - By("running reconcile") - res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey}) - Expect(res).To(Equal(ctrl.Result{})) - Expect(err).To(MatchError(ContainSubstring(`error determining bundle path for entity`))) - - By("fetching updated operator after reconcile") - Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred()) - - By("Checking the status fields") - Expect(operator.Status.ResolvedBundleResource).To(Equal("")) - Expect(operator.Status.InstalledBundleResource).To(Equal("")) - - By("checking the expected conditions") - cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeResolved) - Expect(cond).NotTo(BeNil()) - Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonResolutionFailed)) - Expect(cond.Message).To(ContainSubstring(`error determining bundle path for entity`)) - cond = apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeInstalled) - Expect(cond).NotTo(BeNil()) - Expect(cond.Status).To(Equal(metav1.ConditionUnknown)) - Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonInstallationStatusUnknown)) - Expect(cond.Message).To(Equal("installation has not been attempted as resolution failed")) - }) - }) When("the operator specifies a duplicate package", func() { const pkgName = "prometheus" var dupOperator *operatorsv1alpha1.Operator @@ -1080,41 +1048,62 @@ func verifyConditionsInvariants(op *operatorsv1alpha1.Operator) { } } -var testEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{ - "operatorhub/prometheus/0.37.0": *input.NewEntity("operatorhub/prometheus/0.37.0", map[string]string{ - "olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"`, - "olm.bundle.channelEntry": `{"name":"prometheus.0.37.0"}`, - "olm.channel": `{"channelName":"beta","priority":0}`, - "olm.package": `{"packageName":"prometheus","version":"0.37.0"}`, - "olm.gvk": `[]`, - }), - "operatorhub/prometheus/0.47.0": *input.NewEntity("operatorhub/prometheus/0.47.0", map[string]string{ - "olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed"`, - "olm.bundle.channelEntry": `{"name":"prometheus.0.47.0"}`, - "olm.channel": `{"channelName":"beta","priority":0,"replaces":"prometheusoperator.0.37.0"}`, - "olm.package": `{"packageName":"prometheus","version":"0.47.0"}`, - "olm.gvk": `[]`, - }), - "operatorhub/badimage/0.1.0": *input.NewEntity("operatorhub/badimage/0.1.0", map[string]string{ - "olm.bundle.path": `{"name": "quay.io/operatorhubio/badimage:v0.1.0"}`, - "olm.bundle.channelEntry": `{"name":"badimage.0.1.0"}`, - "olm.package": `{"packageName":"badimage","version":"0.1.0"}`, - "olm.gvk": `[]`, - }), - "operatorhub/plain/0.1.0": *input.NewEntity("operatorhub/plain/0.1.0", map[string]string{ - "olm.bundle.path": `"quay.io/operatorhub/plain@sha256:plain"`, - "olm.bundle.channelEntry": `{"name":"plain.0.1.0"}`, - "olm.channel": `{"channelName":"beta","priority":0}`, - "olm.package": `{"packageName":"plain","version":"0.1.0"}`, - "olm.gvk": `[]`, - "olm.bundle.mediatype": `"plain+v0"`, - }), - "operatorhub/badmedia/0.1.0": *input.NewEntity("operatorhub/badmedia/0.1.0", map[string]string{ - "olm.bundle.path": `"quay.io/operatorhub/badmedia@sha256:badmedia"`, - "olm.bundle.channelEntry": `{"name":"badmedia.0.1.0"}`, - "olm.channel": `{"channelName":"beta","priority":0}`, - "olm.package": `{"packageName":"badmedia","version":"0.1.0"}`, - "olm.gvk": `[]`, - "olm.bundle.mediatype": `"badmedia+v1"`, - }), -}) +var betaChannel = catalogmetadata.Channel{Channel: declcfg.Channel{ + Name: "beta", + Entries: []declcfg.ChannelEntry{ + { + Name: "operatorhub/prometheus/0.37.0", + }, + { + Name: "operatorhub/prometheus/0.47.0", + Replaces: "operatorhub/prometheus/0.37.0", + }, + { + Name: "operatorhub/plain/0.1.0", + }, + { + Name: "operatorhub/badmedia/0.1.0", + }, + }, +}} + +var testBundleList = []*catalogmetadata.Bundle{ + {Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/0.37.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"0.37.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, + }, + }, InChannels: []*catalogmetadata.Channel{&betaChannel}}, + {Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/0.47.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"0.47.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, + }, + }, InChannels: []*catalogmetadata.Channel{&betaChannel}}, + {Bundle: declcfg.Bundle{ + Name: "operatorhub/plain/0.1.0", + Package: "plain", + Image: "quay.io/operatorhub/plain@sha256:plain", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"plain","version":"0.1.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, + {Type: "olm.bundle.mediatype", Value: json.RawMessage(`"plain+v0"`)}, + }, + }, InChannels: []*catalogmetadata.Channel{&betaChannel}}, + {Bundle: declcfg.Bundle{ + Name: "operatorhub/badmedia/0.1.0", + Package: "badmedia", + Image: "quay.io/operatorhub/badmedia@sha256:badmedia", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"badmedia","version":"0.1.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, + {Type: "olm.bundle.mediatype", Value: json.RawMessage(`"badmedia+v1"`)}, + }, + }, InChannels: []*catalogmetadata.Channel{&betaChannel}}, +} diff --git a/internal/controllers/variable_source.go b/internal/controllers/variable_source.go index a4a748111..88608cdfb 100644 --- a/internal/controllers/variable_source.go +++ b/internal/controllers/variable_source.go @@ -21,20 +21,19 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/input" - catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -func NewVariableSource(cl client.Client, catalog catalogclient.CatalogClient) variablesources.NestedVariableSource { +func NewVariableSource(cl client.Client, catalogClient variablesources.BundleProvider) variablesources.NestedVariableSource { return variablesources.NestedVariableSource{ func(inputVariableSource input.VariableSource) (input.VariableSource, error) { - return variablesources.NewOperatorVariableSource(cl, catalog, inputVariableSource), nil + return variablesources.NewOperatorVariableSource(cl, catalogClient, inputVariableSource), nil }, func(inputVariableSource input.VariableSource) (input.VariableSource, error) { - return variablesources.NewBundleDeploymentVariableSource(cl, catalog, inputVariableSource), nil + return variablesources.NewBundleDeploymentVariableSource(cl, catalogClient, inputVariableSource), nil }, func(inputVariableSource input.VariableSource) (input.VariableSource, error) { - return variablesources.NewBundlesAndDepsVariableSource(catalog, inputVariableSource), nil + return variablesources.NewBundlesAndDepsVariableSource(catalogClient, inputVariableSource), nil }, func(inputVariableSource input.VariableSource) (input.VariableSource, error) { return variablesources.NewCRDUniquenessConstraintsVariableSource(inputVariableSource), nil diff --git a/internal/resolution/variables/bundle.go b/internal/resolution/variables/bundle.go index 4be11a634..7b0568ffe 100644 --- a/internal/resolution/variables/bundle.go +++ b/internal/resolution/variables/bundle.go @@ -18,7 +18,7 @@ type BundleVariable struct { dependencies []*catalogmetadata.Bundle } -func (b *BundleVariable) BundleEntity() *catalogmetadata.Bundle { +func (b *BundleVariable) Bundle() *catalogmetadata.Bundle { return b.bundle } diff --git a/internal/resolution/variables/bundle_test.go b/internal/resolution/variables/bundle_test.go index 85d15b762..da29e3742 100644 --- a/internal/resolution/variables/bundle_test.go +++ b/internal/resolution/variables/bundle_test.go @@ -1,37 +1,55 @@ package variables_test import ( + "encoding/json" "testing" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/constraint" - "github.com/operator-framework/deppy/pkg/deppy/input" + "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) func TestBundleVariable(t *testing.T) { - bundleEntity := olmentity.NewBundleEntity(input.NewEntity("bundle-1", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })) - dependencies := []*olmentity.BundleEntity{ - olmentity.NewBundleEntity(input.NewEntity("bundle-2", map[string]string{ - property.TypePackage: `{"packageName": "test-package-2", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - olmentity.NewBundleEntity(input.NewEntity("bundle-3", map[string]string{ - property.TypePackage: `{"packageName": "test-package-3", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - } - bv := olmvariables.NewBundleVariable(bundleEntity, dependencies) - - if bv.BundleEntity() != bundleEntity { - t.Errorf("bundle entity '%v' does not match expected '%v'", bv.BundleEntity(), bundleEntity) + bundle := &catalogmetadata.Bundle{ + InChannels: []*catalogmetadata.Channel{ + {Channel: declcfg.Channel{Name: "stable"}}, + }, + Bundle: declcfg.Bundle{Name: "bundle-1", Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, + }}, } + dependencies := []*catalogmetadata.Bundle{ + { + InChannels: []*catalogmetadata.Channel{ + {Channel: declcfg.Channel{Name: "stable"}}, + }, + Bundle: declcfg.Bundle{Name: "bundle-2", Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, + }}, + }, + { + InChannels: []*catalogmetadata.Channel{ + {Channel: declcfg.Channel{Name: "stable"}}, + }, + Bundle: declcfg.Bundle{Name: "bundle-3", Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "3.0.0"}`)}, + }}, + }, + } + ids := olmvariables.BundleToBundleVariableIDs(bundle) + if len(ids) != len(bundle.InChannels) { + t.Fatalf("bundle should produce one variable ID per channel; received: %d", len(bundle.InChannels)) + } + bv := olmvariables.NewBundleVariable(ids[0], bundle, dependencies) + + if bv.Bundle() != bundle { + t.Errorf("bundle '%v' does not match expected '%v'", bv.Bundle(), bundle) + } + for i, d := range bv.Dependencies() { if d != dependencies[i] { t.Errorf("dependency[%v] '%v' does not match expected '%v'", i, d, dependencies[i]) diff --git a/internal/resolution/variables/installed_package.go b/internal/resolution/variables/installed_package.go index 6ab86126c..35bd87a01 100644 --- a/internal/resolution/variables/installed_package.go +++ b/internal/resolution/variables/installed_package.go @@ -17,7 +17,7 @@ type InstalledPackageVariable struct { bundles []*catalogmetadata.Bundle } -func (r *InstalledPackageVariable) BundleEntities() []*catalogmetadata.Bundle { +func (r *InstalledPackageVariable) Bundles() []*catalogmetadata.Bundle { return r.bundles } diff --git a/internal/resolution/variables/installed_package_test.go b/internal/resolution/variables/installed_package_test.go index edcb9f5a8..850783a6c 100644 --- a/internal/resolution/variables/installed_package_test.go +++ b/internal/resolution/variables/installed_package_test.go @@ -1,43 +1,42 @@ package variables_test import ( + "encoding/json" "fmt" "testing" "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" + "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) func TestInstalledPackageVariable(t *testing.T) { packageName := "test-package" - bundleEntities := []*olmentity.BundleEntity{ - olmentity.NewBundleEntity(input.NewEntity("bundle-1", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - olmentity.NewBundleEntity(input.NewEntity("bundle-2", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - olmentity.NewBundleEntity(input.NewEntity("bundle-3", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "3.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), + + bundles := []*catalogmetadata.Bundle{ + {Bundle: declcfg.Bundle{Name: "bundle-1", Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, + }}}, + {Bundle: declcfg.Bundle{Name: "bundle-2", Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, + }}}, + {Bundle: declcfg.Bundle{Name: "bundle-3", Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "3.0.0"}`)}, + }}}, } - ipv := olmvariables.NewInstalledPackageVariable(packageName, bundleEntities) + ipv := olmvariables.NewInstalledPackageVariable(packageName, bundles) id := deppy.IdentifierFromString(fmt.Sprintf("installed package %s", packageName)) if ipv.Identifier() != id { t.Errorf("package name '%v' does not match expected '%v'", ipv.Identifier(), id) } - for i, e := range ipv.BundleEntities() { - if e != bundleEntities[i] { - t.Errorf("bundle entity[%v] '%v' does not match expected '%v'", i, e, bundleEntities[i]) + for i, e := range ipv.Bundles() { + if e != bundles[i] { + t.Errorf("bundle[%v] '%v' does not match expected '%v'", i, e, bundles[i]) } } } diff --git a/internal/resolution/variables/required_package.go b/internal/resolution/variables/required_package.go index 3aff97743..368d09c6c 100644 --- a/internal/resolution/variables/required_package.go +++ b/internal/resolution/variables/required_package.go @@ -17,7 +17,7 @@ type RequiredPackageVariable struct { bundles []*catalogmetadata.Bundle } -func (r *RequiredPackageVariable) BundleEntities() []*catalogmetadata.Bundle { +func (r *RequiredPackageVariable) Bundles() []*catalogmetadata.Bundle { return r.bundles } diff --git a/internal/resolution/variables/required_package_test.go b/internal/resolution/variables/required_package_test.go index a0872fa6f..d8ece1473 100644 --- a/internal/resolution/variables/required_package_test.go +++ b/internal/resolution/variables/required_package_test.go @@ -1,43 +1,41 @@ package variables_test import ( + "encoding/json" "fmt" "testing" "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" + "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" ) func TestRequiredPackageVariable(t *testing.T) { packageName := "test-package" - bundleEntities := []*olmentity.BundleEntity{ - olmentity.NewBundleEntity(input.NewEntity("bundle-1", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - olmentity.NewBundleEntity(input.NewEntity("bundle-2", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - olmentity.NewBundleEntity(input.NewEntity("bundle-3", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "3.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), + bundles := []*catalogmetadata.Bundle{ + {Bundle: declcfg.Bundle{Name: "bundle-1", Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, + }}}, + {Bundle: declcfg.Bundle{Name: "bundle-2", Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, + }}}, + {Bundle: declcfg.Bundle{Name: "bundle-3", Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "3.0.0"}`)}, + }}}, } - rpv := olmvariables.NewRequiredPackageVariable(packageName, bundleEntities) + rpv := olmvariables.NewRequiredPackageVariable(packageName, bundles) id := deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName)) if rpv.Identifier() != id { t.Errorf("package name '%v' does not match expected '%v'", rpv.Identifier(), id) } - for i, e := range rpv.BundleEntities() { - if e != bundleEntities[i] { - t.Errorf("bundle entity[%v] '%v' does not match expected '%v'", i, e, bundleEntities[i]) + for i, e := range rpv.Bundles() { + if e != bundles[i] { + t.Errorf("bundle entity[%v] '%v' does not match expected '%v'", i, e, bundles[i]) } } diff --git a/internal/resolution/variablesources/bundle_deployment.go b/internal/resolution/variablesources/bundle_deployment.go index 5ce673298..25c287087 100644 --- a/internal/resolution/variablesources/bundle_deployment.go +++ b/internal/resolution/variablesources/bundle_deployment.go @@ -7,22 +7,20 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/input" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" "sigs.k8s.io/controller-runtime/pkg/client" - - catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" ) var _ input.VariableSource = &BundleDeploymentVariableSource{} type BundleDeploymentVariableSource struct { client client.Client - catalog catalogclient.CatalogClient + catalogClient BundleProvider inputVariableSource input.VariableSource } -func NewBundleDeploymentVariableSource(cl client.Client, catalog catalogclient.CatalogClient, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource { +func NewBundleDeploymentVariableSource(cl client.Client, catalogClient BundleProvider, inputVariableSource input.VariableSource) *BundleDeploymentVariableSource { return &BundleDeploymentVariableSource{ client: cl, - catalog: catalog, + catalogClient: catalogClient, inputVariableSource: inputVariableSource, } } @@ -46,7 +44,7 @@ func (o *BundleDeploymentVariableSource) GetVariables(ctx context.Context) ([]de continue } processed[sourceImage.Ref] = struct{}{} - ips, err := NewInstalledPackageVariableSource(o.catalog, bundleDeployment.Spec.Template.Spec.Source.Image.Ref) + ips, err := NewInstalledPackageVariableSource(o.catalogClient, bundleDeployment.Spec.Template.Spec.Source.Image.Ref) if err != nil { return nil, err } diff --git a/internal/resolution/variablesources/bundle_deployment_test.go b/internal/resolution/variablesources/bundle_deployment_test.go index 3eb82321a..0e640210e 100644 --- a/internal/resolution/variablesources/bundle_deployment_test.go +++ b/internal/resolution/variablesources/bundle_deployment_test.go @@ -2,12 +2,18 @@ package variablesources_test import ( "context" + "encoding/json" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/alpha/property" + + "github.com/operator-framework/operator-controller/internal/catalogmetadata" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" + testutil "github.com/operator-framework/operator-controller/test/util" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -16,7 +22,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" ) @@ -47,42 +52,74 @@ func bundleDeployment(name, image string) *rukpakv1alpha1.BundleDeployment { } } -var BundleDeploymentTestEntityCache = map[deppy.Identifier]input.Entity{ - "operatorhub/prometheus/0.37.0": *input.NewEntity("operatorhub/prometheus/0.37.0", map[string]string{ - "olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"`, - "olm.bundle.channelEntry": "{\"name\":\"prometheus.0.37.0\"}", - "olm.channel": "{\"channelName\":\"beta\",\"priority\":0,\"replaces\":\"prometheusoperator.0.32.0\"}", - "olm.gvk": "[{\"group\":\"monitoring.coreos.com\",\"kind\":\"Alertmanager\",\"version\":\"v1\"}, {\"group\":\"monitoring.coreos.com\",\"kind\":\"Prometheus\",\"version\":\"v1\"}]", - "olm.package": "{\"packageName\":\"prometheus\",\"version\":\"0.37.0\"}", - }), - "operatorhub/prometheus/0.47.0": *input.NewEntity("operatorhub/prometheus/0.47.0", map[string]string{ - "olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed"`, - "olm.bundle.channelEntry": "{\"replaces\":\"prometheus.0.37.0\", \"name\":\"prometheus.0.47.0\"}", - "olm.channel": "{\"channelName\":\"beta\",\"priority\":0,\"replaces\":\"prometheusoperator.0.37.0\"}", - "olm.gvk": "[{\"group\":\"monitoring.coreos.com\",\"kind\":\"Alertmanager\",\"version\":\"v1\"}, {\"group\":\"monitoring.coreos.com\",\"kind\":\"Prometheus\",\"version\":\"v1alpha1\"}]", - "olm.package": "{\"packageName\":\"prometheus\",\"version\":\"0.47.0\"}", - }), - "operatorhub/packageA/2.0.0": *input.NewEntity("operatorhub/packageA/2.0.0", map[string]string{ - "olm.bundle.path": `"foo.io/packageA/packageA:v2.0.0"`, - "olm.bundle.channelEntry": "{\"name\":\"packageA.2.0.0\"}", - "olm.channel": "{\"channelName\":\"stable\",\"priority\":0}", - "olm.gvk": "[{\"group\":\"foo.io\",\"kind\":\"Foo\",\"version\":\"v1\"}]", - "olm.package": "{\"packageName\":\"packageA\",\"version\":\"2.0.0\"}", - }), -} - var _ = Describe("BundleDeploymentVariableSource", func() { - var bundleTestEntityCache input.EntitySource + var fakeCatalogClient testutil.FakeCatalogClient + var betaChannel catalogmetadata.Channel + var stableChannel catalogmetadata.Channel + var testBundleList []*catalogmetadata.Bundle BeforeEach(func() { - bundleTestEntityCache = input.NewCacheQuerier(BundleDeploymentTestEntityCache) + betaChannel = catalogmetadata.Channel{Channel: declcfg.Channel{ + Name: "beta", + Entries: []declcfg.ChannelEntry{ + { + Name: "operatorhub/prometheus/0.37.0", + Replaces: "operatorhub/prometheus/0.32.0", + }, + { + Name: "operatorhub/prometheus/0.47.0", + Replaces: "operatorhub/prometheus/0.37.0", + }, + }, + }} + + stableChannel = catalogmetadata.Channel{Channel: declcfg.Channel{ + Name: "beta", + Entries: []declcfg.ChannelEntry{ + { + Name: "operatorhub/packageA/2.0.0", + }, + }, + }} + + testBundleList = []*catalogmetadata.Bundle{ + {Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/0.37.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"0.37.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"monitoring.coreos.com","kind":"Alertmanager","version":"v1"}, {"group":"monitoring.coreos.com","kind":"Prometheus","version":"v1"}]`)}, + }, + }, InChannels: []*catalogmetadata.Channel{&betaChannel}}, + {Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/0.47.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"0.47.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"monitoring.coreos.com","kind":"Alertmanager","version":"v1"}, {"group":"monitoring.coreos.com","kind":"Prometheus","version":"v1alpha1"}]`)}, + }, + }, InChannels: []*catalogmetadata.Channel{&betaChannel}}, + {Bundle: declcfg.Bundle{ + Name: "operatorhub/packageA/2.0.0", + Package: "packageA", + Image: "foo.io/packageA/packageA:v2.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"packageA","version":"2.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + }, + }, InChannels: []*catalogmetadata.Channel{&stableChannel}}, + } + + fakeCatalogClient = testutil.NewFakeCatalogClient(testBundleList) }) It("should produce RequiredPackage variables", func() { cl := BundleDeploymentFakeClient(bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35")) - bdVariableSource := variablesources.NewBundleDeploymentVariableSource(cl, &MockRequiredPackageSource{}) - variables, err := bdVariableSource.GetVariables(context.Background(), bundleTestEntityCache) + bdVariableSource := variablesources.NewBundleDeploymentVariableSource(cl, &fakeCatalogClient, &MockRequiredPackageSource{}) + variables, err := bdVariableSource.GetVariables(context.Background()) Expect(err).ToNot(HaveOccurred()) installedPackageVariable := filterVariables[*olmvariables.InstalledPackageVariable](variables) @@ -90,7 +127,7 @@ var _ = Describe("BundleDeploymentVariableSource", func() { Expect(installedPackageVariable).To(WithTransform(func(bvars []*olmvariables.InstalledPackageVariable) map[deppy.Identifier]int { out := map[deppy.Identifier]int{} for _, variable := range bvars { - out[variable.Identifier()] = len(variable.BundleEntities()) + out[variable.Identifier()] = len(variable.Bundles()) } return out }, Equal(map[deppy.Identifier]int{ @@ -102,8 +139,8 @@ var _ = Describe("BundleDeploymentVariableSource", func() { It("should return an error if the bundleDeployment image doesn't match any operator resource", func() { cl := BundleDeploymentFakeClient(bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:nonexistent")) - bdVariableSource := variablesources.NewBundleDeploymentVariableSource(cl, &MockRequiredPackageSource{}) - _, err := bdVariableSource.GetVariables(context.Background(), bundleTestEntityCache) + bdVariableSource := variablesources.NewBundleDeploymentVariableSource(cl, &fakeCatalogClient, &MockRequiredPackageSource{}) + _, err := bdVariableSource.GetVariables(context.Background()) Expect(err.Error()).To(Equal("bundleImage \"quay.io/operatorhubio/prometheus@sha256:nonexistent\" not found")) }) }) diff --git a/internal/resolution/variablesources/bundle_provider.go b/internal/resolution/variablesources/bundle_provider.go new file mode 100644 index 000000000..c6517069a --- /dev/null +++ b/internal/resolution/variablesources/bundle_provider.go @@ -0,0 +1,14 @@ +package variablesources + +import ( + "context" + + "github.com/operator-framework/operator-controller/internal/catalogmetadata" +) + +// BundleProvider provides the Bundles method through which we can retrieve +// a list of Bundles from any source, generally from a catalog client of +// some kind. +type BundleProvider interface { + Bundles(ctx context.Context) ([]*catalogmetadata.Bundle, error) +} diff --git a/internal/resolution/variablesources/bundles_and_dependencies.go b/internal/resolution/variablesources/bundles_and_dependencies.go index 5f3d6d136..6b8dcd782 100644 --- a/internal/resolution/variablesources/bundles_and_dependencies.go +++ b/internal/resolution/variablesources/bundles_and_dependencies.go @@ -9,7 +9,6 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/input" "github.com/operator-framework/operator-controller/internal/catalogmetadata" - catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" @@ -18,13 +17,13 @@ import ( var _ input.VariableSource = &BundlesAndDepsVariableSource{} type BundlesAndDepsVariableSource struct { - catalog catalogclient.CatalogClient + catalogClient BundleProvider variableSources []input.VariableSource } -func NewBundlesAndDepsVariableSource(catalog catalogclient.CatalogClient, inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource { +func NewBundlesAndDepsVariableSource(catalogClient BundleProvider, inputVariableSources ...input.VariableSource) *BundlesAndDepsVariableSource { return &BundlesAndDepsVariableSource{ - catalog: catalog, + catalogClient: catalogClient, variableSources: inputVariableSources, } } @@ -46,13 +45,13 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]depp for _, variable := range variables { switch v := variable.(type) { case *olmvariables.RequiredPackageVariable: - bundleQueue = append(bundleQueue, v.BundleEntities()...) + bundleQueue = append(bundleQueue, v.Bundles()...) case *olmvariables.InstalledPackageVariable: - bundleQueue = append(bundleQueue, v.BundleEntities()...) + bundleQueue = append(bundleQueue, v.Bundles()...) } } - allBundles, err := b.catalog.Bundles(ctx) + allBundles, err := b.catalogClient.Bundles(ctx) if err != nil { return nil, err } @@ -74,7 +73,7 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]depp // get bundle dependencies dependencies, err := b.filterBundleDependencies(allBundles, head) if err != nil { - return nil, fmt.Errorf("could not determine dependencies for entity with id '%s': %w", id, err) + return nil, fmt.Errorf("could not determine dependencies for bundle with id '%s': %w", id, err) } // add bundle dependencies to queue for processing @@ -96,7 +95,7 @@ func (b *BundlesAndDepsVariableSource) filterBundleDependencies(allBundles []*ca // todo(perdasilva): disambiguate between not found and actual errors requiredPackages, _ := bundle.RequiredPackages() for _, requiredPackage := range requiredPackages { - packageDependencyBundles := catalogfilter.Filter(allBundles, catalogfilter.And(catalogfilter.WithPackageName(requiredPackage.PackageName), catalogfilter.InBlangSemverRange(*requiredPackage.SemverRange))) + packageDependencyBundles := catalogfilter.Filter(allBundles, catalogfilter.And(catalogfilter.WithPackageName(requiredPackage.PackageName), catalogfilter.InBlangSemverRange(requiredPackage.SemverRange))) if len(packageDependencyBundles) == 0 { return nil, fmt.Errorf("could not find package dependencies for bundle '%s'", bundle.Name) } diff --git a/internal/resolution/variablesources/bundles_and_dependencies_test.go b/internal/resolution/variablesources/bundles_and_dependencies_test.go index a7ed680d1..3c0db1c9d 100644 --- a/internal/resolution/variablesources/bundles_and_dependencies_test.go +++ b/internal/resolution/variablesources/bundles_and_dependencies_test.go @@ -2,155 +2,307 @@ package variablesources_test import ( "context" + "encoding/json" "errors" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" + "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" + testutil "github.com/operator-framework/operator-controller/test/util" ) var _ = Describe("BundlesAndDepsVariableSource", func() { var ( - bdvs *variablesources.BundlesAndDepsVariableSource - mockEntitySource input.EntitySource + bdvs *variablesources.BundlesAndDepsVariableSource + testBundleList []*catalogmetadata.Bundle + fakeCatalogClient testutil.FakeCatalogClient ) BeforeEach(func() { + channel := catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}} + testBundleList = []*catalogmetadata.Bundle{ + // required package bundles + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-1", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, + {Type: property.TypeGVKRequired, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-2", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, + {Type: property.TypeGVKRequired, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + {Type: property.TypePackageRequired, Value: json.RawMessage(`[{"packageName": "some-package", "versionRange": ">=1.0.0 <2.0.0"}]`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + + // dependencies + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-4", + Package: "some-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-package", "version": "1.0.0"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-5", + Package: "some-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-package", "version": "1.5.0"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-6", + Package: "some-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-package", "version": "2.0.0"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-7", + Package: "some-other-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-other-package", "version": "1.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-8", + Package: "some-other-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-other-package", "version": "1.5.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + {Type: property.TypeGVKRequired, Value: json.RawMessage(`[{"group":"bar.io","kind":"Bar","version":"v1"}]`)}, + {Type: property.TypePackageRequired, Value: json.RawMessage(`[{"packageName": "another-package", "versionRange": "< 2.0.0"}]`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + + // dependencies of dependencies + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-9", Package: "another-package", Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "another-package", "version": "1.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-10", + Package: "bar-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "bar-package", "version": "1.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"bar.io","kind":"Bar","version":"v1"}]`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-11", + Package: "bar-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "bar-package", "version": "2.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"bar.io","kind":"Bar","version":"v1"}]`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + + // test-package-2 required package - no dependencies + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-15", + Package: "test-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "1.5.0"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-16", + Package: "test-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "2.0.1"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-17", + Package: "test-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "3.16.0"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + + // completely unrelated + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-12", + Package: "unrelated-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "unrelated-package", "version": "2.0.0"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-13", + Package: "unrelated-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "unrelated-package-2", "version": "2.0.0"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-14", + Package: "unrelated-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "unrelated-package-2", "version": "3.0.0"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + } + fakeCatalogClient = testutil.NewFakeCatalogClient(testBundleList) bdvs = variablesources.NewBundlesAndDepsVariableSource( + &fakeCatalogClient, &MockRequiredPackageSource{ ResultSet: []deppy.Variable{ - // must match data in mockEntitySource - olmvariables.NewRequiredPackageVariable("test-package", []*olmentity.BundleEntity{ - olmentity.NewBundleEntity(input.NewEntity("bundle-2", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVKRequired: `[{"group":"foo.io","kind":"Foo","version":"v1"}]`, - property.TypePackageRequired: `[{"packageName": "some-package", "versionRange": ">=1.0.0 <2.0.0"}]`, - })), - olmentity.NewBundleEntity(input.NewEntity("bundle-1", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVKRequired: `[{"group":"foo.io","kind":"Foo","version":"v1"}]`, - })), + // must match data in fakeCatalogClient + olmvariables.NewRequiredPackageVariable("test-package", []*catalogmetadata.Bundle{ + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-2", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, + {Type: property.TypeGVKRequired, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + {Type: property.TypePackageRequired, Value: json.RawMessage(`[{"packageName": "some-package", "versionRange": ">=1.0.0 <2.0.0"}]`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-1", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, + {Type: property.TypeGVKRequired, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, }), }, }, &MockRequiredPackageSource{ ResultSet: []deppy.Variable{ - // must match data in mockEntitySource - olmvariables.NewRequiredPackageVariable("test-package-2", []*olmentity.BundleEntity{ + // must match data in fakeCatalogClient + olmvariables.NewRequiredPackageVariable("test-package-2", []*catalogmetadata.Bundle{ // test-package-2 required package - no dependencies - olmentity.NewBundleEntity(input.NewEntity("bundle-15", map[string]string{ - property.TypePackage: `{"packageName": "test-package-2", "version": "1.5.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - olmentity.NewBundleEntity(input.NewEntity("bundle-16", map[string]string{ - property.TypePackage: `{"packageName": "test-package-2", "version": "2.0.1"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - olmentity.NewBundleEntity(input.NewEntity("bundle-17", map[string]string{ - property.TypePackage: `{"packageName": "test-package-2", "version": "3.16.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-15", + Package: "test-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "1.5.0"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-16", + Package: "test-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "2.0.1"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-17", + Package: "test-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "3.16.0"}`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{&channel}, + }, }), }, }, ) - mockEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{ - // required package bundles - "bundle-1": *input.NewEntity("bundle-1", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVKRequired: `[{"group":"foo.io","kind":"Foo","version":"v1"}]`, - }), - "bundle-2": *input.NewEntity("bundle-2", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVKRequired: `[{"group":"foo.io","kind":"Foo","version":"v1"}]`, - property.TypePackageRequired: `[{"packageName": "some-package", "versionRange": ">=1.0.0 <2.0.0"}]`, - }), - - // dependencies - "bundle-4": *input.NewEntity("bundle-4", map[string]string{ - property.TypePackage: `{"packageName": "some-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - "bundle-5": *input.NewEntity("bundle-5", map[string]string{ - property.TypePackage: `{"packageName": "some-package", "version": "1.5.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - "bundle-6": *input.NewEntity("bundle-6", map[string]string{ - property.TypePackage: `{"packageName": "some-package", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - "bundle-7": *input.NewEntity("bundle-7", map[string]string{ - property.TypePackage: `{"packageName": "some-other-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"foo.io","kind":"Foo","version":"v1"}]`, - }), - "bundle-8": *input.NewEntity("bundle-8", map[string]string{ - property.TypePackage: `{"packageName": "some-other-package", "version": "1.5.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"foo.io","kind":"Foo","version":"v1"}]`, - property.TypeGVKRequired: `[{"group":"bar.io","kind":"Bar","version":"v1"}]`, - property.TypePackageRequired: `[{"packageName": "another-package", "versionRange": "< 2.0.0"}]`, - }), - - // dependencies of dependencies - "bundle-9": *input.NewEntity("bundle-9", map[string]string{ - property.TypePackage: `{"packageName": "another-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"foo.io","kind":"Foo","version":"v1"}]`, - }), - "bundle-10": *input.NewEntity("bundle-10", map[string]string{ - property.TypePackage: `{"packageName": "bar-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"bar.io","kind":"Bar","version":"v1"}]`, - }), - "bundle-11": *input.NewEntity("bundle-11", map[string]string{ - property.TypePackage: `{"packageName": "bar-package", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"bar.io","kind":"Bar","version":"v1"}]`, - }), - - // test-package-2 required package - no dependencies - "bundle-15": *input.NewEntity("bundle-15", map[string]string{ - property.TypePackage: `{"packageName": "test-package-2", "version": "1.5.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - "bundle-16": *input.NewEntity("bundle-16", map[string]string{ - property.TypePackage: `{"packageName": "test-package-2", "version": "2.0.1"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - "bundle-17": *input.NewEntity("bundle-17", map[string]string{ - property.TypePackage: `{"packageName": "test-package-2", "version": "3.16.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - - // completely unrelated - "bundle-12": *input.NewEntity("bundle-12", map[string]string{ - property.TypePackage: `{"packageName": "unrelated-package", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - "bundle-13": *input.NewEntity("bundle-13", map[string]string{ - property.TypePackage: `{"packageName": "unrelated-package-2", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - "bundle-14": *input.NewEntity("bundle-14", map[string]string{ - property.TypePackage: `{"packageName": "unrelated-package-2", "version": "3.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - }) }) It("should return bundle variables with correct dependencies", func() { - variables, err := bdvs.GetVariables(context.TODO(), mockEntitySource) + variables, err := bdvs.GetVariables(context.TODO()) Expect(err).NotTo(HaveOccurred()) var bundleVariables []*olmvariables.BundleVariable @@ -163,37 +315,74 @@ var _ = Describe("BundlesAndDepsVariableSource", func() { // Note: When accounting for Required GVKs (currently not in use), we would expect additional // dependencies (bundles 7, 8, 9, 10, 11) to appear here due to their GVKs being required by // some of the packages. - Expect(bundleVariables).To(WithTransform(CollectBundleVariableIDs, Equal([]string{"bundle-2", "bundle-1", "bundle-15", "bundle-16", "bundle-17", "bundle-5", "bundle-4"}))) + Expect(bundleVariables).To(WithTransform(CollectBundleVariableIDs, Equal([]string{ + "fake-catalog-test-package-stable-bundle-2", + "fake-catalog-test-package-stable-bundle-1", + "fake-catalog-test-package-2-stable-bundle-15", + "fake-catalog-test-package-2-stable-bundle-16", + "fake-catalog-test-package-2-stable-bundle-17", + "fake-catalog-some-package-stable-bundle-5", + "fake-catalog-some-package-stable-bundle-4", + }))) // check dependencies for one of the bundles - bundle2 := VariableWithID("bundle-2")(bundleVariables) + bundle2 := VariableWithName("bundle-2")(bundleVariables) // Note: As above, bundle-2 has GVK requirements satisfied by bundles 7, 8, and 9, but they // will not appear in this list as we are not currently taking Required GVKs into account - Expect(bundle2.Dependencies()).To(WithTransform(CollectDeppyEntities, Equal([]*input.Entity{ - input.NewEntity("bundle-5", map[string]string{ - property.TypePackage: `{"packageName": "some-package", "version": "1.5.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - input.NewEntity("bundle-4", map[string]string{ - property.TypePackage: `{"packageName": "some-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - }))) + Expect(bundle2.Dependencies()).To(HaveLen(2)) + Expect(bundle2.Dependencies()[0].Name).To(Equal("bundle-5")) + Expect(bundle2.Dependencies()[1].Name).To(Equal("bundle-4")) }) It("should return error if dependencies not found", func() { - mockEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{}) - _, err := bdvs.GetVariables(context.TODO(), mockEntitySource) + emptyCatalogClient := testutil.NewFakeCatalogClient(make([]*catalogmetadata.Bundle, 0)) + + bdvs = variablesources.NewBundlesAndDepsVariableSource( + &emptyCatalogClient, + &MockRequiredPackageSource{ + ResultSet: []deppy.Variable{ + // must match data in fakeCatalogClient + olmvariables.NewRequiredPackageVariable("test-package", []*catalogmetadata.Bundle{ + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-2", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, + {Type: property.TypeGVKRequired, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + {Type: property.TypePackageRequired, Value: json.RawMessage(`[{"packageName": "some-package", "versionRange": ">=1.0.0 <2.0.0"}]`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{{Channel: declcfg.Channel{Name: "stable"}}}, + }, + { + CatalogName: "fake-catalog", + Bundle: declcfg.Bundle{ + Name: "bundle-1", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, + {Type: property.TypeGVKRequired, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + }, + }, + InChannels: []*catalogmetadata.Channel{{Channel: declcfg.Channel{Name: "stable"}}}, + }, + }), + }, + }, + ) + _, err := bdvs.GetVariables(context.TODO()) Expect(err).To(HaveOccurred()) - Expect(err).To(MatchError("could not determine dependencies for entity with id 'bundle-2': could not find package dependencies for bundle 'bundle-2'")) + Expect(err.Error()).To(ContainSubstring("could not determine dependencies for bundle with id 'fake-catalog-test-package-stable-bundle-2': could not find package dependencies for bundle 'bundle-2'")) }) It("should return error if an inner variable source returns an error", func() { bdvs = variablesources.NewBundlesAndDepsVariableSource( + &fakeCatalogClient, &MockRequiredPackageSource{Error: errors.New("fake error")}, ) - mockEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{}) - _, err := bdvs.GetVariables(context.TODO(), mockEntitySource) + _, err := bdvs.GetVariables(context.TODO()) Expect(err).To(HaveOccurred()) Expect(err).To(MatchError("fake error")) }) @@ -204,14 +393,14 @@ type MockRequiredPackageSource struct { Error error } -func (m *MockRequiredPackageSource) GetVariables(_ context.Context, _ input.EntitySource) ([]deppy.Variable, error) { +func (m *MockRequiredPackageSource) GetVariables(_ context.Context) ([]deppy.Variable, error) { return m.ResultSet, m.Error } -func VariableWithID(id deppy.Identifier) func(vars []*olmvariables.BundleVariable) *olmvariables.BundleVariable { +func VariableWithName(name string) func(vars []*olmvariables.BundleVariable) *olmvariables.BundleVariable { return func(vars []*olmvariables.BundleVariable) *olmvariables.BundleVariable { for i := 0; i < len(vars); i++ { - if vars[i].Identifier() == id { + if vars[i].Bundle().Name == name { return vars[i] } } @@ -226,11 +415,3 @@ func CollectBundleVariableIDs(vars []*olmvariables.BundleVariable) []string { } return ids } - -func CollectDeppyEntities(vars []*olmentity.BundleEntity) []*input.Entity { - entities := make([]*input.Entity, 0, len(vars)) - for _, v := range vars { - entities = append(entities, v.Entity) - } - return entities -} diff --git a/internal/resolution/variablesources/crd_constraints.go b/internal/resolution/variablesources/crd_constraints.go index 2108a6317..746ed14fa 100644 --- a/internal/resolution/variablesources/crd_constraints.go +++ b/internal/resolution/variablesources/crd_constraints.go @@ -47,7 +47,7 @@ func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Contex for _, variable := range variables { switch v := variable.(type) { case *olmvariables.BundleVariable: - bundles := []*catalogmetadata.Bundle{v.BundleEntity()} + bundles := []*catalogmetadata.Bundle{v.Bundle()} bundles = append(bundles, v.Dependencies()...) for _, bundle := range bundles { for _, id := range olmvariables.BundleToBundleVariableIDs(bundle) { diff --git a/internal/resolution/variablesources/crd_constraints_test.go b/internal/resolution/variablesources/crd_constraints_test.go index d88489666..200d673f8 100644 --- a/internal/resolution/variablesources/crd_constraints_test.go +++ b/internal/resolution/variablesources/crd_constraints_test.go @@ -2,115 +2,181 @@ package variablesources_test import ( "context" + "encoding/json" "fmt" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" + "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) -var bundleSet = map[deppy.Identifier]*input.Entity{ +var channel = catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}} +var bundleSet = map[string]*catalogmetadata.Bundle{ // required package bundles - "bundle-1": input.NewEntity("bundle-1", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVKRequired: `[{"group":"foo.io","kind":"Foo","version":"v1"}]`, - property.TypeGVK: `[{"group":"bit.io","kind":"Bit","version":"v1"}]`, - }), - "bundle-2": input.NewEntity("bundle-2", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVKRequired: `[{"group":"foo.io","kind":"Foo","version":"v1"}]`, - property.TypePackageRequired: `[{"packageName": "some-package", "versionRange": ">=1.0.0 <2.0.0"}]`, - property.TypeGVK: `[{"group":"bit.io","kind":"Bit","version":"v1"}]`, - }), + "bundle-1": {Bundle: declcfg.Bundle{ + Name: "bundle-1", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, + {Type: property.TypeGVKRequired, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"bit.io","kind":"Bit","version":"v1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + "bundle-2": {Bundle: declcfg.Bundle{ + Name: "bundle-2", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, + {Type: property.TypeGVKRequired, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + {Type: property.TypePackageRequired, Value: json.RawMessage(`[{"packageName": "some-package", "versionRange": ">=1.0.0 <2.0.0"}]`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"bit.io","kind":"Bit","version":"v1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, // dependencies - "bundle-3": input.NewEntity("bundle-3", map[string]string{ - property.TypePackage: `{"packageName": "some-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"fiz.io","kind":"Fiz","version":"v1"}]`, - }), - "bundle-4": input.NewEntity("bundle-4", map[string]string{ - property.TypePackage: `{"packageName": "some-package", "version": "1.5.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"fiz.io","kind":"Fiz","version":"v1"}]`, - }), - "bundle-5": input.NewEntity("bundle-5", map[string]string{ - property.TypePackage: `{"packageName": "some-package", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"fiz.io","kind":"Fiz","version":"v1"}]`, - }), - "bundle-6": input.NewEntity("bundle-6", map[string]string{ - property.TypePackage: `{"packageName": "some-other-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"foo.io","kind":"Foo","version":"v1"}]`, - }), - "bundle-7": input.NewEntity("bundle-7", map[string]string{ - property.TypePackage: `{"packageName": "some-other-package", "version": "1.5.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"foo.io","kind":"Foo","version":"v1"}]`, - property.TypeGVKRequired: `[{"group":"bar.io","kind":"Bar","version":"v1"}]`, - property.TypePackageRequired: `[{"packageName": "another-package", "versionRange": "< 2.0.0"}]`, - }), + "bundle-3": {Bundle: declcfg.Bundle{ + Name: "bundle-3", + Package: "some-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-package", "version": "1.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"fiz.io","kind":"Fiz","version":"v1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + "bundle-4": {Bundle: declcfg.Bundle{ + Name: "bundle-4", + Package: "some-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-package", "version": "1.5.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"fiz.io","kind":"Fiz","version":"v1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + "bundle-5": {Bundle: declcfg.Bundle{ + Name: "bundle-5", + Package: "some-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-package", "version": "2.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"fiz.io","kind":"Fiz","version":"v1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + "bundle-6": {Bundle: declcfg.Bundle{ + Name: "bundle-6", + Package: "some-other-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-other-package", "version": "1.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + "bundle-7": {Bundle: declcfg.Bundle{ + Name: "bundle-7", + Package: "some-other-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "some-other-package", "version": "1.5.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + {Type: property.TypeGVKRequired, Value: json.RawMessage(`[{"group":"bar.io","kind":"Bar","version":"v1"}]`)}, + {Type: property.TypePackageRequired, Value: json.RawMessage(`[{"packageName": "another-package", "versionRange": "< 2.0.0"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, // dependencies of dependencies - "bundle-8": input.NewEntity("bundle-8", map[string]string{ - property.TypePackage: `{"packageName": "another-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"foo.io","kind":"Foo","version":"v1"}]`, - }), - "bundle-9": input.NewEntity("bundle-9", map[string]string{ - property.TypePackage: `{"packageName": "bar-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"bar.io","kind":"Bar","version":"v1"}]`, - }), - "bundle-10": input.NewEntity("bundle-10", map[string]string{ - property.TypePackage: `{"packageName": "bar-package", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"bar.io","kind":"Bar","version":"v1"}]`, - }), + "bundle-8": {Bundle: declcfg.Bundle{ + Name: "bundle-8", + Package: "another-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "another-package", "version": "1.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + "bundle-9": {Bundle: declcfg.Bundle{ + Name: "bundle-9", + Package: "bar-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "bar-package", "version": "1.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"bar.io","kind":"Bar","version":"v1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + "bundle-10": {Bundle: declcfg.Bundle{ + Name: "bundle-10", + Package: "bar-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "bar-package", "version": "2.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"bar.io","kind":"Bar","version":"v1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, // test-package-2 required package - no dependencies - "bundle-14": input.NewEntity("bundle-14", map[string]string{ - property.TypePackage: `{"packageName": "test-package-2", "version": "1.5.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"buz.io","kind":"Buz","version":"v1"}]`, - }), - "bundle-15": input.NewEntity("bundle-15", map[string]string{ - property.TypePackage: `{"packageName": "test-package-2", "version": "2.0.1"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"buz.io","kind":"Buz","version":"v1"}]`, - }), - "bundle-16": input.NewEntity("bundle-16", map[string]string{ - property.TypePackage: `{"packageName": "test-package-2", "version": "3.16.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"buz.io","kind":"Buz","version":"v1"}]`, - }), + "bundle-14": {Bundle: declcfg.Bundle{ + Name: "bundle-14", + Package: "test-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "1.5.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"buz.io","kind":"Buz","version":"v1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + "bundle-15": {Bundle: declcfg.Bundle{ + Name: "bundle-15", + Package: "test-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "2.0.1"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"buz.io","kind":"Buz","version":"v1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + "bundle-16": {Bundle: declcfg.Bundle{ + Name: "bundle-16", + Package: "test-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "3.16.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"buz.io","kind":"Buz","version":"v1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, // completely unrelated - "bundle-11": input.NewEntity("bundle-11", map[string]string{ - property.TypePackage: `{"packageName": "unrelated-package", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"buz.io","kind":"Buz","version":"v1alpha1"}]`, - }), - "bundle-12": input.NewEntity("bundle-12", map[string]string{ - property.TypePackage: `{"packageName": "unrelated-package-2", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"buz.io","kind":"Buz","version":"v1alpha1"}]`, - }), - "bundle-13": input.NewEntity("bundle-13", map[string]string{ - property.TypePackage: `{"packageName": "unrelated-package-2", "version": "3.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - property.TypeGVK: `[{"group":"buz.io","kind":"Buz","version":"v1alpha1"}]`, - }), + "bundle-11": {Bundle: declcfg.Bundle{ + Name: "bundle-11", + Package: "unrelated-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "unrelated-package", "version": "2.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"buz.io","kind":"Buz","version":"v1alpha1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + "bundle-12": {Bundle: declcfg.Bundle{ + Name: "bundle-12", + Package: "unrelated-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "unrelated-package-2", "version": "2.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"buz.io","kind":"Buz","version":"v1alpha1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + "bundle-13": {Bundle: declcfg.Bundle{ + Name: "bundle-13", + Package: "unrelated-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "unrelated-package-2", "version": "3.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"buz.io","kind":"Buz","version":"v1alpha1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, } var _ = Describe("CRDUniquenessConstraintsVariableSource", func() { @@ -118,97 +184,93 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() { inputVariableSource *MockInputVariableSource crdConstraintVariableSource *variablesources.CRDUniquenessConstraintsVariableSource ctx context.Context - entitySource input.EntitySource ) BeforeEach(func() { inputVariableSource = &MockInputVariableSource{} crdConstraintVariableSource = variablesources.NewCRDUniquenessConstraintsVariableSource(inputVariableSource) ctx = context.Background() - - // the entity is not used in this variable source - entitySource = &PanicEntitySource{} }) It("should get variables from the input variable source and create global constraint variables", func() { inputVariableSource.ResultSet = []deppy.Variable{ - olmvariables.NewRequiredPackageVariable("test-package", []*olmentity.BundleEntity{ - olmentity.NewBundleEntity(bundleSet["bundle-2"]), - olmentity.NewBundleEntity(bundleSet["bundle-1"]), + olmvariables.NewRequiredPackageVariable("test-package", []*catalogmetadata.Bundle{ + bundleSet["bundle-2"], + bundleSet["bundle-1"], }), - olmvariables.NewRequiredPackageVariable("test-package-2", []*olmentity.BundleEntity{ - olmentity.NewBundleEntity(bundleSet["bundle-14"]), - olmentity.NewBundleEntity(bundleSet["bundle-15"]), - olmentity.NewBundleEntity(bundleSet["bundle-16"]), + olmvariables.NewRequiredPackageVariable("test-package-2", []*catalogmetadata.Bundle{ + bundleSet["bundle-14"], + bundleSet["bundle-15"], + bundleSet["bundle-16"], }), - olmvariables.NewBundleVariable( - olmentity.NewBundleEntity(bundleSet["bundle-2"]), - []*olmentity.BundleEntity{ - olmentity.NewBundleEntity(bundleSet["bundle-3"]), - olmentity.NewBundleEntity(bundleSet["bundle-4"]), - olmentity.NewBundleEntity(bundleSet["bundle-5"]), - olmentity.NewBundleEntity(bundleSet["bundle-6"]), - olmentity.NewBundleEntity(bundleSet["bundle-7"]), + olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-2"])[0], + bundleSet["bundle-2"], + []*catalogmetadata.Bundle{ + bundleSet["bundle-3"], + bundleSet["bundle-4"], + bundleSet["bundle-5"], + bundleSet["bundle-6"], + bundleSet["bundle-7"], }, ), - olmvariables.NewBundleVariable( - olmentity.NewBundleEntity(bundleSet["bundle-1"]), - []*olmentity.BundleEntity{ - olmentity.NewBundleEntity(bundleSet["bundle-6"]), - olmentity.NewBundleEntity(bundleSet["bundle-7"]), - olmentity.NewBundleEntity(bundleSet["bundle-8"]), + olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-1"])[0], + bundleSet["bundle-1"], + []*catalogmetadata.Bundle{ + bundleSet["bundle-6"], + bundleSet["bundle-7"], + bundleSet["bundle-8"], }, ), - olmvariables.NewBundleVariable( - olmentity.NewBundleEntity(bundleSet["bundle-3"]), - []*olmentity.BundleEntity{}, + olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-3"])[0], + bundleSet["bundle-3"], + []*catalogmetadata.Bundle{}, ), - olmvariables.NewBundleVariable( - olmentity.NewBundleEntity(bundleSet["bundle-4"]), - []*olmentity.BundleEntity{}, + olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-4"])[0], + bundleSet["bundle-4"], + []*catalogmetadata.Bundle{}, ), - olmvariables.NewBundleVariable( - olmentity.NewBundleEntity(bundleSet["bundle-5"]), - []*olmentity.BundleEntity{}, + olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-5"])[0], + bundleSet["bundle-5"], + []*catalogmetadata.Bundle{}, ), - olmvariables.NewBundleVariable( - olmentity.NewBundleEntity(bundleSet["bundle-6"]), - []*olmentity.BundleEntity{}, + olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-6"])[0], + bundleSet["bundle-6"], + []*catalogmetadata.Bundle{}, ), - olmvariables.NewBundleVariable( - olmentity.NewBundleEntity(bundleSet["bundle-7"]), - []*olmentity.BundleEntity{ - olmentity.NewBundleEntity(bundleSet["bundle-8"]), - olmentity.NewBundleEntity(bundleSet["bundle-9"]), - olmentity.NewBundleEntity(bundleSet["bundle-10"]), + olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-7"])[0], + bundleSet["bundle-7"], + []*catalogmetadata.Bundle{ + bundleSet["bundle-8"], + bundleSet["bundle-9"], + bundleSet["bundle-10"], }, ), - olmvariables.NewBundleVariable( - olmentity.NewBundleEntity(bundleSet["bundle-8"]), - []*olmentity.BundleEntity{}, + olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-8"])[0], + bundleSet["bundle-8"], + []*catalogmetadata.Bundle{}, ), - olmvariables.NewBundleVariable( - olmentity.NewBundleEntity(bundleSet["bundle-9"]), - []*olmentity.BundleEntity{}, + olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-9"])[0], + bundleSet["bundle-9"], + []*catalogmetadata.Bundle{}, ), - olmvariables.NewBundleVariable( - olmentity.NewBundleEntity(bundleSet["bundle-10"]), - []*olmentity.BundleEntity{}, + olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-10"])[0], + bundleSet["bundle-10"], + []*catalogmetadata.Bundle{}, ), - olmvariables.NewBundleVariable( - olmentity.NewBundleEntity(bundleSet["bundle-14"]), - []*olmentity.BundleEntity{}, + olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-14"])[0], + bundleSet["bundle-14"], + []*catalogmetadata.Bundle{}, ), - olmvariables.NewBundleVariable( - olmentity.NewBundleEntity(bundleSet["bundle-15"]), - []*olmentity.BundleEntity{}, + olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-15"])[0], + bundleSet["bundle-15"], + []*catalogmetadata.Bundle{}, ), - olmvariables.NewBundleVariable( - olmentity.NewBundleEntity(bundleSet["bundle-16"]), - []*olmentity.BundleEntity{}, + olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-16"])[0], + bundleSet["bundle-16"], + []*catalogmetadata.Bundle{}, ), } - variables, err := crdConstraintVariableSource.GetVariables(ctx, entitySource) + variables, err := crdConstraintVariableSource.GetVariables(ctx) Expect(err).ToNot(HaveOccurred()) // Note: When accounting for GVK Uniqueness (which we are currently not doing), we // would expect to have 26 variables from the 5 unique GVKs (Bar, Bit, Buz, Fiz, Foo). @@ -235,38 +297,18 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() { It("should return an error if input variable source returns an error", func() { inputVariableSource = &MockInputVariableSource{Err: fmt.Errorf("error getting variables")} crdConstraintVariableSource = variablesources.NewCRDUniquenessConstraintsVariableSource(inputVariableSource) - _, err := crdConstraintVariableSource.GetVariables(ctx, entitySource) + _, err := crdConstraintVariableSource.GetVariables(ctx) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("error getting variables")) }) }) -var _ input.EntitySource = &PanicEntitySource{} - -type PanicEntitySource struct{} - -func (p PanicEntitySource) Get(_ context.Context, _ deppy.Identifier) (*input.Entity, error) { - return nil, fmt.Errorf("if you are seeing this it is because the global variable source is calling the entity source - this shouldn't happen") -} - -func (p PanicEntitySource) Filter(_ context.Context, _ input.Predicate) (input.EntityList, error) { - return nil, fmt.Errorf("if you are seeing this it is because the global variable source is calling the entity source - this shouldn't happen") -} - -func (p PanicEntitySource) GroupBy(_ context.Context, _ input.GroupByFunction) (input.EntityListMap, error) { - return nil, fmt.Errorf("if you are seeing this it is because the global variable source is calling the entity source - this shouldn't happen") -} - -func (p PanicEntitySource) Iterate(_ context.Context, _ input.IteratorFunction) error { - return fmt.Errorf("if you are seeing this it is because the global variable source is calling the entity source - this shouldn't happen") -} - type MockInputVariableSource struct { ResultSet []deppy.Variable Err error } -func (m *MockInputVariableSource) GetVariables(_ context.Context, _ input.EntitySource) ([]deppy.Variable, error) { +func (m *MockInputVariableSource) GetVariables(_ context.Context) ([]deppy.Variable, error) { if m.Err != nil { return nil, m.Err } diff --git a/internal/resolution/variablesources/installed_package.go b/internal/resolution/variablesources/installed_package.go index 0dbee55ad..3687c04e5 100644 --- a/internal/resolution/variablesources/installed_package.go +++ b/internal/resolution/variablesources/installed_package.go @@ -8,7 +8,6 @@ import ( "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" - catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort" "github.com/operator-framework/operator-controller/internal/resolution/variables" @@ -17,17 +16,17 @@ import ( var _ input.VariableSource = &InstalledPackageVariableSource{} type InstalledPackageVariableSource struct { - catalog catalogclient.CatalogClient - bundleImage string + catalogClient BundleProvider + bundleImage string } func (r *InstalledPackageVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { - allBundles, err := r.catalog.Bundles(ctx) + allBundles, err := r.catalogClient.Bundles(ctx) if err != nil { return nil, err } - // find corresponding bundle entity for the installed content + // find corresponding bundle for the installed content resultSet := catalogfilter.Filter(allBundles, catalogfilter.WithBundleImage(r.bundleImage)) if len(resultSet) == 0 { return nil, r.notFoundError() @@ -60,9 +59,9 @@ func (r *InstalledPackageVariableSource) notFoundError() error { return fmt.Errorf("bundleImage %q not found", r.bundleImage) } -func NewInstalledPackageVariableSource(catalog catalogclient.CatalogClient, bundleImage string) (*InstalledPackageVariableSource, error) { +func NewInstalledPackageVariableSource(catalogClient BundleProvider, bundleImage string) (*InstalledPackageVariableSource, error) { return &InstalledPackageVariableSource{ - catalog: catalog, - bundleImage: bundleImage, + catalogClient: catalogClient, + bundleImage: bundleImage, }, nil } diff --git a/internal/resolution/variablesources/installed_package_test.go b/internal/resolution/variablesources/installed_package_test.go index 5581039a4..1f435f908 100644 --- a/internal/resolution/variablesources/installed_package_test.go +++ b/internal/resolution/variablesources/installed_package_test.go @@ -2,75 +2,117 @@ package variablesources_test import ( "context" + "encoding/json" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" + "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" + testutil "github.com/operator-framework/operator-controller/test/util" ) var _ = Describe("InstalledPackageVariableSource", func() { var ( - ipvs *variablesources.InstalledPackageVariableSource - bundleImage string - mockEntitySource input.EntitySource + ipvs *variablesources.InstalledPackageVariableSource + fakeCatalogClient testutil.FakeCatalogClient + bundleImage string ) BeforeEach(func() { + channel := catalogmetadata.Channel{Channel: declcfg.Channel{ + Name: "stable", + Entries: []declcfg.ChannelEntry{ + { + Name: "test-package.v1.0.0", + }, + { + Name: "test-package.v2.0.0", + Replaces: "test-package.v1.0.0", + }, + { + Name: "test-package.v3.0.0", + Replaces: "test-package.v2.0.0", + }, + { + Name: "test-package.v4.0.0", + Replaces: "test-package.v3.0.0", + }, + { + Name: "test-package.v5.0.0", + Replaces: "test-package.v4.0.0", + }, + }, + }} + bundleList := []*catalogmetadata.Bundle{ + {Bundle: declcfg.Bundle{ + Name: "test-package.v1.0.0", + Package: "test-package", + Image: "registry.io/repo/test-package@v1.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package.v3.0.0", + Package: "test-package", + Image: "registry.io/repo/test-package@v3.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "3.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package.v2.0.0", + Package: "test-package", + Image: "registry.io/repo/test-package@v2.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package.v4.0.0", + Package: "test-package", + Image: "registry.io/repo/test-package@v4.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "4.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package.v5.0.0", + Package: "test-package", + Image: "registry.io/repo/test-package@v5.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "5-0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + } var err error bundleImage = "registry.io/repo/test-package@v2.0.0" - ipvs, err = variablesources.NewInstalledPackageVariableSource(bundleImage) + fakeCatalogClient = testutil.NewFakeCatalogClient(bundleList) + ipvs, err = variablesources.NewInstalledPackageVariableSource(&fakeCatalogClient, bundleImage) Expect(err).NotTo(HaveOccurred()) - - mockEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{ - "test-package.v1.0.0": *input.NewEntity("test-package.v1.0.0test-packagestable", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - olmentity.PropertyBundlePath: `"registry.io/repo/test-package@v1.0.0"`, - olmentity.PropertyBundleChannelEntry: `{"name": "test-package.v1.0.0"}`, - }), - "test-package.v3.0.0": *input.NewEntity("test-package.v3.0.0test-packagestable", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "3.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - olmentity.PropertyBundlePath: `"registry.io/repo/test-package@v3.0.0"`, - olmentity.PropertyBundleChannelEntry: `{"name": "test-package.v3.0.0","replaces": "test-package.v2.0.0"}`, - }), - "test-package.v2.0.0": *input.NewEntity("test-package.v2.0.0test-packagestable", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - olmentity.PropertyBundlePath: `"registry.io/repo/test-package@v2.0.0"`, - olmentity.PropertyBundleChannelEntry: `{"name": "test-package.v2.0.0","replaces": "test-package.v1.0.0"}`, - }), - "test-package.4.0.0": *input.NewEntity("test-package.v4.0.0test-packagestable", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "4.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - olmentity.PropertyBundlePath: `"registry.io/repo/test-package@v4.0.0"`, - olmentity.PropertyBundleChannelEntry: `{"name": "test-package.v4.0.0","replaces": "test-package.v3.0.0"}`, - }), - "test-package.5.0.0": *input.NewEntity("test-package.v5.0.0test-packagestable", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "5-00"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - olmentity.PropertyBundlePath: `"registry.io/repo/test-package@v5.0.0"`, - olmentity.PropertyBundleChannelEntry: `{"name": "test-package.v5.0.0","replaces": "test-package.v4.0.0"}`, - }), - }) }) It("should return the correct package variable", func() { - variables, err := ipvs.GetVariables(context.TODO(), mockEntitySource) + variables, err := ipvs.GetVariables(context.TODO()) Expect(err).NotTo(HaveOccurred()) Expect(variables).To(HaveLen(1)) reqPackageVar, ok := variables[0].(*olmvariables.InstalledPackageVariable) Expect(ok).To(BeTrue()) Expect(reqPackageVar.Identifier()).To(Equal(deppy.IdentifierFromString("installed package test-package"))) - // ensure bundle entities are in version order (high to low) - Expect(reqPackageVar.BundleEntities()[0].ID).To(Equal(deppy.IdentifierFromString("test-package.v3.0.0test-packagestable"))) - Expect(reqPackageVar.BundleEntities()[1].ID).To(Equal(deppy.IdentifierFromString("test-package.v2.0.0test-packagestable"))) + // ensure bundles are in version order (high to low) + Expect(reqPackageVar.Bundles()).To(HaveLen(2)) + Expect(reqPackageVar.Bundles()[0].Name).To(Equal("test-package.v3.0.0")) + Expect(reqPackageVar.Bundles()[1].Name).To(Equal("test-package.v2.0.0")) }) }) diff --git a/internal/resolution/variablesources/operator.go b/internal/resolution/variablesources/operator.go index a57112a4d..19cc88443 100644 --- a/internal/resolution/variablesources/operator.go +++ b/internal/resolution/variablesources/operator.go @@ -4,7 +4,6 @@ import ( "context" operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" - catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" @@ -15,14 +14,14 @@ var _ input.VariableSource = &OperatorVariableSource{} type OperatorVariableSource struct { client client.Client - catalog catalogclient.CatalogClient + catalogClient BundleProvider inputVariableSource input.VariableSource } -func NewOperatorVariableSource(cl client.Client, catalog catalogclient.CatalogClient, inputVariableSource input.VariableSource) *OperatorVariableSource { +func NewOperatorVariableSource(cl client.Client, catalogClient BundleProvider, inputVariableSource input.VariableSource) *OperatorVariableSource { return &OperatorVariableSource{ client: cl, - catalog: catalog, + catalogClient: catalogClient, inputVariableSource: inputVariableSource, } } @@ -41,7 +40,7 @@ func (o *OperatorVariableSource) GetVariables(ctx context.Context) ([]deppy.Vari // build required package variable sources for _, operator := range operatorList.Items { rps, err := NewRequiredPackageVariableSource( - o.catalog, + o.catalogClient, operator.Spec.PackageName, InVersionRange(operator.Spec.Version), InChannel(operator.Spec.Channel), diff --git a/internal/resolution/variablesources/operator_test.go b/internal/resolution/variablesources/operator_test.go index 0920b4961..426cf82c2 100644 --- a/internal/resolution/variablesources/operator_test.go +++ b/internal/resolution/variablesources/operator_test.go @@ -2,21 +2,28 @@ package variablesources_test import ( "context" - "fmt" + "encoding/json" + "errors" + + "github.com/operator-framework/deppy/pkg/deppy" + "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/alpha/property" . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" - operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + testutil "github.com/operator-framework/operator-controller/test/util" + + "github.com/operator-framework/operator-controller/internal/catalogmetadata" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" ) @@ -27,27 +34,6 @@ func FakeClient(objects ...client.Object) client.Client { return fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() } -var testEntityCache = map[deppy.Identifier]input.Entity{ - "operatorhub/prometheus/0.37.0": *input.NewEntity("operatorhub/prometheus/0.37.0", map[string]string{ - "olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"`, - "olm.channel": "{\"channelName\":\"beta\",\"priority\":0,\"replaces\":\"prometheusoperator.0.32.0\"}", - "olm.gvk": "[{\"group\":\"monitoring.coreos.com\",\"kind\":\"Alertmanager\",\"version\":\"v1\"}, {\"group\":\"monitoring.coreos.com\",\"kind\":\"Prometheus\",\"version\":\"v1\"}]", - "olm.package": "{\"packageName\":\"prometheus\",\"version\":\"0.37.0\"}", - }), - "operatorhub/prometheus/0.47.0": *input.NewEntity("operatorhub/prometheus/0.47.0", map[string]string{ - "olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed"`, - "olm.channel": "{\"channelName\":\"beta\",\"priority\":0,\"replaces\":\"prometheusoperator.0.37.0\"}", - "olm.gvk": "[{\"group\":\"monitoring.coreos.com\",\"kind\":\"Alertmanager\",\"version\":\"v1\"}, {\"group\":\"monitoring.coreos.com\",\"kind\":\"Prometheus\",\"version\":\"v1alpha1\"}]", - "olm.package": "{\"packageName\":\"prometheus\",\"version\":\"0.47.0\"}", - }), - "operatorhub/packageA/2.0.0": *input.NewEntity("operatorhub/packageA/2.0.0", map[string]string{ - "olm.bundle.path": `"foo.io/packageA/packageA:v2.0.0"`, - "olm.channel": "{\"channelName\":\"stable\",\"priority\":0}", - "olm.gvk": "[{\"group\":\"foo.io\",\"kind\":\"Foo\",\"version\":\"v1\"}]", - "olm.package": "{\"packageName\":\"packageA\",\"version\":\"2.0.0\"}", - }), -} - func operator(name string) *operatorsv1alpha1.Operator { return &operatorsv1alpha1.Operator{ ObjectMeta: metav1.ObjectMeta{ @@ -60,17 +46,78 @@ func operator(name string) *operatorsv1alpha1.Operator { } var _ = Describe("OperatorVariableSource", func() { - var testEntitySource input.EntitySource + var betaChannel catalogmetadata.Channel + var stableChannel catalogmetadata.Channel + var testBundleList []*catalogmetadata.Bundle BeforeEach(func() { - testEntitySource = input.NewCacheQuerier(testEntityCache) + betaChannel = catalogmetadata.Channel{ + Channel: declcfg.Channel{ + Name: "beta", + Entries: []declcfg.ChannelEntry{ + { + Name: "operatorhub/prometheus/0.37.0", + Replaces: "operatorhub/prometheus/0.32.0", + }, + { + Name: "operatorhub/prometheus/0.47.0", + Replaces: "operatorhub/prometheus/0.37.0", + }, + }, + }, + } + + stableChannel = catalogmetadata.Channel{ + Channel: declcfg.Channel{ + Name: "stable", + Entries: []declcfg.ChannelEntry{ + { + Name: "operatorhub/packageA/2.0.0", + }, + }, + }, + } + + testBundleList = []*catalogmetadata.Bundle{ + {Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/0.37.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"0.37.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"monitoring.coreos.com","kind":"Alertmanager","version":"v1"}, {"group":"monitoring.coreos.com","kind":"Prometheus","version":"v1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&betaChannel}, + }, + {Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/0.47.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"0.47.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"monitoring.coreos.com","kind":"Alertmanager","version":"v1"}, {"group":"monitoring.coreos.com","kind":"Prometheus","version":"v1alpha1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&betaChannel}, + }, + {Bundle: declcfg.Bundle{ + Name: "operatorhub/packageA/2.0.0", + Package: "packageA", + Image: "foo.io/packageA/packageA:v2.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"packageA","version":"2.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[{"group":"foo.io","kind":"Foo","version":"v1"}]`)}, + }}, + InChannels: []*catalogmetadata.Channel{&stableChannel}, + }, + } + }) It("should produce RequiredPackage variables", func() { cl := FakeClient(operator("prometheus"), operator("packageA")) - - opVariableSource := variablesources.NewOperatorVariableSource(cl, &MockRequiredPackageSource{}) - variables, err := opVariableSource.GetVariables(context.Background(), testEntitySource) + fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) + opVariableSource := variablesources.NewOperatorVariableSource(cl, &fakeCatalogClient, &MockRequiredPackageSource{}) + variables, err := opVariableSource.GetVariables(context.Background()) Expect(err).ToNot(HaveOccurred()) packageRequiredVariables := filterVariables[*olmvariables.RequiredPackageVariable](variables) @@ -78,7 +125,7 @@ var _ = Describe("OperatorVariableSource", func() { Expect(packageRequiredVariables).To(WithTransform(func(bvars []*olmvariables.RequiredPackageVariable) map[deppy.Identifier]int { out := map[deppy.Identifier]int{} for _, variable := range bvars { - out[variable.Identifier()] = len(variable.BundleEntities()) + out[variable.Identifier()] = len(variable.Bundles()) } return out }, Equal(map[deppy.Identifier]int{ @@ -89,34 +136,14 @@ var _ = Describe("OperatorVariableSource", func() { It("should return an errors when they occur", func() { cl := FakeClient(operator("prometheus"), operator("packageA")) + fakeCatalogClient := testutil.NewFakeCatalogClientWithError(errors.New("something bad happened")) - opVariableSource := variablesources.NewOperatorVariableSource(cl, nil) - _, err := opVariableSource.GetVariables(context.Background(), FailEntitySource{}) + opVariableSource := variablesources.NewOperatorVariableSource(cl, &fakeCatalogClient, nil) + _, err := opVariableSource.GetVariables(context.Background()) Expect(err).To(HaveOccurred()) }) }) -var _ input.EntitySource = &FailEntitySource{} - -type FailEntitySource struct { -} - -func (f FailEntitySource) Get(_ context.Context, _ deppy.Identifier) (*input.Entity, error) { - return nil, fmt.Errorf("error executing get") -} - -func (f FailEntitySource) Filter(_ context.Context, _ input.Predicate) (input.EntityList, error) { - return nil, fmt.Errorf("error executing filter") -} - -func (f FailEntitySource) GroupBy(_ context.Context, _ input.GroupByFunction) (input.EntityListMap, error) { - return nil, fmt.Errorf("error executing group by") -} - -func (f FailEntitySource) Iterate(_ context.Context, _ input.IteratorFunction) error { - return fmt.Errorf("error executing iterate") -} - func filterVariables[D deppy.Variable](variables []deppy.Variable) []D { var out []D for _, variable := range variables { diff --git a/internal/resolution/variablesources/required_package.go b/internal/resolution/variablesources/required_package.go index 947e6d1f2..568a7ebfa 100644 --- a/internal/resolution/variablesources/required_package.go +++ b/internal/resolution/variablesources/required_package.go @@ -10,7 +10,6 @@ import ( "github.com/operator-framework/deppy/pkg/deppy/input" "github.com/operator-framework/operator-controller/internal/catalogmetadata" - catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client" catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" @@ -47,7 +46,7 @@ func InChannel(channelName string) RequiredPackageVariableSourceOption { } type RequiredPackageVariableSource struct { - catalog catalogclient.CatalogClient + catalogClient BundleProvider packageName string versionRange string @@ -55,12 +54,12 @@ type RequiredPackageVariableSource struct { predicates []catalogfilter.Predicate[catalogmetadata.Bundle] } -func NewRequiredPackageVariableSource(catalog catalogclient.CatalogClient, packageName string, options ...RequiredPackageVariableSourceOption) (*RequiredPackageVariableSource, error) { +func NewRequiredPackageVariableSource(catalogClient BundleProvider, packageName string, options ...RequiredPackageVariableSourceOption) (*RequiredPackageVariableSource, error) { if packageName == "" { return nil, fmt.Errorf("package name must not be empty") } r := &RequiredPackageVariableSource{ - catalog: catalog, + catalogClient: catalogClient, packageName: packageName, predicates: []catalogfilter.Predicate[catalogmetadata.Bundle]{catalogfilter.WithPackageName(packageName)}, @@ -74,7 +73,7 @@ func NewRequiredPackageVariableSource(catalog catalogclient.CatalogClient, packa } func (r *RequiredPackageVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) { - resultSet, err := r.catalog.Bundles(ctx) + resultSet, err := r.catalogClient.Bundles(ctx) if err != nil { return nil, err } diff --git a/internal/resolution/variablesources/required_package_test.go b/internal/resolution/variablesources/required_package_test.go index 834054c70..ae87f874e 100644 --- a/internal/resolution/variablesources/required_package_test.go +++ b/internal/resolution/variablesources/required_package_test.go @@ -2,118 +2,136 @@ package variablesources_test import ( "context" + "encoding/json" + "errors" "fmt" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" + "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" - olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" "github.com/operator-framework/operator-controller/internal/resolution/variablesources" + testutil "github.com/operator-framework/operator-controller/test/util" ) var _ = Describe("RequiredPackageVariableSource", func() { var ( - rpvs *variablesources.RequiredPackageVariableSource - packageName string - mockEntitySource input.EntitySource + rpvs *variablesources.RequiredPackageVariableSource + fakeCatalogClient testutil.FakeCatalogClient + packageName string ) BeforeEach(func() { var err error packageName = "test-package" - rpvs, err = variablesources.NewRequiredPackageVariableSource(packageName) - Expect(err).NotTo(HaveOccurred()) - mockEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{ - "bundle-1": *input.NewEntity("bundle-1", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - "bundle-2": *input.NewEntity("bundle-2", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "3.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - "bundle-3": *input.NewEntity("bundle-3", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - + channel := catalogmetadata.Channel{Channel: declcfg.Channel{ + Name: "stable", + }} + bundleList := []*catalogmetadata.Bundle{ + {Bundle: declcfg.Bundle{ + Name: "test-package.v1.0.0", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package.v3.0.0", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "3.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package.v2.0.0", + Package: "test-package", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "2.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, // add some bundles from a different package - "bundle-4": *input.NewEntity("bundle-4", map[string]string{ - property.TypePackage: `{"packageName": "test-package-2", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - "bundle-5": *input.NewEntity("bundle-5", map[string]string{ - property.TypePackage: `{"packageName": "test-package-2", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - }), - }) + {Bundle: declcfg.Bundle{ + Name: "test-package-2.v1.0.0", + Package: "test-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "1.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + {Bundle: declcfg.Bundle{ + Name: "test-package-2.v2.0.0", + Package: "test-package-2", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package-2", "version": "2.0.0"}`)}, + }}, + InChannels: []*catalogmetadata.Channel{&channel}, + }, + } + fakeCatalogClient = testutil.NewFakeCatalogClient(bundleList) + rpvs, err = variablesources.NewRequiredPackageVariableSource(&fakeCatalogClient, packageName) + Expect(err).NotTo(HaveOccurred()) }) It("should return the correct package variable", func() { - variables, err := rpvs.GetVariables(context.TODO(), mockEntitySource) + variables, err := rpvs.GetVariables(context.TODO()) Expect(err).NotTo(HaveOccurred()) Expect(variables).To(HaveLen(1)) reqPackageVar, ok := variables[0].(*olmvariables.RequiredPackageVariable) Expect(ok).To(BeTrue()) Expect(reqPackageVar.Identifier()).To(Equal(deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName)))) - - // ensure bundle entities are in version order (high to low) - Expect(reqPackageVar.BundleEntities()).To(Equal([]*olmentity.BundleEntity{ - olmentity.NewBundleEntity(input.NewEntity("bundle-2", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "3.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - olmentity.NewBundleEntity(input.NewEntity("bundle-3", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "2.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - olmentity.NewBundleEntity(input.NewEntity("bundle-1", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`})), - })) + Expect(reqPackageVar.Bundles()).To(HaveLen(3)) + // ensure bundles are in version order (high to low) + Expect(reqPackageVar.Bundles()[0].Name).To(Equal("test-package.v3.0.0")) + Expect(reqPackageVar.Bundles()[1].Name).To(Equal("test-package.v2.0.0")) + Expect(reqPackageVar.Bundles()[2].Name).To(Equal("test-package.v1.0.0")) }) It("should filter by version range", func() { // recreate source with version range option var err error - rpvs, err = variablesources.NewRequiredPackageVariableSource(packageName, variablesources.InVersionRange(">=1.0.0 !=2.0.0 <3.0.0")) + rpvs, err = variablesources.NewRequiredPackageVariableSource(&fakeCatalogClient, packageName, variablesources.InVersionRange(">=1.0.0 !=2.0.0 <3.0.0")) Expect(err).NotTo(HaveOccurred()) - variables, err := rpvs.GetVariables(context.TODO(), mockEntitySource) + variables, err := rpvs.GetVariables(context.TODO()) Expect(err).NotTo(HaveOccurred()) Expect(variables).To(HaveLen(1)) reqPackageVar, ok := variables[0].(*olmvariables.RequiredPackageVariable) Expect(ok).To(BeTrue()) Expect(reqPackageVar.Identifier()).To(Equal(deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName)))) - // ensure bundle entities are in version order (high to low) - Expect(reqPackageVar.BundleEntities()).To(Equal([]*olmentity.BundleEntity{ - olmentity.NewBundleEntity(input.NewEntity("bundle-1", map[string]string{ - property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, - property.TypeChannel: `{"channelName":"stable","priority":0}`, - })), - })) + Expect(reqPackageVar.Bundles()).To(HaveLen(1)) + // test-package.v1.0.0 is the only package that matches the provided filter + Expect(reqPackageVar.Bundles()[0].Name).To(Equal("test-package.v1.0.0")) }) It("should fail with bad semver range", func() { - _, err := variablesources.NewRequiredPackageVariableSource(packageName, variablesources.InVersionRange("not a valid semver")) + _, err := variablesources.NewRequiredPackageVariableSource(&fakeCatalogClient, packageName, variablesources.InVersionRange("not a valid semver")) Expect(err).To(HaveOccurred()) }) It("should return an error if package not found", func() { - mockEntitySource := input.NewCacheQuerier(map[deppy.Identifier]input.Entity{}) - _, err := rpvs.GetVariables(context.TODO(), mockEntitySource) + emptyCatalogClient := testutil.NewFakeCatalogClient([]*catalogmetadata.Bundle{}) + rpvs, err := variablesources.NewRequiredPackageVariableSource(&emptyCatalogClient, packageName) + Expect(err).NotTo(HaveOccurred()) + _, err = rpvs.GetVariables(context.TODO()) Expect(err).To(HaveOccurred()) Expect(err).To(MatchError("package 'test-package' not found")) }) - It("should return an error if entity source errors", func() { - _, err := rpvs.GetVariables(context.TODO(), FailEntitySource{}) + It("should return an error if catalog client errors", func() { + testError := errors.New("something bad happened") + emptyCatalogClient := testutil.NewFakeCatalogClientWithError(testError) + rpvs, err := variablesources.NewRequiredPackageVariableSource(&emptyCatalogClient, packageName, variablesources.InVersionRange(">=1.0.0 !=2.0.0 <3.0.0")) + Expect(err).NotTo(HaveOccurred()) + _, err = rpvs.GetVariables(context.TODO()) Expect(err).To(HaveOccurred()) - Expect(err).To(MatchError("error executing filter")) + Expect(err).To(MatchError(testError)) }) }) diff --git a/test/util/fake_catalog_client.go b/test/util/fake_catalog_client.go new file mode 100644 index 000000000..20c3c8b29 --- /dev/null +++ b/test/util/fake_catalog_client.go @@ -0,0 +1,31 @@ +package testutil + +import ( + "context" + + "github.com/operator-framework/operator-controller/internal/catalogmetadata" +) + +type FakeCatalogClient struct { + bundles []*catalogmetadata.Bundle + err error +} + +func NewFakeCatalogClient(b []*catalogmetadata.Bundle) FakeCatalogClient { + return FakeCatalogClient{ + bundles: b, + } +} + +func NewFakeCatalogClientWithError(e error) FakeCatalogClient { + return FakeCatalogClient{ + err: e, + } +} + +func (c *FakeCatalogClient) Bundles(_ context.Context) ([]*catalogmetadata.Bundle, error) { + if c.err != nil { + return nil, c.err + } + return c.bundles, nil +}