Skip to content

Commit 56213a4

Browse files
perdasilvaPer Goncalves da Silva
perdasilva
and
Per Goncalves da Silva
authored
🌱 Deduplicate component generating registry+v1 manifests in appliers (#2251)
* Consolidate regv1 bundle manifest generation across appliers Signed-off-by: Per Goncalves da Silva <[email protected]> * Remove GetWatchNamespace Signed-off-by: Per Goncalves da Silva <[email protected]> --------- Signed-off-by: Per Goncalves da Silva <[email protected]> Co-authored-by: Per Goncalves da Silva <[email protected]>
1 parent 6604f2a commit 56213a4

File tree

10 files changed

+390
-622
lines changed

10 files changed

+390
-622
lines changed

‎cmd/operator-controller/main.go‎

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -448,10 +448,18 @@ func run() error {
448448
return err
449449
}
450450

451+
certProvider := getCertificateProvider()
452+
regv1ManifestProvider := &applier.RegistryV1ManifestProvider{
453+
BundleRenderer: registryv1.Renderer,
454+
CertificateProvider: certProvider,
455+
IsWebhookSupportEnabled: certProvider != nil,
456+
IsSingleOwnNamespaceEnabled: features.OperatorControllerFeatureGate.Enabled(features.SingleOwnNamespaceInstallSupport),
457+
}
458+
451459
if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) {
452-
err = setupBoxcutter(mgr, ceReconciler, preflights)
460+
err = setupBoxcutter(mgr, ceReconciler, preflights, regv1ManifestProvider)
453461
} else {
454-
err = setupHelm(mgr, ceReconciler, preflights, ceController, clusterExtensionFinalizers)
462+
err = setupHelm(mgr, ceReconciler, preflights, ceController, clusterExtensionFinalizers, regv1ManifestProvider)
455463
}
456464
if err != nil {
457465
setupLog.Error(err, "unable to setup lifecycler")
@@ -512,9 +520,12 @@ func getCertificateProvider() render.CertificateProvider {
512520
return nil
513521
}
514522

515-
func setupBoxcutter(mgr manager.Manager, ceReconciler *controllers.ClusterExtensionReconciler, preflights []applier.Preflight) error {
516-
certProvider := getCertificateProvider()
517-
523+
func setupBoxcutter(
524+
mgr manager.Manager,
525+
ceReconciler *controllers.ClusterExtensionReconciler,
526+
preflights []applier.Preflight,
527+
regv1ManifestProvider applier.ManifestProvider,
528+
) error {
518529
coreClient, err := corev1client.NewForConfig(mgr.GetConfig())
519530
if err != nil {
520531
return fmt.Errorf("unable to create core client: %w", err)
@@ -541,11 +552,8 @@ func setupBoxcutter(mgr manager.Manager, ceReconciler *controllers.ClusterExtens
541552
// TODO: better scheme handling - which types do we want to support?
542553
_ = apiextensionsv1.AddToScheme(mgr.GetScheme())
543554
rg := &applier.SimpleRevisionGenerator{
544-
Scheme: mgr.GetScheme(),
545-
BundleRenderer: &applier.RegistryV1BundleRenderer{
546-
BundleRenderer: registryv1.Renderer,
547-
CertificateProvider: certProvider,
548-
},
555+
Scheme: mgr.GetScheme(),
556+
ManifestProvider: regv1ManifestProvider,
549557
}
550558
ceReconciler.Applier = &applier.Boxcutter{
551559
Client: mgr.GetClient(),
@@ -611,6 +619,7 @@ func setupHelm(
611619
preflights []applier.Preflight,
612620
ceController crcontroller.Controller,
613621
clusterExtensionFinalizers crfinalizer.Registerer,
622+
regv1ManifestProvider applier.ManifestProvider,
614623
) error {
615624
coreClient, err := corev1client.NewForConfig(mgr.GetConfig())
616625
if err != nil {
@@ -658,17 +667,12 @@ func setupHelm(
658667
return err
659668
}
660669

661-
certProvider := getCertificateProvider()
662-
663670
// now initialize the helmApplier, assigning the potentially nil preAuth
664671
ceReconciler.Applier = &applier.Helm{
665672
ActionClientGetter: acg,
666673
Preflights: preflights,
667674
HelmChartProvider: &applier.RegistryV1HelmChartProvider{
668-
BundleRenderer: registryv1.Renderer,
669-
CertificateProvider: certProvider,
670-
IsWebhookSupportEnabled: certProvider != nil,
671-
IsSingleOwnNamespaceEnabled: features.OperatorControllerFeatureGate.Enabled(features.SingleOwnNamespaceInstallSupport),
675+
ManifestProvider: regv1ManifestProvider,
672676
},
673677
HelmReleaseToObjectsConverter: &applier.HelmReleaseToObjectsConverter{},
674678
PreAuthorizer: preAuth,

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

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ import (
2727
ocv1 "github.com/operator-framework/operator-controller/api/v1"
2828
"github.com/operator-framework/operator-controller/internal/operator-controller/controllers"
2929
"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
30-
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
31-
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render"
3230
hashutil "github.com/operator-framework/operator-controller/internal/shared/util/hash"
3331
)
3432

@@ -46,8 +44,8 @@ type ClusterExtensionRevisionGenerator interface {
4644
}
4745

4846
type SimpleRevisionGenerator struct {
49-
Scheme *runtime.Scheme
50-
BundleRenderer BundleRenderer
47+
Scheme *runtime.Scheme
48+
ManifestProvider ManifestProvider
5149
}
5250

5351
func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease(
@@ -92,7 +90,7 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
9290
objectLabels, revisionAnnotations map[string]string,
9391
) (*ocv1.ClusterExtensionRevision, error) {
9492
// extract plain manifests
95-
plain, err := r.BundleRenderer.Render(bundleFS, ext)
93+
plain, err := r.ManifestProvider.Get(bundleFS, ext)
9694
if err != nil {
9795
return nil, err
9896
}
@@ -359,34 +357,6 @@ func latestRevisionNumber(prevRevisions []ocv1.ClusterExtensionRevision) int64 {
359357
return prevRevisions[len(prevRevisions)-1].Spec.Revision
360358
}
361359

362-
// TODO: in the next refactor iteration BundleRenderer and RegistryV1BundleRenderer into the RegistryV1ChartProvider
363-
364-
type BundleRenderer interface {
365-
Render(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error)
366-
}
367-
368-
type RegistryV1BundleRenderer struct {
369-
BundleRenderer render.BundleRenderer
370-
CertificateProvider render.CertificateProvider
371-
}
372-
373-
func (r *RegistryV1BundleRenderer) Render(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) {
374-
reg, err := source.FromFS(bundleFS).GetBundle()
375-
if err != nil {
376-
return nil, err
377-
}
378-
379-
if len(reg.CSV.Spec.WebhookDefinitions) > 0 && r.CertificateProvider == nil {
380-
return nil, fmt.Errorf("unsupported bundle: webhookDefinitions are not supported")
381-
}
382-
383-
watchNamespace, err := GetWatchNamespace(ext)
384-
if err != nil {
385-
return nil, err
386-
}
387-
return r.BundleRenderer.Render(reg, ext.Spec.Namespace, render.WithTargetNamespaces(watchNamespace), render.WithCertificateProvider(r.CertificateProvider))
388-
}
389-
390360
func splitManifestDocuments(file string) []string {
391361
//nolint:prealloc
392362
var docs []string

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

Lines changed: 37 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -24,75 +24,12 @@ import (
2424
"sigs.k8s.io/controller-runtime/pkg/client"
2525
"sigs.k8s.io/controller-runtime/pkg/client/fake"
2626

27-
"github.com/operator-framework/api/pkg/operators/v1alpha1"
28-
2927
ocv1 "github.com/operator-framework/operator-controller/api/v1"
3028
"github.com/operator-framework/operator-controller/internal/operator-controller/applier"
3129
"github.com/operator-framework/operator-controller/internal/operator-controller/controllers"
3230
"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
33-
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle"
34-
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render"
35-
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing/bundlefs"
36-
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing/clusterserviceversion"
3731
)
3832

39-
func Test_RegistryV1BundleRenderer_Render_Success(t *testing.T) {
40-
expectedObjs := []client.Object{
41-
&corev1.Service{
42-
ObjectMeta: metav1.ObjectMeta{
43-
Name: "test-service",
44-
},
45-
},
46-
}
47-
r := applier.RegistryV1BundleRenderer{
48-
BundleRenderer: render.BundleRenderer{
49-
ResourceGenerators: []render.ResourceGenerator{
50-
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
51-
require.Equal(t, []string{""}, opts.TargetNamespaces)
52-
require.Equal(t, "some-namespace", opts.InstallNamespace)
53-
return expectedObjs, nil
54-
},
55-
},
56-
},
57-
}
58-
bundleFS := bundlefs.Builder().
59-
WithPackageName("some-package").
60-
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()).
61-
Build()
62-
objs, err := r.Render(bundleFS, &ocv1.ClusterExtension{
63-
Spec: ocv1.ClusterExtensionSpec{
64-
Namespace: "some-namespace",
65-
},
66-
})
67-
require.NoError(t, err)
68-
require.Equal(t, expectedObjs, objs)
69-
}
70-
71-
func Test_RegistryV1BundleRenderer_Render_Failure(t *testing.T) {
72-
var expectedObjs []client.Object
73-
r := applier.RegistryV1BundleRenderer{
74-
BundleRenderer: render.BundleRenderer{
75-
ResourceGenerators: []render.ResourceGenerator{
76-
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
77-
return expectedObjs, fmt.Errorf("some-error")
78-
},
79-
},
80-
},
81-
}
82-
bundleFS := bundlefs.Builder().
83-
WithPackageName("some-package").
84-
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()).
85-
Build()
86-
objs, err := r.Render(bundleFS, &ocv1.ClusterExtension{
87-
Spec: ocv1.ClusterExtensionSpec{
88-
Namespace: "some-namespace",
89-
},
90-
})
91-
require.Nil(t, objs)
92-
require.Error(t, err)
93-
require.Contains(t, err.Error(), "some-error")
94-
}
95-
9633
func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T) {
9734
g := &applier.SimpleRevisionGenerator{}
9835

@@ -175,24 +112,26 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
175112
}
176113

177114
func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
178-
var r mockBundleRenderer = func(_ fs.FS, _ *ocv1.ClusterExtension) ([]client.Object, error) {
179-
return []client.Object{
180-
&corev1.Service{
181-
ObjectMeta: metav1.ObjectMeta{
182-
Name: "test-service",
115+
r := &FakeManifestProvider{
116+
GetFn: func(_ fs.FS, _ *ocv1.ClusterExtension) ([]client.Object, error) {
117+
return []client.Object{
118+
&corev1.Service{
119+
ObjectMeta: metav1.ObjectMeta{
120+
Name: "test-service",
121+
},
183122
},
184-
},
185-
&appsv1.Deployment{
186-
ObjectMeta: metav1.ObjectMeta{
187-
Name: "test-deployment",
123+
&appsv1.Deployment{
124+
ObjectMeta: metav1.ObjectMeta{
125+
Name: "test-deployment",
126+
},
188127
},
189-
},
190-
}, nil
128+
}, nil
129+
},
191130
}
192131

193132
b := applier.SimpleRevisionGenerator{
194-
Scheme: k8scheme.Scheme,
195-
BundleRenderer: r,
133+
Scheme: k8scheme.Scheme,
134+
ManifestProvider: r,
196135
}
197136

198137
ext := &ocv1.ClusterExtension{
@@ -266,15 +205,17 @@ func Test_SimpleRevisionGenerator_Renderer_Integration(t *testing.T) {
266205
Name: "test-extension",
267206
},
268207
}
269-
var r mockBundleRenderer = func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) {
270-
t.Log("by checking renderer was called with the correct parameters")
271-
require.Equal(t, bundleFS, b)
272-
require.Equal(t, ext, e)
273-
return nil, nil
208+
r := &FakeManifestProvider{
209+
GetFn: func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) {
210+
t.Log("by checking renderer was called with the correct parameters")
211+
require.Equal(t, bundleFS, b)
212+
require.Equal(t, ext, e)
213+
return nil, nil
214+
},
274215
}
275216
b := applier.SimpleRevisionGenerator{
276-
Scheme: k8scheme.Scheme,
277-
BundleRenderer: r,
217+
Scheme: k8scheme.Scheme,
218+
ManifestProvider: r,
278219
}
279220

280221
_, err := b.GenerateRevision(bundleFS, ext, map[string]string{}, map[string]string{})
@@ -300,12 +241,15 @@ func Test_SimpleRevisionGenerator_AppliesObjectLabelsAndRevisionAnnotations(t *t
300241
},
301242
},
302243
}
303-
var r mockBundleRenderer = func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) {
304-
return renderedObjs, nil
244+
r := &FakeManifestProvider{
245+
GetFn: func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) {
246+
return renderedObjs, nil
247+
},
305248
}
249+
306250
b := applier.SimpleRevisionGenerator{
307-
Scheme: k8scheme.Scheme,
308-
BundleRenderer: r,
251+
Scheme: k8scheme.Scheme,
252+
ManifestProvider: r,
309253
}
310254

311255
revAnnotations := map[string]string{
@@ -330,12 +274,14 @@ func Test_SimpleRevisionGenerator_AppliesObjectLabelsAndRevisionAnnotations(t *t
330274
}
331275

332276
func Test_SimpleRevisionGenerator_Failure(t *testing.T) {
333-
var r mockBundleRenderer = func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) {
334-
return nil, fmt.Errorf("some-error")
277+
r := &FakeManifestProvider{
278+
GetFn: func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) {
279+
return nil, fmt.Errorf("some-error")
280+
},
335281
}
336282
b := applier.SimpleRevisionGenerator{
337-
Scheme: k8scheme.Scheme,
338-
BundleRenderer: r,
283+
Scheme: k8scheme.Scheme,
284+
ManifestProvider: r,
339285
}
340286

341287
rev, err := b.GenerateRevision(fstest.MapFS{}, &ocv1.ClusterExtension{}, map[string]string{}, map[string]string{})
@@ -928,12 +874,6 @@ func (m *mockBundleRevisionBuilder) GenerateRevisionFromHelmRelease(
928874
return nil, nil
929875
}
930876

931-
type mockBundleRenderer func(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error)
932-
933-
func (f mockBundleRenderer) Render(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) {
934-
return f(bundleFS, ext)
935-
}
936-
937877
type clientMock struct {
938878
mock.Mock
939879
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,13 @@ import (
2929
"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
3030
"github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager"
3131
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
32-
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
3332
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util"
3433
imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image"
3534
)
3635

3736
// HelmChartProvider provides helm charts from bundle sources and cluster extensions
3837
type HelmChartProvider interface {
39-
Get(bundle source.BundleSource, clusterExtension *ocv1.ClusterExtension) (*chart.Chart, error)
38+
Get(bundle fs.FS, clusterExtension *ocv1.ClusterExtension) (*chart.Chart, error)
4039
}
4140

4241
type HelmReleaseToObjectsConverter struct {
@@ -212,7 +211,7 @@ func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*char
212211
)
213212
}
214213
}
215-
return h.HelmChartProvider.Get(source.FromFS(bundleFS), ext)
214+
return h.HelmChartProvider.Get(bundleFS, ext)
216215
}
217216

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

0 commit comments

Comments
 (0)