Skip to content

Commit 2af24e8

Browse files
author
Per Goncalves da Silva
committed
Refactor converter to use validator
Signed-off-by: Per Goncalves da Silva <[email protected]>
1 parent 1282f72 commit 2af24e8

File tree

4 files changed

+69
-36
lines changed

4 files changed

+69
-36
lines changed

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

+30-15
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func RegistryV1ToHelmChart(rv1 fs.FS, installNamespace string, watchNamespace st
4747
return nil, err
4848
}
4949

50-
plain, err := Convert(reg, installNamespace, []string{watchNamespace})
50+
plain, err := PlainConverter.Convert(reg, installNamespace, []string{watchNamespace})
5151
if err != nil {
5252
return nil, err
5353
}
@@ -230,15 +230,26 @@ func saNameOrDefault(saName string) string {
230230
return saName
231231
}
232232

233-
func Convert(in RegistryV1, installNamespace string, targetNamespaces []string) (*Plain, error) {
233+
type Converter struct {
234+
BundleValidator BundleValidator
235+
}
236+
237+
func (c Converter) Convert(rv1 RegistryV1, installNamespace string, targetNamespaces []string) (*Plain, error) {
238+
if c.BundleValidator == nil {
239+
panic("converter has nil bundle validator")
240+
}
241+
if err := c.BundleValidator.Validate(&rv1); err != nil {
242+
return nil, err
243+
}
244+
234245
if installNamespace == "" {
235-
installNamespace = in.CSV.Annotations["operatorframework.io/suggested-namespace"]
246+
installNamespace = rv1.CSV.Annotations["operatorframework.io/suggested-namespace"]
236247
}
237248
if installNamespace == "" {
238-
installNamespace = fmt.Sprintf("%s-system", in.PackageName)
249+
installNamespace = fmt.Sprintf("%s-system", rv1.PackageName)
239250
}
240251
supportedInstallModes := sets.New[string]()
241-
for _, im := range in.CSV.Spec.InstallModes {
252+
for _, im := range rv1.CSV.Spec.InstallModes {
242253
if im.Supported {
243254
supportedInstallModes.Insert(string(im.Type))
244255
}
@@ -255,18 +266,18 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
255266
return nil, err
256267
}
257268

258-
if len(in.CSV.Spec.APIServiceDefinitions.Owned) > 0 {
269+
if len(rv1.CSV.Spec.APIServiceDefinitions.Owned) > 0 {
259270
return nil, fmt.Errorf("apiServiceDefintions are not supported")
260271
}
261272

262-
if len(in.CSV.Spec.WebhookDefinitions) > 0 {
273+
if len(rv1.CSV.Spec.WebhookDefinitions) > 0 {
263274
return nil, fmt.Errorf("webhookDefinitions are not supported")
264275
}
265276

266277
deployments := []appsv1.Deployment{}
267278
serviceAccounts := map[string]corev1.ServiceAccount{}
268-
for _, depSpec := range in.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs {
269-
annotations := util.MergeMaps(in.CSV.Annotations, depSpec.Spec.Template.Annotations)
279+
for _, depSpec := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs {
280+
annotations := util.MergeMaps(rv1.CSV.Annotations, depSpec.Spec.Template.Annotations)
270281
annotations["olm.targetNamespaces"] = strings.Join(targetNamespaces, ",")
271282
depSpec.Spec.Template.Annotations = annotations
272283
deployments = append(deployments, appsv1.Deployment{
@@ -300,8 +311,8 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
300311
clusterRoles := []rbacv1.ClusterRole{}
301312
clusterRoleBindings := []rbacv1.ClusterRoleBinding{}
302313

303-
permissions := in.CSV.Spec.InstallStrategy.StrategySpec.Permissions
304-
clusterPermissions := in.CSV.Spec.InstallStrategy.StrategySpec.ClusterPermissions
314+
permissions := rv1.CSV.Spec.InstallStrategy.StrategySpec.Permissions
315+
clusterPermissions := rv1.CSV.Spec.InstallStrategy.StrategySpec.ClusterPermissions
305316
allPermissions := append(permissions, clusterPermissions...)
306317

307318
// Create all the service accounts
@@ -328,7 +339,7 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
328339
for _, ns := range targetNamespaces {
329340
for _, permission := range permissions {
330341
saName := saNameOrDefault(permission.ServiceAccountName)
331-
name, err := generateName(fmt.Sprintf("%s-%s", in.CSV.Name, saName), permission)
342+
name, err := generateName(fmt.Sprintf("%s-%s", rv1.CSV.Name, saName), permission)
332343
if err != nil {
333344
return nil, err
334345
}
@@ -339,7 +350,7 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
339350

340351
for _, permission := range clusterPermissions {
341352
saName := saNameOrDefault(permission.ServiceAccountName)
342-
name, err := generateName(fmt.Sprintf("%s-%s", in.CSV.Name, saName), permission)
353+
name, err := generateName(fmt.Sprintf("%s-%s", rv1.CSV.Name, saName), permission)
343354
if err != nil {
344355
return nil, err
345356
}
@@ -370,10 +381,10 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
370381
obj := obj
371382
objs = append(objs, &obj)
372383
}
373-
for _, obj := range in.CRDs {
384+
for _, obj := range rv1.CRDs {
374385
objs = append(objs, &obj)
375386
}
376-
for _, obj := range in.Others {
387+
for _, obj := range rv1.Others {
377388
obj := obj
378389
supported, namespaced := registrybundle.IsSupported(obj.GetKind())
379390
if !supported {
@@ -391,6 +402,10 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
391402
return &Plain{Objects: objs}, nil
392403
}
393404

405+
var PlainConverter = Converter{
406+
BundleValidator: RegistryV1BundleValidator,
407+
}
408+
394409
const maxNameLength = 63
395410

396411
func generateName(base string, o interface{}) (string, error) {

‎internal/operator-controller/rukpak/convert/registryv1_test.go

+30-11
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package convert_test
22

33
import (
4+
"errors"
45
"fmt"
56
"io/fs"
67
"os"
@@ -52,6 +53,24 @@ func getCsvAndService() (v1alpha1.ClusterServiceVersion, corev1.Service) {
5253
return csv, svc
5354
}
5455

56+
func TestConverterValidatesBundle(t *testing.T) {
57+
converter := convert.Converter{
58+
BundleValidator: []func(rv1 *convert.RegistryV1) []error{
59+
func(rv1 *convert.RegistryV1) []error {
60+
return []error{errors.New("test error")}
61+
},
62+
},
63+
}
64+
65+
_, err := converter.Convert(convert.RegistryV1{}, "installNamespace", []string{"watchNamespace"})
66+
require.Error(t, err)
67+
require.Contains(t, err.Error(), "test error")
68+
}
69+
70+
func TestPlainConverterUsedRegV1Validator(t *testing.T) {
71+
require.Equal(t, convert.RegistryV1BundleValidator, convert.PlainConverter.BundleValidator)
72+
}
73+
5574
func TestRegistryV1SuiteNamespaceNotAvailable(t *testing.T) {
5675
var targetNamespaces []string
5776

@@ -70,7 +89,7 @@ func TestRegistryV1SuiteNamespaceNotAvailable(t *testing.T) {
7089
}
7190

7291
t.Log("By converting to plain")
73-
plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, targetNamespaces)
92+
plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, targetNamespaces)
7493
require.NoError(t, err)
7594

7695
t.Log("By verifying if plain bundle has required objects")
@@ -104,7 +123,7 @@ func TestRegistryV1SuiteNamespaceAvailable(t *testing.T) {
104123
}
105124

106125
t.Log("By converting to plain")
107-
plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, targetNamespaces)
126+
plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, targetNamespaces)
108127
require.NoError(t, err)
109128

110129
t.Log("By verifying if plain bundle has required objects")
@@ -145,7 +164,7 @@ func TestRegistryV1SuiteNamespaceUnsupportedKind(t *testing.T) {
145164
}
146165

147166
t.Log("By converting to plain")
148-
plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, targetNamespaces)
167+
plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, targetNamespaces)
149168
require.Error(t, err)
150169
require.ErrorContains(t, err, "bundle contains unsupported resource")
151170
require.Nil(t, plainBundle)
@@ -179,7 +198,7 @@ func TestRegistryV1SuiteNamespaceClusterScoped(t *testing.T) {
179198
}
180199

181200
t.Log("By converting to plain")
182-
plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, targetNamespaces)
201+
plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, targetNamespaces)
183202
require.NoError(t, err)
184203

185204
t.Log("By verifying if plain bundle has required objects")
@@ -266,7 +285,7 @@ func TestRegistryV1SuiteGenerateAllNamespace(t *testing.T) {
266285
}
267286

268287
t.Log("By converting to plain")
269-
plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
288+
plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
270289
require.NoError(t, err)
271290

272291
t.Log("By verifying if plain bundle has required objects")
@@ -299,7 +318,7 @@ func TestRegistryV1SuiteGenerateMultiNamespace(t *testing.T) {
299318
}
300319

301320
t.Log("By converting to plain")
302-
plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
321+
plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
303322
require.NoError(t, err)
304323

305324
t.Log("By verifying if plain bundle has required objects")
@@ -332,7 +351,7 @@ func TestRegistryV1SuiteGenerateSingleNamespace(t *testing.T) {
332351
}
333352

334353
t.Log("By converting to plain")
335-
plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
354+
plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
336355
require.NoError(t, err)
337356

338357
t.Log("By verifying if plain bundle has required objects")
@@ -365,7 +384,7 @@ func TestRegistryV1SuiteGenerateOwnNamespace(t *testing.T) {
365384
}
366385

367386
t.Log("By converting to plain")
368-
plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
387+
plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
369388
require.NoError(t, err)
370389

371390
t.Log("By verifying if plain bundle has required objects")
@@ -470,7 +489,7 @@ func TestConvertInstallModeValidation(t *testing.T) {
470489
}
471490

472491
t.Log("By converting to plain")
473-
plainBundle, err := convert.Convert(registryv1Bundle, tc.installNamespace, tc.watchNamespaces)
492+
plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, tc.installNamespace, tc.watchNamespaces)
474493
require.Error(t, err)
475494
require.Nil(t, plainBundle)
476495
})
@@ -559,7 +578,7 @@ func TestRegistryV1SuiteGenerateNoWebhooks(t *testing.T) {
559578
}
560579

561580
t.Log("By converting to plain")
562-
plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
581+
plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
563582
require.Error(t, err)
564583
require.ErrorContains(t, err, "webhookDefinitions are not supported")
565584
require.Nil(t, plainBundle)
@@ -590,7 +609,7 @@ func TestRegistryV1SuiteGenerateNoAPISerciceDefinitions(t *testing.T) {
590609
}
591610

592611
t.Log("By converting to plain")
593-
plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
612+
plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
594613
require.Error(t, err)
595614
require.ErrorContains(t, err, "apiServiceDefintions are not supported")
596615
require.Nil(t, plainBundle)

‎internal/operator-controller/rukpak/convert/validator.go

+7-8
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package convert
22

33
import (
4+
"errors"
45
"fmt"
56
"slices"
67

@@ -9,23 +10,21 @@ import (
910

1011
type BundleValidator []func(v1 *RegistryV1) []error
1112

12-
func (v BundleValidator) Validate(rv1 *RegistryV1) []error {
13+
func (v BundleValidator) Validate(rv1 *RegistryV1) error {
1314
var errs []error
1415
for _, validator := range v {
1516
errs = append(errs, validator(rv1)...)
1617
}
17-
return errs
18+
return errors.Join(errs...)
1819
}
1920

20-
func NewBundleValidator() BundleValidator {
21+
var RegistryV1BundleValidator = BundleValidator{
2122
// NOTE: if you update this list, Test_BundleValidatorHasAllValidationFns will fail until
2223
// you bring the same changes over to that test. This helps ensure all validation rules are executed
2324
// while giving us the flexibility to test each validation function individually
24-
return BundleValidator{
25-
CheckDeploymentSpecUniqueness,
26-
CheckCRDResourceUniqueness,
27-
CheckOwnedCRDExistence,
28-
}
25+
CheckDeploymentSpecUniqueness,
26+
CheckCRDResourceUniqueness,
27+
CheckOwnedCRDExistence,
2928
}
3029

3130
// CheckDeploymentSpecUniqueness checks that each strategy deployment spec in the csv has a unique name.

‎internal/operator-controller/rukpak/convert/validator_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func Test_BundleValidatorHasAllValidationFns(t *testing.T) {
2020
convert.CheckCRDResourceUniqueness,
2121
convert.CheckOwnedCRDExistence,
2222
}
23-
actualValidationFns := convert.NewBundleValidator()
23+
actualValidationFns := convert.RegistryV1BundleValidator
2424

2525
require.Equal(t, len(expectedValidationFns), len(actualValidationFns))
2626
for i := range expectedValidationFns {
@@ -40,7 +40,7 @@ func Test_BundleValidatorCallsAllValidationFnsInOrder(t *testing.T) {
4040
return nil
4141
},
4242
}
43-
require.Empty(t, validator.Validate(nil))
43+
require.NoError(t, validator.Validate(nil))
4444
require.Equal(t, "hi", actual)
4545
}
4646

0 commit comments

Comments
 (0)