Skip to content

Mark GetBundleForChannel as deprecated and trim its response. #769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 1, 2021
Merged

Mark GetBundleForChannel as deprecated and trim its response. #769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pkg/api/registry.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand Down
5 changes: 4 additions & 1 deletion pkg/registry/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion pkg/registry/populator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 7 additions & 1 deletion pkg/server/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,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)))
}

Expand Down
6 changes: 5 additions & 1 deletion pkg/sqlite/configmap_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion pkg/sqlite/directory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
64 changes: 16 additions & 48 deletions pkg/sqlite/query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Member

@timflannagan timflannagan Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL sql.Named is a thing 👍

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) {
Expand Down