Skip to content

Commit

Permalink
fix: incorporate review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mazyu36 committed Jun 2, 2024
1 parent 71da222 commit 8d7c747
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 12 deletions.
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-apprunner-alpha/lib/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand All @@ -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.`);
});

0 comments on commit 8d7c747

Please sign in to comment.