Skip to content

Commit f22ce65

Browse files
authored
Disallow bundles with dependencies and constraints (#781)
Signed-off-by: Mikalai Radchuk <[email protected]>
1 parent e32aa27 commit f22ce65

File tree

2 files changed

+163
-0
lines changed

2 files changed

+163
-0
lines changed

internal/controllers/clusterextension_controller.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"k8s.io/apimachinery/pkg/runtime"
3232
"k8s.io/apimachinery/pkg/types"
3333
utilerrors "k8s.io/apimachinery/pkg/util/errors"
34+
"k8s.io/apimachinery/pkg/util/sets"
3435
"k8s.io/utils/ptr"
3536
ctrl "sigs.k8s.io/controller-runtime"
3637
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -40,6 +41,7 @@ import (
4041

4142
catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
4243
"github.com/operator-framework/operator-registry/alpha/declcfg"
44+
"github.com/operator-framework/operator-registry/alpha/property"
4345
rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2"
4446

4547
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
@@ -140,6 +142,13 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
140142
setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration())
141143
return ctrl.Result{}, err
142144
}
145+
146+
if err := r.validateBundle(bundle); err != nil {
147+
setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
148+
setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration())
149+
return ctrl.Result{}, err
150+
}
151+
143152
bundleProvisioner, err := mapBundleMediaTypeToBundleProvisioner(mediaType)
144153
if err != nil {
145154
setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
@@ -278,6 +287,25 @@ func (r *ClusterExtensionReconciler) installedBundle(ctx context.Context, allBun
278287
return resultSet[0], nil
279288
}
280289

290+
func (r *ClusterExtensionReconciler) validateBundle(bundle *catalogmetadata.Bundle) error {
291+
unsupportedProps := sets.New(
292+
property.TypePackageRequired,
293+
property.TypeGVKRequired,
294+
property.TypeConstraint,
295+
)
296+
for i := range bundle.Properties {
297+
if unsupportedProps.Has(bundle.Properties[i].Type) {
298+
return fmt.Errorf(
299+
"bundle %q has a dependency declared via property %q which is currently not supported",
300+
bundle.Name,
301+
bundle.Properties[i].Type,
302+
)
303+
}
304+
}
305+
306+
return nil
307+
}
308+
281309
func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alpha2.BundleDeployment, ext *ocv1alpha1.ClusterExtension, bundle *catalogmetadata.Bundle) {
282310
bundleDeploymentReady := apimeta.FindStatusCondition(existingTypedBundleDeployment.Status.Conditions, rukpakv1alpha2.TypeInstalled)
283311
if bundleDeploymentReady == nil {
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
package controllers_test
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"fmt"
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
apimeta "k8s.io/apimachinery/pkg/api/meta"
12+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
"k8s.io/apimachinery/pkg/types"
14+
"k8s.io/apimachinery/pkg/util/rand"
15+
ctrl "sigs.k8s.io/controller-runtime"
16+
17+
"github.com/operator-framework/operator-registry/alpha/declcfg"
18+
"github.com/operator-framework/operator-registry/alpha/property"
19+
rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2"
20+
21+
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
22+
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
23+
"github.com/operator-framework/operator-controller/internal/controllers"
24+
testutil "github.com/operator-framework/operator-controller/test/util"
25+
)
26+
27+
func TestClusterExtensionRegistryV1DisallowDependencies(t *testing.T) {
28+
ctx := context.Background()
29+
cl := newClient(t)
30+
31+
for _, tt := range []struct {
32+
name string
33+
bundle *catalogmetadata.Bundle
34+
wantErr string
35+
}{
36+
{
37+
name: "package with no dependencies",
38+
bundle: &catalogmetadata.Bundle{
39+
Bundle: declcfg.Bundle{
40+
Name: "fake-catalog/no-dependencies-package/alpha/1.0.0",
41+
Package: "no-dependencies-package",
42+
Image: "quay.io/fake-catalog/no-dependencies-package@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35",
43+
Properties: []property.Property{
44+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"no-dependencies-package","version":"1.0.0"}`)},
45+
},
46+
},
47+
CatalogName: "fake-catalog",
48+
},
49+
},
50+
{
51+
name: "package with olm.package.required property",
52+
bundle: &catalogmetadata.Bundle{
53+
Bundle: declcfg.Bundle{
54+
Name: "fake-catalog/package-required-test/alpha/1.0.0",
55+
Package: "package-required-test",
56+
Image: "quay.io/fake-catalog/package-required-test@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35",
57+
Properties: []property.Property{
58+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"package-required-test","version":"1.0.0"}`)},
59+
{Type: property.TypePackageRequired, Value: json.RawMessage("content-is-not-relevant")},
60+
},
61+
},
62+
CatalogName: "fake-catalog",
63+
},
64+
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`,
65+
},
66+
{
67+
name: "package with olm.gvk.required property",
68+
bundle: &catalogmetadata.Bundle{
69+
Bundle: declcfg.Bundle{
70+
Name: "fake-catalog/gvk-required-test/alpha/1.0.0",
71+
Package: "gvk-required-test",
72+
Image: "quay.io/fake-catalog/gvk-required-test@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35",
73+
Properties: []property.Property{
74+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"gvk-required-test","version":"1.0.0"}`)},
75+
{Type: property.TypeGVKRequired, Value: json.RawMessage(`content-is-not-relevant`)},
76+
},
77+
},
78+
CatalogName: "fake-catalog",
79+
},
80+
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`,
81+
},
82+
{
83+
name: "package with olm.constraint property",
84+
bundle: &catalogmetadata.Bundle{
85+
Bundle: declcfg.Bundle{
86+
Name: "fake-catalog/constraint-test/alpha/1.0.0",
87+
Package: "constraint-test",
88+
Image: "quay.io/fake-catalog/constraint-test@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35",
89+
Properties: []property.Property{
90+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"constraint-test","version":"1.0.0"}`)},
91+
{Type: property.TypeConstraint, Value: json.RawMessage(`content-is-not-relevant`)},
92+
},
93+
},
94+
CatalogName: "fake-catalog",
95+
},
96+
wantErr: `bundle "fake-catalog/constraint-test/alpha/1.0.0" has a dependency declared via property "olm.constraint" which is currently not supported`,
97+
},
98+
} {
99+
t.Run(tt.name, func(t *testing.T) {
100+
defer func() {
101+
require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
102+
require.NoError(t, cl.DeleteAllOf(ctx, &rukpakv1alpha2.BundleDeployment{}))
103+
}()
104+
105+
fakeCatalogClient := testutil.NewFakeCatalogClient([]*catalogmetadata.Bundle{tt.bundle})
106+
reconciler := &controllers.ClusterExtensionReconciler{
107+
Client: cl,
108+
BundleProvider: &fakeCatalogClient,
109+
}
110+
111+
extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
112+
clusterExtension := &ocv1alpha1.ClusterExtension{
113+
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
114+
Spec: ocv1alpha1.ClusterExtensionSpec{PackageName: tt.bundle.Package},
115+
}
116+
require.NoError(t, cl.Create(ctx, clusterExtension))
117+
118+
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
119+
require.Equal(t, ctrl.Result{}, res)
120+
if tt.wantErr == "" {
121+
assert.NoError(t, err)
122+
} else {
123+
assert.EqualError(t, err, tt.wantErr)
124+
125+
// In case of an error we want it to be included in the installed condition
126+
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
127+
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
128+
require.NotNil(t, cond)
129+
require.Equal(t, metav1.ConditionFalse, cond.Status)
130+
require.Equal(t, ocv1alpha1.ReasonInstallationFailed, cond.Reason)
131+
require.Equal(t, tt.wantErr, cond.Message)
132+
}
133+
})
134+
}
135+
}

0 commit comments

Comments
 (0)