Skip to content

Commit c9df915

Browse files
perdasilvaPer G. da Silva
perdasilva
and
Per G. da Silva
authored
🌱 Refactor rukpack package for configurable registry+v1 rendering behavior (#1968)
* Refactor render package Signed-off-by: Per G. da Silva <[email protected]> * Remove plain converter Signed-off-by: Per G. da Silva <[email protected]> * RegistryV1ToHelmChart to BundleToHelmChartConverter Signed-off-by: Per G. da Silva <[email protected]> * Configure bundle to helm converter based on feature flags Signed-off-by: Per G. da Silva <[email protected]> --------- Signed-off-by: Per G. da Silva <[email protected]> Co-authored-by: Per G. da Silva <[email protected]>
1 parent 6f3a121 commit c9df915

File tree

24 files changed

+851
-816
lines changed

24 files changed

+851
-816
lines changed

‎cmd/operator-controller/main.go

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ 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"
71+
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/certproviders"
72+
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1"
7073
"github.com/operator-framework/operator-controller/internal/operator-controller/scheme"
7174
fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs"
7275
httputil "github.com/operator-framework/operator-controller/internal/shared/util/http"
@@ -189,7 +192,7 @@ func run() error {
189192
secretParts := strings.Split(cfg.globalPullSecret, "/")
190193
if len(secretParts) != 2 {
191194
err := fmt.Errorf("incorrect number of components")
192-
setupLog.Error(err, "value of global-pull-secret should be of the format <namespace>/<name>")
195+
setupLog.Error(err, "Value of global-pull-secret should be of the format <namespace>/<name>")
193196
return err
194197
}
195198
globalPullSecretKey = &k8stypes.NamespacedName{Name: secretParts[1], Namespace: secretParts[0]}
@@ -421,12 +424,25 @@ func run() error {
421424
preAuth = authorization.NewRBACPreAuthorizer(mgr.GetClient())
422425
}
423426

427+
// determine if a certificate provider should be set in the bundle renderer and feature support for the provider
428+
// based on the feature flag
429+
var certProvider render.CertificateProvider
430+
var isWebhookSupportEnabled bool
431+
if features.OperatorControllerFeatureGate.Enabled(features.WebhookProviderCertManager) {
432+
certProvider = certproviders.CertManagerCertificateProvider{}
433+
isWebhookSupportEnabled = true
434+
}
435+
424436
// now initialize the helmApplier, assigning the potentially nil preAuth
425437
helmApplier := &applier.Helm{
426-
ActionClientGetter: acg,
427-
Preflights: preflights,
428-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
429-
PreAuthorizer: preAuth,
438+
ActionClientGetter: acg,
439+
Preflights: preflights,
440+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{
441+
BundleRenderer: registryv1.Renderer,
442+
CertificateProvider: certProvider,
443+
IsWebhookSupportEnabled: isWebhookSupportEnabled,
444+
},
445+
PreAuthorizer: preAuth,
430446
}
431447

432448
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/bundle/source"
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 source.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(source.FromFS(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: 62 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"
@@ -27,6 +26,7 @@ import (
2726
"github.com/operator-framework/operator-controller/internal/operator-controller/applier"
2827
"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
2928
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
29+
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
3030
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert"
3131
)
3232

@@ -197,8 +197,8 @@ func TestApply_Base(t *testing.T) {
197197
t.Run("fails trying to obtain an action client", func(t *testing.T) {
198198
mockAcg := &mockActionGetter{actionClientForErr: errors.New("failed getting action client")}
199199
helmApplier := applier.Helm{
200-
ActionClientGetter: mockAcg,
201-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
200+
ActionClientGetter: mockAcg,
201+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
202202
}
203203

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

218218
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -230,8 +230,8 @@ func TestApply_Installation(t *testing.T) {
230230
dryRunInstallErr: errors.New("failed attempting to dry-run install chart"),
231231
}
232232
helmApplier := applier.Helm{
233-
ActionClientGetter: mockAcg,
234-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
233+
ActionClientGetter: mockAcg,
234+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
235235
}
236236

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

256256
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -266,8 +266,8 @@ func TestApply_Installation(t *testing.T) {
266266
installErr: errors.New("failed installing chart"),
267267
}
268268
helmApplier := applier.Helm{
269-
ActionClientGetter: mockAcg,
270-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
269+
ActionClientGetter: mockAcg,
270+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
271271
}
272272

273273
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -286,8 +286,8 @@ func TestApply_Installation(t *testing.T) {
286286
},
287287
}
288288
helmApplier := applier.Helm{
289-
ActionClientGetter: mockAcg,
290-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
289+
ActionClientGetter: mockAcg,
290+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
291291
}
292292

293293
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -306,8 +306,8 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
306306
dryRunInstallErr: errors.New("failed attempting to dry-run install chart"),
307307
}
308308
helmApplier := applier.Helm{
309-
ActionClientGetter: mockAcg,
310-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
309+
ActionClientGetter: mockAcg,
310+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
311311
}
312312

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

337337
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -350,9 +350,9 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
350350
},
351351
}
352352
helmApplier := applier.Helm{
353-
ActionClientGetter: mockAcg,
354-
PreAuthorizer: &mockPreAuthorizer{nil, errPreAuth},
355-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
353+
ActionClientGetter: mockAcg,
354+
PreAuthorizer: &mockPreAuthorizer{nil, errPreAuth},
355+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
356356
}
357357
// Use a ClusterExtension with valid Spec fields.
358358
validCE := &ocv1.ClusterExtension{
@@ -379,9 +379,9 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
379379
},
380380
}
381381
helmApplier := applier.Helm{
382-
ActionClientGetter: mockAcg,
383-
PreAuthorizer: &mockPreAuthorizer{missingRBAC, nil},
384-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
382+
ActionClientGetter: mockAcg,
383+
PreAuthorizer: &mockPreAuthorizer{missingRBAC, nil},
384+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
385385
}
386386
// Use a ClusterExtension with valid Spec fields.
387387
validCE := &ocv1.ClusterExtension{
@@ -408,9 +408,9 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
408408
},
409409
}
410410
helmApplier := applier.Helm{
411-
ActionClientGetter: mockAcg,
412-
PreAuthorizer: &mockPreAuthorizer{nil, nil},
413-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
411+
ActionClientGetter: mockAcg,
412+
PreAuthorizer: &mockPreAuthorizer{nil, nil},
413+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
414414
}
415415

416416
// Use a ClusterExtension with valid Spec fields.
@@ -442,8 +442,8 @@ func TestApply_Upgrade(t *testing.T) {
442442
dryRunUpgradeErr: errors.New("failed attempting to dry-run upgrade chart"),
443443
}
444444
helmApplier := applier.Helm{
445-
ActionClientGetter: mockAcg,
446-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
445+
ActionClientGetter: mockAcg,
446+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
447447
}
448448

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

472472
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -488,7 +488,7 @@ func TestApply_Upgrade(t *testing.T) {
488488
mockPf := &mockPreflight{}
489489
helmApplier := applier.Helm{
490490
ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf},
491-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
491+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
492492
}
493493

494494
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -509,9 +509,9 @@ func TestApply_Upgrade(t *testing.T) {
509509
}
510510
mockPf := &mockPreflight{}
511511
helmApplier := applier.Helm{
512-
ActionClientGetter: mockAcg,
513-
Preflights: []applier.Preflight{mockPf},
514-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
512+
ActionClientGetter: mockAcg,
513+
Preflights: []applier.Preflight{mockPf},
514+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
515515
}
516516

517517
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -530,8 +530,8 @@ func TestApply_Upgrade(t *testing.T) {
530530
desiredRel: &testDesiredRelease,
531531
}
532532
helmApplier := applier.Helm{
533-
ActionClientGetter: mockAcg,
534-
BundleToHelmChartFn: convert.RegistryV1ToHelmChart,
533+
ActionClientGetter: mockAcg,
534+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
535535
}
536536

537537
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -556,9 +556,11 @@ func TestApply_InstallationWithSingleOwnNamespaceInstallSupportEnabled(t *testin
556556
Manifest: validManifest,
557557
},
558558
},
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)
559+
BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{
560+
fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
561+
require.Equal(t, expectedWatchNamespace, watchNamespace)
562+
return nil, nil
563+
},
562564
},
563565
}
564566

