Merge pull request #618 from dinhxuanvu/declcfg-validate

feat(cmd): Add validate subcommand to opm cmd to validate config files

Author: dinhxuanvu

cmd/opm/root/cmd.go

@@ -8,6 +8,7 @@ import (
 	"github.com/operator-framework/operator-registry/cmd/opm/index"
 	"github.com/operator-framework/operator-registry/cmd/opm/registry"
 	"github.com/operator-framework/operator-registry/cmd/opm/serve"
+	"github.com/operator-framework/operator-registry/cmd/opm/validate"
 	"github.com/operator-framework/operator-registry/cmd/opm/version"
 )
 
@@ -24,7 +25,7 @@ func NewCmd() *cobra.Command {
 		},
 	}
 
-	cmd.AddCommand(registry.NewOpmRegistryCmd(), alpha.NewCmd(), serve.NewCmd(), newAddCmd())
+	cmd.AddCommand(registry.NewOpmRegistryCmd(), alpha.NewCmd(), serve.NewCmd(), newAddCmd(), validate.NewCmd())
 	index.AddCommand(cmd)
 	version.AddCommand(cmd)
 

cmd/opm/validate/validate.go

@@ -0,0 +1,49 @@
+package validate
+
+import (
+	"fmt"
+	"os"
+
+	"github.com/sirupsen/logrus"
+	"github.com/spf13/cobra"
+
+	"github.com/operator-framework/operator-registry/pkg/lib/config"
+)
+
+func NewCmd() *cobra.Command {
+	validate := &cobra.Command{
+		Use:   "validate <directory>",
+		Short: "Validate the declarative index config",
+		Long:  "Validate the declarative config JSON file(s) in a given directory",
+		Args:  cobra.ExactArgs(1),
+		RunE:  configValidate,
+	}
+
+	validate.Flags().BoolP("debug", "d", false, "enable debug log output")
+	return validate
+}
+
+func configValidate(cmd *cobra.Command, args []string) error {
+	debug, err := cmd.Flags().GetBool("debug")
+	if err != nil {
+		return err
+	}
+
+	directory := args[0]
+	if _, err := os.Stat(directory); os.IsNotExist(err) {
+		return err
+	}
+
+	logger := logrus.WithField("cmd", "validate")
+	if debug {
+		logger.Logger.SetLevel(logrus.DebugLevel)
+	}
+
+	err = config.ValidateConfig(directory)
+	if err != nil {
+		logger.Error(err.Error())
+		return fmt.Errorf("failed to validate config: %s", err)
+	}
+
+	return nil
+}

internal/declcfg/declcfg_to_model.go

@@ -41,10 +41,18 @@ func ConvertToModel(cfg DeclarativeConfig) (model.Model, error) {
 			return nil, fmt.Errorf("parse properties for bundle %q: %v", b.Name, err)
 		}
 
+		if len(props.Packages) == 0 {
+			return nil, fmt.Errorf("missing package property for bundle %q", b.Name)
+		}
+
 		if b.Package != props.Packages[0].PackageName {
 			return nil, fmt.Errorf("package %q does not match %q property %q", b.Package, property.TypePackage, props.Packages[0].PackageName)
 		}
 
