Skip to content

Commit 0b14839

Browse files
author
Per G. da Silva
committed
RegistryV1ToHelmChart to BundleToHelmChartConverter
Signed-off-by: Per G. da Silva <[email protected]>
1 parent 443874b commit 0b14839

File tree

13 files changed

+454
-520
lines changed

13 files changed

+454
-520
lines changed

‎cmd/operator-controller/main.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ import (
6767
"github.com/operator-framework/operator-controller/internal/operator-controller/resolve"
6868
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert"
6969
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety"
70+
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1"
7071
"github.com/operator-framework/operator-controller/internal/operator-controller/scheme"
7172
fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs"
7273
httputil "github.com/operator-framework/operator-controller/internal/shared/util/http"
@@ -423,10 +424,12 @@ func run() error {
423424

424425
// now initialize the helmApplier, assigning the potentially nil preAuth
425426
helmApplier := &applier.Helm{
426-
ActionClientGetter: acg,
427-
Preflights: preflights,
428-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
429-
PreAuthorizer: preAuth,
427+
ActionClientGetter: acg,
428+
Preflights: preflights,
429+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{
430+
BundleRenderer: registryv1.Renderer,
431+
},
432+
PreAuthorizer: preAuth,
430433
}
431434

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

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

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626

2727
ocv1 "github.com/operator-framework/operator-controller/api/v1"
2828
"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
29+
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert"
2930
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety"
3031
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util"
3132
)
@@ -53,13 +54,15 @@ type Preflight interface {
5354
Upgrade(context.Context, *release.Release) error
5455
}
5556

56-
type BundleToHelmChartFn func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error)
57+
type BundleToHelmChartConverter interface {
58+
ToHelmChart(bundle convert.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error)
59+
}
5760

5861
type Helm struct {
59-
ActionClientGetter helmclient.ActionClientGetter
60-
Preflights []Preflight
61-
PreAuthorizer authorization.PreAuthorizer
62-
BundleToHelmChartFn BundleToHelmChartFn
62+
ActionClientGetter helmclient.ActionClientGetter
63+
Preflights []Preflight
64+
PreAuthorizer authorization.PreAuthorizer
65+
BundleToHelmChartConverter BundleToHelmChartConverter
6366
}
6467

6568
// shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND
@@ -199,14 +202,14 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
199202
}
200203

