Separate Category Validation

This commit removes the default set of category validation as a part of
the operatorhubio validator. As we have a mechanism for custom
validation, and there is significantly more churn on that specific
validation, this commit separates the default operatorhubio validator
from a distinct default categories validator. This allows users that
want to continue to use the default set of categories to still do so,
and if there are custom categories they would like to include they
are free to use the dynamic categories validation option instead.
This commit accomplishes that by deprecating the existing validator
and creating a v2 version of the operatorhubio validator.

Additionally, this commit adds the 'Observability' category to the list
of default categories used by the new categories validator.

pkg/validation/errors/error.go

@@ -102,6 +102,7 @@ const (
 	ErrorInvalidPackageManifest   ErrorType = "PackageManifestNotValid"
 	ErrorObjectFailedValidation   ErrorType = "ObjectFailedValidation"
 	ErrorPropertiesAnnotationUsed ErrorType = "PropertiesAnnotationUsed"
+	ErrorDeprecatedValidator      ErrorType = "DeprecatedValidator"
 )
 
 func NewError(t ErrorType, detail, field string, v interface{}) Error {
@@ -248,3 +249,7 @@ func WarnInvalidObject(detail string, value interface{}) Error {
 func WarnPropertiesAnnotationUsed(detail string) Error {
 	return Error{ErrorPropertiesAnnotationUsed, LevelWarn, "", "", detail}
 }
+
+func WarnDeprecatedValidator(detail string) Error {
+	return Error{ErrorDeprecatedValidator, LevelWarn, "", "", detail}
+}

pkg/validation/internal/operatorhub.go

@@ -118,7 +118,7 @@ import (
 // `k8s-version` key is allowed. If informed, it will perform the checks against this specific Kubernetes version where the
 // operator bundle is intend to be used and will raise errors instead of warnings.
 // Currently, this check is capable of verifying the removed APIs only for Kubernetes 1.22 version.
-var OperatorHubValidator interfaces.Validator = interfaces.ValidatorFunc(validateOperatorHub)
+var OperatorHubValidator interfaces.Validator = interfaces.ValidatorFunc(validateOperatorHubDeprecated)
 
 var validCapabilities = map[string]struct{}{
 	"Basic Install":     {},
@@ -151,13 +151,15 @@ var validCategories = map[string]struct{}{
 	"Security":                  {},
 	"Storage":                   {},
 	"Streaming & Messaging":     {},
+	"Observability":             {},
 }
 
 const minKubeVersionWarnMessage = "csv.Spec.minKubeVersion is not informed. It is recommended you provide this information. " +
 	"Otherwise, it would mean that your operator project can be distributed and installed in any cluster version " +
 	"available, which is not necessarily the case for all projects."
 
-func validateOperatorHub(objs ...interface{}) (results []errors.ManifestResult) {
+// Warning: this validator is deprecated in favor of validateOperatorHub()
+func validateOperatorHubDeprecated(objs ...interface{}) (results []errors.ManifestResult) {
 
 	// Obtain the k8s version if informed via the objects an optional
 	k8sVersion := ""
@@ -178,6 +180,11 @@ func validateOperatorHub(objs ...interface{}) (results []errors.ManifestResult)
 		}
 	}
 
+	// Add a deprecation warning to the list so that users are aware this validator is deprecated
+	deprecationResultWarning := errors.ManifestResult{}
+	deprecationResultWarning.Add(errors.WarnDeprecatedValidator("OperatorHub Validator deprecated, use OperatorHubV2 validator"))
+	results = append(results, deprecationResultWarning)
+
 	return results
 }
 
@@ -221,7 +228,8 @@ func validateHubCSVSpec(csv v1alpha1.ClusterServiceVersion) CSVChecks {
 	checks = checkSpecProviderName(checks)
 	checks = checkSpecMaintainers(checks)
 	checks = checkSpecLinks(checks)
-	checks = checkAnnotations(checks)
+	checks = checkCapabilities(checks)
+	checks = checkCategories(checks)
 	checks = checkSpecVersion(checks)
 	checks = checkSpecIcon(checks)
 	checks = checkSpecMinKubeVersion(checks)
@@ -257,7 +265,7 @@ func checkSpecVersion(checks CSVChecks) CSVChecks {
 }
 
 // checkAnnotations will validate the values informed via annotations such as; capabilities and categories
-func checkAnnotations(checks CSVChecks) CSVChecks {
+func checkCapabilities(checks CSVChecks) CSVChecks {
 	if checks.csv.GetAnnotations() == nil {
 		checks.csv.SetAnnotations(make(map[string]string))
 	}
@@ -267,6 +275,13 @@ func checkAnnotations(checks CSVChecks) CSVChecks {
 			checks.errs = append(checks.errs, fmt.Errorf("csv.Metadata.Annotations.Capabilities %s is not a valid capabilities level", capability))
 		}
 	}
+	return checks
+}
+
+func checkCategories(checks CSVChecks) CSVChecks {
+	if checks.csv.GetAnnotations() == nil {
+		checks.csv.SetAnnotations(make(map[string]string))
+	}
 
 	if categories, ok := checks.csv.ObjectMeta.Annotations["categories"]; ok {
 		categorySlice := strings.Split(categories, ",")

pkg/validation/internal/operatorhubv2.go

@@ -0,0 +1,81 @@
+package internal
+
+import (
+	"github.com/operator-framework/api/pkg/manifests"
+	"github.com/operator-framework/api/pkg/operators/v1alpha1"
+	"github.com/operator-framework/api/pkg/validation/errors"
+	interfaces "github.com/operator-framework/api/pkg/validation/interfaces"
+)
+
+var OperatorHubValidatorV2 interfaces.Validator = interfaces.ValidatorFunc(validateOperatorHubV2)
+
+func validateOperatorHubV2(objs ...interface{}) (results []errors.ManifestResult) {
+	// Obtain the k8s version if informed via the objects an optional
+	k8sVersion := ""
+	for _, obj := range objs {
+		switch obj.(type) {
+		case map[string]string:
+			k8sVersion = obj.(map[string]string)[k8sVersionKey]
+			if len(k8sVersion) > 0 {
+				break
+			}
+		}
+	}
+
+	for _, obj := range objs {
+		switch v := obj.(type) {
+		case *manifests.Bundle:
+			results = append(results, validateBundleOperatorHubV2(v, k8sVersion))
+		}
+	}
+
+	return results
+}
+
+func validateBundleOperatorHubV2(bundle *manifests.Bundle, k8sVersion string) errors.ManifestResult {
+	result := errors.ManifestResult{Name: bundle.Name}
+
+	if bundle == nil {
+		result.Add(errors.ErrInvalidBundle("Bundle is nil", nil))
+		return result
+	}
+
+	if bundle.CSV == nil {
+		result.Add(errors.ErrInvalidBundle("Bundle csv is nil", bundle.Name))
+		return result
+	}
+
+	csvChecksResult := validateHubCSVSpecV2(*bundle.CSV)
+	for _, err := range csvChecksResult.errs {
+		result.Add(errors.ErrInvalidCSV(err.Error(), bundle.CSV.GetName()))
+	}
+	for _, warn := range csvChecksResult.warns {
+		result.Add(errors.WarnInvalidCSV(warn.Error(), bundle.CSV.GetName()))
+	}
+
+	errs, warns := validateDeprecatedAPIS(bundle, k8sVersion)
+	for _, err := range errs {
+		result.Add(errors.ErrFailedValidation(err.Error(), bundle.CSV.GetName()))
+	}
+	for _, warn := range warns {
+		result.Add(errors.WarnFailedValidation(warn.Error(), bundle.CSV.GetName()))
+	}
+
+	return result
+}
+
+// validateHubCSVSpec will check the CSV against the criteria to publish an
+// operator bundle in the OperatorHub.io
+func validateHubCSVSpecV2(csv v1alpha1.ClusterServiceVersion) CSVChecks {
+	checks := CSVChecks{csv: csv, errs: []error{}, warns: []error{}}
+
+	checks = checkSpecProviderName(checks)
+	checks = checkSpecMaintainers(checks)
+	checks = checkSpecLinks(checks)
+	checks = checkCapabilities(checks)
+	checks = checkSpecVersion(checks)
+	checks = checkSpecIcon(checks)
+	checks = checkSpecMinKubeVersion(checks)
+
+	return checks
+}

pkg/validation/internal/operatorhubv2_test.go

@@ -0,0 +1,59 @@
+package internal
+
+import (
+	"testing"
+
+	"github.com/operator-framework/api/pkg/manifests"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestValidateBundleOperatorHubV2(t *testing.T) {
+	var table = []struct {
+		description string
+		directory   string
+		hasError    bool
+		errStrings  []string
+	}{
+		{
+			description: "registryv1 bundle/valid bundle",
+			directory:   "./testdata/valid_bundle",
+			hasError:    false,
+		},
+		{
+			description: "registryv1 bundle/invald bundle operatorhubio",
+			directory:   "./testdata/invalid_bundle_operatorhub",
+			hasError:    true,
+			errStrings: []string{
+				`Error: Value : (etcdoperator.v0.9.4) csv.Spec.Provider.Name not specified`,
+				`Error: Value : (etcdoperator.v0.9.4) csv.Spec.Maintainers elements should contain both name and email`,
+				`Error: Value : (etcdoperator.v0.9.4) csv.Spec.Maintainers email invalidemail is invalid: mail: missing '@' or angle-addr`,
+				`Error: Value : (etcdoperator.v0.9.4) csv.Spec.Links elements should contain both name and url`,
+				`Error: Value : (etcdoperator.v0.9.4) csv.Spec.Links url https//coreos.com/operators/etcd/docs/latest/ is invalid: parse "https//coreos.com/operators/etcd/docs/latest/": invalid URI for request`,
+				`Error: Value : (etcdoperator.v0.9.4) csv.Metadata.Annotations.Capabilities Installs and stuff is not a valid capabilities level`,
+				`Error: Value : (etcdoperator.v0.9.4) csv.Spec.Icon should only have one element`,
+				`Error: Value : (etcdoperator.v0.9.4) csv.Spec.Version must be set`,
+			},
+		},
+	}
+
+	for _, tt := range table {
+		// Validate the bundle object
+		bundle, err := manifests.GetBundleFromDir(tt.directory)
+		require.NoError(t, err)
+
+		results := OperatorHubValidatorV2.Validate(bundle)
+
+		if len(results) > 0 {
+			require.Equal(t, results[0].HasError(), tt.hasError)
+			if results[0].HasError() {
+				require.Equal(t, len(tt.errStrings), len(results[0].Errors))
+
+				for _, err := range results[0].Errors {
+					errString := err.Error()
+					require.Contains(t, tt.errStrings, errString)
+				}
+			}
+		}
+	}
+}

pkg/validation/internal/standardcategories.go

@@ -0,0 +1,35 @@
+package internal
+
+import (
+	"github.com/operator-framework/api/pkg/manifests"
+	"github.com/operator-framework/api/pkg/validation/errors"
+	interfaces "github.com/operator-framework/api/pkg/validation/interfaces"
+)
+
+var StandardCategoriesValidator interfaces.Validator = interfaces.ValidatorFunc(validateCategories)
+
+func validateCategories(objs ...interface{}) (results []errors.ManifestResult) {
+	for _, obj := range objs {
+		switch v := obj.(type) {
+		case *manifests.Bundle:
+			results = append(results, validateCategoriesBundle(v))
+		}
+	}
+
+	return results
+}
+
+func validateCategoriesBundle(bundle *manifests.Bundle) errors.ManifestResult {
+	result := errors.ManifestResult{Name: bundle.Name}
+	csvCategoryCheck := CSVChecks{csv: *bundle.CSV, errs: []error{}, warns: []error{}}
+
+	csvChecksResult := checkCategories(csvCategoryCheck)
+	for _, err := range csvChecksResult.errs {
+		result.Add(errors.ErrInvalidCSV(err.Error(), bundle.CSV.GetName()))
+	}
+	for _, warn := range csvChecksResult.warns {
+		result.Add(errors.WarnInvalidCSV(warn.Error(), bundle.CSV.GetName()))
+	}
+
+	return result
+}

pkg/validation/internal/standardcategories_test.go

@@ -0,0 +1,52 @@
+package internal
+
+import (
+	"testing"
+
+	"github.com/operator-framework/api/pkg/manifests"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestValidateCategories(t *testing.T) {
+	var table = []struct {
+		description string
+		directory   string
+		hasError    bool
+		errStrings  []string
+	}{
+		{
+			description: "registryv1 bundle/valid bundle",
+			directory:   "./testdata/valid_bundle",
+			hasError:    false,
+		},
+		{
+			description: "registryv1 bundle/invald bundle operatorhubio",
+			directory:   "./testdata/invalid_bundle_operatorhub",
+			hasError:    true,
+			errStrings: []string{
+				`Error: Value : (etcdoperator.v0.9.4) csv.Metadata.Annotations["categories"] value Magic is not in the set of default categories`,
+			},
+		},
+	}
+
+	for _, tt := range table {
+		// Validate the bundle object
+		bundle, err := manifests.GetBundleFromDir(tt.directory)
+		require.NoError(t, err)
+
+		results := StandardCategoriesValidator.Validate(bundle)
+
+		if len(results) > 0 {
+			require.Equal(t, results[0].HasError(), tt.hasError)
+			if results[0].HasError() {
+				require.Equal(t, len(tt.errStrings), len(results[0].Errors))
+
+				for _, err := range results[0].Errors {
+					errString := err.Error()
+					require.Contains(t, tt.errStrings, errString)
+				}
+			}
+		}
+	}
+}

pkg/validation/internal/testdata/valid_bundle/etcdoperator.v0.9.4.clusterserviceversion.yaml

@@ -16,7 +16,7 @@ metadata:
       ],\n      \"storageType\":\"S3\",\n      \"s3\": {\n        \"path\": \"<full-s3-path>\"\
       ,\n        \"awsSecret\": \"<aws-secret>\"\n      }\n    }\n  }\n]\n"
     capabilities: Full Lifecycle
-    categories: Database, Big Data
+    categories: Database, Big Data, Observability
     containerImage: quay.io/coreos/etcd-operator@sha256:66a37fd61a06a43969854ee6d3e21087a98b93838e284a6086b13917f96b0d9b
     createdAt: 2019-02-28 01:03:00
     description: Create and maintain highly-available etcd clusters on Kubernetes

pkg/validation/validation.go

@@ -28,9 +28,17 @@ var CustomResourceDefinitionValidator = internal.CRDValidator
 var BundleValidator = internal.BundleValidator
 
 // OperatorHubValidator implements Validator to validate bundle objects
-// for OperatorHub.io requirements.
+// for OperatorHub.io requirements. This validator is deprecated.
 var OperatorHubValidator = internal.OperatorHubValidator
 
+// OperatorHubValidatorV2 implements Validator to validate bundle objects
+// for OperatorHub.io requirements.
+var OperatorHubValidatorV2 = internal.OperatorHubValidatorV2
+
+// StandardCategoriesValidator implements Validator to validate bundle objects
+// for OperatorHub.io requirements around UI category metadata
+var StandardCategoriesValidator = internal.StandardCategoriesValidator
+
 // Object Validator validates various custom objects in the bundle like PDBs and SCCs.
 // Object validation is optional and not a default-level validation.
 var ObjectValidator = internal.ObjectValidator
@@ -69,7 +77,8 @@ var AllValidators = interfaces.Validators{
 	ClusterServiceVersionValidator,
 	CustomResourceDefinitionValidator,
 	BundleValidator,
-	OperatorHubValidator,
+	OperatorHubValidatorV2,
+	StandardCategoriesValidator,
 	ObjectValidator,
 	OperatorGroupValidator,
 	CommunityOperatorValidator,