Merge pull request #179 from benluddy/bz-2000144

Bug 2000144: Mark GetBundleForChannel as deprecated and trim its response.

Author: openshift-merge-robot

staging/operator-registry/pkg/api/registry.proto

@@ -8,7 +8,9 @@ service Registry {
 	rpc ListPackages(ListPackageRequest) returns (stream PackageName) {}
 	rpc GetPackage(GetPackageRequest) returns (Package) {}
 	rpc GetBundle(GetBundleRequest) returns (Bundle) {}
-	rpc GetBundleForChannel(GetBundleInChannelRequest) returns (Bundle) {}
+	rpc GetBundleForChannel(GetBundleInChannelRequest) returns (Bundle) {
+		option deprecated = true;
+	}
 	rpc GetChannelEntriesThatReplace(GetAllReplacementsRequest) returns (stream ChannelEntry) {}
 	rpc GetBundleThatReplaces(GetReplacementRequest) returns (Bundle) {}
 	rpc GetChannelEntriesThatProvide(GetAllProvidersRequest) returns (stream ChannelEntry) {}

staging/operator-registry/pkg/registry/interface.go

@@ -37,7 +37,10 @@ type GRPCQuery interface {
 	// Get a bundle by its package name, channel name and csv name from the index
 	GetBundle(ctx context.Context, pkgName, channelName, csvName string) (*api.Bundle, error)
 
-	// Get the bundle in the specified package at the head of the specified channel
+	// Get the bundle in the specified package at the head of the
+	// specified channel. DEPRECATED. Returned bundles may have
+	// only the "name" and "csvJson" fields populated in order to
+	// support legacy usage.
 	GetBundleForChannel(ctx context.Context, pkgName string, channelName string) (*api.Bundle, error)
 
 	// Get all channel entries that say they replace this one

staging/operator-registry/pkg/registry/populator_test.go

@@ -171,7 +171,11 @@ func TestQuerierForImage(t *testing.T) {
 			{Group: "testapi.coreos.com", Version: "v1", Kind: "testapi"},
 		},
 	}
-	EqualBundles(t, *expectedBundle, *etcdBundleByChannel)
+	bareGetBundleForChannelResult := &api.Bundle{
+		CsvName: expectedBundle.CsvName,
+		CsvJson: expectedBundle.CsvJson + "\n",
+	}
+	EqualBundles(t, *bareGetBundleForChannelResult, *etcdBundleByChannel)
 
 	etcdBundle, err := store.GetBundle(context.TODO(), "etcd", "alpha", "etcdoperator.v0.9.2")
 	require.NoError(t, err)

staging/operator-registry/pkg/server/server_test.go

@@ -215,7 +215,13 @@ func testGetBundle(addr string, expected *api.Bundle) func(*testing.T) {
 }
 
 func TestGetBundleForChannel(t *testing.T) {
-	t.Run("Sqlite", testGetBundleForChannel(dbAddress, etcdoperator_v0_9_2("alpha", false, false)))
+	{
+		b := etcdoperator_v0_9_2("alpha", false, false)
+		t.Run("Sqlite", testGetBundleForChannel(dbAddress, &api.Bundle{
+			CsvName: b.CsvName,
+			CsvJson: b.CsvJson + "\n",
+		}))
+	}
 	t.Run("DeclarativeConfig", testGetBundleForChannel(cfgAddress, etcdoperator_v0_9_2("alpha", false, true)))
 }
 

staging/operator-registry/pkg/sqlite/configmap_test.go

@@ -175,7 +175,11 @@ func TestQuerierForConfigmap(t *testing.T) {
 			"{\"apiVersion\":\"apiextensions.k8s.io/v1beta1\",\"kind\":\"CustomResourceDefinition\",\"metadata\":{\"creationTimestamp\":null,\"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\",\"versions\":[{\"name\":\"v1beta2\",\"served\":true,\"storage\":true}]},\"status\":{\"acceptedNames\":{\"kind\":\"\",\"plural\":\"\"},\"conditions\":null,\"storedVersions\":null}}",
 			"{\"apiVersion\":\"apiextensions.k8s.io/v1beta1\",\"kind\":\"CustomResourceDefinition\",\"metadata\":{\"creationTimestamp\":null,\"name\":\"etcdrestores.etcd.database.coreos.com\"},\"spec\":{\"group\":\"etcd.database.coreos.com\",\"names\":{\"kind\":\"EtcdRestore\",\"listKind\":\"EtcdRestoreList\",\"plural\":\"etcdrestores\",\"singular\":\"etcdrestore\"},\"scope\":\"Namespaced\",\"version\":\"v1beta2\",\"versions\":[{\"name\":\"v1beta2\",\"served\":true,\"storage\":true}]},\"status\":{\"acceptedNames\":{\"kind\":\"\",\"plural\":\"\"},\"conditions\":null,\"storedVersions\":null}}"},
 	}
