Skip to content

✨ Add package name validation function #1901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion internal/operator-controller/applier/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ func (mag *mockActionGetter) Reconcile(rel *release.Release) error {
var (
// required for unmockable call to convert.RegistryV1ToHelmChart
validFS = fstest.MapFS{
"metadata/annotations.yaml": &fstest.MapFile{Data: []byte("{}")},
"metadata/annotations.yaml": &fstest.MapFile{Data: []byte(`annotations:
operators.operatorframework.io.bundle.package.v1: my-package`)},
"manifests": &fstest.MapFile{Data: []byte(`apiVersion: operators.operatorframework.io/v1alpha1
kind: ClusterServiceVersion
metadata:
Expand Down
18 changes: 12 additions & 6 deletions internal/operator-controller/rukpak/convert/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ var RegistryV1BundleValidator = BundleValidator{
CheckDeploymentSpecUniqueness,
CheckCRDResourceUniqueness,
CheckOwnedCRDExistence,
CheckPackageNameNotEmpty,
}

// CheckDeploymentSpecUniqueness checks that each strategy deployment spec in the csv has a unique name.
Expand All @@ -39,8 +40,7 @@ func CheckDeploymentSpecUniqueness(rv1 *RegistryV1) []error {
deploymentNameSet.Insert(dep.Name)
}

//nolint:prealloc
var errs []error
errs := make([]error, 0, len(duplicateDeploymentNames))
for _, d := range slices.Sorted(slices.Values(duplicateDeploymentNames.UnsortedList())) {
errs = append(errs, fmt.Errorf("cluster service version contains duplicate strategy deployment spec '%s'", d))
}
Expand All @@ -54,15 +54,14 @@ func CheckOwnedCRDExistence(rv1 *RegistryV1) []error {
crdsNames.Insert(crd.Name)
}

//nolint:prealloc
var errs []error
missingCRDNames := sets.Set[string]{}
for _, crd := range rv1.CSV.Spec.CustomResourceDefinitions.Owned {
if !crdsNames.Has(crd.Name) {
missingCRDNames.Insert(crd.Name)
}
}

errs := make([]error, 0, len(missingCRDNames))
for _, crdName := range slices.Sorted(slices.Values(missingCRDNames.UnsortedList())) {
errs = append(errs, fmt.Errorf("cluster service definition references owned custom resource definition '%s' not found in bundle", crdName))
}
Expand All @@ -80,10 +79,17 @@ func CheckCRDResourceUniqueness(rv1 *RegistryV1) []error {
crdsNames.Insert(crd.Name)
}

//nolint:prealloc
var errs []error
errs := make([]error, 0, len(duplicateCRDNames))
for _, crdName := range slices.Sorted(slices.Values(duplicateCRDNames.UnsortedList())) {
errs = append(errs, fmt.Errorf("bundle contains duplicate custom resource definition '%s'", crdName))
}
return errs
}

// CheckPackageNameNotEmpty checks that PackageName is not empty
func CheckPackageNameNotEmpty(rv1 *RegistryV1) []error {
if rv1.PackageName == "" {
return []error{errors.New("package name is empty")}
}
return nil
}
30 changes: 30 additions & 0 deletions internal/operator-controller/rukpak/convert/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ func Test_BundleValidatorHasAllValidationFns(t *testing.T) {
convert.CheckDeploymentSpecUniqueness,
convert.CheckCRDResourceUniqueness,
convert.CheckOwnedCRDExistence,
convert.CheckPackageNameNotEmpty,
}
actualValidationFns := convert.RegistryV1BundleValidator

Expand Down Expand Up @@ -60,6 +61,7 @@ func Test_CheckDeploymentSpecUniqueness(t *testing.T) {
),
),
},
expectedErrs: []error{},
}, {
name: "rejects bundles with duplicate deployment strategy spec names",
bundle: &convert.RegistryV1{
Expand Down Expand Up @@ -114,6 +116,7 @@ func Test_CRDResourceUniqueness(t *testing.T) {
{ObjectMeta: metav1.ObjectMeta{Name: "b.crd.something"}},
},
},
expectedErrs: []error{},
}, {
name: "rejects bundles with duplicate custom resource definition resources",
bundle: &convert.RegistryV1{CRDs: []apiextensionsv1.CustomResourceDefinition{
Expand Down Expand Up @@ -164,6 +167,7 @@ func Test_CheckOwnedCRDExistence(t *testing.T) {
),
),
},
expectedErrs: []error{},
}, {
name: "rejects bundles with missing owned custom resource definition resources",
bundle: &convert.RegistryV1{
Expand Down Expand Up @@ -201,6 +205,32 @@ func Test_CheckOwnedCRDExistence(t *testing.T) {
}
}

func Test_CheckPackageNameNotEmpty(t *testing.T) {
for _, tc := range []struct {
name string
bundle *convert.RegistryV1
expectedErrs []error
}{
{
name: "accepts bundles with non-empty package name",
bundle: &convert.RegistryV1{
PackageName: "not-empty",
},
}, {
name: "rejects bundles with empty package name",
bundle: &convert.RegistryV1{},
expectedErrs: []error{
errors.New("package name is empty"),
},
},
} {
t.Run(tc.name, func(t *testing.T) {
errs := convert.CheckPackageNameNotEmpty(tc.bundle)
require.Equal(t, tc.expectedErrs, errs)
})
}
}

type csvOption func(version *v1alpha1.ClusterServiceVersion)

func withStrategyDeploymentSpecs(strategyDeploymentSpecs ...v1alpha1.StrategyDeploymentSpec) csvOption {
Expand Down
Loading