Skip to content

Conversation

@JivusAyrus
Copy link
Member

@JivusAyrus JivusAyrus commented Jul 23, 2025

Summary by CodeRabbit

  • New Features

    • Plugin & gRPC subgraph support: create, publish, generate, delete, and validate workflows; plugin registry support and Docker multi‑platform publishing; UI shows subgraph types and plugin metadata.
  • Improvements

    • Enforced per‑plan plugin limits and RBAC; composition and repo handling updated for subgraph types; safer plugin image handling and lifecycle.
  • Bug Fixes

    • Routing/validation fixes and improved path sanitization and error messages.
  • Chores

    • DB migration for subgraph types, CI/build updates, dependency refresh, and deployment script updates.
  • Tests

    • Extensive tests covering plugin flows, publishing, feature subgraphs, and access control.

Checklist

COMPLETES ENG-7682
COMPLETES ENG-7758

@coderabbitai
Copy link

coderabbitai bot commented Jul 23, 2025

Walkthrough

This change introduces first-class support for plugin and gRPC subgraphs across the entire stack, including schema, database, control plane, CLI, router, and studio. It adds new subgraph types, plugin lifecycle management, registry integration, plugin image handling, protobuf schema storage, new CLI commands, and comprehensive test coverage for plugin workflows and feature subgraphs.

Changes