201204
func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) {
202-
if h.BundleToHelmChartFn == nil {
203-
return nil, errors.New("BundleToHelmChartFn is nil")
205+
if h.BundleToHelmChartConverter == nil {
206+
return nil, errors.New("BundleToHelmChartConverter is nil")
204207
}
205208
watchNamespace, err := GetWatchNamespace(ext)
206209
if err != nil {
207210
return nil, err
208211
}
209-
return h.BundleToHelmChartFn(bundleFS, ext.Spec.Namespace, watchNamespace)
212+
return h.BundleToHelmChartConverter.ToHelmChart(convert.FSBundleSource{FS: bundleFS}, ext.Spec.Namespace, watchNamespace)
210213
}
211214

212215
func (h *Helm) renderClientOnlyRelease(ctx context.Context, ext *ocv1.ClusterExtension, chrt *chart.Chart, values chartutil.Values, post postrender.PostRenderer) (*release.Release, error) {

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

Lines changed: 61 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"errors"
66
"io"
7-
"io/fs"
87
"os"
98
"testing"
109
"testing/fstest"
@@ -197,8 +196,8 @@ func TestApply_Base(t *testing.T) {
197196
t.Run("fails trying to obtain an action client", func(t *testing.T) {
198197
mockAcg := &mockActionGetter{actionClientForErr: errors.New("failed getting action client")}
199198
helmApplier := applier.Helm{
200-
ActionClientGetter: mockAcg,
201-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
199+
ActionClientGetter: mockAcg,
200+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
202201
}
203202

204203
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -211,8 +210,8 @@ func TestApply_Base(t *testing.T) {
211210
t.Run("fails getting current release and !driver.ErrReleaseNotFound", func(t *testing.T) {
212211
mockAcg := &mockActionGetter{getClientErr: errors.New("failed getting current release")}
213212
helmApplier := applier.Helm{
214-
ActionClientGetter: mockAcg,
215-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
213+
ActionClientGetter: mockAcg,
214+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
216215
}
217216

218217
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -230,8 +229,8 @@ func TestApply_Installation(t *testing.T) {
230229
dryRunInstallErr: errors.New("failed attempting to dry-run install chart"),
231230
}
232231
helmApplier := applier.Helm{
233-
ActionClientGetter: mockAcg,
234-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
232+
ActionClientGetter: mockAcg,
233+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
235234
}
236235

237236
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -248,9 +247,9 @@ func TestApply_Installation(t *testing.T) {
248247
}
249248
mockPf := &mockPreflight{installErr: errors.New("failed during install pre-flight check")}
250249
helmApplier := applier.Helm{
251-
ActionClientGetter: mockAcg,
252-
Preflights: []applier.Preflight{mockPf},
253-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
250+
ActionClientGetter: mockAcg,
251+
Preflights: []applier.Preflight{mockPf},
252+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
254253
}
255254

256255
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -266,8 +265,8 @@ func TestApply_Installation(t *testing.T) {
266265
installErr: errors.New("failed installing chart"),
267266
}
268267
helmApplier := applier.Helm{
269-
ActionClientGetter: mockAcg,
270-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
268+
ActionClientGetter: mockAcg,
269+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
271270
}
272271

273272
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -286,8 +285,8 @@ func TestApply_Installation(t *testing.T) {
286285
},
287286
}
288287
helmApplier := applier.Helm{
289-
ActionClientGetter: mockAcg,
290-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
288+
ActionClientGetter: mockAcg,
289+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
291290
}
292291

293292
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -306,8 +305,8 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
306305
dryRunInstallErr: errors.New("failed attempting to dry-run install chart"),
307306
}
308307
helmApplier := applier.Helm{
309-
ActionClientGetter: mockAcg,
310-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
308+
ActionClientGetter: mockAcg,
309+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
311310
}
312311

313312
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -328,10 +327,10 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
328327
}
329328
mockPf := &mockPreflight{installErr: errors.New("failed during install pre-flight check")}
330329
helmApplier := applier.Helm{
331-
ActionClientGetter: mockAcg,
332-
Preflights: []applier.Preflight{mockPf},
333-
PreAuthorizer: &mockPreAuthorizer{nil, nil},
334-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
330+
ActionClientGetter: mockAcg,
331+
Preflights: []applier.Preflight{mockPf},
332+
PreAuthorizer: &mockPreAuthorizer{nil, nil},
333+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
335334
}
336335

337336
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -350,9 +349,9 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
350349
},
351350
}
352351
helmApplier := applier.Helm{
353-
ActionClientGetter: mockAcg,
354-
PreAuthorizer: &mockPreAuthorizer{nil, errPreAuth},
355-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
352+
ActionClientGetter: mockAcg,
353+
PreAuthorizer: &mockPreAuthorizer{nil, errPreAuth},
354+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
356355
}
357356
// Use a ClusterExtension with valid Spec fields.
358357
validCE := &ocv1.ClusterExtension{
@@ -379,9 +378,9 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
379378
},
380379
}
381380
helmApplier := applier.Helm{
382-
ActionClientGetter: mockAcg,
383-
PreAuthorizer: &mockPreAuthorizer{missingRBAC, nil},
384-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
381+
ActionClientGetter: mockAcg,
382+
PreAuthorizer: &mockPreAuthorizer{missingRBAC, nil},
383+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
385384
}
386385
// Use a ClusterExtension with valid Spec fields.
387386
validCE := &ocv1.ClusterExtension{
@@ -408,9 +407,9 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
408407
},
409408
}
410409
helmApplier := applier.Helm{
411-
ActionClientGetter: mockAcg,
412-
PreAuthorizer: &mockPreAuthorizer{nil, nil},
413-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
410+
ActionClientGetter: mockAcg,
411+
PreAuthorizer: &mockPreAuthorizer{nil, nil},
412+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
414413
}
415414

416415
// Use a ClusterExtension with valid Spec fields.
@@ -442,8 +441,8 @@ func TestApply_Upgrade(t *testing.T) {
442441
dryRunUpgradeErr: errors.New("failed attempting to dry-run upgrade chart"),
443442
}
444443
helmApplier := applier.Helm{
445-
ActionClientGetter: mockAcg,
446-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
444+
ActionClientGetter: mockAcg,
445+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
447446
}
448447

449448
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -464,9 +463,9 @@ func TestApply_Upgrade(t *testing.T) {
464463
}
465464
mockPf := &mockPreflight{upgradeErr: errors.New("failed during upgrade pre-flight check")}
466465
helmApplier := applier.Helm{
467-
ActionClientGetter: mockAcg,
468-
Preflights: []applier.Preflight{mockPf},
469-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
466+
ActionClientGetter: mockAcg,
467+
Preflights: []applier.Preflight{mockPf},
468+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
470469
}
471470

472471
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -488,7 +487,7 @@ func TestApply_Upgrade(t *testing.T) {
488487
mockPf := &mockPreflight{}
489488
helmApplier := applier.Helm{
490489
ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf},
491-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
490+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
492491
}
493492

494493
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -509,9 +508,9 @@ func TestApply_Upgrade(t *testing.T) {
509508
}
510509
mockPf := &mockPreflight{}
511510
helmApplier := applier.Helm{
512-
ActionClientGetter: mockAcg,
513-
Preflights: []applier.Preflight{mockPf},
514-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
511+
ActionClientGetter: mockAcg,
512+
Preflights: []applier.Preflight{mockPf},
513+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
515514
}
516515

517516
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -530,8 +529,8 @@ func TestApply_Upgrade(t *testing.T) {
530529
desiredRel: &testDesiredRelease,
531530
}
532531
helmApplier := applier.Helm{
533-
ActionClientGetter: mockAcg,
534-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
532+
ActionClientGetter: mockAcg,
533+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
535534
}
536535

537536
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -556,9 +555,11 @@ func TestApply_InstallationWithSingleOwnNamespaceInstallSupportEnabled(t *testin
556555
Manifest: validManifest,
557556
},
558557
},
559-
BundleToHelmChartFn: func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) {
560-
require.Equal(t, expectedWatchNamespace, watchNamespace)
561-
return convert.RegistryV1ToHelmChart(rv1, installNamespace, watchNamespace)
558+
BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{
559+
fn: func(bundle convert.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
560+
require.Equal(t, expectedWatchNamespace, watchNamespace)
561+
return nil, nil
562+
},
562563
},
563564
}
564565

