Move bundle validator back to registry under validation package

Move bundle validator from api back to registry and under validation
package. The bundle validator is removed from api.

Signed-off-by: Vu Dinh <[email protected]>

Author: dinhxuanvu

go.mod

@@ -31,7 +31,7 @@ require (
 	github.com/opencontainers/image-spec v1.0.2-0.20190823105129-775207bd45b6
 	github.com/opencontainers/runc v1.0.0-rc9 // indirect
 	github.com/opencontainers/runtime-spec v0.1.2-0.20190618234442-a950415649c7 // indirect
-	github.com/operator-framework/api v0.1.1
+	github.com/operator-framework/api v0.3.4
 	github.com/otiai10/copy v1.0.2
 	github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2
 	github.com/pkg/errors v0.9.1
@@ -45,10 +45,10 @@ require (
 	google.golang.org/grpc v1.26.0
 	gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
 	gopkg.in/yaml.v2 v2.2.8
-	k8s.io/api v0.18.0
-	k8s.io/apiextensions-apiserver v0.18.0
-	k8s.io/apimachinery v0.18.0
-	k8s.io/client-go v0.18.0
+	k8s.io/api v0.18.2
+	k8s.io/apiextensions-apiserver v0.18.2
+	k8s.io/apimachinery v0.18.2
+	k8s.io/client-go v0.18.2
 	k8s.io/klog v1.0.0
 	k8s.io/kubectl v0.18.0
 )

go.sum

@@ -10,6 +10,7 @@ import (
 	v1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
 	v "github.com/operator-framework/api/pkg/validation"
 	"github.com/operator-framework/operator-registry/pkg/containertools"
+	validation "github.com/operator-framework/operator-registry/pkg/lib/validation"
 	"github.com/operator-framework/operator-registry/pkg/registry"
 
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -355,7 +356,7 @@ func (i imageValidator) ValidateBundleContent(manifestDir string) error {
 	// Validate the bundle object
 	if len(unstObjs) > 0 {
 		bundle := registry.NewBundle(csvName, "", nil, unstObjs...)
-		bundleValidator := v.BundleValidator
+		bundleValidator := validation.BundleValidator
 		results := bundleValidator.Validate(bundle)
 		if len(results) > 0 {
 			for _, err := range results[0].Errors {

pkg/lib/bundle/validate.go

@@ -193,14 +193,14 @@ func TestValidateBundleContent(t *testing.T) {
 			mediaType:   RegistryV1Type,
 			directory:   "./testdata/validate/invalid_manifests_bundle/invalid_bundle/",
 			numErrors:   1,
-			errString:   `owned CRD "etcdclusters.etcd.database.coreos.com" not found in bundle`,
+			errString:   "owned CRD etcdclusters.etcd.database.coreos.com/v1beta2 not found in bundle",
 		},
 		{
 			description: "invalid registryv1 bundle/extra crd",
 			mediaType:   RegistryV1Type,
 			directory:   "./testdata/validate/invalid_manifests_bundle/invalid_bundle_2/",
 			numErrors:   1,
-			errString:   `owned CRD "etcdclusters.etcd.database.coreos.com" is present in bundle "etcdoperator.v0.9.4" but not defined in CSV`,
+			errString:   `CRD etcdclusters.etcd.database.coreos.com/v1beta2 is present in bundle "etcdoperator.v0.9.4" but not defined in CSV`,
 		},
 	}
 

pkg/lib/bundle/validate_test.go

@@ -0,0 +1,117 @@
+package validation
+
+import (
+	"fmt"
+
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
+
+	"github.com/operator-framework/api/pkg/validation/errors"
+	interfaces "github.com/operator-framework/api/pkg/validation/interfaces"
+	"github.com/operator-framework/operator-registry/pkg/registry"
+)
+
+var RegistryBundleValidator interfaces.Validator = interfaces.ValidatorFunc(validateBundles)
+
+func validateBundles(objs ...interface{}) (results []errors.ManifestResult) {
+	for _, obj := range objs {
+		switch v := obj.(type) {
+		case *registry.Bundle:
+			results = append(results, validateBundle(v))
+		}
+	}
+	return results
+}
+
+func validateBundle(bundle *registry.Bundle) (result errors.ManifestResult) {
+	csv, err := bundle.ClusterServiceVersion()
+	if err != nil {
+		result.Add(errors.ErrInvalidParse("error getting bundle CSV", err))
+		return result
+	}
+
+	result = validateOwnedCRDs(bundle, csv)
+
+	if result.Name, err = csv.GetVersion(); err != nil {
+		result.Add(errors.ErrInvalidParse("error getting bundle CSV version", err))
+		return result
+	}
+	return result
+}
+
+func validateOwnedCRDs(bundle *registry.Bundle, csv *registry.ClusterServiceVersion) (result errors.ManifestResult) {
+	ownedKeys, _, err := csv.GetCustomResourceDefintions()
+	if err != nil {
+		result.Add(errors.ErrInvalidParse("error getting CSV CRDs", err))
+		return result
+	}
+
+	keySet, rerr := getBundleCRDKeys(bundle)
+	if rerr != (errors.Error{}) {
+		result.Add(rerr)
+		return result
+	}
+
+	// Validate all owned keys, and remove them from the set if seen.
+	for _, ownedKey := range ownedKeys {
+		if _, ok := keySet[*ownedKey]; !ok {
+			result.Add(errors.ErrInvalidBundle(fmt.Sprintf("owned CRD %s not found in bundle %q", keyToString(*ownedKey), bundle.Name), *ownedKey))
+		} else {
+			delete(keySet, *ownedKey)
+		}
+	}
+	// CRDs not defined in the CSV present in the bundle
+	for key := range keySet {
+		result.Add(errors.WarnInvalidBundle(fmt.Sprintf("CRD %s is present in bundle %q but not defined in CSV", keyToString(key), bundle.Name), key))
+	}
+	return result
+}
+
+func getBundleCRDKeys(bundle *registry.Bundle) (map[registry.DefinitionKey]struct{}, errors.Error) {
+	crds, err := bundle.CustomResourceDefinitions()
+	if err != nil {
+		return nil, errors.ErrInvalidParse("error getting bundle CRDs", err)
+	}
+	keySet := map[registry.DefinitionKey]struct{}{}
+	for _, c := range crds {
+		switch crd := c.(type) {
+		case *apiextensionsv1.CustomResourceDefinition:
+			for _, version := range crd.Spec.Versions {
+				// Skip group, which CSVs do not support.
+				key := registry.DefinitionKey{
+					Name:    crd.GetName(),
+					Version: version.Name,
+					Kind:    crd.Spec.Names.Kind,
+				}
+				keySet[key] = struct{}{}
+			}
+		case *apiextensionsv1beta1.CustomResourceDefinition:
+			if crd.Spec.Version != "" {
+				key := registry.DefinitionKey{
+					Name:    crd.GetName(),
+					Version: crd.Spec.Version,
+					Kind:    crd.Spec.Names.Kind,
+				}
+				keySet[key] = struct{}{}
+			} else {
+				for _, version := range crd.Spec.Versions {
+					// Skip group, which CSVs do not support.
+					key := registry.DefinitionKey{
+						Name:    crd.GetName(),
+						Version: version.Name,
+						Kind:    crd.Spec.Names.Kind,
+					}
+					keySet[key] = struct{}{}
+				}
+			}
+		}
+	}
+	return keySet, errors.Error{}
+}
+
+func keyToString(key registry.DefinitionKey) string {
+	if key.Name == "" {
+		return fmt.Sprintf("%s/%s %s", key.Group, key.Version, key.Kind)
+	}
+	return fmt.Sprintf("%s/%s", key.Name, key.Version)
+}

pkg/lib/validation/bundle.go

@@ -0,0 +1,73 @@
+package validation
+
+import (
+	"io/ioutil"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/operator-framework/operator-registry/pkg/registry"
+	"github.com/stretchr/testify/require"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+
+	k8syaml "k8s.io/apimachinery/pkg/util/yaml"
+)
+
+func TestValidateBundle(t *testing.T) {
+	var table = []struct {
+		description string
+		directory   string
+		hasError    bool
+		errString   string
+	}{
+		{
+			description: "registryv1 bundle/valid bundle",
+			directory:   "./testdata/valid_bundle",
+			hasError:    false,
+		},
+		{
+			description: "registryv1 bundle/invalid bundle",
+			directory:   "./testdata/invalid_bundle",
+			hasError:    true,
+			errString:   "owned CRD etcdclusters.etcd.database.coreos.com/v1beta2 not found in bundle",
+		},
+		{
+			description: "registryv1 bundle/invalid bundle 2",
+			directory:   "./testdata/invalid_bundle_2",
+			hasError:    true,
+			errString:   `CRD etcdclusters.etcd.database.coreos.com/v1beta2 is present in bundle "test" but not defined in CSV`,
+		},
+	}
+
+	for _, tt := range table {
+		unstObjs := []*unstructured.Unstructured{}
+
+		// Read all files in manifests directory
+		items, err := ioutil.ReadDir(tt.directory)
+		require.NoError(t, err, "Unable to read directory: %s", tt.description)
+
+		for _, item := range items {
+			fileWithPath := filepath.Join(tt.directory, item.Name())
+			data, err := ioutil.ReadFile(fileWithPath)
+			require.NoError(t, err, "Unable to read file: %s", fileWithPath)
+
+			dec := k8syaml.NewYAMLOrJSONDecoder(strings.NewReader(string(data)), 30)
+			k8sFile := &unstructured.Unstructured{}
+			err = dec.Decode(k8sFile)
+			require.NoError(t, err, "Unable to decode file: %s", fileWithPath)
+
+			unstObjs = append(unstObjs, k8sFile)
+		}
+
+		// Validate the bundle object
+		bundle := registry.NewBundle("test", "", nil, unstObjs...)
+		results := BundleValidator.Validate(bundle)
+
+		if len(results) > 0 {
+			require.Equal(t, tt.hasError, results[0].HasError(), "%s: %s", tt.description, results[0])
+			if results[0].HasError() {
+				require.Contains(t, results[0].Errors[0].Error(), tt.errString)
+			}
+		}
+	}
+}

pkg/lib/validation/bundle_test.go

@@ -0,0 +1,13 @@
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: etcdbackups.etcd.database.coreos.com
+spec:
+  group: etcd.database.coreos.com
+  names:
+    kind: EtcdBackup
+    listKind: EtcdBackupList
+    plural: etcdbackups
+    singular: etcdbackup
+  scope: Namespaced
+  version: v1beta2