Repository: operator-framework/operator-controller
Pull request #1901 (2025-04-04 15:11:36)

:sparkles: Add package name validation function
# Description

Adds a bundle validation check against the package name to ensure it is defined in the RegistryV1 bundle structure. And does some tidying adding array pre-allocations

## Reviewer Checklist

- [ ] API Go Documentation
- [ ] Tests: Unit Tests (and E2E Tests, if appropriate)
- [ ] Comprehensive Commit Messages
- [ ] Links to related GitHub Issue(s)


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1901?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 69.22%. Comparing base [(`115a122`)](https://app.codecov.io/gh/operator-framework/operator-controller/commit/115a1225aa7377f6cc9c1e5887e689d1d49e6ccd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) to head [(`b103288`)](https://app.codecov.io/gh/operator-framework/operator-controller/commit/b1032887a57ac0c555c82a5be8c69df4eabdb31c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework).

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #1901      +/-   ##
==========================================
+ Coverage   69.21%   69.22%   +0.01%     
==========================================
  Files          67       67              
  Lines        5302     5307       +5     
==========================================
+ Hits         3670     3674       +4     
- Misses       1399     1400       +1     
  Partials      233      233              
```

| [Flag](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1901/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | Coverage Δ | |
|---|---|---|
| [e2e](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1901/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | `50.55% <62.50%> (+0.06%)` | :arrow_up: |
| [unit](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1901/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | `57.00% <100.00%> (-0.04%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1901?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework).

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>

Patch:
diff --git a/internal/operator-controller/rukpak/convert/validator.go b/internal/operator-controller/rukpak/convert/validator.go
index 73060fa..e0e8135 100644
--- a/internal/operator-controller/rukpak/convert/validator.go
+++ b/internal/operator-controller/rukpak/convert/validator.go
@@ -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.
@@ -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))
 	}
@@ -54,8 +54,6 @@ 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) {
@@ -63,6 +61,7 @@ func CheckOwnedCRDExistence(rv1 *RegistryV1) []error {
 		}
 	}
 
+	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))
 	}
@@ -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
+}


Test patch:
diff --git a/internal/operator-controller/applier/helm_test.go b/internal/operator-controller/applier/helm_test.go
index faaa417..5b04517 100644
--- a/internal/operator-controller/applier/helm_test.go
+++ b/internal/operator-controller/applier/helm_test.go
@@ -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:
diff --git a/internal/operator-controller/rukpak/convert/validator_test.go b/internal/operator-controller/rukpak/convert/validator_test.go
index 7b218fb..99c78f4 100644
--- a/internal/operator-controller/rukpak/convert/validator_test.go
+++ b/internal/operator-controller/rukpak/convert/validator_test.go
@@ -19,6 +19,7 @@ func Test_BundleValidatorHasAllValidationFns(t *testing.T) {
 		convert.CheckDeploymentSpecUniqueness,
 		convert.CheckCRDResourceUniqueness,
 		convert.CheckOwnedCRDExistence,
+		convert.CheckPackageNameNotEmpty,
 	}
 	actualValidationFns := convert.RegistryV1BundleValidator
 
@@ -60,6 +61,7 @@ func Test_CheckDeploymentSpecUniqueness(t *testing.T) {
 					),
 				),
 			},
+			expectedErrs: []error{},
 		}, {
 			name: "rejects bundles with duplicate deployment strategy spec names",
 			bundle: &convert.RegistryV1{
@@ -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{
@@ -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{
@@ -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 {