diff --git a/pkg/validation/internal/good_practices.go b/pkg/validation/internal/good_practices.go index bdbdbe86f..8cd1ea62e 100644 --- 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: .v 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: .v 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 index af0a90c76..cb008adc0 100644 --- 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: .v 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 index cca7eddac..ac9fd0cf5 100644 --- 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: .v 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: .v 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 index 1c1df7216..6e3406283 100644 --- 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: .v 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