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
m1kola committed May 2, 2024
1 parent e32aa27 commit 46cfcb5
Show file tree
Hide file tree
Showing 2 changed files with 168 additions and 0 deletions.
38 changes: 38 additions & 0 deletions internal/controllers/clusterextension_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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 {
Expand Down
130 changes: 130 additions & 0 deletions internal/controllers/clusterextension_registryv1_validation_test.go
View file
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}

0 comments on commit 46cfcb5

Please sign in to comment.