Skip to content

Commit 998a9fe

Browse files
tmshortjoelanford
tmshort
authored and
joelanford
committed
Boxcutter Preflight
Signed-off-by: Todd Short <[email protected]>
1 parent d87c932 commit 998a9fe

File tree

7 files changed

+171
-99
lines changed

7 files changed

+171
-99
lines changed

‎cmd/operator-controller/main.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,7 @@ func run() error {
458458
CertificateProvider: certProvider,
459459
},
460460
},
461+
Preflights: preflights,
461462
}
462463
ctrlBuilderOpts = append(ctrlBuilderOpts, controllers.WithOwns(&ocv1.ClusterExtensionRevision{}))
463464
} else {
@@ -470,7 +471,8 @@ func run() error {
470471
CertificateProvider: certProvider,
471472
IsWebhookSupportEnabled: certProvider != nil,
472473
},
473-
PreAuthorizer: preAuth,
474+
PreAuthorizer: preAuth,
475+
HelmReleaseToObjectsConverter: &applier.HelmReleaseToObjectsConverter{},
474476
}
475477
}
476478

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

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,24 @@ type Boxcutter struct {
9797
Client client.Client
9898
Scheme *runtime.Scheme
9999
RevisionGenerator ClusterExtensionRevisionGenerator
100+
Preflights []Preflight
100101
}
101102

102103
func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, storageLabels map[string]string) ([]client.Object, string, error) {
103104
objs, err := bc.apply(ctx, contentFS, ext, objectLabels, storageLabels)
104105
return objs, "", err
105106
}
106107

108+
func (bc *Boxcutter) getObjects(rev *ocv1.ClusterExtensionRevision) []client.Object {
109+
var objs []client.Object
110+
for _, phase := range rev.Spec.Phases {
111+
for _, phaseObject := range phase.Objects {
112+
objs = append(objs, &phaseObject.Object)
113+
}
114+
}
115+
return objs
116+
}
117+
107118
func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, _ map[string]string) ([]client.Object, error) {
108119
// Generate desired revision
109120
desiredRevision, err := bc.RevisionGenerator.GenerateRevision(contentFS, ext, objectLabels)
@@ -122,6 +133,7 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
122133
var (
123134
currentRevision *ocv1.ClusterExtensionRevision
124135
)
136+
state := StateNeedsInstall
125137
if len(existingRevisions) > 0 {
126138
maybeCurrentRevision := existingRevisions[len(existingRevisions)-1]
127139
annotations := maybeCurrentRevision.GetAnnotations()
@@ -130,6 +142,27 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
130142
currentRevision = &maybeCurrentRevision
131143
}
132144
}
145+
state = StateNeedsUpgrade
146+
}
147+
148+
// Preflights
149+
plainObjs := bc.getObjects(desiredRevision)
150+
for _, preflight := range bc.Preflights {
151+
if shouldSkipPreflight(ctx, preflight, ext, state) {
152+
continue
153+
}
154+
switch state {
155+
case StateNeedsInstall:
156+
err := preflight.Install(ctx, plainObjs)
157+
if err != nil {
158+
return nil, err
159+
}
160+
case StateNeedsUpgrade:
161+
err := preflight.Upgrade(ctx, plainObjs)
162+
if err != nil {
163+
return nil, err
164+
}
165+
}
133166
}
134167

135168
if currentRevision == nil {
@@ -164,13 +197,7 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
164197
// TODO: Read status from revision.
165198

166199
// Collect objects
167-
var plain []client.Object
168-
for _, phase := range desiredRevision.Spec.Phases {
169-
for _, phaseObject := range phase.Objects {
170-
plain = append(plain, &phaseObject.Object)
171-
}
172-
}
173-
return plain, nil
200+
return bc.getObjects(desiredRevision), nil
174201
}
175202

176203
// getExistingRevisions returns the list of ClusterExtensionRevisions for a ClusterExtension with name extName in revision order (oldest to newest)

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

Lines changed: 28 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -20,71 +20,47 @@ import (
2020
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2121
apimachyaml "k8s.io/apimachinery/pkg/util/yaml"
2222
"sigs.k8s.io/controller-runtime/pkg/client"
23-
"sigs.k8s.io/controller-runtime/pkg/log"
2423

2524
helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
2625

2726
ocv1 "github.com/operator-framework/operator-controller/api/v1"
2827
"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
2928
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
3029
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
31-
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety"
3230
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util"
3331
imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image"
3432
)
3533

