Separate Category and Capability Validation (#304)

This commit removes the default set of category and capability validation
as a part of the operatorhubio validator.

As we have a mechanism for custom category 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 also does the same separation for capability validation.
There is no implementation of custom capability validation (as there is
less churn and no explicit need for that yet) -- adding custom
capability validation should be trivial in a future commit.

This commit accomplishes this by deprecating the existing validator and
creating a v2 version of the operatorhubio validator.

Additionally, this commit adds 'Observability' to the category list.

Author: kevinrizza

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

@@ -6,7 +6,6 @@ import (
 	"io/ioutil"
 	"net/mail"
 	"net/url"
-	"os"
 	"path/filepath"
 	"strings"
 
@@ -118,15 +117,9 @@ 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 validCapabilities = map[string]struct{}{
-	"Basic Install":     {},
-	"Seamless Upgrades": {},
-	"Full Lifecycle":    {},
-	"Deep Insights":     {},
-	"Auto Pilot":        {},
-}
+//
+// Deprecated: Use OperatorHubV2Validator, StandardCapabilitiesValidator and StandardCategoriesValidator for equivalent validation.
+var OperatorHubValidator interfaces.Validator = interfaces.ValidatorFunc(validateOperatorHubDeprecated)
 
 var validMediatypes = map[string]struct{}{
 	"image/gif":     {},
@@ -135,29 +128,12 @@ var validMediatypes = map[string]struct{}{
 	"image/svg+xml": {},
 }
 
-var validCategories = map[string]struct{}{
-	"AI/Machine Learning":       {},
-	"Application Runtime":       {},
-	"Big Data":                  {},
-	"Cloud Provider":            {},
-	"Developer Tools":           {},
-	"Database":                  {},
-	"Integration & Delivery":    {},
-	"Logging & Tracing":         {},
-	"Monitoring":                {},
-	"Modernization & Migration": {},
-	"Networking":                {},
-	"OpenShift Optional":        {},
-	"Security":                  {},
-	"Storage":                   {},
-	"Streaming & Messaging":     {},
-}
-
 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 +154,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(`The "operatorhub" validator is deprecated; for equivalent validation use "operatorhub/v2", "standardcapabilities" and "standardcategories" validators`))
+	results = append(results, deprecationResultWarning)
+
 	return results
 }
 
@@ -221,7 +202,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)
@@ -256,46 +238,6 @@ func checkSpecVersion(checks CSVChecks) CSVChecks {
 	return checks
 }
 
-// checkAnnotations will validate the values informed via annotations such as; capabilities and categories
-func checkAnnotations(checks CSVChecks) CSVChecks {
-	if checks.csv.GetAnnotations() == nil {
-		checks.csv.SetAnnotations(make(map[string]string))
-	}
-
-	if capability, ok := checks.csv.ObjectMeta.Annotations["capabilities"]; ok {
-		if _, ok := validCapabilities[capability]; !ok {
-			checks.errs = append(checks.errs, fmt.Errorf("csv.Metadata.Annotations.Capabilities %s is not a valid capabilities level", capability))
-		}
-	}
-
-	if categories, ok := checks.csv.ObjectMeta.Annotations["categories"]; ok {
-		categorySlice := strings.Split(categories, ",")
-
-		// use custom categories for validation if provided
-		customCategoriesPath := os.Getenv("OPERATOR_BUNDLE_CATEGORIES")
-		if customCategoriesPath != "" {
-			customCategories, err := extractCategories(customCategoriesPath)
-			if err != nil {
-				checks.errs = append(checks.errs, fmt.Errorf("could not extract custom categories from categories %#v: %s", customCategories, err))
-			} else {
-				for _, category := range categorySlice {
-					if _, ok := customCategories[strings.TrimSpace(category)]; !ok {
-						checks.errs = append(checks.errs, fmt.Errorf("csv.Metadata.Annotations[\"categories\"] value %s is not in the set of custom categories", category))
-					}
-				}
-			}
-		} else {
-			// use default categories
-			for _, category := range categorySlice {
-				if _, ok := validCategories[strings.TrimSpace(category)]; !ok {
-					checks.errs = append(checks.errs, fmt.Errorf("csv.Metadata.Annotations[\"categories\"] value %s is not in the set of default categories", category))
-				}
-			}
-		}
-	}
-	return checks
-}
-
 // checkSpecIcon will validate if the CSV.spec.Icon was informed and is correct
 func checkSpecIcon(checks CSVChecks) CSVChecks {
 	if checks.csv.Spec.Icon != nil {

pkg/validation/internal/operatorhub_test.go

@@ -32,9 +32,9 @@ func TestValidateBundleOperatorHub(t *testing.T) {
 				`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.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.Metadata.Annotations["categories"] value Magic is not in the set of default categories`,
+				`Error: Value : (etcdoperator.v0.9.4) csv.Metadata.Annotations["categories"] value "Magic" is not in the set of standard categories`,
 				`Error: Value : (etcdoperator.v0.9.4) csv.Spec.Version must be set`,
 			},
 		},

pkg/validation/internal/operatorhubv2.go

@@ -0,0 +1,80 @@
+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 OperatorHubV2Validator 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 = checkSpecVersion(checks)
+	checks = checkSpecIcon(checks)
+	checks = checkSpecMinKubeVersion(checks)
+
+	return checks
+}

pkg/validation/internal/operatorhubv2_test.go

@@ -0,0 +1,58 @@
+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.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 := OperatorHubV2Validator.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/standardcapabilities.go

@@ -0,0 +1,59 @@
+package internal
+
+import (
+	"fmt"
+
+	"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 StandardCapabilitiesValidator interfaces.Validator = interfaces.ValidatorFunc(validateCapabilities)
+
+var validCapabilities = map[string]struct{}{
+	"Basic Install":     {},
+	"Seamless Upgrades": {},
+	"Full Lifecycle":    {},
+	"Deep Insights":     {},
+	"Auto Pilot":        {},
+}
+
+func validateCapabilities(objs ...interface{}) (results []errors.ManifestResult) {
+	for _, obj := range objs {
+		switch v := obj.(type) {
+		case *manifests.Bundle:
+			results = append(results, validateCapabilitiesBundle(v))
+		}
+	}
+
+	return results
+}
+
+func validateCapabilitiesBundle(bundle *manifests.Bundle) errors.ManifestResult {
+	result := errors.ManifestResult{Name: bundle.Name}
+	csvCategoryCheck := CSVChecks{csv: *bundle.CSV, errs: []error{}, warns: []error{}}
+
+	csvChecksResult := checkCapabilities(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
+}
+
+// checkAnnotations will validate the values informed via annotations such as; capabilities and categories
+func checkCapabilities(checks CSVChecks) CSVChecks {
+	if checks.csv.GetAnnotations() == nil {
+		checks.csv.SetAnnotations(make(map[string]string))
+	}
+
+	if capability, ok := checks.csv.ObjectMeta.Annotations["capabilities"]; ok {
+		if _, ok := validCapabilities[capability]; !ok {
+			checks.errs = append(checks.errs, fmt.Errorf("csv.Metadata.Annotations.Capabilities %q is not a valid capabilities level", capability))
+		}
+	}
+	return checks
+}