Cohort / File(s) Change Summary
Database & Migrations
controlplane/migrations/0128_nifty_romulus.sql, controlplane/src/db/schema.ts, controlplane/src/db/models.ts, controlplane/migrations/meta/_journal.json
Adds subgraph_type enum (standard, grpc_plugin, grpc_service), new tables plugin_image_versions and protobuf_schema_versions, and a non-null type column on subgraphs. Updates models and migration journal.
Protobuf & Connect Definitions
proto/wg/cosmo/platform/v1/platform.proto, connect/src/wg/cosmo/platform/v1/platform_pb.ts, connect/src/wg/cosmo/platform/v1/platform_connect.ts, connect/src/wg/cosmo/platform/v1/platform-PlatformService_connectquery.ts, proto/wg/cosmo/node/v1/node.proto, connect/src/wg/cosmo/node/v1/node_pb.ts
Adds SubgraphType enum, ProtoInput message, plugin PluginData, ValidateAndFetchPluginData request/response and RPC, and ImageReference message + field in node plugin config.
Control Plane: Types & Utilities
controlplane/src/types/index.ts, controlplane/src/core/util.ts, controlplane/src/core/crypto/jwt.ts, controlplane/src/core/composition/composer.ts
Introduces DB/DTO type changes (ProtoSubgraph, DBSubgraphType, organizationId), plugin JWT audience, plugin-version validation, and conversions between enum/string subgraph types; composer extended for plugin/gRPC subgraphs and image references.
Control Plane: Repositories & Schema Handling
controlplane/src/core/repositories/SubgraphRepository.ts, controlplane/src/core/repositories/PluginRepository.ts, controlplane/src/core/repositories/FederatedGraphRepository.ts, controlplane/src/core/repositories/FeatureFlagRepository.ts, controlplane/src/core/repositories/OrganizationRepository.ts, controlplane/src/core/repositories/GraphCompositionRepository.ts
Subgraph repo now stores/returns type and optional proto/plugin data, adds plugin image/proto version persistence; new PluginRepository.count to count plugins; federated graph returns include organizationId; organization features include plugins.
Control Plane: API Handlers (bufservices)
controlplane/src/core/bufservices/subgraph/createFederatedSubgraph.ts, .../publishFederatedSubgraph.ts, .../getSubgraphById.ts, .../getSubgraphByName.ts, .../getSubgraphs.ts, .../federated-graph/*.ts, .../feature-flag/*.ts, controlplane/src/core/bufservices/plugin/validateAndFetchPluginData.ts, controlplane/src/core/bufservices/PlatformService.ts, controlplane/src/core/bufservices/monograph/createMonograph.ts
Extends handlers to accept/validate type and proto, enforce plugin limits, derive feature-subgraph types from base, add validateAndFetchPluginData RPC implementation, and map DB type to API responses.
Control Plane: Composition & Router Config
controlplane/src/core/composition/composer.ts, shared/src/router-config/builder.ts
Composer supports plugin/grpc subgraphs, includes schemaVersionId, parses gRPC mappings, and builds PluginConfiguration with imageReference.
Control Plane: Services, Billing & Build
controlplane/src/core/services/ApolloMigrator.ts, controlplane/src/bin/billing.json, controlplane/package.json, controlplane/Dockerfile
Migration ensures subgraph type on import; billing plans gain plugins feature/limits; controlplane now depends on protographic and includes it in image build.
CLI: Plugin Commands & Templates
cli/src/commands/router/commands/plugin/*, cli/src/commands/router/commands/plugin/index.ts, cli/src/commands/router/commands/plugin/templates/plugin.ts, cli/.env.example, cli/src/core/config.ts, cli/src/core/types/types.ts, .github/workflows/controlplane-ci.yaml
Adds generate, create, publish, delete, init plugin CLI commands; updates templates (Makefile, Dockerfile), adds PLUGIN_REGISTRY_URL config, fixes protobuf import paths, and includes protographic in CI build filters.
CLI: Subgraph & Feature-subgraph Commands
cli/src/commands/feature-subgraph/commands/*.ts, cli/src/commands/subgraph/commands/*.ts
Import/usage of SubgraphType and make routing URL optional where appropriate; publish/create accept explicit type and proto input.
Router: Plugin & gRPC Refactor + OCI Support
router/core/graph_server.go, router/pkg/grpcconnector/* (multiple files moved/refactored to grpccommon, grpcremote, grpcplugin, grpcpluginoci), router/pkg/grpcconnector/grpcpluginoci/*, router/pkg/grpcconnector/grpcremote/grpc_remote.go, router/pkg/grpcconnector/provider.go, router/pkg/grpcconnector/connector.go, router/pkg/config/*, router/go.mod, router-tests/go.mod
Large refactor: provider interface simplified (removed Name()), common startup/command preparation refactored into grpccommon, OCI-based plugin provider (grpcpluginoci) added (image pull, unpack, run), remote provider simplified, registry config added, dependency updates with selective pins.
Studio: UI & SDL Viewer
studio/src/components/subgraphs-table.tsx, studio/src/pages/.../subgraph/.../index.tsx, studio/src/pages/.../compositions/.../index.tsx, studio/src/components/schema/sdl-viewer.tsx
Adds subgraph type badges and plugin metadata display; adds protobuf syntax highlighting support and download handling in SDL viewer.
Tests & Test Data
controlplane/test/* (many), controlplane/test/test-data/plugin/*, router/pkg/grpcconnector/grpcpluginoci/*_test.go, router/pkg/grpcconnector/grpcpluginoci/image_unpack_test.go, router/pkg/grpcconnector/grpccommon/*_test.go
Adds extensive tests for plugin workflows: validate/fetch plugin data, plugin create/publish/delete flows, feature-subgraph interactions, composition with plugin subgraphs, and unit tests for OCI plugin unpack/prep and related helpers. Includes proto, mapping, and lock test files.
Miscellaneous
.gitignore, demo/deploy.sh, demo/pkg/subgraphs/projects/Dockerfile, docker-compose.full.yml, composition-go/shim/tsup.config.ts, controlplane/src/core/repositories/analytics/MetricsRepository.ts, controlplane/test/organization-groups.test.ts
Various updates: ignore pattern, demo/docker paths and Dockerfile, compose Dockerfile paths, tsup regex broadened, minor SQL string formatting, and test import path fix.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~90+ minutes

Complexity: cross-cutting, schema and migration changes, new DB tables and enum, protobuf contract changes, new RPCs, significant router plugin lifecycle refactor (OCI image handling), CLI surface additions, and many new/updated tests. Review should include DB migration safety, protobuf compatibility, security checks for image unpacking and JWTs, RBAC and quota enforcement, and integration flows between router, control plane, and CLI.

Possibly related PRs

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch suvij/eng-7509-router-plugins-with-cosmo-integration

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link

github-actions bot commented Jul 23, 2025

Router image scan passed

✅ No security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-ff4b4e02a5b609c306d286989261ccf13ee20bad

@JivusAyrus JivusAyrus marked this pull request as ready for review July 24, 2025 10:50
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (4)
proto/wg/cosmo/platform/v1/platform.proto (3)

377-401: Subgraph.type should be optional + minor PluginData nit

  1. The same “unset vs default” problem exists here:
-  SubgraphType type = 18;
+  optional SubgraphType type = 18;
  1. PluginData.platforms looks like an enumeration of well-known runtimes (linux/arm64, alpine, …). Consider an enum to prevent free-form strings.

168-169: Make type field optional for presence detection
Same issue previously raised still applies: without optional, servers cannot distinguish “unset” from the default enum value (STANDARD = 0).

-  SubgraphType type = 13;
+  optional SubgraphType type = 13;

1840-1841: Mark subgraphType as optional
GraphCompositionSubgraph.subgraphType should be declared optional for the same reason as above.

-  SubgraphType subgraphType = 7;
+  optional SubgraphType subgraphType = 7;
controlplane/src/core/repositories/SubgraphRepository.ts (1)

542-543: Validation implemented but could be more explicit.

The implementation correctly validates that plugin data is only processed for plugin-type subgraphs (lines 571-577), addressing the past review concern. However, the validation could be more explicit about rejecting invalid combinations.

The current implementation validates the subgraph type before processing plugin data:

if (data.proto.pluginData && subgraph.type === 'grpc_plugin') {

This prevents plugin data insertion for non-plugin subgraphs, but it silently ignores the plugin data rather than throwing an error. Consider making this more explicit:

+      if (data.proto.pluginData && subgraph.type !== 'grpc_plugin') {
+        throw new Error('Plugin data can only be provided for plugin-type subgraphs');
+      }
+
       if (data.proto.pluginData && subgraph.type === 'grpc_plugin') {

Also applies to: 544-544, 550-580

🧹 Nitpick comments (3)
proto/wg/cosmo/platform/v1/platform.proto (3)

370-374: Add an explicit UNSPECIFIED = 0 value
While STANDARD = 0 currently doubles as “unspecified”, a dedicated UNSPECIFIED (or SUBGRAPH_TYPE_UNSPECIFIED) makes intent clearer and leaves room for future re-ordering without breaking wire compatibility.


2844-2855: Treat pushToken as sensitive – document or rename
pushToken will likely carry bearer-token style secrets. Make sure callers do not log this field and consider renaming to push_token (snake-case) for consistency with the rest of the schema.
[security]


3221-3223: Add idempotency annotation if RPC is read-only
If ValidateAndFetchPluginData merely validates & returns data without mutating state, add

option idempotency_level = NO_SIDE_EFFECTS;

to the method options for better gRPC tooling support.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 30ce39c and 2ebff3a.

⛔ Files ignored due to path filters (1)
  • connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.go is excluded by !**/*.pb.go, !**/gen/**
📒 Files selected for processing (25)
  • cli/src/commands/router/commands/plugin/commands/create.ts (1 hunks)
  • cli/src/commands/router/commands/plugin/commands/publish.ts (1 hunks)
  • connect/src/wg/cosmo/platform/v1/platform_pb.ts (12 hunks)
  • controlplane/migrations/0128_nifty_romulus.sql (1 hunks)
  • controlplane/migrations/meta/_journal.json (1 hunks)
  • controlplane/src/core/bufservices/plugin/validateAndFetchPluginData.ts (1 hunks)
  • controlplane/src/core/bufservices/subgraph/createFederatedSubgraph.ts (7 hunks)
  • controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts (9 hunks)
  • controlplane/src/core/composition/composer.ts (4 hunks)
  • controlplane/src/core/repositories/FeatureFlagRepository.ts (5 hunks)
  • controlplane/src/core/repositories/PluginRepository.ts (1 hunks)
  • controlplane/src/core/repositories/SubgraphRepository.ts (14 hunks)
  • controlplane/src/core/util.ts (4 hunks)
  • controlplane/src/db/schema.ts (3 hunks)
  • controlplane/test/feature-flag/feature-flag-with-plugin-fs.test.ts (1 hunks)
  • controlplane/test/feature-subgraph/create-feature-subgraph.test.ts (5 hunks)
  • controlplane/test/feature-subgraph/publish-feature-subgraph.test.ts (1 hunks)
  • controlplane/test/plugin/validate-and-fetch-plugin-data.test.ts (1 hunks)
  • controlplane/test/subgraph/create-subgraph.test.ts (16 hunks)
  • controlplane/test/subgraph/delete-subgraph.test.ts (2 hunks)
  • controlplane/test/subgraph/publish-subgraph.test.ts (10 hunks)
  • proto/wg/cosmo/platform/v1/platform.proto (8 hunks)
  • studio/src/components/subgraphs-table.tsx (8 hunks)
  • studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/compositions/[compositionId]/index.tsx (3 hunks)
  • studio/src/pages/[organizationSlug]/[namespace]/subgraph/[subgraphSlug]/index.tsx (5 hunks)
✅ Files skipped from review due to trivial changes (1)
  • controlplane/migrations/meta/_journal.json
🚧 Files skipped from review as they are similar to previous changes (16)
  • studio/src/pages/[organizationSlug]/[namespace]/subgraph/[subgraphSlug]/index.tsx
  • controlplane/test/subgraph/delete-subgraph.test.ts
  • controlplane/src/core/repositories/PluginRepository.ts
  • studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/compositions/[compositionId]/index.tsx
  • controlplane/src/core/util.ts
  • cli/src/commands/router/commands/plugin/commands/create.ts
  • controlplane/src/core/repositories/FeatureFlagRepository.ts
  • cli/src/commands/router/commands/plugin/commands/publish.ts
  • controlplane/src/core/bufservices/plugin/validateAndFetchPluginData.ts
  • controlplane/test/feature-flag/feature-flag-with-plugin-fs.test.ts
  • controlplane/test/feature-subgraph/publish-feature-subgraph.test.ts
  • controlplane/test/plugin/validate-and-fetch-plugin-data.test.ts
  • studio/src/components/subgraphs-table.tsx
  • controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts
  • controlplane/test/feature-subgraph/create-feature-subgraph.test.ts
  • controlplane/src/db/schema.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: in the cosmo codebase, database transactions are typically managed at the service layer (e.g., in bu...
Learnt from: wilsonrivera
PR: wundergraph/cosmo#1919
File: controlplane/src/core/repositories/OrganizationGroupRepository.ts:193-224
Timestamp: 2025-07-01T13:53:54.146Z
Learning: In the Cosmo codebase, database transactions are typically managed at the service layer (e.g., in buf services like deleteOrganizationGroup.ts), where repositories are instantiated with the transaction handle and all operations within those repositories are automatically part of the same transaction.

Applied to files:

  • controlplane/src/core/bufservices/subgraph/createFederatedSubgraph.ts
📚 Learning: in the cosmo router project, when extending json schema validation for security-sensitive fields lik...
Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.

Applied to files:

  • controlplane/src/core/composition/composer.ts
📚 Learning: in the cosmo router project, required field validation for jwks configuration (secret, algorithm, ke...
Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.

Applied to files:

  • controlplane/src/core/composition/composer.ts
📚 Learning: in the wundergraph/cosmo project, anonymous admin access in grafana (gf_auth_anonymous_enabled=true ...
Learnt from: endigma
PR: wundergraph/cosmo#2009
File: docker-compose.yml:102-103
Timestamp: 2025-07-03T10:33:38.291Z
Learning: In the wundergraph/cosmo project, anonymous admin access in Grafana (GF_AUTH_ANONYMOUS_ENABLED=true and GF_AUTH_ANONYMOUS_ORG_ROLE=Admin) is intentionally configured for development environments in docker-compose.yml files.

Applied to files:

  • controlplane/test/subgraph/create-subgraph.test.ts
🧬 Code Graph Analysis (3)
controlplane/test/subgraph/create-subgraph.test.ts (5)
controlplane/test/test-util.ts (3)
  • SetupTest (66-417)
  • DEFAULT_NAMESPACE (52-52)
  • DEFAULT_SUBGRAPH_URL_ONE (49-49)
controlplane/src/core/test-util.ts (4)
  • genID (53-55)
  • createTestRBACEvaluator (173-175)
  • createTestGroup (181-198)
  • genUniqueLabel (57-59)
connect-go/gen/proto/wg/cosmo/common/common.pb.go (4)
  • EnumStatusCode (23-23)
  • EnumStatusCode (103-105)
  • EnumStatusCode (107-109)
  • EnumStatusCode (116-118)
studio/src/lib/constants.ts (1)
  • OrganizationRole (284-284)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.go (4)
  • SubgraphType (73-73)
  • SubgraphType (105-107)
  • SubgraphType (109-111)
  • SubgraphType (118-120)
controlplane/test/subgraph/publish-subgraph.test.ts (2)
controlplane/src/core/test-util.ts (4)
  • genUniqueLabel (57-59)
  • genID (53-55)
  • createTestRBACEvaluator (173-175)
  • createTestGroup (181-198)
controlplane/test/test-util.ts (3)
  • SetupTest (66-417)
  • createSubgraph (703-715)
  • subgraphSDL (753-757)
controlplane/src/core/repositories/SubgraphRepository.ts (2)
controlplane/src/db/models.ts (1)
  • DBSubgraphType (41-41)
controlplane/src/types/index.ts (2)
  • SubgraphDTO (117-138)
  • ProtoSubgraph (107-115)
🔇 Additional comments (44)
controlplane/src/core/bufservices/subgraph/createFederatedSubgraph.ts (6)

7-7: LGTM!

The import additions properly support the new plugin subgraph functionality with appropriate type definitions and repository dependencies.

Also applies to: 26-28


48-49: LGTM!

Setting a default STANDARD type ensures backward compatibility and defensive programming practices.


65-90: Comprehensive feature subgraph validation logic.

The implementation properly validates feature subgraph requirements:

  • Ensures base subgraph name is provided
  • Validates base subgraph exists in the namespace
  • Correctly sets baseSubgraphID and aligns feature subgraph type with base subgraph

This maintains referential integrity and type consistency for feature subgraphs.


137-137: Appropriate routing URL validation exemption.

Plugin subgraphs correctly bypass routing URL validation since they execute via OCI images rather than HTTP endpoints, similar to event-driven graphs.


211-226: Proper plugin limit enforcement.

The implementation correctly enforces billing plan plugin limits by:

  • Counting existing plugins in the namespace
  • Retrieving organization's plugin feature limit
  • Handling unlimited plans appropriately
  • Returning proper error response when limits are exceeded

248-248: LGTM!

Properly formats and passes the subgraph type to the repository using the utility function.

controlplane/test/subgraph/publish-subgraph.test.ts (5)

17-21: Excellent use of real test data files.

Reading actual proto schema, mappings, and lock files from disk provides more realistic test coverage compared to mock data.


391-404: Well-organized plugin test suite structure.

The nested describe block with reusable test data definitions provides clear separation and maintainable test organization.


406-474: Comprehensive plugin publishing test coverage.

The tests effectively cover both publishing scenarios:

  • Publishing to existing plugin subgraph
  • Creating and publishing in one step

Both tests properly validate stored plugin metadata and use appropriate billing plan configuration.


476-664: Thorough plugin validation and error handling tests.

Excellent coverage of error scenarios:

  • Plugin limit enforcement via billing plans
  • Name conflict detection with regular subgraphs
  • Type validation and mismatch handling
  • Required field validation for proto data
  • Version format validation

All tests use appropriate error codes and meaningful error messages.


666-762: Comprehensive RBAC test coverage for plugins.

The tests effectively validate role-based permissions for:

  • Plugin creation (limited to admin/developer/subgraph-admin roles)
  • Publishing to existing plugins (includes subgraph-publisher role)

Clear separation between creation and publishing permissions aligns with security best practices.

controlplane/migrations/0128_nifty_romulus.sql (4)

1-1: Proper enum definition for subgraph types.

The subgraph_type enum correctly defines the three supported types with appropriate naming conventions.


2-8: Well-designed plugin version metadata table.

The plugin_image_versions table structure appropriately supports plugin versioning with:

  • UUID primary key with auto-generation
  • Foreign key relationship to schema_versions
  • Flexible text version field and platform array
  • Proper timestamp handling

10-17: Appropriate protobuf schema storage design.

The protobuf_schema_versions table correctly stores protobuf data with separate fields for schema, mappings, and lock information, linked to the schema versioning system.


19-30: Robust column addition and constraint handling.

The implementation correctly:

  • Adds type column with backward-compatible default
  • Uses exception handling for graceful constraint creation
  • Implements cascade delete for proper referential integrity

The migration is resilient to re-runs and maintains data consistency.

controlplane/src/core/composition/composer.ts (4)

14-28: Proper type system extension for plugin support.

The import additions and ComposedSubgraph type extension correctly incorporate the new plugin and gRPC subgraph variants while adding the required schemaVersionId field for consistency.

Also applies to: 113-117


119-126: Proper error handling for JSON parsing.

The parseGRPCMapping function correctly wraps JSON parsing in try-catch with meaningful error messages, addressing potential parsing failures.


146-190: Comprehensive plugin and gRPC subgraph composition logic.

The implementation properly handles different subgraph types:

  • Validates required data presence for each type
  • Constructs appropriate composed subgraph objects
  • Builds correct ImageReference for plugin subgraphs
  • Maintains type safety and error handling

The composition logic integrates well with the existing federation system.


226-226: Correct function signature update.

Properly passes federatedGraph.organizationId to match the updated function signature required for plugin ImageReference construction.

controlplane/test/subgraph/create-subgraph.test.ts (6)

3-3: Good addition of SubgraphType import.

The import correctly adds support for the new plugin subgraph type enumeration, which is essential for the plugin creation tests.


12-12: Appropriate addition of genUniqueLabel utility.

The genUniqueLabel import will be used effectively in the new plugin tests for generating unique labels, which helps prevent test interference.


52-74: Good refactoring to parameterized test.

The conversion to test.each improves test maintainability by eliminating code duplication while testing multiple RBAC roles consistently.


539-862: Excellent comprehensive plugin subgraph test suite.

The new plugin test suite provides thorough coverage of plugin subgraph functionality including:

  • ✅ Billing plan restrictions (developer vs launch plans)
  • ✅ Plugin limit enforcement (max 3 on launch plan)
  • ✅ Name collision prevention between plugin and regular subgraphs
  • ✅ Complete RBAC permission matrix
  • ✅ Multiple label support
  • ✅ Verification that plugin limits are isolated from regular subgraph limits

The tests are well-structured, use appropriate test data generation, and verify both success and failure scenarios with proper error codes and messages.

A few notable strengths:

  • Line 567: Proper validation that created subgraphs have the correct GRPC_PLUGIN type
  • Lines 601-613: Efficient loop-based creation for testing limits
  • Lines 831-858: Clever verification that plugin limits don't affect other subgraph types
  • Lines 704-733, 735-770: Comprehensive RBAC coverage for both positive and negative cases

588-589: Consistent and appropriate error handling.

The error messages and status codes are consistent across plugin limit scenarios, providing clear feedback about billing plan restrictions.

Also applies to: 626-627


661-663: Proper name collision error messages.

The error messages clearly indicate the conflict and include both the subgraph name and namespace, which aids in debugging and user experience.

Also applies to: 697-699

proto/wg/cosmo/platform/v1/platform.proto (1)

67-69: 👍 Optional qualifiers added – resolves presence-detection problem
PublishFederatedSubgraphRequest now marks both type and proto as optional. This fixes the silent-default issue noted earlier.

connect/src/wg/cosmo/platform/v1/platform_pb.ts (12)

33-57: Well-structured enum definition for subgraph types.

The SubgraphType enum is properly defined with clear semantic names and sequential numbering. The distinction between GRPC_PLUGIN and GRPC_SERVICE provides good separation of concerns for different gRPC-based subgraph types.


618-677: Complete protobuf message implementation for ProtoInput.

The ProtoInput message class is well-structured with appropriate field types and proper protobuf conventions. The field ordering is logical: schema, mappings, lock, platforms array, and version.


754-762: Appropriate optional fields for plugin support in PublishFederatedSubgraphRequest.

The addition of optional type and proto fields maintains backward compatibility while enabling plugin subgraph publishing functionality.


783-784: Correct field definitions for optional plugin fields.

The field definitions use proper protobuf field numbers (13, 14) and correct typing with optional flags.


1413-1416: Good default value for subgraph type in CreateFederatedSubgraphRequest.

Setting the default to SubgraphType.STANDARD ensures backward compatibility and provides sensible defaults for existing workflows.


1437-1437: Proper non-optional field definition for required subgraph type.

The type field is correctly defined as non-optional in CreateFederatedSubgraphRequest, ensuring type information is always present.


3040-3048: Well-designed plugin data extension to Subgraph message.

The addition of type field with STANDARD default and optional pluginData field provides a clean way to extend subgraph metadata for plugin types while maintaining backward compatibility.


3075-3076: Correct field definitions for plugin-related fields.

Field numbers 18 and 19 are properly sequenced, and the types are correctly specified with appropriate optional flags.


3096-3137: Complete nested message implementation for plugin metadata.

The Subgraph_PluginData nested message follows protobuf conventions properly with version string and platforms array. The implementation includes all necessary serialization methods.


14257-14260: Consistent subgraph type field in GraphCompositionSubgraph.

Adding the subgraphType field with STANDARD default maintains consistency across the platform API for composition handling.


14276-14276: Proper field definition for composition subgraph type.

Field number 7 continues the sequence correctly with proper enum type reference.


22347-22449: Complete implementation of plugin validation messages.

Both ValidateAndFetchPluginDataRequest and ValidateAndFetchPluginDataResponse are properly implemented with appropriate field types. The request includes name, namespace, and labels for plugin identification, while the response provides validation results and plugin metadata.

controlplane/src/core/repositories/SubgraphRepository.ts (6)

12-12: LGTM! Proper imports for new type system.

The imports for DBSubgraphType and ProtoSubgraph are correctly added to support the new subgraph type classification and protobuf schema functionality.

Also applies to: 32-32


95-96: LGTM! Type parameter properly integrated.

The type parameter is correctly added to the create method signature, stored in the subgraphs table, and included in the returned SubgraphDTO. The implementation aligns with the new subgraph type system.

Also applies to: 136-137, 190-194


212-213: LGTM! Proto parameter handling is well-structured.

The proto parameter is properly added to the update method and passed to addSchemaVersion. The conditional check for grpc_plugin type ensures schema versions are added for plugin subgraphs regardless of schema equality, which is appropriate for plugin lifecycle management.

Also applies to: 248-255


563-579: LGTM! Protobuf schema handling is well-implemented.

The protobuf schema version insertion is properly scoped to gRPC service and plugin types, and the plugin image version data is correctly handled only for plugin types. The database operations are appropriately wrapped in the existing transaction.


605-605: LGTM! Return type includes new type field.

The addSchemaVersion method correctly includes the type field in the returned SubgraphDTO.


928-928: LGTM! Proto data retrieval is comprehensive.

The getSubgraph method properly:

  • Selects the type field from the database
  • Initializes the proto variable
  • Conditionally fetches protobuf schema and plugin data for gRPC types
  • Includes both fields in the returned SubgraphDTO

The implementation correctly handles the relationship between protobuf schema versions and plugin image versions.

Also applies to: 943-943, 955-976, 997-998

Copy link
Member Author

@JivusAyrus JivusAyrus left a comment

Choose a reason for hiding this comment

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

Should modify the registry url to point to prod before merging

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
cli/src/commands/router/commands/plugin/commands/publish.ts (2)

16-19: DRY: Deduplicate the supportedPlatforms definition

supportedPlatforms is declared twice (Lines 16-19 and 186-189). Define it once at module scope and reuse it to avoid divergence when the supported set changes.

Also applies to: 186-198


101-103: Sanitise pluginName derived from directory name

pluginName falls back to path.basename(pluginDir), which can include characters disallowed by the backend or Docker tag rules (e.g., spaces, uppercase letters). Add a regex whitelist and fail fast if the name violates it.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between afaa2f1 and ddb07b4.

📒 Files selected for processing (1)
  • cli/src/commands/router/commands/plugin/commands/publish.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-21T14:46:34.879Z
Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.

Applied to files:

  • cli/src/commands/router/commands/plugin/commands/publish.ts
📚 Learning: 2025-07-21T15:06:36.664Z
Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.

Applied to files:

  • cli/src/commands/router/commands/plugin/commands/publish.ts
📚 Learning: 2025-08-07T12:05:06.744Z
Learnt from: StarpTech
PR: wundergraph/cosmo#2079
File: proto/wg/cosmo/platform/v1/platform.proto:39-45
Timestamp: 2025-08-07T12:05:06.744Z
Learning: In the Cosmo project, the proto fields for schema, mappings, and lock in ProtoInput are intentionally kept as string types rather than bytes because the team works with text data and wants it to be UTF-8 encoded for readability and text processing purposes.

Applied to files:

  • cli/src/commands/router/commands/plugin/commands/publish.ts

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (3)
proto/wg/cosmo/platform/v1/platform.proto (3)

168-168: Make CreateFederatedSubgraphRequest.type optional for presence detection (duplicate)

Without optional, down-level clients are indistinguishable from those explicitly setting STANDARD (0). This can defeat validation and migration logic.

-  SubgraphType type = 13;
+  optional SubgraphType type = 13;

399-401: Make Subgraph.type optional for presence detection (duplicate)

Same rationale as creation: without presence, existing clients get coerced to STANDARD (0) and servers cannot tell unset from explicitly set.

-  SubgraphType type = 18;
+  optional SubgraphType type = 18;

1842-1842: Make GraphCompositionSubgraph.subgraphType optional (duplicate)

Maintains presence semantics in composition results and prevents defaulting ambiguity.

-  SubgraphType subgraphType = 7;
+  optional SubgraphType subgraphType = 7;
🧹 Nitpick comments (6)
proto/wg/cosmo/platform/v1/platform.proto (6)

39-45: ProtoInput: keep strings (UTF-8) — add brief field docs for clarity

Aligned with the team decision to keep these as string. Consider documenting expected contents and formats to aid SDK users.

 message ProtoInput {
-  string schema = 1;
-  string mappings = 2;
-  string lock = 3;
-  repeated string platforms = 4;
-  string version = 5;
+  // Text contents of the protobuf schema (UTF-8)
+  string schema = 1;
+  // Text contents of plugin mappings (UTF-8)
+  string mappings = 2;
+  // Text contents of the lock file (UTF-8)
+  string lock = 3;
+  // Supported build targets (e.g., "linux/amd64", "linux/arm64", "wasm")
+  repeated string platforms = 4;
+  // Plugin version (e.g., semver)
+  string version = 5;
 }

67-68: PublishFederatedSubgraphRequest: clarify relation between type and proto

Please document the precondition so clients know how to populate these fields. Server-side must enforce it.

   optional bool disable_resolvability_validation = 12;
-  optional SubgraphType type = 13;
-  optional ProtoInput proto = 14;
+  // If type != STANDARD, 'proto' must be provided; for STANDARD subgraphs, 'proto' must be omitted.
+  optional SubgraphType type = 13;
+  optional ProtoInput proto = 14;

370-374: SubgraphType: add short comments per value

Names look good. Add one-line docs to reduce ambiguity across SDKs.

 enum SubgraphType {
-  STANDARD = 0;
-  GRPC_PLUGIN = 1;
-  GRPC_SERVICE = 2;
+  // Standard GraphQL subgraph
+  STANDARD = 0;
+  // gRPC plugin running within the router
+  GRPC_PLUGIN = 1;
+  // External gRPC service subgraph
+  GRPC_SERVICE = 2;
 }

377-381: Subgraph.PluginData: add field docs

Mirror ProtoInput docs so the meaning of platforms and version remains consistent across request/response types.

 message Subgraph {
   message PluginData{
-    string version = 1;
-    repeated string platforms = 2;
+    // Plugin version (e.g., semver)
+    string version = 1;
+    // Supported target platforms (e.g., "linux/amd64", "linux/arm64", "wasm")
+    repeated string platforms = 2;
   }

3223-3225: Annotate RPC as NO_SIDE_EFFECTS

This RPC validates and fetches metadata; marking as no side effects can improve client behavior and proxies.

-  rpc ValidateAndFetchPluginData(ValidateAndFetchPluginDataRequest) returns (ValidateAndFetchPluginDataResponse) {}
+  rpc ValidateAndFetchPluginData(ValidateAndFetchPluginDataRequest) returns (ValidateAndFetchPluginDataResponse) {
+    option idempotency_level = NO_SIDE_EFFECTS;
+  }

2846-2850: ValidateAndFetchPluginDataRequest: confirm call timing & strengthen identity

I didn’t find any implementation or call sites for ValidateAndFetchPluginData in the service layer—please verify whether this RPC can be invoked before a subgraph record exists. If it can, adding a unique identifier (for example, a type or dedicated id field) will help avoid misclassification down-the-line.

Please review proto/wg/cosmo/platform/v1/platform.proto:

message ValidateAndFetchPluginDataRequest {
  string name = 1;
  string namespace = 2;
  repeated Label labels = 3;
  // Optional: include when request may occur pre-subgraph creation
  optional SubgraphType type = 4;
}

– If the RPC is always called post-creation, confirm here.
– If pre-creation calls are possible, please add and document the new field.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b0ec824 and e23121b.

⛔ Files ignored due to path filters (3)
  • connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.go is excluded by !**/*.pb.go, !**/gen/**
  • router-tests/go.sum is excluded by !**/*.sum
  • router/go.sum is excluded by !**/*.sum
📒 Files selected for processing (4)
  • connect/src/wg/cosmo/platform/v1/platform_pb.ts (12 hunks)
  • proto/wg/cosmo/platform/v1/platform.proto (8 hunks)
  • router-tests/go.mod (6 hunks)
  • router/go.mod (6 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • router/go.mod
  • router-tests/go.mod
  • connect/src/wg/cosmo/platform/v1/platform_pb.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-07T12:05:06.744Z
Learnt from: StarpTech
PR: wundergraph/cosmo#2079
File: proto/wg/cosmo/platform/v1/platform.proto:39-45
Timestamp: 2025-08-07T12:05:06.744Z
Learning: In the Cosmo project, the proto fields for schema, mappings, and lock in ProtoInput are intentionally kept as string types rather than bytes because the team works with text data and wants it to be UTF-8 encoded for readability and text processing purposes.

Applied to files:

  • proto/wg/cosmo/platform/v1/platform.proto
📚 Learning: 2025-07-21T15:06:36.664Z
Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.

Applied to files:

  • proto/wg/cosmo/platform/v1/platform.proto
📚 Learning: 2025-07-21T14:46:34.879Z
Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.

Applied to files:

  • proto/wg/cosmo/platform/v1/platform.proto
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
  • GitHub Check: build-router
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: image_scan (nonroot)
  • GitHub Check: integration_test (./events)
  • GitHub Check: integration_test (./telemetry)
  • GitHub Check: build_push_image
  • GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
  • GitHub Check: image_scan
  • GitHub Check: build_push_image (nonroot)
  • GitHub Check: build_test
  • GitHub Check: build_test
  • GitHub Check: build_test
  • GitHub Check: build_push_image
  • GitHub Check: build_push_image
  • GitHub Check: build_test
  • GitHub Check: build_test

Copy link
Contributor

@StarpTech StarpTech left a comment

Choose a reason for hiding this comment

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

LGTM

@JivusAyrus JivusAyrus merged commit 05c923a into main Aug 12, 2025
49 checks passed
@JivusAyrus JivusAyrus deleted the suvij/eng-7509-router-plugins-with-cosmo-integration branch August 12, 2025 13:47
@coderabbitai coderabbitai bot mentioned this pull request Aug 12, 2025
5 tasks
@Noroth Noroth mentioned this pull request Sep 30, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants