Skip to content

Commit dcae8e9

Browse files
Per Goncalves da Silvaperdasilva
Per Goncalves da Silva
authored and
perdasilva
committed
Make bundle->chart function configurable in applier and add tests
Signed-off-by: Per Goncalves da Silva <[email protected]>
1 parent 04e324c commit dcae8e9

File tree

6 files changed

+176
-44
lines changed

6 files changed

+176
-44
lines changed

‎cmd/operator-controller/main.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ import (
6767
"github.com/operator-framework/operator-controller/internal/operator-controller/finalizers"
6868
"github.com/operator-framework/operator-controller/internal/operator-controller/httputil"
6969
"github.com/operator-framework/operator-controller/internal/operator-controller/resolve"
70+
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert"
7071
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety"
7172
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/source"
7273
"github.com/operator-framework/operator-controller/internal/operator-controller/scheme"
@@ -381,8 +382,9 @@ func main() {
381382
}
382383

383384
helmApplier := &applier.Helm{
384-
ActionClientGetter: acg,
385-
Preflights: preflights,
385+
ActionClientGetter: acg,
386+
Preflights: preflights,
387+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
386388
}
387389

388390
cm := contentmanager.NewManager(clientRestConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper())

‎internal/operator-controller/applier/helm.go‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424

2525
ocv1 "github.com/operator-framework/operator-controller/api/v1"
2626
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
27-
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert"
2827
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety"
2928
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util"
3029
)
@@ -52,9 +51,12 @@ type Preflight interface {
5251
Upgrade(context.Context, *release.Release) error
5352
}
5453

54+
type BundleToHelmChartFn func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error)
55+
5556
type Helm struct {
56-
ActionClientGetter helmclient.ActionClientGetter
57-
Preflights []Preflight
57+
ActionClientGetter helmclient.ActionClientGetter
58+
Preflights []Preflight
59+
BundleToHelmChartFn BundleToHelmChartFn
5860
}
5961

6062
// shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND
@@ -78,11 +80,7 @@ func shouldSkipPreflight(ctx context.Context, preflight Preflight, ext *ocv1.Clu
7880
}
7981

8082
func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) ([]client.Object, string, error) {
81-
watchNamespace, err := GetWatchNamespace(ext)
82-
if err != nil {
83-
return nil, "", err
84-
}
85-
chrt, err := convert.RegistryV1ToHelmChart(ctx, contentFS, ext.Spec.Namespace, []string{watchNamespace})
83+
chrt, err := h.buildHelmChart(contentFS, ext)
8684
if err != nil {
8785
return nil, "", err
8886
}
@@ -155,6 +153,17 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
155153
return relObjects, state, nil
156154
}
157155

