fix: incorporate review comments

aws · Jun 2, 2024 · b93cd9b · b93cd9b
1 parent 71da222
commit b93cd9b
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';
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,15 +145,25 @@ 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;
  this.observabilityConfigurationRevision = resource.attrObservabilityConfigurationRevision;
  this.observabilityConfigurationName = resource.ref;
  }
-}
+}
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.`);
+});