Skip to content

Commit

Permalink
Disallow bundles with dependencies and constraints
Browse files Browse the repository at this point in the history
Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
  • Loading branch information
m1kola committed May 2, 2024
1 parent e32aa27 commit 4813cd3
Show file tree
Hide file tree
Showing 2 changed files with 163 additions and 0 deletions.
28 changes: 28 additions & 0 deletions internal/controllers/clusterextension_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/utils/ptr"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -40,6 +41,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"
Expand Down Expand Up @@ -140,6 +142,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())
Expand Down Expand Up @@ -278,6 +287,25 @@ func (r *ClusterExtensionReconciler) installedBundle(ctx context.Context, allBun
return resultSet[0], nil
}

func (r *ClusterExtensionReconciler) validateBundle(bundle *catalogmetadata.Bundle) error {
unsupportedProps := sets.New(
property.TypePackageRequired,
property.TypeGVKRequired,
property.TypeConstraint,
)
for i := range bundle.Properties {
if unsupportedProps.Has(bundle.Properties[i].Type) {
return fmt.Errorf(
"bundle %q has a dependency declared via property %q which is currently not supported",
bundle.Name,
bundle.Properties[i].Type,
)
}
}

return nil
}

func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alpha2.BundleDeployment, ext *ocv1alpha1.ClusterExtension, bundle *catalogmetadata.Bundle) {
bundleDeploymentReady := apimeta.FindStatusCondition(existingTypedBundleDeployment.Status.Conditions, rukpakv1alpha2.TypeInstalled)
if bundleDeploymentReady == nil {
Expand Down
135 changes: 135 additions & 0 deletions internal/controllers/clusterextension_registryv1_validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
package controllers_test

import (
"context"
"encoding/json"
"fmt"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
apimeta "k8s.io/apimachinery/pkg/api/meta"
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: 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: 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: 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: 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.NoError(t, err)
} else {
assert.EqualError(t, err, tt.wantErr)

// In case of an error we want it to be included in the installed condition
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1alpha1.ReasonInstallationFailed, cond.Reason)
require.Equal(t, tt.wantErr, cond.Message)
}
})
}
}

0 comments on commit 4813cd3

Please sign in to comment.