@@ -587,9 +589,11 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) {
587589
Manifest: validManifest,
588590
},
589591
},
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)
592+
BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{
593+
fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
594+
require.Equal(t, expectedWatchNamespace, watchNamespace)
595+
return nil, nil
596+
},
593597
},
594598
}
595599

@@ -605,12 +609,22 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) {
605609
Manifest: validManifest,
606610
},
607611
},
608-
BundleToHelmChartFn: func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) {
609-
return nil, errors.New("some error")
612+
BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{
613+
fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
614+
return nil, errors.New("some error")
615+
},
610616
},
611617
}
612618

613619
_, _, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
614620
require.Error(t, err)
615621
})
616622
}
623+
624+
type fakeBundleToHelmChartConverter struct {
625+
fn func(source.BundleSource, string, string) (*chart.Chart, error)
626+
}
627+
628+
func (f fakeBundleToHelmChartConverter) ToHelmChart(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) {
629+
return f.fn(bundle, installNamespace, watchNamespace)
630+
}

‎internal/operator-controller/rukpak/bundle/registryv1.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package bundle
2+
3+
import (
4+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
5+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
6+
7+
"github.com/operator-framework/api/pkg/operators/v1alpha1"
8+
)
9+
10+
type RegistryV1 struct {
11+
PackageName string
12+
CSV v1alpha1.ClusterServiceVersion
13+
CRDs []apiextensionsv1.CustomResourceDefinition
14+
Others []unstructured.Unstructured
15+
}

0 commit comments

Comments
 (0)