✨ chnange: move check bundle name to the Good Practices validator (#238)

operator-framework · Jun 24, 2022 · 1c4cba2 · 1c4cba2
1 parent cb9dc0e
commit 1c4cba2
Show file tree

Hide file tree

Showing 4 changed files with 111 additions and 116 deletions.
diff --git a/pkg/validation/internal/good_practices.go b/pkg/validation/internal/good_practices.go
@@ -1,10 +1,13 @@
 package internal
 
 import (
-	goerrors "errors"
 	"fmt"
+	"regexp"
 	"strings"
 
+	goerrors "errors"
+	"github.com/blang/semver/v4"
+
 	"github.com/operator-framework/api/pkg/manifests"
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
@@ -30,6 +33,9 @@ import (
 // b) "An Operator shouldn't deploy or manage other operators (such patterns are known as meta or super operators or include CRDs in its Operands). It's the Operator Lifecycle Manager's job to manage the deployment and lifecycle of operators. For further information check Dependency Resolution: https://olm.operatorframework.io/docs/concepts/olm-architecture/dependency-resolution/"
 //
 // WARNING: if you create CRD's via the reconciliations or via the Operands then, OLM cannot handle CRDs migration and update, validation.
+// - The bundle name (CSV.metadata.name) does not follow the naming convention: <operator-name>.v<semver> e.g. memcached-operator.v0.0.1
+//
+// NOTE: The bundle name must be 63 characters or less because it will be used as k8s ownerref label which only allows max of 63 characters.
 var GoodPracticesValidator interfaces.Validator = interfaces.ValidatorFunc(goodPracticesValidator)
 
 func goodPracticesValidator(objs ...interface{}) (results []errors.ManifestResult) {
@@ -60,6 +66,8 @@ func validateGoodPracticesFrom(bundle *manifests.Bundle) errors.ManifestResult {
 	warns = append(warns, validateCrdDescriptions(bundle.CSV.Spec.CustomResourceDefinitions)...)
 	warns = append(warns, validateHubChannels(bundle))
 	warns = append(warns, validateRBACForCRDsWith(bundle.CSV))
+	warns = append(warns, checkBundleName(bundle.CSV)...)
+
 	for _, err := range errs {
 		if err != nil {
 			result.Add(errors.ErrFailedValidation(err.Error(), bundle.CSV.GetName()))
@@ -96,6 +104,34 @@ func validateResourceRequests(csv *operatorsv1alpha1.ClusterServiceVersion) (err
 	return errs, warns
 }
 
+// checkBundleName will validate the operator bundle name informed via CSV.metadata.name.
+// The motivation for the following check is to ensure that operators authors knows that operator bundles names should
+// follow a name and versioning convention
+func checkBundleName(csv *operatorsv1alpha1.ClusterServiceVersion) []error {
+	var warns []error
+	// Check if is following the semver
+	re := regexp.MustCompile("([0-9]+)\\.([0-9]+)\\.([0-9]+)(?:-([0-9A-Za-z-]+(?:\\.[0-9A-Za-z-]+)*))?(?:\\+[0-9A-Za-z-]+)?$")
+	match := re.FindStringSubmatch(csv.Name)
+
+	if len(match) > 0 {
+		if _, err := semver.Parse(match[0]); err != nil {
+			warns = append(warns, fmt.Errorf("csv.metadata.Name %v is not following the versioning "+
+				"convention (MAJOR.MINOR.PATCH e.g 0.0.1): https://semver.org/", csv.Name))
+		}
+	} else {
+		warns = append(warns, fmt.Errorf("csv.metadata.Name %v is not following the versioning "+
+			"convention (MAJOR.MINOR.PATCH e.g 0.0.1): https://semver.org/", csv.Name))
+	}
+
+	// Check if its following the name convention
+	if len(strings.Split(csv.Name, ".v")) != 2 {
+		warns = append(warns, fmt.Errorf("csv.metadata.Name %v is not following the recommended "+
+			"naming convention: <operator-name>.v<semver> e.g. memcached-operator.v0.0.1", csv.Name))
+	}
+
+	return warns
+}
+
 // validateHubChannels will check the channels. The motivation for the following check is to ensure that operators
 // authors knows if their operator bundles are or not respecting the Naming Convention Rules.
 // However, the operator authors still able to choose the names as please them.

diff --git a/pkg/validation/internal/good_practices_test.go b/pkg/validation/internal/good_practices_test.go
@@ -242,3 +242,77 @@ func TestValidateRBACForCRDsWith(t *testing.T) {
 		})
 	}
 }
+
+func TestCheckBundleName(t *testing.T) {
+	type args struct {
+		bundleName string
+	}
+	tests := []struct {
+		name        string
+		args        args
+		wantWarning bool
+		errStrings  []string
+		warnStrings []string
+	}{
+		{
+			name: "should work with valid bundle name",
+			args: args{bundleName: "memcached-operator.v0.9.2"},
+		},
+		{
+			name:        "should return a warning when the bundle name is not following the convention",
+			args:        args{bundleName: "memcached-operator0.9.2"},
+			wantWarning: true,
+			warnStrings: []string{"csv.metadata.Name memcached-operator0.9.2 is not following the recommended naming " +
+				"convention: <operator-name>.v<semver> e.g. memcached-operator.v0.0.1"},
+		},
+		{
+			name:        "should return a warning when the bundle name version is not following semver",
+			args:        args{bundleName: "memcached-operator.v1"},
+			wantWarning: true,
+			warnStrings: []string{"csv.metadata.Name memcached-operator.v1 is not following the versioning convention " +
+				"(MAJOR.MINOR.PATCH e.g 0.0.1): https://semver.org/"},
+		},
+		{
+			name:        "should return a warning when the bundle name version is not following semver",
+			args:        args{bundleName: "memcached-operator.v1"},
+			wantWarning: true,
+			warnStrings: []string{"csv.metadata.Name memcached-operator.v1 is not following the " +
+				"versioning convention (MAJOR.MINOR.PATCH e.g 0.0.1): https://semver.org/"},
+		},
+		{
+			name:        "should return a warning when the bundle name version is not following semver",
+			args:        args{bundleName: "memcached-operator.v1--1.0"},
+			wantWarning: true,
+			warnStrings: []string{"csv.metadata.Name memcached-operator.v1--1.0 is not following the " +
+				"versioning convention (MAJOR.MINOR.PATCH e.g 0.0.1): https://semver.org/"},
+		},
+		{
+			name:        "should return a warning when the bundle name version is not following semver",
+			args:        args{bundleName: "memcached-operator.v1.3"},
+			wantWarning: true,
+			warnStrings: []string{"csv.metadata.Name memcached-operator.v1.3 is not following the " +
+				"versioning convention (MAJOR.MINOR.PATCH e.g 0.0.1): https://semver.org/"},
+		},
+		{
+			name: "should not warning for patch releases",
+			args: args{bundleName: "memcached-operator.v0.9.2+alpha"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+
+			csv := operatorsv1alpha1.ClusterServiceVersion{}
+			csv.Name = tt.args.bundleName
+			result := checkBundleName(&csv)
+
+			require.Equal(t, tt.wantWarning, len(result) > 0)
+			if tt.wantWarning {
+				require.Equal(t, len(tt.warnStrings), len(result))
+				for _, w := range result {
+					wString := w.Error()
+					require.Contains(t, tt.warnStrings, wString)
+				}
+			}
+		})
+	}
+}
diff --git a/pkg/validation/internal/operatorhub.go b/pkg/validation/internal/operatorhub.go
@@ -8,7 +8,6 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strings"
 
 	"github.com/blang/semver/v4"
@@ -111,10 +110,6 @@ import (
 // - If informed ONLY, check if the value csv.Spec.MinKubeVersion is parsable according to semver (https://semver.org/)
 // Also, this validator will raise warnings when:
 //
-// - The bundle name (CSV.metadata.name) does not follow the naming convention: <operator-name>.v<semver> e.g. memcached-operator.v0.0.1
-//
-// NOTE: The bundle name must be 63 characters or less because it will be used as k8s ownerref label which only allows max of 63 characters.
-//
 // - The channel names seems are not following the convention https://olm.operatorframework.io/docs/best-practices/channel-naming/
 //
 // - The usage of the removed APIs on Kubernetes 1.22 is found. More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22
@@ -230,7 +225,6 @@ func validateHubCSVSpec(csv v1alpha1.ClusterServiceVersion) CSVChecks {
 	checks = checkSpecVersion(checks)
 	checks = checkSpecIcon(checks)
 	checks = checkSpecMinKubeVersion(checks)
-	checks = checkBundleName(checks)
 
 	return checks
 }
@@ -241,34 +235,6 @@ type CSVChecks struct {
 	warns []error
 }
 
-// checkBundleName will validate the operator bundle name informed via CSV.metadata.name.
-// The motivation for the following check is to ensure that operators authors knows that operator bundles names should
-// follow a name and versioning convention
-func checkBundleName(checks CSVChecks) CSVChecks {
-
-	// Check if is following the semver
-	re := regexp.MustCompile("([0-9]+)\\.([0-9]+)\\.([0-9]+)(?:-([0-9A-Za-z-]+(?:\\.[0-9A-Za-z-]+)*))?(?:\\+[0-9A-Za-z-]+)?$")
-	match := re.FindStringSubmatch(checks.csv.Name)
-
-	if len(match) > 0 {
-		if _, err := semver.Parse(match[0]); err != nil {
-			checks.warns = append(checks.warns, fmt.Errorf("csv.metadata.Name %v is not following the versioning "+
-				"convention (MAJOR.MINOR.PATCH e.g 0.0.1): https://semver.org/", checks.csv.Name))
-		}
-	} else {
-		checks.warns = append(checks.warns, fmt.Errorf("csv.metadata.Name %v is not following the versioning "+
-			"convention (MAJOR.MINOR.PATCH e.g 0.0.1): https://semver.org/", checks.csv.Name))
-	}
-
-	// Check if its following the name convention
-	if len(strings.Split(checks.csv.Name, ".v")) < 2 {
-		checks.warns = append(checks.errs, fmt.Errorf("csv.metadata.Name %v is not following the recommended "+
-			"naming convention: <operator-name>.v<semver> e.g. memcached-operator.v0.0.1", checks.csv.Name))
-	}
-
-	return checks
-}
-
 // checkSpecMinKubeVersion will validate the spec minKubeVersion informed via CSV.spec.minKubeVersion
 func checkSpecMinKubeVersion(checks CSVChecks) CSVChecks {
 	if len(strings.TrimSpace(checks.csv.Spec.MinKubeVersion)) == 0 {

diff --git a/pkg/validation/internal/operatorhub_test.go b/pkg/validation/internal/operatorhub_test.go
@@ -204,87 +204,6 @@ func TestCheckSpecIcon(t *testing.T) {
 	}
 }
 
-func TestCheckBundleName(t *testing.T) {
-	type args struct {
-		bundleName string
-	}
-	tests := []struct {
-		name        string
-		args        args
-		wantError   bool
-		wantWarning bool
-		errStrings  []string
-		warnStrings []string
-	}{
-		{
-			name: "should work with valid bundle name",
-			args: args{bundleName: "memcached-operator.v0.9.2"},
-		},
-		{
-			name:        "should return a warning when the bundle name is not following the convention",
-			args:        args{bundleName: "memcached-operator0.9.2"},
-			wantWarning: true,
-			warnStrings: []string{"csv.metadata.Name memcached-operator0.9.2 is not following the recommended naming " +
-				"convention: <operator-name>.v<semver> e.g. memcached-operator.v0.0.1"},
-		},
-		{
-			name:        "should return a warning when the bundle name version is not following semver",
-			args:        args{bundleName: "memcached-operator.v1"},
-			wantWarning: true,
-			warnStrings: []string{"csv.metadata.Name memcached-operator.v1 is not following the versioning convention " +
-				"(MAJOR.MINOR.PATCH e.g 0.0.1): https://semver.org/"},
-		},
-		{
-			name:        "should return a warning when the bundle name version is not following semver",
-			args:        args{bundleName: "memcached-operator.v1"},
-			wantWarning: true,
-			warnStrings: []string{"csv.metadata.Name memcached-operator.v1 is not following the " +
-				"versioning convention (MAJOR.MINOR.PATCH e.g 0.0.1): https://semver.org/"},
-		},
-		{
-			name:        "should return a warning when the bundle name version is not following semver",
-			args:        args{bundleName: "memcached-operator.v1--1.0"},
-			wantWarning: true,
-			warnStrings: []string{"csv.metadata.Name memcached-operator.v1--1.0 is not following the " +
-				"versioning convention (MAJOR.MINOR.PATCH e.g 0.0.1): https://semver.org/"},
-		},
-		{
-			name:        "should return a warning when the bundle name version is not following semver",
-			args:        args{bundleName: "memcached-operator.v1.3"},
-			wantWarning: true,
-			warnStrings: []string{"csv.metadata.Name memcached-operator.v1.3 is not following the " +
-				"versioning convention (MAJOR.MINOR.PATCH e.g 0.0.1): https://semver.org/"},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-
-			csv := v1alpha1.ClusterServiceVersion{}
-			csv.Name = tt.args.bundleName
-			checks := CSVChecks{csv: csv, errs: []error{}, warns: []error{}}
-			result := checkBundleName(checks)
-
-			require.Equal(t, tt.wantWarning, len(result.warns) > 0)
-			if tt.wantWarning {
-				require.Equal(t, len(tt.warnStrings), len(result.warns))
-				for _, w := range result.warns {
-					wString := w.Error()
-					require.Contains(t, tt.warnStrings, wString)
-				}
-			}
-
-			require.Equal(t, tt.wantError, len(result.errs) > 0)
-			if tt.wantError {
-				require.Equal(t, len(tt.errStrings), len(result.errs))
-				for _, err := range result.errs {
-					errString := err.Error()
-					require.Contains(t, tt.errStrings, errString)
-				}
-			}
-		})
-	}
-}
-
 func TestCheckSpecMinKubeVersion(t *testing.T) {
 	type args struct {
 		minKubeVersion string