Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(core): a number of resources are not taggable with Tags.of() #28989

Merged
merged 7 commits into from
Feb 23, 2024
Merged

fix(core): a number of resources are not taggable with Tags.of() #28989

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dbe614c
fix(spec2cdk): enable modern tagging style with ITaggableV2
GavinZZ Feb 5, 2024
c134260
Add a comment
GavinZZ Feb 5, 2024
5ff4515
Update code generation
GavinZZ Feb 10, 2024
e805aea
Remove unused boolean HAS_25610
GavinZZ Feb 22, 2024
1c374cd
Update render tags
GavinZZ Feb 22, 2024
07529e6
Give property name in render tags
GavinZZ Feb 23, 2024
5cce6f5
Merge branch 'main' into yuanhaoz/tagv2
mergify[bot] Feb 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions tools/@aws-cdk/spec2cdk/lib/cdk/resource-decider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import { attributePropertyName, cloudFormationDocLink, propertyNameFromCloudForm
import { splitDocumentation } from '../util';

// Depends on https://github.com/aws/aws-cdk/pull/25610
export const HAS_25610 = false;
// Eanbling this feature since PR 25610 was merged.
export const HAS_25610 = true;

// This convenience typewriter builder is used all over the place
const $this = $E(expr.this_());
Expand Down Expand Up @@ -184,7 +185,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));
GavinZZ marked this conversation as resolved.
Show resolved Hide resolved
const originalType = this.converter.typeFromProperty(prop);

this.propsProperties.push({
propertySpec: {
Expand Down Expand Up @@ -217,11 +218,11 @@ export class ResourceDecider {
new CDK_CORE.TagManager(
this.tagManagerVariant(variant),
expr.lit(this.resource.cloudFormationType),
HAS_25610 ? expr.UNDEFINED : $E(props)[originalName],
$E(props)[originalName],
GavinZZ marked this conversation as resolved.
Show resolved Hide resolved
expr.object({ tagPropertyName: expr.lit(originalName) }),
),
cfnValueToRender: {
[originalName]: $this.tags.renderTags(...(HAS_25610 ? [$this[originalName]] : [])),
GavinZZ marked this conversation as resolved.
Show resolved Hide resolved
[originalName]: $this.cdkTagManager.renderTags(),
},
},
{
Expand Down