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

feat(lib): cross-stack references #1416

Merged
merged 24 commits into from
Jan 12, 2022
Merged

feat(lib): cross-stack references #1416

merged 24 commits into from
Jan 12, 2022

Conversation

DanielMSchmidt
Copy link
Contributor

@DanielMSchmidt DanielMSchmidt commented Dec 13, 2021

Closes #651

General

This PR adds support for cross stack references (initially for the local and Terraform Cloud backend being used in the source stack).
A Cross Stack Reference is a reference that is used in a different stack than it's created, e.g.:

export class SourceStack extends TerraformStack {
  public diceRoll: Integer;
  constructor(scope: Construct, id: string) {
    super(scope, id);

    new RandomProvider(this, "random");

    this.diceRoll =  new Integer(this, "int", {
      min: 0,
      max: 6,
    });
  }
}

export class ConsumerStack extends TerraformStack {
  constructor(
    scope: Construct,
    id: string,
    inputs: { randomValue: number }
  ) {
    super(scope, id);

    new LocalProvider(this, "local");

    new File(this, "dice-roll-result", {
        filename: "dice-roll-result.txt",
        content: String(inputs.randomValue)
    })
  }
}

const app = new App();
const src = new SourceStack(app, "source");
new ConsumerStack(app, "destination", {
  randomValue: src.diceRoll.result, // we use the reference from the source stack here
});
app.synth();

How to review

I'd recommend going commmit by commit, they should all make sense by themselves

@DanielMSchmidt DanielMSchmidt force-pushed the cross-stack-references branch 7 times, most recently from 64abbbc to fd2f6a6 Compare December 15, 2021 14:44
@jsteinich
Copy link
Collaborator

Does this supersede #1179? There were some comments there that may still apply here.

@DanielMSchmidt
Copy link
Contributor Author

Yes it will, I left the other PR open so that I don't forget to look through the comments

@DanielMSchmidt DanielMSchmidt force-pushed the cross-stack-references branch 7 times, most recently from f15983d to 3d560ea Compare January 5, 2022 11:38
website/docs/cdktf/concepts/stacks.mdx Show resolved Hide resolved
examples/typescript/aws-kubernetes/cdktf.json Outdated Show resolved Hide resolved
examples/typescript/aws-kubernetes/main.ts Show resolved Hide resolved
packages/cdktf-cli/bin/cmds/helper/stack-dependencies.ts Outdated Show resolved Hide resolved
packages/cdktf/lib/terraform-stack.ts Outdated Show resolved Hide resolved
packages/cdktf/lib/tfExpression.ts Show resolved Hide resolved
test/typescript/terraform-cloud/test.ts Outdated Show resolved Hide resolved
@DanielMSchmidt DanielMSchmidt force-pushed the cross-stack-references branch 4 times, most recently from f439f63 to 524cb51 Compare January 7, 2022 17:15
@ansgarm
Copy link
Member

ansgarm commented Jan 10, 2022

Just came across this when reading a former proposal for cross stack references I did a while back: aws/aws-cdk#12778
We should at least create a follow-up task to check whether this could happen to us as well.

@DanielMSchmidt DanielMSchmidt force-pushed the cross-stack-references branch 4 times, most recently from f59e3f4 to 1c3f787 Compare January 10, 2022 15:10
@DanielMSchmidt DanielMSchmidt added this to the 0.9 milestone Jan 10, 2022
@DanielMSchmidt
Copy link
Contributor Author

@skorfmann does this test case cover your concern around complex computed lists or did I do sth wrong here? 103a5be

@skorfmann
Copy link
Contributor

What happens if I have deployed a producing and consuming stack. After the deployment, I remove the reference and hence the automatically created output. I have a hunch that I might run into problems planning / deploying the consuming stack once the output of the producing stack is gone.

That's a great point, @ansgarm brought that up as well, this is sth the AWS CDK struggles with. I created a test to check and it does not cause problems for TF: https://github.com/hashicorp/terraform-cdk/pull/1416/files#diff-99c6047d4d933587477aa2e3622ea8bc6fa07034cc9b32861c3f3c07d74ba734R81 and here is the main.ts for that test: https://github.com/hashicorp/terraform-cdk/pull/1416/files#diff-5162569d3dc22ac1cf854a3aaaf4b4cc41eb9a7ece4f578006ce94b6fcb321ceR140

The links appear to be broken. But anyway, if it works that's ok.

@skorfmann
Copy link
Contributor

The output of the producing stack should contain the fully resolved value of Terraform. That a function is involved is not a concern of the consuming stack.

Is that more of a general guideline or would you say that's important in the first iteration? We could do a follow-up issue for this, would that work for you?

I think it would be good to have this properly working from the beginning, seems very unexpected to me.

@skorfmann
Copy link
Contributor

@skorfmann does this test case cover your concern around complex computed lists or did I do sth wrong here? 103a5be

It's not really straightforward to understand what's actually being tested there. Here's my case: I was passing the entire domain validation options object (ComplexComputedList), not a single element. Not even sure if that's a thing which should be working or which should be caught with an error.

@skorfmann
Copy link
Contributor

I wonder if this is a real use-case though, wouldn't folks use the attributes of a resource? I think this is never a good idea and would argue for disallowing passing these into outputs and terraform functions in general

Passing complete resources into an output is definitely valid. Terraform accepts that and there was a ticket fairly recently where a user requested the functionality and we made it work. It's not possible to pass to any other resource though and also seems a bit far fetched to pass into another stack's output.

So, this should raise an error when referencing across stacks then, correct?

Copy link
Contributor

@skorfmann skorfmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't gone through the code since others did this already. As laid out in the comments I'd add a few more explicit errors for things which are not working (yet). Plus, the functions magically appearing in a referencing stack should be fixed. Other than that, I think it's good and works as expected 👍

@DanielMSchmidt DanielMSchmidt merged commit 58b5239 into main Jan 12, 2022
@DanielMSchmidt DanielMSchmidt deleted the cross-stack-references branch January 12, 2022 15:27
@DanielMSchmidt
Copy link
Contributor Author

I'm going to create follow up issues for the outstanding work and mention them here 👍

@DanielMSchmidt
Copy link
Contributor Author

@dhmistry3
Copy link

Is there an example of how to do this in python?

@DanielMSchmidt
Copy link
Contributor Author

DanielMSchmidt commented Feb 2, 2022

We have a test case in Python, hope that helps :) https://github.com/hashicorp/terraform-cdk/blob/main/test/python/cross-stack-references/main.py

The usage is pretty similar to Typescript, passing a value created in one stack into another one does the trick

@dhmistry3
Copy link

@DanielMSchmidt Thanks that is helpful. However, I am trying to pass an aws ARN from one stack to another. When i set it in one stack and reference it in another, the variable gets set as

${module.s3_bucket_id.s3_bucket_arn}

and then i get an error of

│ No module call named "s3_bucket_id" is declared in the root module.

@DanielMSchmidt
Copy link
Contributor Author

@dhmistry3 Could you create an issue for this with a reproducible example so that I can take a look?

@dhmistry3
Copy link

@DanielMSchmidt So it worked in the simplified version i made (https://github.com/dhmistry3/cdktf-cross-stack) so there must be something wrong i am doing in my main code. Thanks, i'll ask if i have any more questions.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 1, 2022

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cross Stack References
5 participants