commands/.../scorecard,Gopkg.lock: add crdsHaveValidation func (#979)

* commands/.../scorecard: add crdsHaveValidation func

Author: AlexNPavel

commands/operator-sdk/cmd/scorecard.go

@@ -18,6 +18,8 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/operator-framework/operator-sdk/pkg/scaffold"
+
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
@@ -40,6 +42,7 @@ type scorecardConfig struct {
 	crManifest         string
 	proxyImage         string
 	proxyPullPolicy    string
+	crdsDir            string
 	verbose            bool
 }
 
@@ -67,6 +70,7 @@ func NewScorecardCmd() *cobra.Command {
 	scorecardCmd.Flags().StringVar(&scConf.crManifest, scorecard.CRManifestOpt, "", "Path to manifest for Custom Resource (required)")
 	scorecardCmd.Flags().StringVar(&scConf.proxyImage, scorecard.ProxyImageOpt, fmt.Sprintf("quay.io/operator-framework/scorecard-proxy:%s", strings.TrimSuffix(version.Version, "+git")), "Image name for scorecard proxy")
 	scorecardCmd.Flags().StringVar(&scConf.proxyPullPolicy, scorecard.ProxyPullPolicyOpt, "Always", "Pull policy for scorecard proxy image")
+	scorecardCmd.Flags().StringVar(&scConf.crdsDir, "crds-dir", scaffold.CRDsDir, "Directory containing CRDs (all CRD manifest filenames must have the suffix 'crd.yaml')")
 	scorecardCmd.Flags().BoolVar(&scConf.verbose, scorecard.VerboseOpt, false, "Enable verbose logging")
 
 	if err := viper.BindPFlags(scorecardCmd.Flags()); err != nil {

commands/operator-sdk/cmd/scorecard/olm_tests.go

@@ -16,13 +16,123 @@ package scorecard
 
 import (
 	"context"
+	"fmt"
+	"io/ioutil"
+	"path/filepath"
+	"strings"
+
+	"github.com/operator-framework/operator-sdk/pkg/scaffold"
 
 	olmapiv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
+	log "github.com/sirupsen/logrus"
+	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+func getCRDs(crdsDir string) ([]apiextv1beta1.CustomResourceDefinition, error) {
+	files, err := ioutil.ReadDir(crdsDir)
+	if err != nil {
+		return nil, fmt.Errorf("could not read deploy directory: (%v)", err)
+	}
+	crds := []apiextv1beta1.CustomResourceDefinition{}
+	for _, file := range files {
+		if strings.HasSuffix(file.Name(), "crd.yaml") {
+			obj, err := yamlToUnstructured(filepath.Join(scaffold.CRDsDir, file.Name()))
+			if err != nil {
+				return nil, err
+			}
+			crd, err := unstructuredToCRD(obj)
+			if err != nil {
+				return nil, err
+			}
+			crds = append(crds, *crd)
+		}
+	}
+	return crds, nil
+}
+
+func matchKind(kind1, kind2 string) bool {
+	singularKind1, err := restMapper.ResourceSingularizer(kind1)
+	if err != nil {
+		singularKind1 = kind1
+		log.Warningf("could not find singular version of %s", kind1)
+	}
+	singularKind2, err := restMapper.ResourceSingularizer(kind2)
+	if err != nil {
+		singularKind2 = kind2
+		log.Warningf("could not find singular version of %s", kind2)
+	}
+	return strings.EqualFold(singularKind1, singularKind2)
+}
+
+// matchVersion checks if a CRD contains a specified version in a case insensitive manner
+func matchVersion(version string, crd apiextv1beta1.CustomResourceDefinition) bool {
+	if strings.EqualFold(version, crd.Spec.Version) {
+		return true
+	}
+	// crd.Spec.Version is deprecated, so check in crd.Spec.Versions as well
+	for _, currVer := range crd.Spec.Versions {
+		if strings.EqualFold(version, currVer.Name) {
+			return true
+		}
+	}
+	return false
+}
+
+// crdsHaveValidation makes sure that all CRDs have a validation block
+func crdsHaveValidation(crdsDir string, runtimeClient client.Client, obj *unstructured.Unstructured) error {
+	test := scorecardTest{testType: olmIntegration, name: "Provided APIs have validation"}
+	crds, err := getCRDs(crdsDir)
+	if err != nil {
+		return fmt.Errorf("failed to get CRDs in %s directory: %v", crdsDir, err)
+	}
+	err = runtimeClient.Get(context.TODO(), types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}, obj)
+	if err != nil {
+		return err
+	}
+	// TODO: we need to make this handle multiple CRs better/correctly
+	for _, crd := range crds {
+		test.maximumPoints++
+		if crd.Spec.Validation == nil {
+			scSuggestions = append(scSuggestions, fmt.Sprintf("Add CRD validation for %s/%s", crd.Spec.Names.Kind, crd.Spec.Version))
+			continue
+		}
+		// check if the CRD matches the testing CR
+		gvk := obj.GroupVersionKind()
+		// Only check the validation block if the CRD and CR have the same Kind and Version
+		if !(matchVersion(gvk.Version, crd) && matchKind(gvk.Kind, crd.Spec.Names.Kind)) {
+			test.earnedPoints++
+			continue
+		}
+		failed := false
+		if obj.Object["spec"] != nil {
+			spec := obj.Object["spec"].(map[string]interface{})
+			for key := range spec {
+				if _, ok := crd.Spec.Validation.OpenAPIV3Schema.Properties["spec"].Properties[key]; !ok {
+					failed = true
+					scSuggestions = append(scSuggestions, fmt.Sprintf("Add CRD validation for spec field `%s` in %s/%s", key, gvk.Kind, gvk.Version))
+				}
+			}
+		}
+		if obj.Object["status"] != nil {
+			status := obj.Object["status"].(map[string]interface{})
+			for key := range status {
+				if _, ok := crd.Spec.Validation.OpenAPIV3Schema.Properties["status"].Properties[key]; !ok {
+					failed = true
+					scSuggestions = append(scSuggestions, fmt.Sprintf("Add CRD validation for status field `%s` in %s/%s", key, gvk.Kind, gvk.Version))
+				}
+			}
+		}
+		if !failed {
+			test.earnedPoints++
+		}
+	}
+	scTests = append(scTests, test)
+	return nil
+}
+
 // crdsHaveResources checks to make sure that all owned CRDs have resources listed
 func crdsHaveResources(csv *olmapiv1alpha1.ClusterServiceVersion) {
 	test := scorecardTest{testType: olmIntegration, name: "Owned CRDs have resources listed"}

commands/operator-sdk/cmd/scorecard/resource_handler.go

@@ -31,6 +31,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	appsv1 "k8s.io/api/apps/v1"
 	v1 "k8s.io/api/core/v1"
+	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -225,6 +226,24 @@ func addProxyContainer(dep *appsv1.Deployment) {
 	})
 }
 
+// unstructuredToCRD converts an unstructured object to a CRD
+func unstructuredToCRD(obj *unstructured.Unstructured) (*apiextv1beta1.CustomResourceDefinition, error) {
+	jsonByte, err := obj.MarshalJSON()
+	if err != nil {
+		return nil, fmt.Errorf("failed to convert CRD to json: %v", err)
+	}
+	crdObj, _, err := dynamicDecoder.Decode(jsonByte, nil, nil)
+	if err != nil {
+		return nil, fmt.Errorf("failed to decode CRD object: %v", err)
+	}
+	switch o := crdObj.(type) {
+	case *apiextv1beta1.CustomResourceDefinition:
+		return o, nil
+	default:
+		return nil, fmt.Errorf("conversion of runtime object to CRD failed (resulting runtime object not CRD type)")
+	}
+}
+
 // unstructuredToDeployment converts an unstructured object to a deployment
 func unstructuredToDeployment(obj *unstructured.Unstructured) (*appsv1.Deployment, error) {
 	jsonByte, err := obj.MarshalJSON()

commands/operator-sdk/cmd/scorecard/scorecard.go

@@ -55,6 +55,7 @@ const (
 	CRManifestOpt         = "cr-manifest"
 	ProxyImageOpt         = "proxy-image"
 	ProxyPullPolicyOpt    = "proxy-pull-policy"
+	CRDsDirOpt            = "crds-dir"
 	VerboseOpt            = "verbose"
 )
 
@@ -234,6 +235,10 @@ func ScorecardTests(cmd *cobra.Command, args []string) error {
 		default:
 			return fmt.Errorf("provided yaml file not of ClusterServiceVersion type")
 		}
+		fmt.Println("Checking if all CRDs have validation")
+		if err := crdsHaveValidation(viper.GetString(CRDsDirOpt), runtimeClient, obj); err != nil {
+			return err
+		}
 		fmt.Println("Checking for CRD resources")
 		crdsHaveResources(csv)
 		fmt.Println("Checking for existence of example CRs")

doc/test-framework/scorecard.md

@@ -77,6 +77,12 @@ API server, indicating that it is modifying resources. This test has a maximum s
 
 ### OLM Integration
 
+#### Provided APIs have validation
+
+This test verifies that all the CRDs in the CRDs folder contain a validation section. If the CRD matches the kind and version of the
+CR currently being tested, it will also verify that there is a validation for each spec and status field in that CR. This test has a
+maximum score of 1.
+
 #### Owned CRDs Have Resources Listed
 
 This test makes sure that the CRDs listed in the [`owned` CRDs section][owned-crds] of the CSV have a `resources` subsection. This

hack/tests/scorecard-subcommand.sh

@@ -10,8 +10,8 @@ set -ex
 # the test framework directory has all the manifests needed to run the cluster
 pushd test/test-framework
 commandoutput="$(operator-sdk scorecard --cr-manifest deploy/crds/cache_v1alpha1_memcached_cr.yaml --init-timeout 60 --csv-path deploy/memcachedoperator.0.0.2.csv.yaml --verbose --proxy-image $DEST_IMAGE --proxy-pull-policy Never 2>&1)"
-echo $commandoutput | grep "Total Score: 5/7 points"
+echo $commandoutput | grep "Total Score: 6/8 points"
 # test config file
 commandoutput2="$(operator-sdk scorecard --proxy-image $DEST_IMAGE)"
-echo $commandoutput2 | grep "Total Score: 5/7 points"
+echo $commandoutput2 | grep "Total Score: 6/8 points"
 popd

test/test-framework/deploy/namespace-init.yaml

@@ -2,7 +2,9 @@ apiVersion: v1
 kind: ServiceAccount
 metadata:
   name: memcached-operator
+
 ---
+
 apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
 metadata:
@@ -36,7 +38,9 @@ rules:
   - '*'
   verbs:
   - '*'
+
 ---
+
 kind: RoleBinding
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
@@ -48,36 +52,42 @@ roleRef:
   kind: Role
   name: memcached-operator
   apiGroup: rbac.authorization.k8s.io
+
 ---
+
 apiVersion: apps/v1
 kind: Deployment
 metadata:
+  creationTimestamp: null
   name: memcached-operator
 spec:
   replicas: 1
   selector:
     matchLabels:
       name: memcached-operator
+  strategy: {}
   template:
     metadata:
+      creationTimestamp: null
       labels:
         name: memcached-operator
     spec:
-      serviceAccountName: memcached-operator
       containers:
-        - name: memcached-operator
-          # Replace this with the built image name
-          image: quay.io/coreos/operator-sdk-dev:test-framework-operator-runtime
-          ports:
-          - containerPort: 60000
-            name: metrics
-          command:
-          - memcached-operator
-          imagePullPolicy: Always
-          env:
-            - name: WATCH_NAMESPACE
-              valueFrom:
-                fieldRef:
-                  fieldPath: metadata.namespace
-            - name: OPERATOR_NAME
-              value: "memcached-operator"
+      - command:
+        - memcached-operator
+        env:
+        - name: WATCH_NAMESPACE
+          valueFrom:
+            fieldRef:
+              fieldPath: metadata.namespace
+        - name: OPERATOR_NAME
+          value: memcached-operator
+        image: quay.io/coreos/operator-sdk-dev:test-framework-operator-runtime
+        imagePullPolicy: Always
+        name: memcached-operator
+        ports:
+        - containerPort: 60000
+          name: metrics
+        resources: {}
+      serviceAccountName: memcached-operator
+status: {}