Skip to content

Commit

Permalink
fix(core): a number of resources are not taggable with Tags.of() (#…
Browse files Browse the repository at this point in the history
…28989)

### Issue # (if applicable)

Closes #28862

### Reason for this change

CFN resources that has array format `tags` cannot use `Tags.of()` to add tags.

### Description of changes

Enable modern style tags `ITaggableV2`

### Description of how you validated changes

The generated looks correct and shouldn't cause breaking changes.

*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 Feb 23, 2024
1 parent aa186ac commit 7a4c189
Showing 1 changed file with 10 additions and 15 deletions.
25 changes: 10 additions & 15 deletions tools/@aws-cdk/spec2cdk/lib/cdk/resource-decider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ import { TypeConverter } from './type-converter';
import { attributePropertyName, cloudFormationDocLink, propertyNameFromCloudFormation } from '../naming';
import { splitDocumentation } from '../util';

// Depends on https://github.com/aws/aws-cdk/pull/25610
export const HAS_25610 = false;

// This convenience typewriter builder is used all over the place
const $this = $E(expr.this_());

Expand All @@ -21,7 +18,7 @@ export class ResourceDecider {
const taggability = resourceTaggabilityStyle(resource);
return taggability?.style === 'legacy'
? [CDK_CORE.ITaggable]
: taggability?.style === 'modern' && HAS_25610
: taggability?.style === 'modern'
? [CDK_CORE.ITaggableV2]
: [];
}
Expand Down Expand Up @@ -51,10 +48,8 @@ export class ResourceDecider {
this.handleTagPropertyLegacy(name, prop, this.taggability.variant);
continue;
case 'modern':
if (HAS_25610) {
this.handleTagPropertyModern(name, prop, this.taggability.variant);
continue;
}
this.handleTagPropertyModern(name, prop, this.taggability.variant);
continue;
}
} else {
this.handleTypeHistoryTypes(prop);
Expand Down Expand Up @@ -158,15 +153,15 @@ export class ResourceDecider {
summary: 'Tag Manager which manages the tags for this resource',
},
},
initializer: (props: Expression) =>
initializer: (_: Expression) =>
new CDK_CORE.TagManager(
this.tagManagerVariant(variant),
expr.lit(this.resource.cloudFormationType),
HAS_25610 ? expr.UNDEFINED : $E(props)[originalName],
expr.UNDEFINED,
expr.object({ tagPropertyName: expr.lit(originalName) }),
),
cfnValueToRender: {
[originalName]: $this.tags.renderTags(...(HAS_25610 ? [$this[rawTagsPropName]] : [])),
[originalName]: $this.tags.renderTags($this[rawTagsPropName]),
},
},
{
Expand All @@ -184,7 +179,7 @@ export class ResourceDecider {

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

this.propsProperties.push({
propertySpec: {
Expand Down Expand Up @@ -213,15 +208,15 @@ export class ResourceDecider {
summary: 'Tag Manager which manages the tags for this resource',
},
},
initializer: (props: Expression) =>
initializer: (_: Expression) =>
new CDK_CORE.TagManager(
this.tagManagerVariant(variant),
expr.lit(this.resource.cloudFormationType),
HAS_25610 ? expr.UNDEFINED : $E(props)[originalName],
expr.UNDEFINED,
expr.object({ tagPropertyName: expr.lit(originalName) }),
),
cfnValueToRender: {
[originalName]: $this.tags.renderTags(...(HAS_25610 ? [$this[originalName]] : [])),
[originalName]: $this.cdkTagManager.renderTags($this[originalName]),
},
},
{
Expand Down

0 comments on commit 7a4c189

Please sign in to comment.