Skip to content

Commit 3bc7496

Browse files
perdasilvaPer Goncalves da Silva
perdasilva
and
Per Goncalves da Silva
authored
✨ Add registry+v1 bundle validation framework (#1885)
* Add CRDs field to RegistryV1 struct Signed-off-by: Per Goncalves da Silva <[email protected]> * Add registry+v1 bundlvalidators Signed-off-by: Per Goncalves da Silva <[email protected]> * Refactor converter to use validator 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 e490bd9 commit 3bc7496

File tree

5 files changed

+381
-26
lines changed

5 files changed

+381
-26
lines changed

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

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
appsv1 "k8s.io/api/apps/v1"
1414
corev1 "k8s.io/api/core/v1"
1515
rbacv1 "k8s.io/api/rbac/v1"
16+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1617
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1718
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1819
"k8s.io/apimachinery/pkg/runtime"
@@ -32,6 +33,7 @@ import (
3233
type RegistryV1 struct {
3334
PackageName string
3435
CSV v1alpha1.ClusterServiceVersion
36+
CRDs []apiextensionsv1.CustomResourceDefinition
3537
Others []unstructured.Unstructured
3638
}
3739

@@ -45,7 +47,7 @@ func RegistryV1ToHelmChart(rv1 fs.FS, installNamespace string, watchNamespace st
4547
return nil, err
4648
}
4749

48-
plain, err := Convert(reg, installNamespace, []string{watchNamespace})
50+
plain, err := PlainConverter.Convert(reg, installNamespace, []string{watchNamespace})
4951
if err != nil {
5052
return nil, err
5153
}
@@ -121,6 +123,12 @@ func ParseFS(rv1 fs.FS) (RegistryV1, error) {
121123
}
122124
reg.CSV = csv
123125
foundCSV = true
126+
case "CustomResourceDefinition":
127+
crd := apiextensionsv1.CustomResourceDefinition{}
128+
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(info.Object.(*unstructured.Unstructured).Object, &crd); err != nil {
129+
return err
130+
}
131+
reg.CRDs = append(reg.CRDs, crd)
124132
default:
125133
reg.Others = append(reg.Others, *info.Object.(*unstructured.Unstructured))
126134
}
@@ -222,15 +230,23 @@ func saNameOrDefault(saName string) string {
222230
return saName
223231
}
224232

225-
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+
226242
if installNamespace == "" {
227-
installNamespace = in.CSV.Annotations["operatorframework.io/suggested-namespace"]
243+
installNamespace = rv1.CSV.Annotations["operatorframework.io/suggested-namespace"]
228244
}
229245
if installNamespace == "" {
230-
installNamespace = fmt.Sprintf("%s-system", in.PackageName)
246+
installNamespace = fmt.Sprintf("%s-system", rv1.PackageName)
231247
}
232248
supportedInstallModes := sets.New[string]()
233-
for _, im := range in.CSV.Spec.InstallModes {
249+
for _, im := range rv1.CSV.Spec.InstallModes {
234250
if im.Supported {
235251
supportedInstallModes.Insert(string(im.Type))
236252
}
@@ -247,18 +263,18 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
247263
return nil, err
248264
}
249265

250-
if len(in.CSV.Spec.APIServiceDefinitions.Owned) > 0 {
266+
if len(rv1.CSV.Spec.APIServiceDefinitions.Owned) > 0 {
251267
return nil, fmt.Errorf("apiServiceDefintions are not supported")
252268
}
253269

254-
if len(in.CSV.Spec.WebhookDefinitions) > 0 {
270+
if len(rv1.CSV.Spec.WebhookDefinitions) > 0 {
255271
return nil, fmt.Errorf("webhookDefinitions are not supported")
256272
}
257273

258274
deployments := []appsv1.Deployment{}
259275
serviceAccounts := map[string]corev1.ServiceAccount{}
260-
for _, depSpec := range in.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs {
261-
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)
262278
annotations["olm.targetNamespaces"] = strings.Join(targetNamespaces, ",")
263279
depSpec.Spec.Template.Annotations = annotations
264280
deployments = append(deployments, appsv1.Deployment{
@@ -292,8 +308,8 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
292308
clusterRoles := []rbacv1.ClusterRole{}
293309
clusterRoleBindings := []rbacv1.ClusterRoleBinding{}
294310

295-
permissions := in.CSV.Spec.InstallStrategy.StrategySpec.Permissions
296-
clusterPermissions := in.CSV.Spec.InstallStrategy.StrategySpec.ClusterPermissions
311+
permissions := rv1.CSV.Spec.InstallStrategy.StrategySpec.Permissions
312+
clusterPermissions := rv1.CSV.Spec.InstallStrategy.StrategySpec.ClusterPermissions
297313
allPermissions := append(permissions, clusterPermissions...)
298314

299315
// Create all the service accounts
@@ -320,7 +336,7 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
320336
for _, ns := range targetNamespaces {
321337
for _, permission := range permissions {
322338
saName := saNameOrDefault(permission.ServiceAccountName)
323-
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)
324340
if err != nil {
325341
return nil, err
326342
}
@@ -331,7 +347,7 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
331347

332348
for _, permission := range clusterPermissions {
333349
saName := saNameOrDefault(permission.ServiceAccountName)
334-
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)
335351
if err != nil {
336352
return nil, err
337353
}
@@ -362,7 +378,10 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
362378
obj := obj
363379
objs = append(objs, &obj)
364380
}
365-
for _, obj := range in.Others {
381+
for _, obj := range rv1.CRDs {
382+
objs = append(objs, &obj)
383+
}
384+
for _, obj := range rv1.Others {
366385
obj := obj
367386
supported, namespaced := registrybundle.IsSupported(obj.GetKind())
368387
if !supported {
@@ -380,6 +399,10 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
380399
return &Plain{Objects: objs}, nil
381400
}
382401

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

385408
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: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
package convert
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"slices"
7+
8+
"k8s.io/apimachinery/pkg/util/sets"
9+
)
10+
11+
type BundleValidator []func(v1 *RegistryV1) []error
12+
13+
func (v BundleValidator) Validate(rv1 *RegistryV1) error {
14+
var errs []error
15+
for _, validator := range v {
16+
errs = append(errs, validator(rv1)...)
17+
}
18+
return errors.Join(errs...)
19+
}
20+
21+
var RegistryV1BundleValidator = BundleValidator{
22+
// NOTE: if you update this list, Test_BundleValidatorHasAllValidationFns will fail until
23+
// you bring the same changes over to that test. This helps ensure all validation rules are executed
24+
// while giving us the flexibility to test each validation function individually
25+
CheckDeploymentSpecUniqueness,
26+
CheckCRDResourceUniqueness,
27+
CheckOwnedCRDExistence,
28+
}
29+
30+
// CheckDeploymentSpecUniqueness checks that each strategy deployment spec in the csv has a unique name.
31+
// Errors are sorted by deployment name.
32+
func CheckDeploymentSpecUniqueness(rv1 *RegistryV1) []error {
33+
deploymentNameSet := sets.Set[string]{}
34+
duplicateDeploymentNames := sets.Set[string]{}
35+
for _, dep := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs {
36+
if deploymentNameSet.Has(dep.Name) {
37+
duplicateDeploymentNames.Insert(dep.Name)
38+
}
39+
deploymentNameSet.Insert(dep.Name)
40+
}
41+
42+
//nolint:prealloc
43+
var errs []error
44+
for _, d := range slices.Sorted(slices.Values(duplicateDeploymentNames.UnsortedList())) {
45+
errs = append(errs, fmt.Errorf("cluster service version contains duplicate strategy deployment spec '%s'", d))
46+
}
47+
return errs
48+
}
49+
50+
// CheckOwnedCRDExistence checks bundle owned custom resource definitions declared in the csv exist in the bundle
51+
func CheckOwnedCRDExistence(rv1 *RegistryV1) []error {
52+
crdsNames := sets.Set[string]{}
53+
for _, crd := range rv1.CRDs {
54+
crdsNames.Insert(crd.Name)
55+
}
56+
57+
//nolint:prealloc
58+
var errs []error
59+
missingCRDNames := sets.Set[string]{}
60+
for _, crd := range rv1.CSV.Spec.CustomResourceDefinitions.Owned {
61+
if !crdsNames.Has(crd.Name) {
62+
missingCRDNames.Insert(crd.Name)
63+
}
64+
}
65+
66+
for _, crdName := range slices.Sorted(slices.Values(missingCRDNames.UnsortedList())) {
67+
errs = append(errs, fmt.Errorf("cluster service definition references owned custom resource definition '%s' not found in bundle", crdName))
68+
}
69+
return errs
70+
}
71+
72+
// CheckCRDResourceUniqueness checks that the bundle CRD names are unique
73+
func CheckCRDResourceUniqueness(rv1 *RegistryV1) []error {
74+
crdsNames := sets.Set[string]{}
75+
duplicateCRDNames := sets.Set[string]{}
76+
for _, crd := range rv1.CRDs {
77+
if crdsNames.Has(crd.Name) {
78+
duplicateCRDNames.Insert(crd.Name)
79+
}
80+
crdsNames.Insert(crd.Name)
81+
}
82+
83+
//nolint:prealloc
84+
var errs []error
85+
for _, crdName := range slices.Sorted(slices.Values(duplicateCRDNames.UnsortedList())) {
86+
errs = append(errs, fmt.Errorf("bundle contains duplicate custom resource definition '%s'", crdName))
87+
}
88+
return errs
89+
}

0 commit comments

Comments
 (0)