From 46cfcb59c78e6ceabcf9277a27258caa56a1ef72 Mon Sep 17 00:00:00 2001 From: Mikalai Radchuk Date: Wed, 24 Apr 2024 17:40:07 +0200 Subject: [PATCH] Disallow bundles with dependencies and constraints Signed-off-by: Mikalai Radchuk --- .../clusterextension_controller.go | 38 +++++ ...terextension_registryv1_validation_test.go | 130 ++++++++++++++++++ 2 files changed, 168 insertions(+) create mode 100644 internal/controllers/clusterextension_registryv1_validation_test.go diff --git a/internal/controllers/clusterextension_controller.go b/internal/controllers/clusterextension_controller.go index 42913e77e..9686766d1 100644 --- a/internal/controllers/clusterextension_controller.go +++ b/internal/controllers/clusterextension_controller.go @@ -40,6 +40,7 @@ import ( catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/alpha/property" rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2" ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" @@ -140,6 +141,13 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration()) return ctrl.Result{}, err } + + if err := r.validateBundle(bundle); err != nil { + setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) + setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration()) + return ctrl.Result{}, err + } + bundleProvisioner, err := mapBundleMediaTypeToBundleProvisioner(mediaType) if err != nil { setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration()) @@ -278,6 +286,36 @@ func (r *ClusterExtensionReconciler) installedBundle(ctx context.Context, allBun return resultSet[0], nil } +func (r *ClusterExtensionReconciler) validateBundle(bundle *catalogmetadata.Bundle) error { + mediaType, err := bundle.MediaType() + if err != nil { + return fmt.Errorf("unable to determine type of bundle %q: %s", bundle.Name, err) + } + + if mediaType != catalogmetadata.MediaTypeRegistry { + // We only validate registry+v1 bundles + return nil + } + + for i := range bundle.Properties { + for _, propType := range []string{ + property.TypePackageRequired, + property.TypeGVKRequired, + property.TypeConstraint, + } { + if bundle.Properties[i].Type == propType { + return fmt.Errorf( + "bundle %q has a dependency declared via property %q which is currently not supported", + bundle.Name, + propType, + ) + } + } + } + + return nil +} + func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alpha2.BundleDeployment, ext *ocv1alpha1.ClusterExtension, bundle *catalogmetadata.Bundle) { bundleDeploymentReady := apimeta.FindStatusCondition(existingTypedBundleDeployment.Status.Conditions, rukpakv1alpha2.TypeInstalled) if bundleDeploymentReady == nil { diff --git a/internal/controllers/clusterextension_registryv1_validation_test.go b/internal/controllers/clusterextension_registryv1_validation_test.go new file mode 100644 index 000000000..0e7ed5d6e --- /dev/null +++ b/internal/controllers/clusterextension_registryv1_validation_test.go @@ -0,0 +1,130 @@ +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/types" + "k8s.io/apimachinery/pkg/util/rand" + ctrl "sigs.k8s.io/controller-runtime" + + "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/operator-framework/operator-registry/alpha/property" + rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2" + + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/internal/catalogmetadata" + "github.com/operator-framework/operator-controller/internal/controllers" + testutil "github.com/operator-framework/operator-controller/test/util" +) + +func TestClusterExtensionRegistryV1DisallowDependencies(t *testing.T) { + ctx := context.Background() + cl := newClient(t) + + for _, tt := range []struct { + name string + bundle *catalogmetadata.Bundle + wantErr string + }{ + { + name: "package with no dependencies", + bundle: &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "fake-catalog/no-dependencies-package/alpha/1.0.0", + Package: "no-dependencies-package", + Image: "quay.io/fake-catalog/no-dependencies-package@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", + Properties: []property.Property{ + {Type: catalogmetadata.PropertyBundleMediaType, Value: json.RawMessage(`"registry+v1"`)}, + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"no-dependencies-package","version":"1.0.0"}`)}, + }, + }, + CatalogName: "fake-catalog", + }, + }, + { + name: "package with olm.package.required property", + bundle: &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "fake-catalog/package-required-test/alpha/1.0.0", + Package: "package-required-test", + Image: "quay.io/fake-catalog/package-required-test@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", + Properties: []property.Property{ + {Type: catalogmetadata.PropertyBundleMediaType, Value: json.RawMessage(`"registry+v1"`)}, + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"package-required-test","version":"1.0.0"}`)}, + {Type: property.TypePackageRequired, Value: json.RawMessage("content-is-not-relevant")}, + }, + }, + CatalogName: "fake-catalog", + }, + wantErr: `bundle "fake-catalog/package-required-test/alpha/1.0.0" has a dependency declared via property "olm.package.required" which is currently not supported`, + }, + { + name: "package with olm.gvk.required property", + bundle: &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "fake-catalog/gvk-required-test/alpha/1.0.0", + Package: "gvk-required-test", + Image: "quay.io/fake-catalog/gvk-required-test@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", + Properties: []property.Property{ + {Type: catalogmetadata.PropertyBundleMediaType, Value: json.RawMessage(`"registry+v1"`)}, + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"gvk-required-test","version":"1.0.0"}`)}, + {Type: property.TypeGVKRequired, Value: json.RawMessage(`content-is-not-relevant`)}, + }, + }, + CatalogName: "fake-catalog", + }, + wantErr: `bundle "fake-catalog/gvk-required-test/alpha/1.0.0" has a dependency declared via property "olm.gvk.required" which is currently not supported`, + }, + { + name: "package with olm.constraint property", + bundle: &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "fake-catalog/constraint-test/alpha/1.0.0", + Package: "constraint-test", + Image: "quay.io/fake-catalog/constraint-test@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", + Properties: []property.Property{ + {Type: catalogmetadata.PropertyBundleMediaType, Value: json.RawMessage(`"registry+v1"`)}, + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"constraint-test","version":"1.0.0"}`)}, + {Type: property.TypeConstraint, Value: json.RawMessage(`content-is-not-relevant`)}, + }, + }, + CatalogName: "fake-catalog", + }, + wantErr: `bundle "fake-catalog/constraint-test/alpha/1.0.0" has a dependency declared via property "olm.constraint" which is currently not supported`, + }, + } { + t.Run(tt.name, func(t *testing.T) { + defer func() { + require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) + require.NoError(t, cl.DeleteAllOf(ctx, &rukpakv1alpha2.BundleDeployment{})) + }() + + fakeCatalogClient := testutil.NewFakeCatalogClient([]*catalogmetadata.Bundle{tt.bundle}) + reconciler := &controllers.ClusterExtensionReconciler{ + Client: cl, + BundleProvider: &fakeCatalogClient, + } + + extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} + clusterExtension := &ocv1alpha1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, + Spec: ocv1alpha1.ClusterExtensionSpec{PackageName: tt.bundle.Package}, + } + require.NoError(t, cl.Create(ctx, clusterExtension)) + + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) + require.Equal(t, ctrl.Result{}, res) + if tt.wantErr != "" { + assert.EqualError(t, err, tt.wantErr) + } else { + assert.NoError(t, err) + } + }) + } +}