@@ -587,9 +588,11 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) {
587588
Manifest: validManifest,
588589
},
589590
},
590-
BundleToHelmChartFn: func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) {
591-
require.Equal(t, expectedWatchNamespace, watchNamespace)
592-
return convert.RegistryV1ToHelmChart(rv1, installNamespace, watchNamespace)
591+
BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{
592+
fn: func(bundle convert.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
593+
require.Equal(t, expectedWatchNamespace, watchNamespace)
594+
return nil, nil
595+
},
593596
},
594597
}
595598

@@ -605,12 +608,22 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) {
605608
Manifest: validManifest,
606609
},
607610
},
608-
BundleToHelmChartFn: func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) {
609-
return nil, errors.New("some error")
611+
BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{
612+
fn: func(bundle convert.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
613+
return nil, errors.New("some error")
614+
},
610615
},
611616
}
612617

613618
_, _, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
614619
require.Error(t, err)
615620
})
616621
}
622+
623+
type fakeBundleToHelmChartConverter struct {
624+
fn func(convert.BundleSource, string, string) (*chart.Chart, error)
625+
}
626+
627+
func (f fakeBundleToHelmChartConverter) ToHelmChart(bundle convert.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
628+
return f.fn(bundle, installNamespace, watchNamespace)
629+
}

0 commit comments

Comments
 (0)