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.

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

fix(lib): Fix handling of replaceTriggeredBy lifecycle attribute #3322

Merged
merged 2 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/cdktf/lib/terraform-data-source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
TerraformResourceConfig,
TerraformResourceLifecycle,
ITerraformResource,
lifecycleToTerraform,
} from "./terraform-resource";
import { keysToSnakeCase, deepMerge, processDynamicAttributes } from "./util";
import { ITerraformDependable } from "./terraform-dependable";
Expand Down Expand Up @@ -115,7 +116,7 @@ export class TerraformDataSource
? this.count.toTerraform()
: this.count,
provider: this.provider?.fqn,
lifecycle: this.lifecycle,
lifecycle: lifecycleToTerraform(this.lifecycle),
forEach: this.forEach?._getForEachExpression(),
};
}
Expand Down
31 changes: 30 additions & 1 deletion packages/cdktf/lib/terraform-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,34 @@ export interface TerraformResourceLifecycle {
readonly postcondition?: Postcondition[];
}

/**
* prepares a lifecycle object for being rendered as JSON
* currently this function:
* - converts all replaceTriggeredBy items that are ITerraformDependables to strings
*/
export function lifecycleToTerraform(
lifecycle?: TerraformResourceLifecycle
): TerraformResourceLifecycle | undefined {
if (!lifecycle) {
return undefined;
}

return {
...lifecycle,
...(lifecycle?.replaceTriggeredBy?.length
? {
replaceTriggeredBy: lifecycle?.replaceTriggeredBy?.map((x) => {
if (typeof x === "string") {
return x;
} else {
return x.fqn;
}
}),
}
: undefined),
};
}

export interface TerraformMetaArguments {
readonly dependsOn?: ITerraformDependable[];
readonly count?: number | TerraformCount;
Expand Down Expand Up @@ -190,13 +218,14 @@ export class TerraformResource
!this.forEach || typeof this.count === "undefined",
`forEach and count are both set, but they are mutually exclusive. You can only use either of them. Check the resource at path: ${this.node.path}`
);

return {
dependsOn: this.dependsOn,
count: TerraformCount.isTerraformCount(this.count)
? this.count.toTerraform()
: this.count,
provider: this.provider?.fqn,
lifecycle: this.lifecycle,
lifecycle: lifecycleToTerraform(this.lifecycle),
forEach: this.forEach?._getForEachExpression(),
connection: this.connection,
};
Expand Down
34 changes: 34 additions & 0 deletions packages/cdktf/test/__snapshots__/resource.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,40 @@ exports[`supports local-exec provisioner 1`] = `
}"
`;

exports[`supports resource and attribute references in lifecycle.replaceTriggeredBy 1`] = `
"{
"provider": {
"test": [
{
}
]
},
"resource": {
"test_resource": {
"other": {
"name": "other"
},
"simple": {
"lifecycle": {
"replace_triggered_by": [
"\${test_resource.other}",
"\${test_resource.other.string_value}"
]
},
"name": "foo"
}
}
},
"terraform": {
"required_providers": {
"test": {
"version": "~> 2.0"
}
}
}
}"
`;

exports[`with complex computed list 1`] = `
"{
"provider": {
Expand Down
17 changes: 17 additions & 0 deletions packages/cdktf/test/resource.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,23 @@ it("maintains the same order of provisioner", () => {
expect(Testing.synth(stack)).toMatchSnapshot();
});

it("supports resource and attribute references in lifecycle.replaceTriggeredBy", () => {
const app = Testing.app();
const stack = new TerraformStack(app, "test");
new TestProvider(stack, "provider", {});

const other = new TestResource(stack, "other", { name: "other" });

new TestResource(stack, "simple", {
name: "foo",
lifecycle: {
replaceTriggeredBy: [other, other.stringValue],
},
});

expect(Testing.synth(stack)).toMatchSnapshot();
});

test("includes import block when import is present", () => {
const app = Testing.app();
const stack = new TerraformStack(app, "test");
Expand Down