36-
const (
37-
StateNeedsInstall string = "NeedsInstall"
38-
StateNeedsUpgrade string = "NeedsUpgrade"
39-
StateUnchanged string = "Unchanged"
40-
StateError string = "Error"
41-
maxHelmReleaseHistory = 10
42-
)
43-
44-
// Preflight is a check that should be run before making any changes to the cluster
45-
type Preflight interface {
46-
// Install runs checks that should be successful prior
47-
// to installing the Helm release. It is provided
48-
// a Helm release and returns an error if the
49-
// check is unsuccessful
50-
Install(context.Context, *release.Release) error
51-
52-
// Upgrade runs checks that should be successful prior
53-
// to upgrading the Helm release. It is provided
54-
// a Helm release and returns an error if the
55-
// check is unsuccessful
56-
Upgrade(context.Context, *release.Release) error
57-
}
58-
5934
type BundleToHelmChartConverter interface {
6035
ToHelmChart(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error)
6136
}
6237

63-
type Helm struct {
64-
ActionClientGetter helmclient.ActionClientGetter
65-
Preflights []Preflight
66-
PreAuthorizer authorization.PreAuthorizer
67-
BundleToHelmChartConverter BundleToHelmChartConverter
38+
type HelmReleaseToObjectsConverter struct {
39+
Mock bool
6840
}
6941

70-
// shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND
71-
// if it is set to enforcement None.
72-
func shouldSkipPreflight(ctx context.Context, preflight Preflight, ext *ocv1.ClusterExtension, state string) bool {
73-
l := log.FromContext(ctx)
74-
hasCRDUpgradeSafety := ext.Spec.Install != nil && ext.Spec.Install.Preflight != nil && ext.Spec.Install.Preflight.CRDUpgradeSafety != nil
75-
_, isCRDUpgradeSafetyInstance := preflight.(*crdupgradesafety.Preflight)
42+
type HelmReleaseToObjectsConverterInterface interface {
43+
GetObjectsFromRelease(rel *release.Release) ([]client.Object, error)
44+
}
7645

77-
if hasCRDUpgradeSafety && isCRDUpgradeSafetyInstance {
78-
if state == StateNeedsInstall || state == StateNeedsUpgrade {
79-
l.Info("crdUpgradeSafety ", "policy", ext.Spec.Install.Preflight.CRDUpgradeSafety.Enforcement)
80-
}
81-
if ext.Spec.Install.Preflight.CRDUpgradeSafety.Enforcement == ocv1.CRDUpgradeSafetyEnforcementNone {
82-
// Skip this preflight check because it is of type *crdupgradesafety.Preflight and the CRD Upgrade Safety
83-
// policy is set to None
84-
return true
85-
}
46+
func (h HelmReleaseToObjectsConverter) GetObjectsFromRelease(rel *release.Release) ([]client.Object, error) {
47+
if rel == nil || h.Mock {
48+
return nil, nil
49+
}
50+
51+
relObjects, err := util.ManifestObjects(strings.NewReader(rel.Manifest), fmt.Sprintf("%s-release-manifest", rel.Name))
52+
if err != nil {
53+
return nil, fmt.Errorf("parsing release %q objects: %w", rel.Name, err)
8654
}
87-
return false
55+
return relObjects, nil
56+
}
57+
58+
type Helm struct {
59+
ActionClientGetter helmclient.ActionClientGetter
60+
Preflights []Preflight
61+
PreAuthorizer authorization.PreAuthorizer
62+
BundleToHelmChartConverter BundleToHelmChartConverter
63+
HelmReleaseToObjectsConverter HelmReleaseToObjectsConverterInterface
8864
}
8965

9066
// runPreAuthorizationChecks performs pre-authorization checks for a Helm release
@@ -149,19 +125,23 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
149125
if err != nil {
150126
return nil, "", fmt.Errorf("failed to get release state using server-side dry-run: %w", err)
151127
}
128+
objs, err := h.HelmReleaseToObjectsConverter.GetObjectsFromRelease(desiredRel)
129+
if err != nil {
130+
return nil, state, err
131+
}
152132

153133
for _, preflight := range h.Preflights {
154134
if shouldSkipPreflight(ctx, preflight, ext, state) {
155135
continue
156136
}
157137
switch state {
158138
case StateNeedsInstall:
159-
err := preflight.Install(ctx, desiredRel)
139+
err := preflight.Install(ctx, objs)
160140
if err != nil {
161141
return nil, state, err
162142
}
163143
case StateNeedsUpgrade:
164-
err := preflight.Upgrade(ctx, desiredRel)
144+
err := preflight.Upgrade(ctx, objs)
165145
if err != nil {
166146
return nil, state, err
167147
}

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

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,11 @@ func (p *mockPreAuthorizer) PreAuthorize(
4848
return p.missingRules, p.returnError
4949
}
5050

51-
func (mp *mockPreflight) Install(context.Context, *release.Release) error {
51+
func (mp *mockPreflight) Install(context.Context, []client.Object) error {
5252
return mp.installErr
5353
}
5454

55-
func (mp *mockPreflight) Upgrade(context.Context, *release.Release) error {
55+
func (mp *mockPreflight) Upgrade(context.Context, []client.Object) error {
5656
return mp.upgradeErr
5757
}
5858

@@ -248,9 +248,10 @@ 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-
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
251+
ActionClientGetter: mockAcg,
252+
Preflights: []applier.Preflight{mockPf},
253+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
254+
HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true},
254255
}
255256

256257
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -266,8 +267,9 @@ func TestApply_Installation(t *testing.T) {
266267
installErr: errors.New("failed installing chart"),
267268
}
268269
helmApplier := applier.Helm{
269-
ActionClientGetter: mockAcg,
270-
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
270+
ActionClientGetter: mockAcg,
271+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
272+
HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true},
271273
}
272274

273275
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -286,8 +288,9 @@ func TestApply_Installation(t *testing.T) {
286288
},
287289
}
288290
helmApplier := applier.Helm{
289-
ActionClientGetter: mockAcg,
290-
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
291+
ActionClientGetter: mockAcg,
292+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
293+
HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true},
291294
}
292295

293296
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -328,10 +331,11 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
328331
}
329332
mockPf := &mockPreflight{installErr: errors.New("failed during install pre-flight check")}
330333
helmApplier := applier.Helm{
331-
ActionClientGetter: mockAcg,
332-
Preflights: []applier.Preflight{mockPf},
333-
PreAuthorizer: &mockPreAuthorizer{nil, nil},
334-
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
334+
ActionClientGetter: mockAcg,
335+
Preflights: []applier.Preflight{mockPf},
336+
PreAuthorizer: &mockPreAuthorizer{nil, nil},
337+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
338+
HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true},
335339
}
336340

