Skip to content

Commit be70c82

Browse files
fix(core): isTaggable function can return undefined instead of false (#31600)
Closes #26495 ### Reason for this change The `isTaggable` function of the `TagManager` class is currently broken in Python, as it can return `undefined` instead of `true` or `false`. ### Description of changes In JS/TS, the logical AND operator (`&&`) returns the first falsy value it encounters, even if that value is `undefined` instead of `false` - so the current implementation of `isTaggable` allows for `undefined` to be returned if `tags` is undefined: ```ts public static isTaggable(construct: any): construct is ITaggable { const tags = (construct as any).tags; return tags && typeof tags === 'object' && (tags as any)[TAG_MANAGER_SYM]; } ``` The fix is simply changing the return line to the following to handle cases where tags is `null` or `undefined`: ```ts return tags !== undefined && tags !== null && typeof tags === 'object' && (tags as any)[TAG_MANAGER_SYM]; ``` ### Description of how you validated changes Added a unit test to assert that `isTaggable` returns `false`, not `undefined` for a non-taggable Construct (and still returns `true` for a taggable construct). ### 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*
1 parent 0755561 commit be70c82

File tree

2 files changed

+38
-1
lines changed

2 files changed

+38
-1
lines changed

packages/aws-cdk-lib/core/lib/tag-manager.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ export class TagManager {
303303
*/
304304
public static isTaggable(construct: any): construct is ITaggable {
305305
const tags = (construct as any).tags;
306-
return tags && typeof tags === 'object' && (tags as any)[TAG_MANAGER_SYM];
306+
return tags !== undefined && tags !== null && typeof tags === 'object' && (tags as any)[TAG_MANAGER_SYM];
307307
}
308308

309309
/**

packages/aws-cdk-lib/core/test/tag-manager.test.ts

+37
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { Construct } from 'constructs';
2+
import * as cdk from '../../../aws-cdk-lib';
13
import { TagType } from '../lib/cfn-resource';
24
import { TagManager } from '../lib/tag-manager';
35

@@ -225,4 +227,39 @@ describe('tag manager', () => {
225227
expect(true).toEqual(mgr.applyTagAspectHere(['AWS::Fake::Resource'], []));
226228
expect(false).toEqual(mgr.applyTagAspectHere(['AWS::Wrong::Resource'], []));
227229
});
230+
231+
test('isTaggable function works as expected', () => {
232+
const app = new cdk.App();
233+
const taggableConstruct = new TaggableConstruct(app, 'MyConstruct');
234+
const nonTaggableConstruct = new NonTaggableConstruct(app, 'NonTaggableConstruct');
235+
236+
// Assert that isTaggable returns true for a taggable construct
237+
expect(TagManager.isTaggable(taggableConstruct)).toEqual(true);
238+
239+
// Assert that isTaggable returns false (not undefined) for a non-taggable construct
240+
expect(TagManager.isTaggable(nonTaggableConstruct)).not.toEqual(undefined);
241+
expect(TagManager.isTaggable(nonTaggableConstruct)).toEqual(false);
242+
});
228243
});
244+
245+
// `Stack` extends ITaggable so this construct is Taggable by default
246+
class TaggableConstruct extends cdk.Stack {
247+
constructor(scope: Construct, id: string) {
248+
super(scope, id);
249+
new cdk.CfnResource(this, 'Resource', {
250+
type: 'Whatever::The::Type',
251+
properties: {
252+
Tags: this.tags.renderedTags,
253+
},
254+
});
255+
}
256+
}
257+
258+
// Simple Construct that does not extend ITaggable
259+
class NonTaggableConstruct extends Construct {
260+
public readonly id: string;
261+
constructor(scope: Construct, id: string) {
262+
super(scope, id);
263+
this.id = id;
264+
}
265+
}

0 commit comments

Comments
 (0)