Allow kafka:// URIs in webhook subscription TOML validation#7817
Draft
dpeacock wants to merge 1 commit into
Draft
Allow kafka:// URIs in webhook subscription TOML validation#7817dpeacock wants to merge 1 commit into
dpeacock wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Shopify CLI webhook TOML validation to accept Kafka delivery URIs, unblocking internal apps that use Kafka from adopting declarative webhooks.
Changes:
- Added
kafka://{topic}shape support toWebhookSubscriptionUriValidationand updated the validation error message to mention Kafka. - Updated/added tests to cover valid/invalid Kafka URIs and included Kafka in the combined-URI sorting/expansion test.
- Added a changeset to publish the validation update as a patch release for
@shopify/app.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/app/src/cli/models/extensions/specifications/validation/common.ts | Adds a Kafka URI regex and allows kafka://... in the webhook subscription URI validator + updates the error message. |
| packages/app/src/cli/models/app/loader.test.ts | Updates expected validation error strings and adds Kafka URI test coverage (including combined-URI expansion order). |
| .changeset/add-kafka-webhook-uri-validation.md | Announces the patch release and describes the new Kafka URI validation behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
17
to
+21
| export const WebhookSubscriptionUriValidation = zod.string({invalid_type_error: 'Value must be string'}).refine( | ||
| (uri) => { | ||
| if (uri.startsWith('/')) return true | ||
|
|
||
| return httpsRegex.test(uri) || pubSubRegex.test(uri) || arnRegex.test(uri) | ||
| return httpsRegex.test(uri) || pubSubRegex.test(uri) || arnRegex.test(uri) || kafkaRegex.test(uri) |
Comment on lines
+2905
to
+2919
| test('throws an error if kafka topic contains invalid characters', async () => { | ||
| const webhookConfig: WebhooksConfig = { | ||
| api_version: '2021-07', | ||
| subscriptions: [{uri: 'kafka://invalid topic!', topics: ['products/create']}], | ||
| } | ||
| const errorObj = { | ||
| code: zod.ZodIssueCode.custom, | ||
| message: | ||
| "URI format isn't correct. Valid formats include: relative path starting with a slash, HTTPS URL, pubsub://{project-id}:{topic-id}, kafka://{topic-id} or Eventbridge ARN", | ||
| path: ['webhooks', 'subscriptions', 0, 'uri'], | ||
| } | ||
|
|
||
| const result = await setupParsing(errorObj, webhookConfig) | ||
| expect(result.threw).toBe(true) | ||
| }) |
Comment on lines
3052
to
3055
| code: zod.ZodIssueCode.custom, | ||
| message: | ||
| "URI format isn't correct. Valid formats include: relative path starting with a slash, HTTPS URL, pubsub://{project-id}:{topic-id} or Eventbridge ARN", | ||
| "URI format isn't correct. Valid formats include: relative path starting with a slash, HTTPS URL, pubsub://{project-id}:{topic-id}, kafka://{topic-id} or Eventbridge ARN", | ||
| path: ['webhooks', 'subscriptions', 0, 'uri'], |
| // example Eventbridge ARN - arn:aws:events:{region}::event-source/aws.partner/shopify.com/{app_id}/{path} | ||
| const arnRegex = | ||
| /^arn:aws:events:(?<aws_region>[a-z]{2}-[a-z]+-[0-9]+)::event-source\/aws\.partner\/shopify\.com(\.test)?\/(?<api_client_id>\d+)\/(?<event_source_name>.+)$/ | ||
| // example Kafka URI - kafka://{topic}. Restricted to internal apps; the platform enforces authorization. |
| '@shopify/app': patch | ||
| --- | ||
|
|
||
| Accept `kafka://{topic}` URIs in `[[webhooks.subscriptions]]` TOML validation. Kafka delivery is restricted to internal Shopify apps; the platform enforces authorization at subscription time. |
The webhook subscription URI validator in @shopify/app previously rejected kafka:// URIs, which blocked internal Shopify apps (Email, Shop, Flow, Collabs, Stocky, Marketplace, Collective, etc.) from declaring their webhook subscriptions in the app TOML. Adds a kafka:// regex mirroring the server-side topic character set ([a-zA-Z0-9_.-]+) so the CLI rejects exactly what the platform rejects for shape. Authorization (internal-app-only) is enforced by the platform at subscription creation time via api_client.internal_graphql_access?; the CLI intentionally validates URI shape only, matching how pubsub:// and Eventbridge ARNs are handled today. Updates the validation error message to mention kafka and adds tests for accepting a well-formed kafka URI, rejecting invalid topic characters, and including kafka in the combined-URI subscriptions test (which expands and sorts alphabetically by URI). Resolves shop/issues-event-foundations#252.
dpeacock
force-pushed
the
add-kafka-webhook-uri-validation
branch
from
14cd554 to
4c7f7ae
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WHY are these changes introduced?
Fixes shop/issues-event-foundations#252.
The webhook subscription URI validator (
WebhookSubscriptionUriValidationincommon.ts) only accepted relative paths, HTTPS URLs,pubsub://URIs, and Eventbridge ARNs.kafka://URIs were rejected at TOML validation time, which blocked the internal Shopify apps that currently rely on Kafka delivery (Email, Shop, Flow, Collabs, Stocky, Marketplace, Collective, …) from adopting declarative webhooks.What is this change?
kafka://{topic}regex tocommon.ts. The topic character set ([a-zA-Z0-9_.-]+) mirrors the server-side validator atcomponents/apps/webhook_foundations/app/validators/webhook_uri_validator.rbso the CLI rejects exactly what the platform rejects on shape.kafkaRegex.test(uri)in the.refine()predicate and update the error message to mentionkafka://{topic-id}.kafka://my_topic.name-1URI; rejectingkafka://invalid topic!; and including kafka in the combined-URI subscriptions test (whose expansion sorts alphabetically by URI: arn → https → kafka → pubsub).loader.test.tsthat assert the previous error string to includekafka://{topic-id}.Authorization is enforced server-side, not in the CLI
The CLI validates URI shape only. Internal-app-only enforcement happens at the platform:
webhook_uri_validator.rb#validate_kafka_topicchecksrecord.api_client.internal_graphql_access?and returns"uses delivery method 'kafka' and is not supported"for non-internal clients. This mirrors howpubsub://and Eventbridge ARNs are validated today — the CLI doesn't reproduce server authorization rules, it only filters obviously malformed input.Checklist