337341
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -408,9 +412,10 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
408412
},
409413
}
410414
helmApplier := applier.Helm{
411-
ActionClientGetter: mockAcg,
412-
PreAuthorizer: &mockPreAuthorizer{nil, nil},
413-
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
415+
ActionClientGetter: mockAcg,
416+
PreAuthorizer: &mockPreAuthorizer{nil, nil},
417+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
418+
HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true},
414419
}
415420

416421
// Use a ClusterExtension with valid Spec fields.
@@ -464,9 +469,10 @@ func TestApply_Upgrade(t *testing.T) {
464469
}
465470
mockPf := &mockPreflight{upgradeErr: errors.New("failed during upgrade pre-flight check")}
466471
helmApplier := applier.Helm{
467-
ActionClientGetter: mockAcg,
468-
Preflights: []applier.Preflight{mockPf},
469-
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
472+
ActionClientGetter: mockAcg,
473+
Preflights: []applier.Preflight{mockPf},
474+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
475+
HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true},
470476
}
471477

472478
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -488,7 +494,8 @@ func TestApply_Upgrade(t *testing.T) {
488494
mockPf := &mockPreflight{}
489495
helmApplier := applier.Helm{
490496
ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf},
491-
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
497+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
498+
HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true},
492499
}
493500

494501
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -509,9 +516,10 @@ func TestApply_Upgrade(t *testing.T) {
509516
}
510517
mockPf := &mockPreflight{}
511518
helmApplier := applier.Helm{
512-
ActionClientGetter: mockAcg,
513-
Preflights: []applier.Preflight{mockPf},
514-
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
519+
ActionClientGetter: mockAcg,
520+
Preflights: []applier.Preflight{mockPf},
521+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
522+
HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true},
515523
}
516524

517525
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -530,8 +538,9 @@ func TestApply_Upgrade(t *testing.T) {
530538
desiredRel: &testDesiredRelease,
531539
}
532540
helmApplier := applier.Helm{
533-
ActionClientGetter: mockAcg,
534-
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
541+
ActionClientGetter: mockAcg,
542+
BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{},
543+
HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true},
535544
}
536545

537546
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
@@ -562,6 +571,7 @@ func TestApply_InstallationWithSingleOwnNamespaceInstallSupportEnabled(t *testin
562571
return nil, nil
563572
},
564573
},
574+
HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true},
565575
}
566576

567577
testExt := &ocv1.ClusterExtension{
@@ -595,6 +605,7 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) {
595605
return nil, nil
596606
},
597607
},
608+
HelmReleaseToObjectsConverter: applier.HelmReleaseToObjectsConverter{Mock: true},
598609
}
599610

600611
_, _, _ = helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)

0 commit comments

Comments
 (0)