From 24cf7c469e2474a261e35deb31da123133753a01 Mon Sep 17 00:00:00 2001 From: Simon Heather Date: Tue, 21 Apr 2026 12:42:51 +0100 Subject: [PATCH 1/3] Fix ForExpression missing iteratorContext and OperatorExpression drops falsy right operands --- packages/cdktn/lib/tfExpression.ts | 8 +++++-- packages/cdktn/test/iterator.test.ts | 23 +++++++++++++++++++ .../cdktn/test/terraform-operator.test.ts | 15 ++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/packages/cdktn/lib/tfExpression.ts b/packages/cdktn/lib/tfExpression.ts index 4d5ab08da..53a62d2e5 100644 --- a/packages/cdktn/lib/tfExpression.ts +++ b/packages/cdktn/lib/tfExpression.ts @@ -322,7 +322,10 @@ export class OperatorExpression extends TFExpression { context.suppressBraces = true; const left = this.resolveArg(context, this.left); - const right = this.right ? this.resolveArg(context, this.right) : undefined; + const right = + this.right !== undefined + ? this.resolveArg(context, this.right) + : undefined; let expr = ""; switch (this.operator) { @@ -331,7 +334,7 @@ export class OperatorExpression extends TFExpression { break; } case "-": { - if (right) { + if (right !== undefined) { // subtraction expr = `(${left} - ${right})`; } else { @@ -402,6 +405,7 @@ class ForExpression extends TFExpression { public resolve(context: IResolveContext): string { const suppressBraces = context.suppressBraces; context.suppressBraces = true; + context.iteratorContext = "FOR_EXPRESSION"; const key = this.resolveArg(context, FOR_EXPRESSION_KEY); const value = this.resolveArg(context, FOR_EXPRESSION_VALUE); const input = this.resolveArg(context, this.input); diff --git a/packages/cdktn/test/iterator.test.ts b/packages/cdktn/test/iterator.test.ts index 344c15b53..c563fbbae 100644 --- a/packages/cdktn/test/iterator.test.ts +++ b/packages/cdktn/test/iterator.test.ts @@ -529,3 +529,26 @@ test("for expressions from iterators", () => { "${[ for key, val in toset(var.list): key]}", ); }); + +test("forExpressionForList with fromComplexList uses raw list, not map conversion", () => { + const app = Testing.app(); + const stack = new TerraformStack(app, "test"); + const variable = new TerraformVariable(stack, "list", {}); + const it = TerraformIterator.fromComplexList( + variable.listValue as any, + "name", + ); + + new TestResource(stack, "test", { + name: Token.asString( + it.forExpressionForList(`val.id if val.name == "foo"`), + ), + }); + + const synth = JSON.parse(Testing.synth(stack)); + // Should iterate the raw list, not the map conversion + expect(synth).toHaveProperty( + "resource.test_resource.test.name", + '${[ for key, val in var.list: val.id if val.name == "foo"]}', + ); +}); diff --git a/packages/cdktn/test/terraform-operator.test.ts b/packages/cdktn/test/terraform-operator.test.ts index d72d2c579..a2bac6d44 100644 --- a/packages/cdktn/test/terraform-operator.test.ts +++ b/packages/cdktn/test/terraform-operator.test.ts @@ -78,3 +78,18 @@ test("Op.or renders correctly", () => { `"\${(true || true)}"`, ); }); + +// Regression tests for falsy right-hand operands +test("Op.gt renders correctly with 0 as right operand", () => { + expect(resolveExpression(Op.gt(1, 0))).toMatchInlineSnapshot(`"\${(1 > 0)}"`); +}); +test("Op.eq renders correctly with false as right operand", () => { + expect(resolveExpression(Op.eq(true, false))).toMatchInlineSnapshot( + `"\${(true == false)}"`, + ); +}); +test("Op.sub renders correctly with 0 as right operand", () => { + expect(resolveExpression(Op.sub(5, 0))).toMatchInlineSnapshot( + `"\${(5 - 0)}"`, + ); +}); From 79f2297eff803e7f89ab71c5606e61267a207c82 Mon Sep 17 00:00:00 2001 From: Simon Heather Date: Wed, 22 Apr 2026 17:10:49 +0100 Subject: [PATCH 2/3] Fix test --- packages/cdktn/test/tfExpression.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cdktn/test/tfExpression.test.ts b/packages/cdktn/test/tfExpression.test.ts index 6f379e204..a1eb3fb79 100644 --- a/packages/cdktn/test/tfExpression.test.ts +++ b/packages/cdktn/test/tfExpression.test.ts @@ -200,7 +200,7 @@ test("string index expression argument renders correctly", () => { test("null expression argument renders correctly", () => { expect(resolveExpression(Op.or(true, null))).toMatchInlineSnapshot( - `"\${(true || undefined)}"`, + `"\${(true || null)}"`, ); }); From edd76e3b0a6cbf65d2662056af256f22cb0323d4 Mon Sep 17 00:00:00 2001 From: Simon Heather Date: Thu, 23 Apr 2026 13:08:23 +0100 Subject: [PATCH 3/3] Refactor bug 1 fix --- .../cdktn/lib/terraform-dynamic-expression.ts | 2 +- packages/cdktn/lib/terraform-iterator.ts | 70 +++++++++------ packages/cdktn/test/iterator.test.ts | 90 +++++++++++++++++++ 3 files changed, 133 insertions(+), 29 deletions(-) diff --git a/packages/cdktn/lib/terraform-dynamic-expression.ts b/packages/cdktn/lib/terraform-dynamic-expression.ts index a0921ea95..b0e95207b 100644 --- a/packages/cdktn/lib/terraform-dynamic-expression.ts +++ b/packages/cdktn/lib/terraform-dynamic-expression.ts @@ -49,7 +49,7 @@ export class TerraformDynamicExpression implements IResolvable { // context.resolve is required for the iteratorContext to be correctly passed // to Lazy values within this.content return context.resolve( - forExpression(this.iterator._getForEachExpression(), this.content), + forExpression(this.iterator._getForExpressionInput(), this.content), ); }, }); diff --git a/packages/cdktn/lib/terraform-iterator.ts b/packages/cdktn/lib/terraform-iterator.ts index e6d437d47..3fadb0756 100644 --- a/packages/cdktn/lib/terraform-iterator.ts +++ b/packages/cdktn/lib/terraform-iterator.ts @@ -33,6 +33,13 @@ export interface ITerraformIterator { * @internal used by TerraformResource to set the for_each expression */ _getForEachExpression(): any; + + /** + * @internal input used when this iterator appears inside a for-expression + * body (keys / values / pluckProperty / forExpressionForList / + * forExpressionForMap / dynamic). + */ + _getForExpressionInput(): any; } type ListType = @@ -66,6 +73,17 @@ export abstract class TerraformIterator implements ITerraformIterator { */ abstract _getForEachExpression(): any; + /** + * @internal input used when this iterator appears inside a for-expression + * body (keys / values / pluckProperty / forExpressionForList / + * forExpressionForMap / dynamic). Defaults to the for_each expression; + * subclasses that need a different shape in for-expression bodies + * (e.g. DynamicListTerraformIterator) can override. + */ + public _getForExpressionInput(): any { + return this._getForEachExpression(); + } + /** * Creates a new iterator from a list */ @@ -303,7 +321,7 @@ export abstract class TerraformIterator implements ITerraformIterator { */ public keys(): IResolvable { return Token.asAny( - forExpression(this._getForEachExpression(), FOR_EXPRESSION_KEY), + forExpression(this._getForExpressionInput(), FOR_EXPRESSION_KEY), ); } @@ -315,7 +333,7 @@ export abstract class TerraformIterator implements ITerraformIterator { */ public values(): IResolvable { return Token.asAny( - forExpression(this._getForEachExpression(), FOR_EXPRESSION_VALUE), + forExpression(this._getForExpressionInput(), FOR_EXPRESSION_VALUE), ); } @@ -328,7 +346,7 @@ export abstract class TerraformIterator implements ITerraformIterator { public pluckProperty(property: string): IResolvable { return Token.asAny( forExpression( - this._getForEachExpression(), + this._getForExpressionInput(), propertyAccess(FOR_EXPRESSION_VALUE, [property]), ), ); @@ -349,7 +367,9 @@ export abstract class TerraformIterator implements ITerraformIterator { * @param expression The expression to use in the for mapping */ public forExpressionForList(expression: string | IResolvable) { - return Token.asAny(forExpression(this._getForEachExpression(), expression)); + return Token.asAny( + forExpression(this._getForExpressionInput(), expression), + ); } /** @@ -373,7 +393,7 @@ export abstract class TerraformIterator implements ITerraformIterator { ) { return Token.asAny( forExpression( - this._getForEachExpression(), + this._getForExpressionInput(), valueExpression, keyExpression, ), @@ -491,31 +511,25 @@ export class DynamicListTerraformIterator extends MapTerraformIterator { } /** - * @internal used by TerraformResource to set the for_each expression + * @internal used by TerraformResource to set the for_each expression. + * Always returns the map-wrapped form so each.key is the mapKeyAttributeName + * and entries are uniquely keyed. */ public _getForEachExpression(): any { - // uses a Lazy value to be able to render a conversion into a map in the context of a TerraformResource - return Lazy.anyValue( - { - produce: (context) => { - switch (context.iteratorContext) { - case "FOR_EXPRESSION": - return this.list; - case "DYNAMIC_BLOCK": // fallthrough - default: // same as dynamic block, as this is the case when a iterator is passed to the root level of e.g. a resource - // Turn list into a map - // { for k,v in : => } - return forExpression( - this.list, // input - FOR_EXPRESSION_VALUE, // valueExpression - Fn.lookupNested(FOR_EXPRESSION_VALUE, [ - this.mapKeyAttributeName, - ]), // keyExpression - ); - } - }, - }, - { displayHint: "" }, + // Turn list into a map: { for k,v in : => } + return forExpression( + this.list, + FOR_EXPRESSION_VALUE, + Fn.lookupNested(FOR_EXPRESSION_VALUE, [this.mapKeyAttributeName]), ); } + + /** + * @internal inside a for-expression body we walk the raw list so + * `[ for key, val in : ... ]` iterates list elements once, + * rather than the doubly-wrapped form the for_each conversion produces. + */ + public _getForExpressionInput(): any { + return this.list; + } } diff --git a/packages/cdktn/test/iterator.test.ts b/packages/cdktn/test/iterator.test.ts index c563fbbae..01e547306 100644 --- a/packages/cdktn/test/iterator.test.ts +++ b/packages/cdktn/test/iterator.test.ts @@ -552,3 +552,93 @@ test("forExpressionForList with fromComplexList uses raw list, not map conversio '${[ for key, val in var.list: val.id if val.name == "foo"]}', ); }); + +test("keys() on fromComplexList yields raw list indices", () => { + const app = Testing.app(); + const stack = new TerraformStack(app, "test"); + const variable = new TerraformVariable(stack, "list", {}); + const it = TerraformIterator.fromComplexList( + variable.listValue as any, + "name", + ); + + new TestResource(stack, "test", { + name: Token.asString(it.keys()), + }); + + const synth = JSON.parse(Testing.synth(stack)); + expect(synth).toHaveProperty( + "resource.test_resource.test.name", + "${[ for key, val in var.list: key]}", + ); +}); + +test("values() on fromComplexList iterates raw list", () => { + const app = Testing.app(); + const stack = new TerraformStack(app, "test"); + const variable = new TerraformVariable(stack, "list", {}); + const it = TerraformIterator.fromComplexList( + variable.listValue as any, + "name", + ); + + new TestResource(stack, "test", { + name: Token.asString(it.values()), + }); + + const synth = JSON.parse(Testing.synth(stack)); + expect(synth).toHaveProperty( + "resource.test_resource.test.name", + "${[ for key, val in var.list: val]}", + ); +}); + +test("pluckProperty on fromComplexList iterates raw list", () => { + const app = Testing.app(); + const stack = new TerraformStack(app, "test"); + const variable = new TerraformVariable(stack, "list", {}); + const it = TerraformIterator.fromComplexList( + variable.listValue as any, + "name", + ); + + new TestResource(stack, "test", { + name: Token.asString(it.pluckProperty("id")), + }); + + const synth = JSON.parse(Testing.synth(stack)); + expect(synth).toHaveProperty( + "resource.test_resource.test.name", + "${[ for key, val in var.list: val.id]}", + ); +}); + +test("fromComplexList renders map-wrapped for_each and raw-list for-expressions on the same iterator", () => { + const app = Testing.app(); + const stack = new TerraformStack(app, "test"); + const variable = new TerraformVariable(stack, "list", {}); + const it = TerraformIterator.fromComplexList( + variable.listValue as any, + "name", + ); + + new TestResource(stack, "test", { + forEach: it, + name: it.getString("id"), + tags: { ids: Token.asString(it.pluckProperty("id")) }, + }); + + const synth = JSON.parse(Testing.synth(stack)); + expect(synth).toHaveProperty( + "resource.test_resource.test.for_each", + "${{ for key, val in var.list: val.name => val }}", + ); + expect(synth).toHaveProperty( + "resource.test_resource.test.name", + "${each.value.id}", + ); + expect(synth).toHaveProperty( + "resource.test_resource.test.tags.ids", + "${[ for key, val in var.list: val.id]}", + ); +});