Skip to content

Commit 87717a5

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 87717a5

File tree

4 files changed

+66
-36
lines changed

4 files changed

+66
-36
lines changed

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

Lines changed: 27 additions & 15 deletions
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,23 @@ 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 err := c.BundleValidator.Validate(&rv1); err != nil {
239+
return nil, err
240+
}
241+
234242
if installNamespace == "" {
235-
installNamespace = in.CSV.Annotations["operatorframework.io/suggested-namespace"]
243+
installNamespace = rv1.CSV.Annotations["operatorframework.io/suggested-namespace"]
236244
}
237245
if installNamespace == "" {
238-
installNamespace = fmt.Sprintf("%s-system", in.PackageName)
246+
installNamespace = fmt.Sprintf("%s-system", rv1.PackageName)
239247
}
240248
supportedInstallModes := sets.New[string]()
241-
for _, im := range in.CSV.Spec.InstallModes {
249+
for _, im := range rv1.CSV.Spec.InstallModes {
242250
if im.Supported {
243251
supportedInstallModes.Insert(string(im.Type))
244252
}
@@ -255,18 +263,18 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
255263
return nil, err
256264
}
257265

258-
if len(in.CSV.Spec.APIServiceDefinitions.Owned) > 0 {
266+
if len(rv1.CSV.Spec.APIServiceDefinitions.Owned) > 0 {
259267
return nil, fmt.Errorf("apiServiceDefintions are not supported")
260268
}
261269

262-
if len(in.CSV.Spec.WebhookDefinitions) > 0 {
270+
if len(rv1.CSV.Spec.WebhookDefinitions) > 0 {
263271
return nil, fmt.Errorf("webhookDefinitions are not supported")
264272
}
265273

266274
deployments := []appsv1.Deployment{}
267275
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)
276+
for _, depSpec := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs {
277+
annotations := util.MergeMaps(rv1.CSV.Annotations, depSpec.Spec.Template.Annotations)
270278
annotations["olm.targetNamespaces"] = strings.Join(targetNamespaces, ",")
271279
depSpec.Spec.Template.Annotations = annotations
272280
deployments = append(deployments, appsv1.Deployment{
@@ -300,8 +308,8 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
300308
clusterRoles := []rbacv1.ClusterRole{}
301309
clusterRoleBindings := []rbacv1.ClusterRoleBinding{}
302310

303-
permissions := in.CSV.Spec.InstallStrategy.StrategySpec.Permissions
304-
clusterPermissions := in.CSV.Spec.InstallStrategy.StrategySpec.ClusterPermissions
311+
permissions := rv1.CSV.Spec.InstallStrategy.StrategySpec.Permissions
312+
clusterPermissions := rv1.CSV.Spec.InstallStrategy.StrategySpec.ClusterPermissions
305313
allPermissions := append(permissions, clusterPermissions...)
306314

307315
// Create all the service accounts
@@ -328,7 +336,7 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
328336
for _, ns := range targetNamespaces {
329337
for _, permission := range permissions {
330338
saName := saNameOrDefault(permission.ServiceAccountName)
331-
name, err := generateName(fmt.Sprintf("%s-%s", in.CSV.Name, saName), permission)
339+
name, err := generateName(fmt.Sprintf("%s-%s", rv1.CSV.Name, saName), permission)
332340
if err != nil {
333341
return nil, err
334342
}
@@ -339,7 +347,7 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
339347

340348
for _, permission := range clusterPermissions {
341349
saName := saNameOrDefault(permission.ServiceAccountName)
342-
name, err := generateName(fmt.Sprintf("%s-%s", in.CSV.Name, saName), permission)
350+
name, err := generateName(fmt.Sprintf("%s-%s", rv1.CSV.Name, saName), permission)
343351
if err != nil {
344352
return nil, err
345353
}
@@ -370,10 +378,10 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
370378
obj := obj
371379
objs = append(objs, &obj)
372380
}
373-
for _, obj := range in.CRDs {
381+
for _, obj := range rv1.CRDs {
374382
objs = append(objs, &obj)
375383
}
376-
for _, obj := range in.Others {
384+
for _, obj := range rv1.Others {
377385
obj := obj
378386
supported, namespaced := registrybundle.IsSupported(obj.GetKind())
379387
if !supported {
@@ -391,6 +399,10 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
391399
return &Plain{Objects: objs}, nil
392400
}
393401

402+
var PlainConverter = Converter{
403+
BundleValidator: RegistryV1BundleValidator,
404+
}
405+
394406
const maxNameLength = 63
395407

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

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

Lines changed: 30 additions & 11 deletions
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

Lines changed: 7 additions & 8 deletions
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

Lines changed: 2 additions & 2 deletions
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)