Address feedbacks (second round)

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

Author: dinhxuanvu

bundles/prometheus.0.22.2/metadata/dependencies.yaml

@@ -1,4 +1,4 @@
 dependencies:
   - type: olm.package
-    packageName: test-operator
-    version: ">0.2.0"
+    packageName: testoperator
+    version: "> 0.2.0"

pkg/api/registry.pb.go

@@ -12,7 +12,7 @@ service Registry {
 	rpc GetChannelEntriesThatProvide(GetAllProvidersRequest) returns (stream ChannelEntry) {}
 	rpc GetLatestChannelEntriesThatProvide(GetLatestProvidersRequest) returns (stream ChannelEntry) {}
 	rpc GetDefaultBundleThatProvides(GetDefaultProviderRequest) returns (Bundle) {}
-  rpc ListBundles(ListBundlesRequest) returns (stream Bundle) {}
+      rpc ListBundles(ListBundlesRequest) returns (stream Bundle) {}
 }
 
 
@@ -40,10 +40,7 @@ message GroupVersionKind{
 
 message Dependency{
   string type = 1;
-  string name = 2;
-  string group = 3;
-  string version = 4;
-  string kind = 5;
+  string value = 2;
 }
 
 message Bundle{
@@ -57,7 +54,7 @@ message Bundle{
 	repeated GroupVersionKind requiredApis = 8;
 	string version = 9;
 	string skipRange = 10;
-  repeated Dependency dependencies = 11;
+      repeated Dependency dependencies = 11;
 }
 
 message ChannelEntry{

pkg/api/registry.proto

@@ -35,7 +35,7 @@ type Bundle struct {
 	BundleImage  string
 	csv          *ClusterServiceVersion
 	crds         []*v1beta1.CustomResourceDefinition
-	Dependencies []Dependency
+	Dependencies []*Dependency
 	cacheStale   bool
 }
 

pkg/registry/bundle.go

@@ -2,6 +2,7 @@ package registry
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io/ioutil"
@@ -16,9 +17,9 @@ import (
 	"github.com/operator-framework/operator-registry/pkg/image"
 )
 
-const (
-	DependenciesFileName = "dependencies.yaml"
-)
+type Dependencies struct {
+	RawMessage []map[string]string `json:"dependencies" yaml:"dependencies"`
+}
 
 // DirectoryPopulator loads an unpacked operator bundle from a directory into the database.
 type DirectoryPopulator struct {
@@ -62,13 +63,12 @@ func (i *DirectoryPopulator) Populate(mode Mode) error {
 			log.Info("found annotations file searching for csv")
 			continue
 		}
-		if f.Name() == DependenciesFileName {
-			err = decodeFile(filepath.Join(metadata, f.Name()), dependenciesFile)
-			if err != nil {
-				log.Info("unable to parse dependencies.yaml file")
-			} else {
-				log.Info("found dependencies file searching for csv")
-			}
+
+		err = parseDependenciesFile(filepath.Join(metadata, f.Name()), dependenciesFile)
+		if err != nil || len(dependenciesFile.Dependencies) < 1 {
+			continue
+		} else {
+			log.Info("found dependencies file searching for csv")
 		}
 	}
 
@@ -112,7 +112,7 @@ func (i *DirectoryPopulator) loadManifests(manifests string, annotationsFile *An
 	// set the bundleimage on the bundle
 	bundle.BundleImage = i.to.String()
 	// set the dependencies on the bundle
-	bundle.Dependencies = dependenciesFile.Dependencies
+	bundle.Dependencies = dependenciesFile.GetDependencies()
 
 	bundle.Name = csvName
 	bundle.Package = annotationsFile.Annotations.PackageName
@@ -340,3 +340,36 @@ func decodeFile(path string, into interface{}) error {
 
 	return decoder.Decode(into)
 }
+
+func parseDependenciesFile(path string, depFile *DependenciesFile) error {
+	deps := Dependencies{}
+	err := decodeFile(path, &deps)
+	if err != nil || len(deps.RawMessage) == 0 {
+		return fmt.Errorf("Unable to decode the dependencies file %s", path)
+	}
+	depList := []Dependency{}
+	for _, v := range deps.RawMessage {
+		// convert map to json
+		jsonStr, _ := json.Marshal(v)
+		fmt.Println(string(jsonStr))
+
+		// Check dependency type
+		dep := Dependency{}
+		err := json.Unmarshal(jsonStr, &dep)
+		if err != nil {
+			return err
+		}
+
+		switch dep.GetType() {
+		case "olm.gvk", "olm.package":
+			dep.Value = string(jsonStr)
+		default:
+			return fmt.Errorf("Unsupported dependency type %s", dep.GetType())
+		}
+		depList = append(depList, dep)
+	}
+
+	depFile.Dependencies = depList
+
+	return nil
+}

pkg/registry/populator.go

@@ -118,10 +118,8 @@ func TestQuerierForImage(t *testing.T) {
 		SkipRange: "",
 		Dependencies: []*api.Dependency{
 			{
-				Type:    "olm.gvk",
-				Group:   "testapi.coreos.com",
-				Kind:    "testapi",
-				Version: "v1",
+				Type:  "olm.gvk",
+				Value: `{"group":"testapi.coreos.com","kind":"testapi","type":"olm.gvk","version":"v1"}`,
 			},
 		},
 		ProvidedApis: []*api.GroupVersionKind{
@@ -351,15 +349,12 @@ func TestQuerierForDependencies(t *testing.T) {
 
 	expectedDependencies := []*api.Dependency{
 		{
-			Type:    "olm.package",
-			Name:    "test-operator",
-			Version: ">0.2.0",
+			Type:  "olm.package",
+			Value: `{"packageName":"testoperator","type":"olm.package","version":"\u003e 0.2.0"}`,
 		},
 		{
-			Type:    "olm.gvk",
-			Group:   "testapi.coreos.com",
-			Kind:    "testapi",
-			Version: "v1",
+			Type:  "olm.gvk",
+			Value: `{"group":"testapi.coreos.com","kind":"testapi","type":"olm.gvk","version":"v1"}`,
 		},
 	}
 
@@ -401,15 +396,12 @@ func TestListBundles(t *testing.T) {
 
 	expectedDependencies := []*api.Dependency{
 		{
-			Type:    "olm.package",
-			Name:    "test-operator",
-			Version: ">0.2.0",
+			Type:  "olm.package",
+			Value: `{"packageName":"testoperator","type":"olm.package","version":"\u003e 0.2.0"}`,
 		},
 		{
-			Type:    "olm.gvk",
-			Group:   "testapi.coreos.com",
-			Kind:    "testapi",
-			Version: "v1",
+			Type:  "olm.gvk",
+			Value: `{"group":"testapi.coreos.com","kind":"testapi","type":"olm.gvk","version":"v1"}`,
 		},
 	}
 
@@ -418,7 +410,6 @@ func TestListBundles(t *testing.T) {
 	require.NoError(t, err)
 	for _, b := range bundles {
 		dep := b.Dependencies
-		fmt.Printf("Dep: %v\n", dep)
 		dependencies = append(dependencies, dep...)
 	}
 	require.ElementsMatch(t, expectedDependencies, dependencies)

pkg/registry/populator_test.go

@@ -126,9 +126,11 @@ type Dependency struct {
 	// dependency or `olm.gvk` for gvk based dependency. This field is required.
 	Type string `json:"type" yaml:"type"`
 
-	// The name of dependency such as 'etcd'
-	Name string `json:"packageName" yaml:"packageName"`
+	// The value of the dependency (either GVKDependency or PackageDependency)
+	Value string `json:"value" yaml:"value"`
+}
 
+type GVKDependency struct {
 	// The group of GVK based dependency
 	Group string `json:"group" yaml:"group"`
 
@@ -139,42 +141,27 @@ type Dependency struct {
 	Version string `json:"version" yaml:"version"`
 }
 
-// GetType returns the type of dependency
-func (e *Dependency) GetType() string {
-	if e.Type != "" {
-		return e.Type
-	}
-	return ""
-}
-
-// GetName returns the package name of dependency
-func (e *Dependency) GetName() string {
-	if e.Name != "" {
-		return e.Name
-	}
-	return ""
-}
+type PackageDependency struct {
+	// The name of dependency such as 'etcd'
+	PackageName string `json:"packageName" yaml:"packageName"`
 
-// GetGroup returns the group name of dependency
-func (e *Dependency) GetGroup() string {
-	if e.Group != "" {
-		return e.Group
-	}
-	return ""
+	// The version of dependency in semver format
+	Version string `json:"version" yaml:"version"`
 }
 
-// GetKind returns the kind of dependency
-func (e *Dependency) GetKind() string {
-	if e.Kind != "" {
-		return e.Kind
+// GetDependencies returns the list of dependency
+func (d *DependenciesFile) GetDependencies() []*Dependency {
+	var dependencies []*Dependency
+	for _, item := range d.Dependencies {
+		dependencies = append(dependencies, &item)
 	}
-	return ""
+	return dependencies
 }
 
-// GetVersion returns the version of dependency
-func (e *Dependency) GetVersion() string {
-	if e.Version != "" {
-		return e.Version
+// GetType returns the type of dependency
+func (e *Dependency) GetType() string {
+	if e.Type != "" {
+		return e.Type
 	}
 	return ""
 }

pkg/registry/types.go

@@ -755,7 +755,7 @@ func (s *SQLLoader) AddBundlePackageChannels(manifest registry.PackageManifest,
 }
 
 func (s *SQLLoader) addDependencies(tx *sql.Tx, bundle *registry.Bundle) error {
-	addDep, err := tx.Prepare("insert into dependencies(type, package_name, group_name, version, kind, operatorbundle_name, operatorbundle_version, operatorbundle_path) values(?, ?, ?, ?, ?, ?, ?, ?)")
+	addDep, err := tx.Prepare("insert into dependencies(type, value, operatorbundle_name, operatorbundle_version, operatorbundle_path) values(?, ?, ?, ?, ?)")
 	if err != nil {
 		return err
 	}
@@ -770,7 +770,7 @@ func (s *SQLLoader) addDependencies(tx *sql.Tx, bundle *registry.Bundle) error {
 		return sql.NullString{String: s, Valid: s != ""}
 	}
 	for _, dep := range bundle.Dependencies {
-		if _, err := addDep.Exec(dep.Type, dep.Name, dep.Group, dep.Version, dep.Kind, bundle.Name, sqlString(bundleVersion), sqlString(bundle.BundleImage)); err != nil {
+		if _, err := addDep.Exec(dep.Type, dep.Value, bundle.Name, sqlString(bundleVersion), sqlString(bundle.BundleImage)); err != nil {
 			return err
 		}
 	}

pkg/sqlite/load.go

@@ -3,6 +3,7 @@ package migrations
 import (
 	"context"
 	"database/sql"
+	"encoding/json"
 
 	"github.com/operator-framework/operator-registry/pkg/registry"
 )
@@ -20,10 +21,7 @@ var dependenciesMigration = &Migration{
 		sql := `
 		CREATE TABLE IF NOT EXISTS dependencies (
 			type TEXT,
-			package_name TEXT,
-			group_name TEXT,
-			version TEXT,
-			kind TEXT,
+			value TEXT,
 			operatorbundle_name TEXT,
 			operatorbundle_version TEXT,
 			operatorbundle_path TEXT,
@@ -35,15 +33,25 @@ var dependenciesMigration = &Migration{
 			return err
 		}
 
-		insertRequired := `INSERT INTO dependencies(type, package_name, group_name, version, kind, operatorbundle_name, operatorbundle_version, operatorbundle_path) VALUES (?, ?, ?, ?, ?, ?, ?, ?)`
+		insertRequired := `INSERT INTO dependencies(type, value, operatorbundle_name, operatorbundle_version, operatorbundle_path) VALUES (?, ?, ?, ?, ?)`
 
 		bundleApis, err := getRequiredAPIs(ctx, tx)
 		if err != nil {
 			return err
 		}
 		for bundle, apis := range bundleApis {
 			for required := range apis.required {
-				_, err := tx.ExecContext(ctx, insertRequired, "olm.gvk", nil, required.Group, required.Version, required.Kind, bundle.CsvName, bundle.Version, bundle.BundlePath)
+				valueMap := map[string]string{
+					"type":    "olm.gvk",
+					"group":   required.Group,
+					"version": required.Version,
+					"kind":    required.Kind,
+				}
+				value, err := json.Marshal(valueMap)
+				if err != nil {
+					return err
+				}
+				_, err = tx.ExecContext(ctx, insertRequired, "olm.gvk", value, bundle.CsvName, bundle.Version, bundle.BundlePath)
 				if err != nil {
 					return err
 				}

pkg/sqlite/migrations/008_dependencies.go

@@ -31,24 +31,18 @@ func TestDependenciesUp(t *testing.T) {
 	err = migrator.Up(context.TODO(), migrations.Only(migrations.DependenciesMigrationKey))
 	require.NoError(t, err)
 
-	depQuery := `SELECT DISTINCT type, package_name, group_name, version, kind FROM dependencies
+	depQuery := `SELECT DISTINCT type, value FROM dependencies
 	WHERE operatorbundle_name=? AND operatorbundle_version=? AND operatorbundle_path=?`
 
 	rows, err := db.Query(depQuery, "etcdoperator.v0.6.1", "0.6.1", "quay.io/image")
 	require.NoError(t, err)
 	defer rows.Close()
 	rows.Next()
 	var typeName sql.NullString
-	var name sql.NullString
-	var group sql.NullString
-	var version sql.NullString
-	var kind sql.NullString
-	require.NoError(t, rows.Scan(&typeName, &name, &group, &version, &kind))
+	var value sql.NullString
+	require.NoError(t, rows.Scan(&typeName, &value))
 	require.Equal(t, typeName.String, "olm.gvk")
-	require.Equal(t, name.String, "")
-	require.Equal(t, group.String, "test.coreos.com")
-	require.Equal(t, version.String, "v1")
-	require.Equal(t, kind.String, "testapi")
+	require.Equal(t, value.String, `{"group":"test.coreos.com","kind":"testapi","type":"olm.gvk","version":"v1"}`)
 	require.NoError(t, rows.Close())
 }
 
@@ -67,35 +61,30 @@ func TestDependenciesDown(t *testing.T) {
 	insert := "insert into operatorbundle(name, csv, bundle, bundlepath, version, skiprange, replaces, skips) values(?, ?, ?, ?, ?, ?, ?, ?)"
 	_, err = db.Exec(insert, "etcdoperator.v0.6.1", testCSV, testBundle, "quay.io/image", "0.6.1", ">0.5.0 <0.6.1", "0.9.0", "0.9.1,0.9.2")
 	require.NoError(t, err)
-	_, err = db.Exec("insert into dependencies(type, package_name, group_name, version, kind, operatorbundle_name, operatorbundle_version, operatorbundle_path) VALUES (?, ?, ?, ?, ?, ?, ?, ?)", "olm.package", "etcd-operator", "", "0.6.0", "", "etcdoperator.v0.6.1", "0.6.1", "quay.io/image")
+	valueStr := `{"packageName":"etcd-operator","type":"olm.package","version":">0.6.0"}`
+	_, err = db.Exec("insert into dependencies(type, value, operatorbundle_name, operatorbundle_version, operatorbundle_path) VALUES (?, ?, ?, ?, ?)", "olm.package", valueStr, "etcdoperator.v0.6.1", "0.6.1", "quay.io/image")
 	require.NoError(t, err)
 	require.NoError(t, tx.Commit())
 
-	depQuery := `SELECT DISTINCT type, package_name, group_name, version, kind FROM dependencies
+	depQuery := `SELECT DISTINCT type, value FROM dependencies
 	WHERE operatorbundle_name=? AND operatorbundle_version=? AND operatorbundle_path=?`
 
 	rows, err := db.Query(depQuery, "etcdoperator.v0.6.1", "0.6.1", "quay.io/image")
 	require.NoError(t, err)
 	defer rows.Close()
 	rows.Next()
 	var typeName sql.NullString
-	var name sql.NullString
-	var group sql.NullString
-	var version sql.NullString
-	var kind sql.NullString
-	require.NoError(t, rows.Scan(&typeName, &name, &group, &version, &kind))
+	var value sql.NullString
+	require.NoError(t, rows.Scan(&typeName, &value))
 	require.Equal(t, typeName.String, "olm.package")
-	require.Equal(t, name.String, "etcd-operator")
-	require.Equal(t, group.String, "")
-	require.Equal(t, version.String, "0.6.0")
-	require.Equal(t, kind.String, "")
+	require.Equal(t, value.String, valueStr)
 	require.NoError(t, rows.Close())
 
 	// run down migration
 	err = migrator.Down(context.TODO(), migrations.Only(migrations.DependenciesMigrationKey))
 	require.NoError(t, err)
 
 	// check that no dependencies were extracted.
-	rows, err = db.Query(depQuery, "etcdoperator.v0.6.1", "0.6.1", "quay.io/image")
+	rows, _ = db.Query(depQuery, "etcdoperator.v0.6.1", "0.6.1", "quay.io/image")
 	require.False(t, rows.Next())
 }

pkg/sqlite/migrations/008_dependencies_test.go

@@ -783,14 +783,13 @@ func (s *SQLQuerier) GetCurrentCSVNameForChannel(ctx context.Context, pkgName, c
 
 func (s *SQLQuerier) ListBundles(ctx context.Context) (bundles []*api.Bundle, err error) {
 	query := `SELECT DISTINCT channel_entry.entry_id, operatorbundle.bundle, operatorbundle.bundlepath,
-	MIN(channel_entry.depth), channel_entry.operatorbundle_name, channel_entry.package_name,
-	channel_entry.channel_name, channel_entry.replaces, operatorbundle.version, operatorbundle.skiprange,
-	dependencies.type, dependencies.package_name, dependencies.group_name, dependencies.version, dependencies.kind
+	channel_entry.operatorbundle_name, channel_entry.package_name, channel_entry.channel_name, channel_entry.replaces,
+	operatorbundle.version, operatorbundle.skiprange,
+	dependencies.type, dependencies.value
 	FROM channel_entry
 	INNER JOIN operatorbundle ON operatorbundle.name = channel_entry.operatorbundle_name
 	INNER JOIN dependencies ON dependencies.operatorbundle_name = channel_entry.operatorbundle_name
-	INNER JOIN package ON package.name = channel_entry.package_name
-	GROUP BY channel_entry.package_name, channel_entry.channel_name`
+	INNER JOIN package ON package.name = channel_entry.package_name`
 
 	rows, err := s.db.QueryContext(ctx, query)
 	if err != nil {
@@ -804,19 +803,15 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) (bundles []*api.Bundle, er
 		var entryID sql.NullInt64
 		var bundle sql.NullString
 		var bundlePath sql.NullString
-		var minDepth sql.NullInt64
 		var bundleName sql.NullString
 		var pkgName sql.NullString
 		var channelName sql.NullString
 		var replaces sql.NullString
 		var version sql.NullString
 		var skipRange sql.NullString
 		var depType sql.NullString
-		var depPkgName sql.NullString
-		var depGroup sql.NullString
-		var depVersion sql.NullString
-		var depKind sql.NullString
-		if err := rows.Scan(&entryID, &bundle, &bundlePath, &minDepth, &bundleName, &pkgName, &channelName, &replaces, &version, &skipRange, &depType, &depPkgName, &depGroup, &depVersion, &depKind); err != nil {
+		var depValue sql.NullString
+		if err := rows.Scan(&entryID, &bundle, &bundlePath, &bundleName, &pkgName, &channelName, &replaces, &version, &skipRange, &depType, &depValue); err != nil {
 			return nil, err
 		}
 
@@ -826,10 +821,7 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) (bundles []*api.Bundle, er
 			// Create new dependency object
 			dep := &api.Dependency{}
 			dep.Type = depType.String
-			dep.Name = depPkgName.String
-			dep.Group = depGroup.String
-			dep.Version = depVersion.String
-			dep.Kind = depKind.String
+			dep.Value = depValue.String
 
 			// Add new dependency to the existing list
 			existingDeps := bundleItem.Dependencies
@@ -863,10 +855,7 @@ func (s *SQLQuerier) ListBundles(ctx context.Context) (bundles []*api.Bundle, er
 			dep := &api.Dependency{}
 			dependencies := []*api.Dependency{}
 			dep.Type = depType.String
-			dep.Name = depPkgName.String
-			dep.Group = depGroup.String
-			dep.Version = depVersion.String
-			dep.Kind = depKind.String
+			dep.Value = depValue.String
 			dependencies = append(dependencies, dep)
 			out.Dependencies = dependencies
 
@@ -889,7 +878,7 @@ func unique(deps []*api.Dependency) []*api.Dependency {
 	keys := make(map[string]bool)
 	list := []*api.Dependency{}
 	for _, entry := range deps {
-		depKey := fmt.Sprintf("%s/%s/%s/%s/%s", entry.Type, entry.Name, entry.Group, entry.Version, entry.Kind)
+		depKey := fmt.Sprintf("%s/%s", entry.Type, entry.Value)
 		if _, value := keys[depKey]; !value {
 			keys[depKey] = true
 			list = append(list, entry)
@@ -899,7 +888,7 @@ func unique(deps []*api.Dependency) []*api.Dependency {
 }
 
 func (s *SQLQuerier) GetDependenciesForBundle(ctx context.Context, name, version, path string) (dependencies []*api.Dependency, err error) {
-	depQuery := `SELECT DISTINCT dependencies.type, dependencies.package_name, dependencies.group_name, dependencies.version, dependencies.kind FROM dependencies
+	depQuery := `SELECT DISTINCT dependencies.type, dependencies.value FROM dependencies
 	WHERE dependencies.operatorbundle_name=? AND dependencies.operatorbundle_version=? AND dependencies.operatorbundle_path=?`
 
 	rows, err := s.db.QueryContext(ctx, depQuery, name, version, path)
@@ -909,23 +898,17 @@ func (s *SQLQuerier) GetDependenciesForBundle(ctx context.Context, name, version
 	dependencies = []*api.Dependency{}
 	for rows.Next() {
 		var typeName sql.NullString
-		var packageName sql.NullString
-		var groupName sql.NullString
-		var versionName sql.NullString
-		var kindName sql.NullString
+		var value sql.NullString
 
-		if err := rows.Scan(&typeName, &packageName, &groupName, &versionName, &kindName); err != nil {
+		if err := rows.Scan(&typeName, &value); err != nil {
 			return nil, err
 		}
-		if !typeName.Valid || !packageName.Valid || !groupName.Valid || !versionName.Valid || !kindName.Valid {
+		if !typeName.Valid || !value.Valid {
 			return nil, err
 		}
 		dependencies = append(dependencies, &api.Dependency{
-			Type:    typeName.String,
-			Name:    packageName.String,
-			Group:   groupName.String,
-			Version: versionName.String,
-			Kind:    kindName.String,
+			Type:  typeName.String,
+			Value: value.String,
 		})
 	}
 	if err := rows.Close(); err != nil {