diff --git a/controlplane/src/core/bufservices/subgraph/createFederatedSubgraph.ts b/controlplane/src/core/bufservices/subgraph/createFederatedSubgraph.ts index 05920951e2..d26d155969 100644 --- a/controlplane/src/core/bufservices/subgraph/createFederatedSubgraph.ts +++ b/controlplane/src/core/bufservices/subgraph/createFederatedSubgraph.ts @@ -20,7 +20,7 @@ import { getLogger, handleError, isValidGraphName, - isValidGrpcNamingScheme, + isValidGrpcSubgraphRoutingURL, isValidLabels, } from '../../util.js'; import { UnauthorizedError } from '../../errors/errors.js'; @@ -167,14 +167,19 @@ export function createFederatedSubgraph( admissionErrors: [], }; } - // For GRPC_SERVICE subgraphs, validate that routing URL follows gRPC naming scheme - if (req.type === SubgraphType.GRPC_SERVICE && !isValidGrpcNamingScheme(routingUrl)) { + // For GRPC_SERVICE subgraphs, the routing URL may use either a gRPC + // naming scheme (for native gRPC dialing) or http(s):// (for ConnectRPC). + // The router selects the actual transport at request time via the + // `grpc_protocol` configuration block. + if (req.type === SubgraphType.GRPC_SERVICE && !isValidGrpcSubgraphRoutingURL(routingUrl)) { return { response: { code: EnumStatusCode.ERR, details: - `Routing URL must follow gRPC naming scheme. ` + - `See https://grpc.io/docs/guides/custom-name-resolution/ for examples.`, + `Routing URL "${routingUrl}" is not a valid gRPC subgraph URL. ` + + `Use http(s)://host:port for ConnectRPC or a gRPC naming scheme ` + + `(e.g. dns:///host:port) for native gRPC. ` + + `See https://grpc.io/docs/guides/custom-name-resolution/ for the gRPC schemes.`, }, compositionErrors: [], admissionErrors: [], diff --git a/controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts b/controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts index 3b8b71fb81..78f03d5952 100644 --- a/controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts +++ b/controlplane/src/core/bufservices/subgraph/publishFederatedSubgraph.ts @@ -32,7 +32,7 @@ import { getLogger, handleError, isValidGraphName, - isValidGrpcNamingScheme, + isValidGrpcSubgraphRoutingURL, isValidLabels, isValidPluginVersion, } from '../../util.js'; @@ -402,14 +402,18 @@ export function publishFederatedSubgraph( proposalMatchMessage, }; } - // For GRPC_SERVICE subgraphs, validate that routing URL follows gRPC naming scheme - if (req.type === SubgraphType.GRPC_SERVICE && !isValidGrpcNamingScheme(routingUrl)) { + // For GRPC_SERVICE subgraphs the routing URL may use either a gRPC + // naming scheme (native gRPC) or http(s):// (ConnectRPC). The router + // selects the actual transport via the `grpc_protocol` config block. + if (req.type === SubgraphType.GRPC_SERVICE && !isValidGrpcSubgraphRoutingURL(routingUrl)) { return { response: { code: EnumStatusCode.ERR, details: - `Routing URL must follow gRPC naming scheme. ` + - `See https://grpc.io/docs/guides/custom-name-resolution/ for examples.`, + `Routing URL "${routingUrl}" is not a valid gRPC subgraph URL. ` + + `Use http(s)://host:port for ConnectRPC or a gRPC naming scheme ` + + `(e.g. dns:///host:port) for native gRPC. ` + + `See https://grpc.io/docs/guides/custom-name-resolution/ for the gRPC schemes.`, }, compositionErrors: [], deploymentErrors: [], diff --git a/controlplane/src/core/bufservices/subgraph/updateSubgraph.ts b/controlplane/src/core/bufservices/subgraph/updateSubgraph.ts index 5849b4b211..0d816b0016 100644 --- a/controlplane/src/core/bufservices/subgraph/updateSubgraph.ts +++ b/controlplane/src/core/bufservices/subgraph/updateSubgraph.ts @@ -21,7 +21,7 @@ import { formatWebsocketSubprotocol, getLogger, handleError, - isValidGrpcNamingScheme, + isValidGrpcSubgraphRoutingURL, isValidLabels, } from '../../util.js'; import { OrganizationWebhookService } from '../../webhooks/OrganizationWebhookService.js'; @@ -152,18 +152,22 @@ export function updateSubgraph( compositionWarnings: [], }; } - // For GRPC_SERVICE subgraphs, validate that routing URL follows gRPC naming scheme + // For GRPC_SERVICE subgraphs the routing URL may use either a gRPC + // naming scheme (native gRPC) or http(s):// (ConnectRPC). The router + // selects the actual transport via the `grpc_protocol` config block. if ( req.routingUrl !== undefined && subgraph.type === formatSubgraphType(SubgraphType.GRPC_SERVICE) && - !isValidGrpcNamingScheme(req.routingUrl) + !isValidGrpcSubgraphRoutingURL(req.routingUrl) ) { return { response: { code: EnumStatusCode.ERR, details: - `Routing URL must follow gRPC naming scheme. ` + - `See https://grpc.io/docs/guides/custom-name-resolution/ for examples.`, + `Routing URL "${req.routingUrl}" is not a valid gRPC subgraph URL. ` + + `Use http(s)://host:port for ConnectRPC or a gRPC naming scheme ` + + `(e.g. dns:///host:port) for native gRPC. ` + + `See https://grpc.io/docs/guides/custom-name-resolution/ for the gRPC schemes.`, }, compositionErrors: [], deploymentErrors: [], diff --git a/controlplane/src/core/util.test.ts b/controlplane/src/core/util.test.ts index 3d378ca7ec..8c0dcd4e71 100644 --- a/controlplane/src/core/util.test.ts +++ b/controlplane/src/core/util.test.ts @@ -3,6 +3,7 @@ import { extractOperationNames, hasLabelsChanged, isValidGrpcNamingScheme, + isValidGrpcSubgraphRoutingURL, isValidLabels, isValidNamespaceName, normalizePagination, @@ -524,3 +525,38 @@ describe('isValidGrpcNamingScheme', () => { }); }); }); + +describe('isValidGrpcSubgraphRoutingURL', () => { + test('accepts http URLs (ConnectRPC)', () => { + expect(isValidGrpcSubgraphRoutingURL('http://localhost:8080')).toBe(true); + expect(isValidGrpcSubgraphRoutingURL('http://example.com:443')).toBe(true); + expect(isValidGrpcSubgraphRoutingURL('http://example.com:443/grpc')).toBe(true); + }); + + test('accepts https URLs (ConnectRPC)', () => { + expect(isValidGrpcSubgraphRoutingURL('https://api.example.com')).toBe(true); + expect(isValidGrpcSubgraphRoutingURL('https://api.example.com:8443/v1')).toBe(true); + }); + + test('case-insensitive on http(s) scheme', () => { + expect(isValidGrpcSubgraphRoutingURL('HTTP://localhost:8080')).toBe(true); + expect(isValidGrpcSubgraphRoutingURL('HTTPS://api.example.com')).toBe(true); + }); + + test('rejects malformed http URLs', () => { + expect(isValidGrpcSubgraphRoutingURL('http://')).toBe(false); + expect(isValidGrpcSubgraphRoutingURL('https://')).toBe(false); + }); + + test('delegates to isValidGrpcNamingScheme for non-http schemes', () => { + expect(isValidGrpcSubgraphRoutingURL('dns:///example.com:8080')).toBe(true); + expect(isValidGrpcSubgraphRoutingURL('localhost:8080')).toBe(true); + expect(isValidGrpcSubgraphRoutingURL('unix:/tmp/sock')).toBe(true); + expect(isValidGrpcSubgraphRoutingURL('ftp://example.com')).toBe(false); + }); + + test('rejects empty / whitespace-only', () => { + expect(isValidGrpcSubgraphRoutingURL('')).toBe(false); + expect(isValidGrpcSubgraphRoutingURL(' ')).toBe(false); + }); +}); diff --git a/controlplane/src/core/util.ts b/controlplane/src/core/util.ts index 0b6e405b5f..8adaf7fbb9 100644 --- a/controlplane/src/core/util.ts +++ b/controlplane/src/core/util.ts @@ -902,3 +902,34 @@ export function isValidGrpcNamingScheme(url: string): boolean { } } } + +/** + * Validates a routing URL accepted for a GRPC_SERVICE subgraph. The router + * can reach a gRPC subgraph over either native gRPC (via the gRPC name + * resolver schemes) or ConnectRPC over HTTP/1.1, and the protocol is chosen + * at request time by the router via the `grpc_protocol` configuration block. + * + * Accepts: + * - any URL using one of the gRPC naming schemes recognised by + * isValidGrpcNamingScheme (dns:, unix:, unix-abstract:, vsock:, ipv4:, ipv6:), + * - any well-formed http:// or https:// URL. + */ +export function isValidGrpcSubgraphRoutingURL(url: string): boolean { + const value = url.trim(); + if (!value) { + return false; + } + + const lower = value.toLowerCase(); + if (lower.startsWith('http://') || lower.startsWith('https://')) { + try { + // eslint-disable-next-line no-new + new URL(value); + return true; + } catch { + return false; + } + } + + return isValidGrpcNamingScheme(value); +} diff --git a/controlplane/test/subgraph/create-subgraph.test.ts b/controlplane/test/subgraph/create-subgraph.test.ts index 1a63f20ff3..afc95650c1 100644 --- a/controlplane/test/subgraph/create-subgraph.test.ts +++ b/controlplane/test/subgraph/create-subgraph.test.ts @@ -930,7 +930,7 @@ describe('Create subgraph tests', () => { expect(createGrpcServiceSubgraphResp.response?.details).toBe('Routing URL "invalid-url" is not a valid URL'); }); - test('Should not allow creating a GRPC service subgraph with HTTP/HTTPS routing URL', async (testContext) => { + test('Should allow creating a GRPC service subgraph with an HTTP/HTTPS routing URL (ConnectRPC)', async (testContext) => { const { client, server } = await SetupTest({ dbname, }); @@ -938,7 +938,8 @@ describe('Create subgraph tests', () => { const grpcServiceLabel = genUniqueLabel('service'); - // Test HTTP URL + // ConnectRPC subgraphs serve over plain HTTP, so http:// must be a + // legitimate routing URL even though the subgraph type is GRPC_SERVICE. const createGrpcServiceSubgraphRespHttp = await client.createFederatedSubgraph({ name: genID('grpc-service-http'), namespace: DEFAULT_NAMESPACE, @@ -947,12 +948,8 @@ describe('Create subgraph tests', () => { labels: [grpcServiceLabel], }); - expect(createGrpcServiceSubgraphRespHttp.response?.code).toBe(EnumStatusCode.ERR); - expect(createGrpcServiceSubgraphRespHttp.response?.details).toContain( - 'Routing URL must follow gRPC naming scheme', - ); + expect(createGrpcServiceSubgraphRespHttp.response?.code).toBe(EnumStatusCode.OK); - // Test HTTPS URL const createGrpcServiceSubgraphRespHttps = await client.createFederatedSubgraph({ name: genID('grpc-service-https'), namespace: DEFAULT_NAMESPACE, @@ -961,10 +958,7 @@ describe('Create subgraph tests', () => { labels: [grpcServiceLabel], }); - expect(createGrpcServiceSubgraphRespHttps.response?.code).toBe(EnumStatusCode.ERR); - expect(createGrpcServiceSubgraphRespHttps.response?.details).toContain( - 'Routing URL must follow gRPC naming scheme', - ); + expect(createGrpcServiceSubgraphRespHttps.response?.code).toBe(EnumStatusCode.OK); }); test('Should allow creating a GRPC service subgraph with valid gRPC naming scheme URLs', async (testContext) => { diff --git a/controlplane/test/subgraph/publish-subgraph.test.ts b/controlplane/test/subgraph/publish-subgraph.test.ts index ad24f88ac1..4a22383436 100644 --- a/controlplane/test/subgraph/publish-subgraph.test.ts +++ b/controlplane/test/subgraph/publish-subgraph.test.ts @@ -1100,7 +1100,7 @@ describe('Publish subgraph tests', () => { }, ); - test('Should not allow publishing a GRPC service subgraph with HTTP/HTTPS routing URL', async (testContext) => { + test('Should allow publishing a GRPC service subgraph with an HTTP/HTTPS routing URL (ConnectRPC)', async (testContext) => { const { client, server } = await SetupTest({ dbname, }); @@ -1108,7 +1108,9 @@ describe('Publish subgraph tests', () => { const grpcServiceLabel = genUniqueLabel('grpc-service'); - // Test HTTP URL when creating and publishing in one step + // ConnectRPC subgraphs serve over plain HTTP, so the create-and-publish + // path must accept http:// (and https://) URLs even though the subgraph + // type is GRPC_SERVICE. const publishResponseHttp = await client.publishFederatedSubgraph({ name: genID('grpc-service-http'), namespace: 'default', @@ -1119,10 +1121,8 @@ describe('Publish subgraph tests', () => { labels: [grpcServiceLabel], }); - expect(publishResponseHttp.response?.code).toBe(EnumStatusCode.ERR); - expect(publishResponseHttp.response?.details).toContain('Routing URL must follow gRPC naming scheme'); + expect(publishResponseHttp.response?.code).toBe(EnumStatusCode.OK); - // Test HTTPS URL when creating and publishing in one step const publishResponseHttps = await client.publishFederatedSubgraph({ name: genID('grpc-service-https'), namespace: 'default', @@ -1133,8 +1133,7 @@ describe('Publish subgraph tests', () => { labels: [grpcServiceLabel], }); - expect(publishResponseHttps.response?.code).toBe(EnumStatusCode.ERR); - expect(publishResponseHttps.response?.details).toContain('Routing URL must follow gRPC naming scheme'); + expect(publishResponseHttps.response?.code).toBe(EnumStatusCode.OK); }); test('Should allow publishing a GRPC service subgraph with valid gRPC naming scheme URLs', async (testContext) => { diff --git a/controlplane/test/subgraph/update-subgraph.test.ts b/controlplane/test/subgraph/update-subgraph.test.ts index 798bf0a98a..4168b7bfd1 100644 --- a/controlplane/test/subgraph/update-subgraph.test.ts +++ b/controlplane/test/subgraph/update-subgraph.test.ts @@ -226,14 +226,14 @@ describe('Update subgraph tests', () => { }); describe('GRPC Service subgraph update tests', () => { - test('Should not allow updating a GRPC service subgraph with HTTP/HTTPS routing URL', async (testContext) => { + test('Should allow updating a GRPC service subgraph with an HTTP/HTTPS routing URL (ConnectRPC)', async (testContext) => { const { client, server } = await SetupTest({ dbname }); testContext.onTestFinished(() => server.close()); const grpcServiceName = genID('grpc-service'); const grpcServiceLabel = genUniqueLabel('grpc-service'); - // First create a GRPC service subgraph with valid gRPC naming scheme + // Create with a gRPC naming scheme URL first. const createResp = await client.createFederatedSubgraph({ name: grpcServiceName, namespace: DEFAULT_NAMESPACE, @@ -244,25 +244,23 @@ describe('Update subgraph tests', () => { expect(createResp.response?.code).toBe(EnumStatusCode.OK); - // Try to update with HTTP URL + // Switching to ConnectRPC means the subgraph is now reachable over + // plain HTTP, so an http:// routing URL is the right thing to set. const updateResponseHttp = await client.updateSubgraph({ name: grpcServiceName, namespace: DEFAULT_NAMESPACE, routingUrl: 'http://localhost:8080', }); - expect(updateResponseHttp.response?.code).toBe(EnumStatusCode.ERR); - expect(updateResponseHttp.response?.details).toContain('Routing URL must follow gRPC naming scheme'); + expect(updateResponseHttp.response?.code).toBe(EnumStatusCode.OK); - // Try to update with HTTPS URL const updateResponseHttps = await client.updateSubgraph({ name: grpcServiceName, namespace: DEFAULT_NAMESPACE, routingUrl: 'https://example.com:8080', }); - expect(updateResponseHttps.response?.code).toBe(EnumStatusCode.ERR); - expect(updateResponseHttps.response?.details).toContain('Routing URL must follow gRPC naming scheme'); + expect(updateResponseHttps.response?.code).toBe(EnumStatusCode.OK); }); test('Should allow updating a GRPC service subgraph with valid gRPC naming scheme URLs', async (testContext) => {