+		if len(props.Channels) == 0 {
+			return nil, fmt.Errorf("bundle %q is missing channel information", b.Name)
+		}
+
 		for _, bundleChannel := range props.Channels {
 			pkgChannel, ok := mpkg.Channels[bundleChannel.Name]
 			if !ok {
@@ -75,7 +83,7 @@ func ConvertToModel(cfg DeclarativeConfig) (model.Model, error) {
 
 	for _, mpkg := range mpkgs {
 		defaultChannelName := defaultChannels[mpkg.Name]
-		if mpkg.DefaultChannel == nil {
+		if defaultChannelName != "" && mpkg.DefaultChannel == nil {
 			dch := &model.Channel{
 				Package: mpkg,
 				Name:    defaultChannelName,

internal/declcfg/declcfg_to_model_test.go

@@ -28,11 +28,11 @@ func TestConvertToModel(t *testing.T) {
 			assertion: require.Error,
 			cfg: DeclarativeConfig{
 				Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
-				Bundles:  []Bundle{newTestBundle("bar", "0.1.0")},
+				Bundles:  []Bundle{newTestBundle("bar", "0.1.0", withChannel("alpha", ""))},
 			},
 		},
 		{
-			name:      "Error/FailedModelValidation",
+			name:      "Error/BundleMissingChannel",
 			assertion: require.Error,
 			cfg: DeclarativeConfig{
 				Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
@@ -47,6 +47,38 @@ func TestConvertToModel(t *testing.T) {
 				Bundles:  []Bundle{newTestBundle("foo", "0.1.0", withChannel("alpha", "1"), withChannel("alpha", "2"))},
 			},
 		},
+		{
+			name:      "Error/BundleMissingDefaultChannel",
+			assertion: require.Error,
+			cfg: DeclarativeConfig{
+				Packages: []Package{newTestPackage("foo", "", svgSmallCircle)},
+				Bundles:  []Bundle{newTestBundle("foo", "0.1.0", withChannel("alpha", ""))},
+			},
+		},
+		{
+			name:      "Error/BundleMissingImageAndData",
+			assertion: require.Error,
+			cfg: DeclarativeConfig{
+				Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
+				Bundles:  []Bundle{newTestBundle("foo", "0.1.0", withChannel("alpha", ""), withNoBundleImage(), withNoBundleData())},
+			},
+		},
+		{
+			name:      "NoError/BundleMissingProperties",
+			assertion: require.Error,
+			cfg: DeclarativeConfig{
+				Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
+				Bundles:  []Bundle{newTestBundle("foo", "0.1.0", withChannel("alpha", ""), withNoProperties())},
+			},
+		},
+		{
+			name:      "NoError/BundleWithDataButMissingImage",
+			assertion: require.NoError,
+			cfg: DeclarativeConfig{
+				Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
+				Bundles:  []Bundle{newTestBundle("foo", "0.1.0", withChannel("alpha", ""), withNoBundleImage())},
+			},
+		},
 		{
 			name:      "Success/ValidModel",
 			assertion: require.NoError,

internal/declcfg/helpers_test.go

@@ -79,6 +79,25 @@ func withSkips(name string) func(*Bundle) {
 	}
 }
 
+func withNoProperties() func(*Bundle) {
+	return func(b *Bundle) {
+		b.Properties = []property.Property{}
+	}
+}
+
+func withNoBundleImage() func(*Bundle) {
+	return func(b *Bundle) {
+		b.Image = ""
+	}
+}
+
+func withNoBundleData() func(*Bundle) {
+	return func(b *Bundle) {
+		b.Objects = []string{}
+		b.CsvJSON = ""
+	}
+}
+
 func newTestBundle(packageName, version string, opts ...bundleOpt) Bundle {
 	csvJson := fmt.Sprintf(`{"kind": "ClusterServiceVersion", "apiVersion": "operators.coreos.com/v1alpha1", "metadata":{"name":%q}}`, testBundleName(packageName, version))
 	b := Bundle{

internal/model/model.go

@@ -54,14 +54,14 @@ func (m *Package) Validate() error {
 		result = multierror.Append(result, fmt.Errorf("invalid icon: %v", err))
 	}
 
-	if len(m.Channels) == 0 {
-		result = multierror.Append(result, fmt.Errorf("package must contain at least one channel"))
-	}
-
 	if m.DefaultChannel == nil {
 		result = multierror.Append(result, fmt.Errorf("default channel must be set"))
 	}
 
+	if len(m.Channels) == 0 {
+		result = multierror.Append(result, fmt.Errorf("package must contain at least one channel"))
+	}
+
 	foundDefault := false
 	for name, ch := range m.Channels {
 		if name != ch.Name {
@@ -260,6 +260,10 @@ func (b *Bundle) Validate() error {
 		result = multierror.Append(result, fmt.Errorf("must be exactly one property with type %q", property.TypePackage))
 	}
 
+	if b.Image == "" && len(b.Objects) == 0 {
+		result = multierror.Append(result, errors.New("bundle image must be set"))
+	}
+
 	return result.ErrorOrNil()
 }
 

internal/model/model_test.go

@@ -362,7 +362,7 @@ func TestValidators(t *testing.T) {
 				Package:  pkg,
 				Channel:  ch,
 				Name:     "anakin.v0.1.0",
-				Image:    "",
+				Image:    "registry.io/image",
 				Replaces: "anakin.v0.0.1",
 				Skips:    []string{"anakin.v0.0.2"},
 				Properties: []property.Property{
@@ -375,6 +375,39 @@ func TestValidators(t *testing.T) {
 			},
 			assertion: require.NoError,
 		},
+		{
+			name: "Bundle/Success/NoBundleImage/HaveBundleData",
+			v: &Bundle{
+				Package: pkg,
+				Channel: ch,
+				Name:    "anakin.v0.1.0",
+				Image:   "",
+				Properties: []property.Property{
+					property.MustBuildPackage("anakin", "0.1.0"),
+					property.MustBuildGVK("skywalker.me", "v1alpha1", "PodRacer"),
+					property.MustBuildChannel("light", "anakin.v0.0.1"),
+					property.MustBuildBundleObjectRef("path/to/data"),
+				},
+				Objects: []string{"testdata"},
+				CsvJSON: "CSVjson",
+			},
+			assertion: require.NoError,
+		},
+		{
+			name: "Bundle/Error/NoBundleImage/NoBundleData",
+			v: &Bundle{
+				Package: pkg,
+				Channel: ch,
+				Name:    "anakin.v0.1.0",
+				Image:   "",
+				Properties: []property.Property{
+					property.MustBuildPackage("anakin", "0.1.0"),
+					property.MustBuildGVK("skywalker.me", "v1alpha1", "PodRacer"),
+					property.MustBuildChannel("light", "anakin.v0.0.1"),
+				},
+			},
+			assertion: require.Error,
+		},
 		{
 			name:      "Bundle/Error/NoName",
 			v:         &Bundle{},

pkg/lib/config/validate.go

@@ -0,0 +1,29 @@
+package config
+
+import (
+	"github.com/operator-framework/operator-registry/internal/declcfg"
+)
+
+// ValidateConfig takes a directory containing the declarative config file(s)
+// 1. Validate if declarative config file(s) are valid based on specified schema
+// 2. Validate the `replaces` chains of the upgrade graph
+// Inputs:
+// directory: the directory where declarative config file(s) exist
+// Outputs:
+// error: a wrapped error that contains a list of error strings
+func ValidateConfig(directory string) error {
+	// Load config files and convert them to declcfg objects
+	cfg, err := declcfg.LoadDir(directory)
+	if err != nil {
+		return err
+	}
+	// Validate the config using model validation:
+	// This will convert declcfg objects to intermediate model objects that are
+	// also used for serve and add commands. The conversion process will run
+	// validation for the model objects and ensure they are valid.
+	_, err = declcfg.ConvertToModel(*cfg)
+	if err != nil {
+		return err
+	}
+	return nil
+}