Skip to content

Commit

Permalink
fix(spec2cdk): use modern type when building tag type (aws#29389)
Browse files Browse the repository at this point in the history
### Issue # (if applicable)

Closes aws#29388

### Reason for this change

Some of the modern tags failed to run `cdk synth` due to type misconfiguration.

### Description of changes

Always default to use the latest type for modern tags.

### Description of how you validated changes

Fixed for failed resources.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
GavinZZ authored Mar 7, 2024
1 parent d33caff commit 3fb0254
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 1 deletion.
41 changes: 41 additions & 0 deletions packages/aws-cdk-lib/aws-xray/test/xray.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { Match, Template } from '../../assertions';
import * as iam from '../../aws-iam';
import * as kms from '../../aws-kms';
import { CfnParameter, Duration, Stack, App, Token, Tags } from '../../core';
import * as xray from '../lib';

/* eslint-disable quote-props */

test('able to add tags to XRay CfnGroup', () => {
const stack = new Stack();
new xray.CfnGroup(stack, 'Group', {
groupName: 'GroupName',
tags: [{
key: 'Key',
value: 'Value',
}],
});

Template.fromStack(stack).hasResourceProperties('AWS::XRay::Group', {
Tags: [{
Key: 'Key',
Value: 'Value',
}],
});
});

test('able to add tags through Tags.of()... to XRay CfnGroup', () => {
const stack = new Stack();
new xray.CfnGroup(stack, 'Group', {
groupName: 'GroupName',
});

Tags.of(stack).add('Key', 'Value');

Template.fromStack(stack).hasResourceProperties('AWS::XRay::Group', {
Tags: [{
Key: 'Key',
Value: 'Value',
}],
});
});
2 changes: 1 addition & 1 deletion tools/@aws-cdk/spec2cdk/lib/cdk/resource-decider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ export class ResourceDecider {

private handleTagPropertyModern(cfnName: string, prop: Property, variant: TagVariant) {
const originalName = propertyNameFromCloudFormation(cfnName);
const originalType = this.converter.typeFromProperty(prop);
const originalType = this.converter.typeFromPropertyForModernTags(prop);

this.propsProperties.push({
propertySpec: {
Expand Down
10 changes: 10 additions & 0 deletions tools/@aws-cdk/spec2cdk/lib/cdk/type-converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,16 @@ export class TypeConverter {
return this.typeFromSpecType(this.typeHistoryFromProperty(property)[0]);
}

/**
* Return the appropriate typewriter type for a servicespec type for modern tags
* Unlike typeFromProperty, we want to default to use the newest type instead.
*/
public typeFromPropertyForModernTags(property: Property): Type {
// For backwards compatibility reasons we always have to use the original type
const types = this.typeHistoryFromProperty(property);
return this.typeFromSpecType(types[types.length - 1]);
}

/**
* Return the full type history for a servicespec property
*/
Expand Down

0 comments on commit 3fb0254

Please sign in to comment.