Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions staging/operator-registry/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ The server exposes the following gRPC services:
- `GetLatestChannelEntriesThatProvide(group, version, kind)` - Stream latest entries providing an API
- `GetDefaultBundleThatProvides(group, version, kind)` - Get default bundle providing an API
- `ListBundles()` - Stream all bundles
- `ListPackageCustomSchemas(schema, packageName)` - Stream raw FBC blobs for a non-standard schema scoped to a package

**Health Service:**
- Standard gRPC health check service for monitoring
Expand Down
4 changes: 2 additions & 2 deletions staging/operator-registry/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ fix-lint: $(GOLANGCI_LINT)

.PHONY: codegen
codegen: $(PROTOC) $(PROTOC_GEN_GO_GRPC)
$(PROTOC) --plugin=protoc-gen-go=$(PROTOC_GEN_GO_GRPC) -I pkg/api/ --go_out=pkg/api pkg/api/*.proto
$(PROTOC) --plugin=protoc-gen-go-grpc=$(PROTOC_GEN_GO_GRPC) -I pkg/api/ --go-grpc_out=pkg/api pkg/api/*.proto
$(PROTOC) --plugin=protoc-gen-go=$(PROTOC_GEN_GO_GRPC) -I pkg/api/ -I ./tools/bin/include --go_out=pkg/api pkg/api/*.proto
$(PROTOC) --plugin=protoc-gen-go-grpc=$(PROTOC_GEN_GO_GRPC) -I pkg/api/ -I ./tools/bin/include --go-grpc_out=pkg/api pkg/api/*.proto

.PHONY: generate-fakes
generate-fakes:
Expand Down
1 change: 1 addition & 0 deletions staging/operator-registry/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ GetBundleForChannel
GetBundleThatReplaces
GetChannelEntriesThatProvide
GetChannelEntriesThatReplace
ListPackageCustomSchemas
GetDefaultBundleThatProvides
GetLatestChannelEntriesThatProvide
GetPackage
Expand Down
1,058 changes: 376 additions & 682 deletions staging/operator-registry/pkg/api/registry.pb.go

Large diffs are not rendered by default.

11 changes: 10 additions & 1 deletion staging/operator-registry/pkg/api/registry.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ syntax = "proto3";

option go_package = ".;api";

import "google/protobuf/struct.proto";

package api;

service Registry {
Expand All @@ -17,6 +19,7 @@ service Registry {
rpc GetLatestChannelEntriesThatProvide(GetLatestProvidersRequest) returns (stream ChannelEntry) {}
rpc GetDefaultBundleThatProvides(GetDefaultProviderRequest) returns (Bundle) {}
rpc ListBundles(ListBundlesRequest) returns (stream Bundle) {}
rpc ListPackageCustomSchemas(ListPackageCustomSchemasRequest) returns (stream google.protobuf.Struct) {}
}

message Channel{
Expand Down Expand Up @@ -130,4 +133,10 @@ message GetDefaultProviderRequest{

message Deprecation{
string message = 1;
}
}

message ListPackageCustomSchemasRequest{
string schema = 1;
string packageName = 2;
}

65 changes: 65 additions & 0 deletions staging/operator-registry/pkg/api/registry_grpc.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 28 additions & 0 deletions staging/operator-registry/pkg/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ type Cache interface {
Build(ctx context.Context, fbc fs.FS) error
Load(ctc context.Context) error
Close() error

ListPackageCustomSchemas(ctx context.Context, schema, packageName string, sender func([]byte) error) error
}

type backend interface {
Expand All @@ -45,6 +47,9 @@ type backend interface {
GetBundle(context.Context, bundleKey) (*api.Bundle, error)
PutBundle(context.Context, bundleKey, *api.Bundle) error

PutMeta(context.Context, metaKey, []byte) error
SendMetas(context.Context, metaKey, func([]byte) error) error

GetDigest(context.Context) (string, error)
ComputeDigest(context.Context, fs.FS) (string, error)
PutDigest(context.Context, string) error
Expand Down Expand Up @@ -237,6 +242,14 @@ func (c *cache) GetBundleThatProvides(ctx context.Context, group, version, kind
return c.packageIndex.GetBundleThatProvides(ctx, c, group, version, kind)
}

func (c *cache) ListPackageCustomSchemas(ctx context.Context, schema, packageName string, sender func([]byte) error) error {
mk, err := newValidatedMetaKey(schema, packageName)
if err != nil {
return fmt.Errorf("invalid custom schema query: %v", err)
}
Comment on lines +245 to +249
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# First, check if the file exists and its line count
wc -l staging/operator-registry/pkg/cache/cache.go

Repository: openshift/operator-framework-olm

Length of output: 125


🏁 Script executed:

# Examine lines around 245-249
sed -n '245,249p' staging/operator-registry/pkg/cache/cache.go

Repository: openshift/operator-framework-olm

Length of output: 334


🏁 Script executed:

# Examine lines around 310-316
sed -n '310,316p' staging/operator-registry/pkg/cache/cache.go

Repository: openshift/operator-framework-olm

Length of output: 357


🏁 Script executed:

# Check for all fmt.Errorf usages with %v in this file to understand the pattern
rg 'fmt\.Errorf\(.*%v' staging/operator-registry/pkg/cache/cache.go -A 1 -n

Repository: openshift/operator-framework-olm

Length of output: 1395


Use %w instead of %v for error wrapping in new error paths.

Lines 248, 312, and 315 use %v with errors, which prevents callers from inspecting root causes with errors.Is() and errors.As(). Replace with %w to preserve error chains as required by the coding guidelines.

  • Line 248: "invalid custom schema query: %v" → use %w
  • Line 312: "invalid custom schema meta: %v" → use %w
  • Line 315: "store custom schema meta %v: %v" → use %w for the error (second %v)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@staging/operator-registry/pkg/cache/cache.go` around lines 245 - 249, Replace
the fmt.Errorf uses that currently use %v for error values with %w so the error
chain is preserved; specifically update the error construction in
ListPackageCustomSchemas (change "invalid custom schema query: %v" to use %w),
and the other two error returns that read "invalid custom schema meta: %v" and
"store custom schema meta %v: %v" so the final %v for the underlying error
becomes %w, ensuring callers can use errors.Is()/errors.As() to inspect root
causes.

return c.backend.SendMetas(ctx, mk, sender)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

func (c *cache) CheckIntegrity(ctx context.Context, fbc fs.FS) error {
existingDigest, err := c.backend.GetDigest(ctx)
if err != nil {
Expand Down Expand Up @@ -288,6 +301,21 @@ func (c *cache) Build(ctx context.Context, fbcFsys fs.FS) error {
packageName = meta.Name
}

switch meta.Schema {
case declcfg.SchemaPackage, declcfg.SchemaChannel, declcfg.SchemaBundle, declcfg.SchemaDeprecation:
default:
if packageName == "" {
return nil
}
mk, err := newValidatedMetaKey(meta.Schema, packageName)
if err != nil {
return fmt.Errorf("invalid custom schema meta: %v", err)
}
if err := c.backend.PutMeta(ctx, mk, meta.Blob); err != nil {
return fmt.Errorf("store custom schema meta %v: %v", mk, err)
}
}

walkMu.Lock()
defer walkMu.Unlock()
if _, err := tmpFile.Write(meta.Blob); err != nil {
Expand Down
140 changes: 140 additions & 0 deletions staging/operator-registry/pkg/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,146 @@ func TestCache_ListPackages(t *testing.T) {
}
}

var customSchemaFS = fstest.MapFS{
".": &fstest.MapFile{
Mode: fs.ModeDir,
},
"catalog.json": &fstest.MapFile{
Data: []byte(`{
"schema": "olm.package",
"name": "testpkg",
"defaultChannel": "stable"
}
{
"schema": "olm.channel",
"package": "testpkg",
"name": "stable",
"entries": [{"name": "testpkg.v1.0.0"}]
}
{
"schema": "olm.bundle",
"name": "testpkg.v1.0.0",
"package": "testpkg",
"image": "quay.io/test/testpkg:v1.0.0",
"properties": [{"type": "olm.package", "value": {"packageName": "testpkg", "version": "1.0.0"}}]
}
{
"schema": "custom.operator.io",
"package": "testpkg",
"name": "my-custom-resource",
"customField": "customValue"
}
{
"schema": "custom.operator.io",
"package": "testpkg",
"name": "another-custom-resource",
"data": {"key": "value"}
}
{
"schema": "other.custom.schema",
"package": "testpkg",
"name": "other-custom",
"info": "test"
}
`),
},
}

func TestCache_ListPackageCustomSchemas(t *testing.T) {
for name, testCache := range genTestCaches(t, customSchemaFS) {
t.Run(name, func(t *testing.T) {
collect := func(schema, pkg string) []string {
t.Helper()
var blobs []string
err := testCache.ListPackageCustomSchemas(context.TODO(), schema, pkg,
func(blob []byte) error {
blobs = append(blobs, string(blob))
return nil
})
require.NoError(t, err)
return blobs
}

// Multiple blobs for same (schema, package)
blobs := collect("custom.operator.io", "testpkg")
require.Len(t, blobs, 2)

// Different schema
blobs = collect("other.custom.schema", "testpkg")
require.Len(t, blobs, 1)
require.Contains(t, blobs[0], `"info"`)

// Nonexistent schema returns empty
blobs = collect("nonexistent.schema", "testpkg")
require.Empty(t, blobs)

// Nonexistent package returns empty
blobs = collect("custom.operator.io", "nonexistent")
require.Empty(t, blobs)
})
}
}

func TestCache_ListPackageCustomSchemas_PackagelessBlobSkipped(t *testing.T) {
packagelessFS := fstest.MapFS{
".": &fstest.MapFile{
Mode: fs.ModeDir,
},
"catalog.json": &fstest.MapFile{
Data: []byte(`{
"schema": "olm.package",
"name": "testpkg",
"defaultChannel": "stable"
}
{
"schema": "olm.channel",
"package": "testpkg",
"name": "stable",
"entries": [{"name": "testpkg.v1.0.0"}]
}
{
"schema": "olm.bundle",
"name": "testpkg.v1.0.0",
"package": "testpkg",
"image": "quay.io/test/testpkg:v1.0.0",
"properties": [{"type": "olm.package", "value": {"packageName": "testpkg", "version": "1.0.0"}}]
}
{
"schema": "custom.packageless",
"data": "no-package-blob"
}
`),
},
}
for name, testCache := range genTestCaches(t, packagelessFS) {
t.Run(name, func(t *testing.T) {
var blobs []string
err := testCache.ListPackageCustomSchemas(context.TODO(), "custom.packageless", "testpkg",
func(blob []byte) error {
blobs = append(blobs, string(blob))
return nil
})
require.NoError(t, err)
require.Empty(t, blobs)
})
}
}

func TestCache_ListPackageCustomSchemas_NoCustomSchemas(t *testing.T) {
for name, testCache := range genTestCaches(t, validFS) {
t.Run(name, func(t *testing.T) {
var blobs []string
err := testCache.ListPackageCustomSchemas(context.TODO(), "custom.operator.io", "cockroachdb",
func(blob []byte) error {
blobs = append(blobs, string(blob))
return nil
})
require.NoError(t, err)
require.Empty(t, blobs)
})
}
}

func genTestCaches(t *testing.T, fbcFS fs.FS) map[string]Cache {
t.Helper()

Expand Down
Loading