fix: incorporate review comments

aws · Jun 2, 2024 · 8d7c747 · 8d7c747
1 parent 71da222
commit 8d7c747
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 12 deletions.
diff --git 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';
+export * from './vpc-connector';
diff --git 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
@@ -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.`);
+});