156+
func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) {
157+
if h.BundleToHelmChartFn == nil {
158+
return nil, errors.New("BundleToHelmChartFn is nil")
159+
}
160+
watchNamespace, err := GetWatchNamespace(ext)
161+
if err != nil {
162+
return nil, err
163+
}
164+
return h.BundleToHelmChartFn(bundleFS, ext.Spec.Namespace, watchNamespace)
165+
}
166+
158167
func (h *Helm) getReleaseState(cl helmclient.ActionInterface, ext *ocv1.ClusterExtension, chrt *chart.Chart, values chartutil.Values, post postrender.PostRenderer) (*release.Release, *release.Release, string, error) {
159168
currentRelease, err := cl.Get(ext.GetName())
160169
if errors.Is(err, driver.ErrReleaseNotFound) {

‎internal/operator-controller/applier/helm_test.go‎

Lines changed: 145 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package applier_test
33
import (
44
"context"
55
"errors"
6+
"io/fs"
67
"os"
78
"testing"
89
"testing/fstest"
@@ -13,6 +14,8 @@ import (
1314
"helm.sh/helm/v3/pkg/chart"
1415
"helm.sh/helm/v3/pkg/release"
1516
"helm.sh/helm/v3/pkg/storage/driver"
17+
corev1 "k8s.io/api/core/v1"
18+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1619
featuregatetesting "k8s.io/component-base/featuregate/testing"
1720
"sigs.k8s.io/controller-runtime/pkg/client"
1821

@@ -21,6 +24,7 @@ import (
2124
v1 "github.com/operator-framework/operator-controller/api/v1"
2225
"github.com/operator-framework/operator-controller/internal/operator-controller/applier"
2326
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
27+
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert"
2428
)
2529

2630
type mockPreflight struct {
@@ -106,6 +110,10 @@ metadata:
106110
olm.properties: '[{"type":"from-csv-annotations-key", "value":"from-csv-annotations-value"}]'
107111
spec:
108112
installModes:
113+
- type: SingleNamespace
114+
supported: true
115+
- type: OwnNamespace
116+
supported: true
109117
- type: AllNamespaces
110118
supported: true`)},
111119
}
@@ -144,7 +152,10 @@ func TestApply_Base(t *testing.T) {
144152

145153
t.Run("fails trying to obtain an action client", func(t *testing.T) {
146154
mockAcg := &mockActionGetter{actionClientForErr: errors.New("failed getting action client")}
147-
helmApplier := applier.Helm{ActionClientGetter: mockAcg}
155+
helmApplier := applier.Helm{
156+
ActionClientGetter: mockAcg,
157+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
158+
}
148159

149160
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
150161
require.Error(t, err)
@@ -155,7 +166,10 @@ func TestApply_Base(t *testing.T) {
155166

156167
t.Run("fails getting current release and !driver.ErrReleaseNotFound", func(t *testing.T) {
157168
mockAcg := &mockActionGetter{getClientErr: errors.New("failed getting current release")}
158-
helmApplier := applier.Helm{ActionClientGetter: mockAcg}
169+
helmApplier := applier.Helm{
170+
ActionClientGetter: mockAcg,
171+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
172+
}
159173

160174
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
161175
require.Error(t, err)
@@ -171,7 +185,10 @@ func TestApply_Installation(t *testing.T) {
171185
getClientErr: driver.ErrReleaseNotFound,
172186
dryRunInstallErr: errors.New("failed attempting to dry-run install chart"),
173187
}
174-
helmApplier := applier.Helm{ActionClientGetter: mockAcg}
188+
helmApplier := applier.Helm{
189+
ActionClientGetter: mockAcg,
190+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
191+
}
175192

176193
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
177194
require.Error(t, err)
@@ -186,7 +203,11 @@ func TestApply_Installation(t *testing.T) {
186203
installErr: errors.New("failed installing chart"),
187204
}
188205
mockPf := &mockPreflight{installErr: errors.New("failed during install pre-flight check")}
189-
helmApplier := applier.Helm{ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}}
206+
helmApplier := applier.Helm{
207+
ActionClientGetter: mockAcg,
208+
Preflights: []applier.Preflight{mockPf},
209+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
210+
}
190211

191212
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
192213
require.Error(t, err)
@@ -200,7 +221,10 @@ func TestApply_Installation(t *testing.T) {
200221
getClientErr: driver.ErrReleaseNotFound,
201222
installErr: errors.New("failed installing chart"),
202223
}
203-
helmApplier := applier.Helm{ActionClientGetter: mockAcg}
224+
helmApplier := applier.Helm{
225+
ActionClientGetter: mockAcg,
226+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
227+
}
204228

205229
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
206230
require.Error(t, err)
@@ -217,7 +241,10 @@ func TestApply_Installation(t *testing.T) {
217241
Manifest: validManifest,
218242
},
219243
}
220-
helmApplier := applier.Helm{ActionClientGetter: mockAcg}
244+
helmApplier := applier.Helm{
245+
ActionClientGetter: mockAcg,
246+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
247+
}
221248

222249
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
223250
require.NoError(t, err)
@@ -236,7 +263,10 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
236263
getClientErr: driver.ErrReleaseNotFound,
237264
dryRunInstallErr: errors.New("failed attempting to dry-run install chart"),
238265
}
239-
helmApplier := applier.Helm{ActionClientGetter: mockAcg}
266+
helmApplier := applier.Helm{
267+
ActionClientGetter: mockAcg,
268+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
269+
}
240270

241271
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
242272
require.Error(t, err)
@@ -251,7 +281,11 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
251281
installErr: errors.New("failed installing chart"),
252282
}
253283
mockPf := &mockPreflight{installErr: errors.New("failed during install pre-flight check")}
254-
helmApplier := applier.Helm{ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}}
284+
helmApplier := applier.Helm{
285+
ActionClientGetter: mockAcg,
286+
Preflights: []applier.Preflight{mockPf},
287+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
288+
}
255289

256290
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
257291
require.Error(t, err)
@@ -265,7 +299,10 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
265299
getClientErr: driver.ErrReleaseNotFound,
266300
installErr: errors.New("failed installing chart"),
267301
}
268-
helmApplier := applier.Helm{ActionClientGetter: mockAcg}
302+
helmApplier := applier.Helm{
303+
ActionClientGetter: mockAcg,
304+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
305+
}
269306

270307
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
271308
require.Error(t, err)
@@ -282,7 +319,10 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
282319
Manifest: validManifest,
283320
},
284321
}
285-
helmApplier := applier.Helm{ActionClientGetter: mockAcg}
322+
helmApplier := applier.Helm{
323+
ActionClientGetter: mockAcg,
324+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
325+
}
286326

287327
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
288328
require.NoError(t, err)
@@ -302,7 +342,10 @@ func TestApply_Upgrade(t *testing.T) {
302342
mockAcg := &mockActionGetter{
303343
dryRunUpgradeErr: errors.New("failed attempting to dry-run upgrade chart"),
304344
}
305-
helmApplier := applier.Helm{ActionClientGetter: mockAcg}
345+
helmApplier := applier.Helm{
346+
ActionClientGetter: mockAcg,
347+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
348+
}
306349

307350
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
308351
require.Error(t, err)
@@ -321,7 +364,11 @@ func TestApply_Upgrade(t *testing.T) {
321364
desiredRel: &testDesiredRelease,
322365
}
323366
mockPf := &mockPreflight{upgradeErr: errors.New("failed during upgrade pre-flight check")}
324-
helmApplier := applier.Helm{ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}}
367+
helmApplier := applier.Helm{
368+
ActionClientGetter: mockAcg,
369+
Preflights: []applier.Preflight{mockPf},
370+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
371+
}
325372

326373
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
327374
require.Error(t, err)
@@ -340,7 +387,10 @@ func TestApply_Upgrade(t *testing.T) {
340387
desiredRel: &testDesiredRelease,
341388
}
342389
mockPf := &mockPreflight{}
343-
helmApplier := applier.Helm{ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}}
390+
helmApplier := applier.Helm{
391+
ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf},
392+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
393+
}
344394

345395
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
346396
require.Error(t, err)
@@ -359,7 +409,11 @@ func TestApply_Upgrade(t *testing.T) {
359409
desiredRel: &testDesiredRelease,
360410
}
361411
mockPf := &mockPreflight{}
362-
helmApplier := applier.Helm{ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}}
412+
helmApplier := applier.Helm{
413+
ActionClientGetter: mockAcg,
414+
Preflights: []applier.Preflight{mockPf},
415+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
416+
}
363417

364418
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
365419
require.Error(t, err)
@@ -376,7 +430,10 @@ func TestApply_Upgrade(t *testing.T) {
376430
currentRel: testCurrentRelease,
377431
desiredRel: &testDesiredRelease,
378432
}
379-
helmApplier := applier.Helm{ActionClientGetter: mockAcg}
433+
helmApplier := applier.Helm{
434+
ActionClientGetter: mockAcg,
435+
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
436+
}
380437

381438
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
382439
require.NoError(t, err)
@@ -386,3 +443,76 @@ func TestApply_Upgrade(t *testing.T) {
386443
assert.Equal(t, "service-b", objs[1].GetName())
387444
})
388445
}
446+
447+
func TestApply_InstallationWithSingleOwnNamespaceInstallSupportEnabled(t *testing.T) {
448+
featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.SingleOwnNamespaceInstallSupport, true)
449+
450+
t.Run("generates bundle resources in SingleNamespace install mode when watch namespace is configured", func(t *testing.T) {
451+
var expectedWatchNamespace = "watch-namespace"
452+
453+
helmApplier := applier.Helm{
454+
ActionClientGetter: &mockActionGetter{
455+
getClientErr: driver.ErrReleaseNotFound,
456+
desiredRel: &release.Release{
457+
Info: &release.Info{Status: release.StatusDeployed},
458+
Manifest: validManifest,
459+
},
460+
},
461+
BundleToHelmChartFn: func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) {
462+
require.Equal(t, expectedWatchNamespace, watchNamespace)
463+
return convert.RegistryV1ToHelmChart(rv1, installNamespace, watchNamespace)
464+
},
465+
}
466+
467+
testExt := &v1.ClusterExtension{
468+
ObjectMeta: metav1.ObjectMeta{
469+
Name: "testExt",
470+
Annotations: map[string]string{
471+
applier.AnnotationClusterExtensionWatchNamespace: expectedWatchNamespace,
472+
},
473+
},
474+
}
475+
476+
_, _, _ = helmApplier.Apply(context.TODO(), validFS, testExt, testObjectLabels, testStorageLabels)
477+
})
478+
}
479+
480+
func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) {
481+
t.Run("generates bundle resources in AllNamespaces install mode", func(t *testing.T) {
482+
var expectedWatchNamespace = corev1.NamespaceAll
483+
484+
helmApplier := applier.Helm{
485+
ActionClientGetter: &mockActionGetter{
486+
getClientErr: driver.ErrReleaseNotFound,
487+
desiredRel: &release.Release{
488+
Info: &release.Info{Status: release.StatusDeployed},
489+
Manifest: validManifest,
490+
},
491+
},
492+
BundleToHelmChartFn: func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) {
493+
require.Equal(t, expectedWatchNamespace, watchNamespace)
494+
return convert.RegistryV1ToHelmChart(rv1, installNamespace, watchNamespace)
495+
},
496+
}
497+
498+
_, _, _ = helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
499+
})
500+
501+
t.Run("surfaces chart generation errors", func(t *testing.T) {
502+
helmApplier := applier.Helm{
503+
ActionClientGetter: &mockActionGetter{
504+
getClientErr: driver.ErrReleaseNotFound,
505+
desiredRel: &release.Release{
506+
Info: &release.Info{Status: release.StatusDeployed},
507+
Manifest: validManifest,
508+
},
509+
},
510+
BundleToHelmChartFn: func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) {
511+
return nil, errors.New("some error")
512+
},
513+
}
514+
515+
_, _, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
516+
require.Error(t, err)
517+
})
518+
}

‎internal/operator-controller/applier/watchnamespace_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package applier_test
22

33
import (
4-
"github.com/operator-framework/operator-controller/internal/operator-controller/applier"
54
"testing"
65

76
"github.com/stretchr/testify/require"
@@ -10,6 +9,7 @@ import (
109
featuregatetesting "k8s.io/component-base/featuregate/testing"
1110

1211
v1 "github.com/operator-framework/operator-controller/api/v1"
12+
"github.com/operator-framework/operator-controller/internal/operator-controller/applier"
1313
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
1414
)
1515

0 commit comments

Comments
 (0)