From 22eb931e25c9c50dc1b3713aab91a9557bd64345 Mon Sep 17 00:00:00 2001 From: kevinrizza Date: Wed, 1 Nov 2023 17:35:07 -0400 Subject: [PATCH] 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 | 5 ++ pkg/validation/internal/operatorhub.go | 23 +++++- pkg/validation/internal/operatorhubv2.go | 81 +++++++++++++++++++ pkg/validation/internal/operatorhubv2_test.go | 59 ++++++++++++++ pkg/validation/internal/standardcategories.go | 35 ++++++++ .../internal/standardcategories_test.go | 52 ++++++++++++ ...operator.v0.9.4.clusterserviceversion.yaml | 2 +- pkg/validation/validation.go | 13 ++- 8 files changed, 263 insertions(+), 7 deletions(-) create mode 100644 pkg/validation/internal/operatorhubv2.go create mode 100644 pkg/validation/internal/operatorhubv2_test.go create mode 100644 pkg/validation/internal/standardcategories.go create mode 100644 pkg/validation/internal/standardcategories_test.go diff --git a/pkg/validation/errors/error.go b/pkg/validation/errors/error.go index 9b4391658..4e22d6286 100644 --- a/pkg/validation/errors/error.go +++ b/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} +} diff --git a/pkg/validation/internal/operatorhub.go b/pkg/validation/internal/operatorhub.go index e82d10287..fea1bf077 100644 --- a/pkg/validation/internal/operatorhub.go +++ b/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, ",") diff --git a/pkg/validation/internal/operatorhubv2.go b/pkg/validation/internal/operatorhubv2.go new file mode 100644 index 000000000..000870f7c --- /dev/null +++ b/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 +} diff --git a/pkg/validation/internal/operatorhubv2_test.go b/pkg/validation/internal/operatorhubv2_test.go new file mode 100644 index 000000000..5a09d03ed --- /dev/null +++ b/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) + } + } + } + } +} diff --git a/pkg/validation/internal/standardcategories.go b/pkg/validation/internal/standardcategories.go new file mode 100644 index 000000000..f798e1cee --- /dev/null +++ b/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 +} diff --git a/pkg/validation/internal/standardcategories_test.go b/pkg/validation/internal/standardcategories_test.go new file mode 100644 index 000000000..9433b68a5 --- /dev/null +++ b/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) + } + } + } + } +} diff --git a/pkg/validation/internal/testdata/valid_bundle/etcdoperator.v0.9.4.clusterserviceversion.yaml b/pkg/validation/internal/testdata/valid_bundle/etcdoperator.v0.9.4.clusterserviceversion.yaml index 29d1f682a..b4dc61e58 100644 --- a/pkg/validation/internal/testdata/valid_bundle/etcdoperator.v0.9.4.clusterserviceversion.yaml +++ b/pkg/validation/internal/testdata/valid_bundle/etcdoperator.v0.9.4.clusterserviceversion.yaml @@ -16,7 +16,7 @@ metadata: ],\n \"storageType\":\"S3\",\n \"s3\": {\n \"path\": \"\"\ ,\n \"awsSecret\": \"\"\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 diff --git a/pkg/validation/validation.go b/pkg/validation/validation.go index 2f3f61eab..c1f119a64 100644 --- a/pkg/validation/validation.go +++ b/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,