-	EqualBundles(t, *expectedBundle, *etcdBundleByChannel)
+	bareGetBundleForChannelResult := &api.Bundle{
+		CsvName: expectedBundle.CsvName,
+		CsvJson: expectedBundle.CsvJson + "\n",
+	}
+	EqualBundles(t, *bareGetBundleForChannelResult, *etcdBundleByChannel)
 
 	etcdBundle, err := store.GetBundle(context.TODO(), "etcd", "alpha", "etcdoperator.v0.9.2")
 	require.NoError(t, err)

staging/operator-registry/pkg/sqlite/directory_test.go

@@ -180,7 +180,11 @@ func TestQuerierForDirectory(t *testing.T) {
 			{Group: "etcd.database.coreos.com", Version: "v1beta2", Kind: "EtcdCluster", Plural: "etcdclusters"},
 		},
 	}
-	EqualBundles(t, *expectedBundle, *etcdBundleByChannel)
+	bareGetBundleForChannelResult := &api.Bundle{
+		CsvName: expectedBundle.CsvName,
+		CsvJson: expectedBundle.CsvJson + "\n",
+	}
+	EqualBundles(t, *bareGetBundleForChannelResult, *etcdBundleByChannel)
 
 	etcdBundle, err := store.GetBundle(context.TODO(), "etcd", "alpha", "etcdoperator.v0.9.2")
 	require.NoError(t, err)

staging/operator-registry/pkg/sqlite/query.go

@@ -287,64 +287,32 @@ func (s *SQLQuerier) GetBundle(ctx context.Context, pkgName, channelName, csvNam
 	return out, nil
 }
 
-func (s *SQLQuerier) GetBundleForChannel(ctx context.Context, pkgName string, channelName string) (*api.Bundle, error) {
-	query := `SELECT DISTINCT channel_entry.entry_id, operatorbundle.name, operatorbundle.bundle, operatorbundle.bundlepath, operatorbundle.version, operatorbundle.skiprange FROM channel
-              INNER JOIN operatorbundle ON channel.head_operatorbundle_name=operatorbundle.name
-              INNER JOIN channel_entry ON (channel_entry.channel_name = channel.name and channel_entry.package_name=channel.package_name and channel_entry.operatorbundle_name=operatorbundle.name)
-              WHERE channel.package_name=? AND channel.name=? LIMIT 1`
-	rows, err := s.db.QueryContext(ctx, query, pkgName, channelName)
+func (s *SQLQuerier) GetBundleForChannel(ctx context.Context, pkg string, channel string) (*api.Bundle, error) {
+	query := `
+SELECT operatorbundle.name, operatorbundle.csv FROM operatorbundle INNER JOIN channel
+ON channel.head_operatorbundle_name = operatorbundle.name
+WHERE channel.name = :channel AND channel.package_name = :package`
+	rows, err := s.db.QueryContext(ctx, query, sql.Named("channel", channel), sql.Named("package", pkg))
 	if err != nil {
 		return nil, err
 	}
 	defer rows.Close()
 
 	if !rows.Next() {
-		return nil, fmt.Errorf("no entry found for %s %s", pkgName, channelName)
-	}
-	var entryId sql.NullInt64
-	var name sql.NullString
-	var bundle sql.NullString
-	var bundlePath sql.NullString
-	var version sql.NullString
-	var skipRange sql.NullString
-	if err := rows.Scan(&entryId, &name, &bundle, &bundlePath, &version, &skipRange); err != nil {
-		return nil, err
-	}
-
-	out := &api.Bundle{}
-	if bundle.Valid && bundle.String != "" {
-		out, err = registry.BundleStringToAPIBundle(bundle.String)
-		if err != nil {
-			return nil, err
-		}
-	}
-	out.CsvName = name.String
-	out.PackageName = pkgName
-	out.ChannelName = channelName
-	out.BundlePath = bundlePath.String
-	out.Version = version.String
-	out.SkipRange = skipRange.String
-
-	provided, required, err := s.GetApisForEntry(ctx, entryId.Int64)
-	if err != nil {
-		return nil, err
-	}
-	out.ProvidedApis = provided
-	out.RequiredApis = required
-
-	dependencies, err := s.GetDependenciesForBundle(ctx, name.String, version.String, bundlePath.String)
-	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("no entry found for %s %s", pkg, channel)
 	}
-	out.Dependencies = dependencies
-
-	properties, err := s.GetPropertiesForBundle(ctx, name.String, version.String, bundlePath.String)
-	if err != nil {
+	var (
+		name sql.NullString
+		csv  sql.NullString
+	)
+	if err := rows.Scan(&name, &csv); err != nil {
 		return nil, err
 	}
-	out.Properties = properties
 
-	return out, nil
+	return &api.Bundle{
+		CsvName: name.String,
+		CsvJson: csv.String,
+	}, nil
 }
 
 func (s *SQLQuerier) GetChannelEntriesThatReplace(ctx context.Context, name string) (entries []*registry.ChannelEntry, err error) {