Conversation
tools/client-subscriptions-management/src/__tests__/client-subscription-builder.test.ts
Show resolved
Hide resolved
43d5c7d to
ef886ce
Compare
93f424c to
3e034dc
Compare
fc910ea to
4970864
Compare
77e971a to
2df6ac9
Compare
There was a problem hiding this comment.
Pull request overview
Adds end-to-end client subscription configuration support: a new CLI tool to write/read subscription configs in S3, shared model exports for status/channel enums, and runtime subscription filtering in client-transform-filter-lambda (with S3-backed config loading + caching) to suppress callbacks when no subscription matches.
Changes:
- Introduces
tools/client-subscriptions-managementTypeScript CLI (get/put subscriptions) backed by S3, with unit tests and docs. - Extends
client-transform-filter-lambdato load/validate per-client subscription config from S3 (cached) and filter transformed events before delivery. - Refactors
src/modelsto export*_STATUSES/CHANNEL_TYPESconstants and tightens config typing to use those enums.
Reviewed changes
Copilot reviewed 61 out of 63 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds a repo-level tsconfig extending the base config and including tool/lambda/test sources. |
| tools/client-subscriptions-management/tsconfig.json | TS config for the new CLI workspace. |
| tools/client-subscriptions-management/src/types.ts | Defines subscription configuration types for the CLI tool. |
| tools/client-subscriptions-management/src/infra/s3-repository.ts | Adds S3 read/write wrapper with stream-to-string handling. |
| tools/client-subscriptions-management/src/infra/client-subscription-repository.ts | Implements get/put subscription operations with AJV validation and S3 persistence. |
| tools/client-subscriptions-management/src/index.ts | Exposes the repository factory from the tool package. |
| tools/client-subscriptions-management/src/entrypoint/cli/put-message-status.ts | CLI entrypoint to upsert message-status subscriptions. |
| tools/client-subscriptions-management/src/entrypoint/cli/put-channel-status.ts | CLI entrypoint to upsert channel-status subscriptions. |
| tools/client-subscriptions-management/src/entrypoint/cli/helper.ts | CLI formatting + env resolution helpers. |
| tools/client-subscriptions-management/src/entrypoint/cli/get-client-subscriptions.ts | CLI entrypoint to fetch and print a client’s subscriptions. |
| tools/client-subscriptions-management/src/domain/client-subscription-builder.ts | Builds subscription config payloads (EventSource/EventDetail/Targets). |
| tools/client-subscriptions-management/src/container.ts | Wires S3 client + repositories for the CLI tool. |
| tools/client-subscriptions-management/src/constants.ts | Introduces tool-local status/channel constants for CLI choices and validation. |
| tools/client-subscriptions-management/src/tests/s3-repository.test.ts | Unit tests for S3Repository body handling and errors. |
| tools/client-subscriptions-management/src/tests/put-message-status.test.ts | Unit tests for message-status CLI argument parsing and behavior. |
| tools/client-subscriptions-management/src/tests/put-channel-status.test.ts | Unit tests for channel-status CLI argument parsing and behavior. |
| tools/client-subscriptions-management/src/tests/helper.test.ts | Unit tests for helper formatting and env resolution. |
| tools/client-subscriptions-management/src/tests/get-client-subscriptions.test.ts | Unit tests for get CLI output behavior. |
| tools/client-subscriptions-management/src/tests/container.test.ts | Unit tests for tool container wiring. |
| tools/client-subscriptions-management/src/tests/container-s3-config.test.ts | Unit tests for S3 client config (forcePathStyle on localhost). |
| tools/client-subscriptions-management/src/tests/constants.test.ts | Unit tests asserting tool constants are exposed. |
| tools/client-subscriptions-management/src/tests/client-subscription-repository.test.ts | Unit tests for repository put/get, dry-run, and AJV validation behavior. |
| tools/client-subscriptions-management/src/tests/client-subscription-builder.test.ts | Unit tests for config builder output and event-source resolution. |
| tools/client-subscriptions-management/package.json | New workspace package definition and scripts for the CLI tool. |
| tools/client-subscriptions-management/jest.config.ts | Jest config for the tool workspace. |
| tools/client-subscriptions-management/README.md | Usage documentation for the new CLI tool. |
| tests/integration/event-bus-to-webhook.test.ts | Adjusts helper import path. |
| src/models/src/status-types.ts | Replaces union types with exported *_STATUSES const arrays + derived types. |
| src/models/src/index.ts | Re-exports status/channel const arrays for consumers. |
| src/models/src/client-config.ts | Tightens config types to use Channel/MessageStatus/ChannelStatus/SupplierStatus arrays. |
| src/models/src/channel-types.ts | Exports CHANNEL_TYPES const array + derived Channel type. |
| scripts/deploy_client_subscriptions.sh | Adds a deployment helper script to run the CLI and optionally apply Terraform. |
| scripts/config/sonar-scanner.properties | Updates sonar coverage exclusion patterns to include tool/script tests and jest configs. |
| package.json | Adds tool workspace, adds aws-sdk S3 dep at root, and adjusts minimatch overrides. |
| lambdas/client-transform-filter-lambda/src/services/validators/config-validator.ts | Adds Zod-based validation for subscription config loaded from S3. |
| lambdas/client-transform-filter-lambda/src/services/transform-pipeline.ts | Adds subscription filter evaluation routing by event type. |
| lambdas/client-transform-filter-lambda/src/services/observability.ts | Extends batch-completed logging context to include filtered. |
| lambdas/client-transform-filter-lambda/src/services/filters/message-status-filter.ts | Adds message-status subscription matching (client/status + event-pattern). |
| lambdas/client-transform-filter-lambda/src/services/filters/event-pattern.ts | Implements EventSource/EventDetail JSON pattern matching. |
| lambdas/client-transform-filter-lambda/src/services/filters/channel-status-filter.ts | Adds channel-status subscription matching including change detection and event-pattern checks. |
| lambdas/client-transform-filter-lambda/src/services/config-loader.ts | Loads config from S3, validates it, caches it, and logs outcomes. |
| lambdas/client-transform-filter-lambda/src/services/config-cache.ts | Adds TTL-based in-memory config cache. |
| lambdas/client-transform-filter-lambda/src/index.ts | Wires S3 config loader/cache into handler execution and exposes test helpers. |
| lambdas/client-transform-filter-lambda/src/handler.ts | Filters transformed events by subscription config before delivery initiation/return. |
| lambdas/client-transform-filter-lambda/src/tests/validators/event-validator.test.ts | Updates expected validation error string. |
| lambdas/client-transform-filter-lambda/src/tests/validators/config-validator.test.ts | Adds tests for config schema validation failures/success. |
| lambdas/client-transform-filter-lambda/src/tests/services/transform-pipeline.test.ts | Tests evaluateSubscriptionFilters for different event/config cases. |
| lambdas/client-transform-filter-lambda/src/tests/services/subscription-filter.test.ts | Adds comprehensive filter matching tests for message/channel subscriptions. |
| lambdas/client-transform-filter-lambda/src/tests/services/filters/event-pattern.test.ts | Tests EventSource/EventDetail matching behavior. |
| lambdas/client-transform-filter-lambda/src/tests/services/config-update.integration.test.ts | Tests cache expiry leading to config reload. |
| lambdas/client-transform-filter-lambda/src/tests/services/config-loader.test.ts | Tests S3 body decoding, caching, missing keys, and validation failures. |
| lambdas/client-transform-filter-lambda/src/tests/services/config-cache.test.ts | Tests TTL cache get/set/expiry/clear behavior. |
| lambdas/client-transform-filter-lambda/src/tests/services/callback-logger.test.ts | Updates test description text to match event naming. |
| lambdas/client-transform-filter-lambda/src/tests/index.test.ts | Updates expected validation error string and handler wiring assertion. |
| lambdas/client-transform-filter-lambda/src/tests/index.s3-config.test.ts | Tests lambda S3 client forcePathStyle behavior. |
| lambdas/client-transform-filter-lambda/src/tests/index.reset-loader.test.ts | Tests resetConfigLoader behavior with/without bucket and prefix. |
| lambdas/client-transform-filter-lambda/src/tests/index.integration.test.ts | End-to-end handler test with mocked S3 subscription filtering enabled/disabled. |
| lambdas/client-transform-filter-lambda/src/tests/index.config-prefix.test.ts | Tests default vs configured S3 key prefix wiring. |
| lambdas/client-transform-filter-lambda/src/tests/index.cache-ttl-valid.test.ts | Tests TTL parsing when configured value is valid. |
| lambdas/client-transform-filter-lambda/src/tests/index.cache-ttl-invalid.test.ts | Tests TTL parsing fallback when configured value is invalid. |
| lambdas/client-transform-filter-lambda/package.json | Adds S3 client dependency for lambda config loading. |
| infrastructure/terraform/components/callbacks/module_transform_filter_lambda.tf | Supplies new env vars for config bucket/prefix and cache TTL to the lambda. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tools/client-subscriptions-management/src/domain/client-subscription-builder.ts
Show resolved
Hide resolved
tools/client-subscriptions-management/src/infra/client-subscription-repository.ts
Show resolved
Hide resolved
2df6ac9 to
b8a8bda
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 60 out of 62 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tools/client-subscriptions-management/src/infra/client-subscription-repository.ts
Show resolved
Hide resolved
infrastructure/terraform/components/callbacks/module_transform_filter_lambda.tf
Show resolved
Hide resolved
tools/client-subscriptions-management/src/entrypoint/cli/put-message-status.ts
Show resolved
Hide resolved
tools/client-subscriptions-management/src/entrypoint/cli/put-channel-status.ts
Show resolved
Hide resolved
tools/client-subscriptions-management/src/infra/client-subscription-repository.ts
Show resolved
Hide resolved
f02ce04 to
0ba9f60
Compare
* CCM-14637 - Set up integration tests * CCM-14637 - Attempt to use AWS_ACCOUNT_ID env var instead of fetching it via STS
0ba9f60 to
fc5991e
Compare
mjewildnhs
left a comment
There was a problem hiding this comment.
Posting feedback I have - though not made it too far
| observability: ObservabilityService, | ||
| stats: BatchStats, | ||
| ): Promise<TransformedEvent[]> { | ||
| const uniqueClientIds = [ |
There was a problem hiding this comment.
pMap takes an iterable so should be able to pass the Set directly
| const config = configByClientId.get(clientId); | ||
| const filterResult = evaluateSubscriptionFilters(event, config); | ||
|
|
||
| if (filterResult.matched) { |
There was a problem hiding this comment.
Do we want some metrics here as we have them for all the other stages e.g. recordProcessingStarted, recordDeliveryInitiated, recordCallbackGenerated all emit metric events?
recordTransformationStarted doesn't emit an event metric but it would be nice to follow the pattern by having filterBatch call into the observability service to record a lifecycle event.
| const getConfigLoader = (): ConfigLoader | undefined => { | ||
| const bucketName = process.env.CLIENT_SUBSCRIPTION_CONFIG_BUCKET; | ||
| if (!bucketName) { | ||
| // Config bucket not configured - subscription filtering disabled |
There was a problem hiding this comment.
Do we need/want to support no subscription filtering? I think we always want it.
resetConfigLoader throws if its not set
|
|
||
| // Exported for testing - resets the cached loader (and clears the config cache) to allow | ||
| // clean state between tests, with optional custom S3Client injection | ||
| export const resetConfigLoader = (s3Client?: S3Client): void => { |
There was a problem hiding this comment.
Could we wrap some of this ConfigLoader / ConfigCache code together in a standalone service to simplify some of this stuff.
Its quite a lot of code for the lambda entry point.
It would be nice to use HandlerDependencies like we do for the ObservabilityService for DI.
And hide resetConfigLoader away somewhere if its test only
Description
Context
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.