From 8d7c74736586c1946827bce2e09266d60148ad39 Mon Sep 17 00:00:00 2001 From: maz Date: Sun, 2 Jun 2024 10:55:23 +0900 Subject: [PATCH] fix: incorporate review comments --- .../@aws-cdk/aws-apprunner-alpha/lib/index.ts | 4 +- .../lib/observability-configuration.ts | 20 ++++++-- .../test/obserbability-configuration.test.ts | 47 ++++++++++++++++--- 3 files changed, 59 insertions(+), 12 deletions(-) diff --git a/packages/@aws-cdk/aws-apprunner-alpha/lib/index.ts b/packages/@aws-cdk/aws-apprunner-alpha/lib/index.ts index 072abf0e89c03..82aa712c086f1 100644 --- a/packages/@aws-cdk/aws-apprunner-alpha/lib/index.ts +++ b/packages/@aws-cdk/aws-apprunner-alpha/lib/index.ts @@ -1,4 +1,4 @@ // AWS::AppRunner CloudFormation Resources: +export * from './observability-configuration'; export * from './service'; -export * from './vpc-connector'; -export * from './observability-configuration'; \ No newline at end of file +export * from './vpc-connector'; \ No newline at end of file diff --git a/packages/@aws-cdk/aws-apprunner-alpha/lib/observability-configuration.ts b/packages/@aws-cdk/aws-apprunner-alpha/lib/observability-configuration.ts index 44c28f85e3375..b0914cadb76d4 100644 --- a/packages/@aws-cdk/aws-apprunner-alpha/lib/observability-configuration.ts +++ b/packages/@aws-cdk/aws-apprunner-alpha/lib/observability-configuration.ts @@ -26,7 +26,9 @@ export interface ObservabilityConfigurationProps { /** * The implementation provider chosen for tracing App Runner services. * - * @default Vendor.AWSXRAY + * You can not attach ObservabilityConfiguration with no vendor to the App Runner Service. + * + * @default - tracing is not enabled for the service */ readonly vendor?: Vendor; } @@ -143,11 +145,21 @@ export class ObservabilityConfiguration extends cdk.Resource implements IObserva physicalName: props.observabilityConfigurationName, }); + if (props.observabilityConfigurationName !== undefined) { + if (props.observabilityConfigurationName.length < 4 || props.observabilityConfigurationName.length > 32) { + throw new Error(`observabilityConfigurationName must be between 4 and 32 characters long, but it has ${props.observabilityConfigurationName.length} characters.`); + } + + if (!/^[A-Za-z0-9][A-Za-z0-9\-_]{3,31}$/.test(props.observabilityConfigurationName)) { + throw new Error(`observabilityConfigurationName ${props.observabilityConfigurationName} must start with a letter or number, and can contain only letters, numbers, hyphens, and underscores.`); + } + } + const resource = new CfnObservabilityConfiguration(this, 'Resource', { observabilityConfigurationName: props.observabilityConfigurationName, - traceConfiguration: { - vendor: props.vendor ?? Vendor.AWSXRAY, - }, + traceConfiguration: props.vendor ? { + vendor: props.vendor, + } : undefined, }); this.observabilityConfigurationArn = resource.attrObservabilityConfigurationArn; diff --git a/packages/@aws-cdk/aws-apprunner-alpha/test/obserbability-configuration.test.ts b/packages/@aws-cdk/aws-apprunner-alpha/test/obserbability-configuration.test.ts index dfcf73ff02832..8a9fde25b8a68 100644 --- a/packages/@aws-cdk/aws-apprunner-alpha/test/obserbability-configuration.test.ts +++ b/packages/@aws-cdk/aws-apprunner-alpha/test/obserbability-configuration.test.ts @@ -7,22 +7,37 @@ beforeEach(() => { stack = new cdk.Stack(); }); -test('create a Observability Configuration with all properties', () => { +test.each([ + ['MyObservabilityConfiguration'], + ['my-observability-configuration_1'], +])('create a ObservabilityConfiguration with all properties (name: %s)', (observabilityConfigurationName: string) => { // WHEN new ObservabilityConfiguration(stack, 'ObservabilityConfiguration', { - observabilityConfigurationName: 'MyObservabilityConfiguration', + observabilityConfigurationName, vendor: Vendor.AWSXRAY, }); // THEN Template.fromStack(stack).hasResourceProperties('AWS::AppRunner::ObservabilityConfiguration', { - ObservabilityConfigurationName: 'MyObservabilityConfiguration', + ObservabilityConfigurationName: observabilityConfigurationName, TraceConfiguration: { Vendor: 'AWSXRAY', }, }); }); +test('create a ObservabilityConfiguration without all properties', () => { + // WHEN + new ObservabilityConfiguration(stack, 'ObservabilityConfiguration', { + }); + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::AppRunner::ObservabilityConfiguration', { + ObservabilityConfigurationName: Match.absent(), + TraceConfiguration: Match.absent(), + }); +}); + test('create a Observability Configuration without all properties', () => { // WHEN new ObservabilityConfiguration(stack, 'ObservabilityConfiguration', { @@ -31,8 +46,28 @@ test('create a Observability Configuration without all properties', () => { // THEN Template.fromStack(stack).hasResourceProperties('AWS::AppRunner::ObservabilityConfiguration', { ObservabilityConfigurationName: Match.absent(), - TraceConfiguration: { - Vendor: 'AWSXRAY', - }, + TraceConfiguration: Match.absent(), }); }); + +test.each([ + ['tes'], + ['test-observability-configuration-name-over-limitation'], +])('observabilityConfigurationName over length limitation (name: %s)', (observabilityConfigurationName: string) => { + expect(() => { + new ObservabilityConfiguration(stack, 'ObservabilityConfiguration', { + observabilityConfigurationName, + }); + }).toThrow(`observabilityConfigurationName must be between 4 and 32 characters long, but it has ${observabilityConfigurationName.length} characters.`); +}); + +test.each([ + ['-test'], + ['test-?'], +])('invalid observabilityConfigurationName (name: %s)', (observabilityConfigurationName: string) => { + expect(() => { + new ObservabilityConfiguration(stack, 'ObservabilityConfiguration', { + observabilityConfigurationName, + }); + }).toThrow(`observabilityConfigurationName ${observabilityConfigurationName} must start with a letter or number, and can contain only letters, numbers, hyphens, and underscores.`); +}); \ No newline at end of file