From 4815978b0a9cd5f0a48a2716af3567047f1bb136 Mon Sep 17 00:00:00 2001 From: Mikalai Radchuk Date: Tue, 31 Oct 2023 16:13:25 +0000 Subject: [PATCH] Refactor controller variable source Signed-off-by: Mikalai Radchuk --- internal/controllers/variable_source.go | 47 +++++--- internal/controllers/variable_source_test.go | 114 +++++++++++++++++++ 2 files changed, 144 insertions(+), 17 deletions(-) create mode 100644 internal/controllers/variable_source_test.go diff --git a/internal/controllers/variable_source.go b/internal/controllers/variable_source.go index c1976a1cc..aaf766f5b 100644 --- a/internal/controllers/variable_source.go +++ b/internal/controllers/variable_source.go @@ -22,7 +22,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/operator-framework/deppy/pkg/deppy" - "github.com/operator-framework/deppy/pkg/deppy/input" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" @@ -64,21 +63,35 @@ func (v *VariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, er return nil, err } - // We are in process of getting rid of extra variable sources. - // See this for progress: https://github.com/operator-framework/operator-controller/issues/437 - vs := variablesources.NestedVariableSource{ - func(inputVariableSource input.VariableSource) (input.VariableSource, error) { - return variablesources.NewOperatorVariableSource(operatorList.Items, allBundles, inputVariableSource), nil - }, - func(inputVariableSource input.VariableSource) (input.VariableSource, error) { - return variablesources.NewBundleDeploymentVariableSource(bundleDeploymentList.Items, allBundles, inputVariableSource), nil - }, - func(inputVariableSource input.VariableSource) (input.VariableSource, error) { - return variablesources.NewBundlesAndDepsVariableSource(allBundles, inputVariableSource), nil - }, - func(inputVariableSource input.VariableSource) (input.VariableSource, error) { - return variablesources.NewCRDUniquenessConstraintsVariableSource(inputVariableSource), nil - }, + requiredPackages, err := variablesources.MakeRequiredPackageVariables(allBundles, operatorList.Items) + if err != nil { + return nil, err + } + + installedPackages, err := variablesources.MakeInstalledPackageVariables(allBundles, bundleDeploymentList.Items) + if err != nil { + return nil, err + } + + bundles, err := variablesources.MakeBundleVariables(allBundles, requiredPackages, installedPackages) + if err != nil { + return nil, err + } + + bundleUniqueness := variablesources.MakeBundleUniquenessVariables(bundles) + + result := []deppy.Variable{} + for _, v := range requiredPackages { + result = append(result, v) + } + for _, v := range installedPackages { + result = append(result, v) + } + for _, v := range bundles { + result = append(result, v) + } + for _, v := range bundleUniqueness { + result = append(result, v) } - return vs.GetVariables(ctx) + return result, nil } diff --git a/internal/controllers/variable_source_test.go b/internal/controllers/variable_source_test.go new file mode 100644 index 000000000..d0277ef8d --- /dev/null +++ b/internal/controllers/variable_source_test.go @@ -0,0 +1,114 @@ +package controllers_test + +import ( + "context" + "encoding/json" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/rand" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/utils/pointer" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/alpha/property" + rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" + + 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" + olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables" + testutil "github.com/operator-framework/operator-controller/test/util" +) + +func TestVariableSource(t *testing.T) { + sch := runtime.NewScheme() + utilruntime.Must(operatorsv1alpha1.AddToScheme(sch)) + utilruntime.Must(rukpakv1alpha1.AddToScheme(sch)) + + stableChannel := catalogmetadata.Channel{Channel: declcfg.Channel{ + Name: "stable", + Entries: []declcfg.ChannelEntry{ + { + Name: "packageA.v2.0.0", + }, + }, + }} + testBundleList := []*catalogmetadata.Bundle{ + { + Bundle: declcfg.Bundle{ + Name: "packageA.v2.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"}`)}, + }, + }, + CatalogName: "fake-catalog", + InChannels: []*catalogmetadata.Channel{&stableChannel}, + }, + } + + pkgName := "packageA" + opName := fmt.Sprintf("operator-test-%s", rand.String(8)) + operator := &operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{Name: opName}, + Spec: operatorsv1alpha1.OperatorSpec{ + PackageName: pkgName, + Channel: "stable", + Version: "2.0.0", + }, + } + + bd := &rukpakv1alpha1.BundleDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: opName, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: operatorsv1alpha1.GroupVersion.String(), + Kind: "Operator", + Name: operator.Name, + UID: operator.UID, + Controller: pointer.Bool(true), + BlockOwnerDeletion: pointer.Bool(true), + }, + }, + }, + Spec: rukpakv1alpha1.BundleDeploymentSpec{ + ProvisionerClassName: "core-rukpak-io-plain", + Template: &rukpakv1alpha1.BundleTemplate{ + Spec: rukpakv1alpha1.BundleSpec{ + ProvisionerClassName: "core-rukpak-io-registry", + Source: rukpakv1alpha1.BundleSource{ + Type: rukpakv1alpha1.SourceTypeImage, + Image: &rukpakv1alpha1.ImageSource{ + Ref: "foo.io/packageA/packageA:v2.0.0", + }, + }, + }, + }, + }, + } + fakeClient := fake.NewClientBuilder().WithScheme(sch).WithObjects(operator, bd).Build() + fakeCatalogClient := testutil.NewFakeCatalogClient(testBundleList) + + vs := controllers.NewVariableSource(fakeClient, &fakeCatalogClient) + + vars, err := vs.GetVariables(context.Background()) + require.NoError(t, err) + + require.Len(t, vars, 4) + assert.Equal(t, "required package packageA", vars[0].Identifier().String()) + assert.IsType(t, &olmvariables.RequiredPackageVariable{}, vars[0]) + assert.Equal(t, "installed package packageA", vars[1].Identifier().String()) + assert.IsType(t, &olmvariables.InstalledPackageVariable{}, vars[1]) + assert.Equal(t, "fake-catalog-packageA-packageA.v2.0.0", vars[2].Identifier().String()) + assert.IsType(t, &olmvariables.BundleVariable{}, vars[2]) + assert.Equal(t, "packageA package uniqueness", vars[3].Identifier().String()) + assert.IsType(t, &olmvariables.BundleUniquenessVariable{}, vars[3]) +}