From a7ebc81c9577a300814448a901edae74c6e6e44e Mon Sep 17 00:00:00 2001
From: Ryan King <ryantking@protonmail.com>
Date: Tue, 5 Apr 2022 12:01:56 -0400
Subject: [PATCH] Add CRD description check validation (#234)

This adds a check to the good practices validator that ensures all CRDs
declared in the bundle have non-empty descriptions. Both owned and
required CRDs will be checked. Note that this validates the CRD
definition in the CSV, not any field in the CRD itself.

Signed-off-by: Ryan King <ryking@redhat.com>
---
 pkg/validation/internal/good_practices.go      | 18 ++++++++++++++++++
 pkg/validation/internal/good_practices_test.go | 14 +++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/pkg/validation/internal/good_practices.go b/pkg/validation/internal/good_practices.go
index 246c4e26f..09fe5fc90 100644
--- a/pkg/validation/internal/good_practices.go
+++ b/pkg/validation/internal/good_practices.go
@@ -54,6 +54,9 @@ func validateGoodPracticesFrom(bundle *manifests.Bundle) errors.ManifestResult {
 	for _, warn := range warns {
 		result.Add(errors.WarnFailedValidation(warn.Error(), bundle.CSV.GetName()))
 	}
+	for _, warn := range validateCrdDescriptions(bundle.CSV.Spec.CustomResourceDefinitions) {
+		result.Add(errors.WarnFailedValidation(warn.Error(), bundle.CSV.GetName()))
+	}
 
 	channels := append(bundle.Channels, bundle.DefaultChannel)
 	if warn := validateHubChannels(channels); warn != nil {
@@ -126,3 +129,18 @@ func getUniqueValues(array []string) []string {
 	}
 	return result
 }
+
+// validateCrdDescrptions ensures that all CRDs defined in the bundle have non-empty descriptions.
+func validateCrdDescriptions(crds operatorsv1alpha1.CustomResourceDefinitions) []error {
+	f := func(crds []operatorsv1alpha1.CRDDescription, relation string) []error {
+		errors := make([]error, 0, len(crds))
+		for _, crd := range crds {
+			if crd.Description == "" {
+				errors = append(errors, fmt.Errorf("%s CRD %q has an empty description", relation, crd.Name))
+			}
+		}
+		return errors
+	}
+
+	return append(f(crds.Owned, "owned"), f(crds.Required, "required")...)
+}
diff --git a/pkg/validation/internal/good_practices_test.go b/pkg/validation/internal/good_practices_test.go
index 86ef36d81..908514e78 100644
--- a/pkg/validation/internal/good_practices_test.go
+++ b/pkg/validation/internal/good_practices_test.go
@@ -1,15 +1,19 @@
 package internal
 
 import (
+	"testing"
+
 	"github.com/operator-framework/api/pkg/manifests"
 	"github.com/stretchr/testify/require"
-	"testing"
 )
 
 func Test_ValidateGoodPractices(t *testing.T) {
 	bundleWithDeploymentSpecEmpty, _ := manifests.GetBundleFromDir("./testdata/valid_bundle")
 	bundleWithDeploymentSpecEmpty.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs = nil
 
+	bundleWithMissingCrdDescription, _ := manifests.GetBundleFromDir("./testdata/valid_bundle")
+	bundleWithMissingCrdDescription.CSV.Spec.CustomResourceDefinitions.Owned[0].Description = ""
+
 	type args struct {
 		bundleDir string
 		bundle    *manifests.Bundle
@@ -70,6 +74,14 @@ func Test_ValidateGoodPractices(t *testing.T) {
 			},
 			warnStrings: []string{"Warning: Value memcached-operator.v0.0.1: channel(s) [\"alpha\"] are not following the recommended naming convention: https://olm.operatorframework.io/docs/best-practices/channel-naming"},
 		},
+		{
+			name:        "should raise a warn when a CRD does not have a description",
+			wantWarning: true,
+			args: args{
+				bundle: bundleWithMissingCrdDescription,
+			},
+			warnStrings: []string{"Warning: Value etcdoperator.v0.9.4: owned CRD \"etcdclusters.etcd.database.coreos.com\" has an empty description"},
+		},
 	}
 
 	for _, tt := range tests {