aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.72k stars 3.94k forks source link

(opensearch): Trying to define an Opensearch domain in one stack and reference it in another causes cyclic reference errors #20256

Open SamStephens opened 2 years ago

SamStephens commented 2 years ago

Describe the bug

I want to use the classic pattern where you have an infrastructure stack and a code stack.

I define an Opensearch domain in my infrastructure stack in a VPC. I then define a code stack, and attempt to wire up a Lambda that uses that domain.

This works when all the resources are in one stack; or if the code stack is nested in the infrastructure stack. But if they're peer stacks I get a cyclic reference stack.

For the reproduction provided below, the error is:

'InfrastructureStack' depends on 'CodeStack' (InfrastructureStack -> CodeStack/LambdaFunction/SecurityGroup/Resource.GroupId). Adding this dependency (CodeStack -> InfrastructureStack/OpensearchDomain/Resource.Arn) would create a cyclic reference.

Expected Behavior

For all the resources related to my Lambda to be defined in the Code stack, so there's no cyclic references. That's what I'd do if I was writing the Cloudformation directly.

Current Behavior

The lambda function itself is defined in the code stack Cloudformation. However, the AWS::EC2::SecurityGroupIngress allowing the lambda function ingress to the Opensearch domain is defined in the infrastructure stack Cloudformation.

Reproduction Steps

The simplest reproduction I could come up with is to generate a new python application using cdk init app --language python, and then update app.py to be:

#!/usr/bin/env python3

import aws_cdk as cdk
from aws_cdk import aws_opensearchservice, aws_lambda, aws_ec2
from constructs import Construct

class InfrastructureStack(cdk.Stack):
    def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        self._vpc = aws_ec2.Vpc(
            scope=self,
            id="Vpc",
            cidr="10.64.0.0/16",
        )

        self._domain = aws_opensearchservice.Domain(
            scope=self,
            id="OpensearchDomain",
            version=aws_opensearchservice.EngineVersion.OPENSEARCH_1_2,
            vpc=self._vpc,
        )

    @property
    def domain(self) -> aws_opensearchservice.Domain:
        return self._domain

    @property
    def vpc(self) -> aws_ec2.IVpc:
        return self._vpc

class CodeStack(cdk.Stack):
    def __init__(self,
                 scope: Construct,
                 construct_id: str,
                 domain: aws_opensearchservice.Domain,
                 vpc: aws_ec2.IVpc,
                 **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        self._lambda_function = aws_lambda.Function(
            scope=self,
            id="LambdaFunction",
            handler="index.handler",
            runtime=aws_lambda.Runtime.PYTHON_3_9,
            code=aws_lambda.Code.from_inline("handler code goes here"),
            environment={
                "OPENSEARCH_ENDPOINT": domain.domain_endpoint,
            },
            vpc=vpc,
        )

        domain.connections.allow_from(self._lambda_function, aws_ec2.Port.tcp(443))
        domain.grant_read_write(self._lambda_function)

app = cdk.App()

infrastructure_stack = InfrastructureStack(app, "InfrastructureStack")
CodeStack(app, "CodeStack", infrastructure_stack.domain, infrastructure_stack.vpc)

app.synth()

Possible Solution

For all the resources related to my Lambda to be defined in the Code stack, so there's no cyclic references. That's what I'd do if I was writing the Cloudformation directly.

Additional Information/Context

No response

CDK CLI Version

2.23.0 (build 50444aa)

Framework Version

2.24.0

Node.js Version

14.17.0

OS

Ubuntu (Windows Subsystem for Linux)

Language

Python

Language Version

3.9.7

Other information

No response

kaizencc commented 2 years ago

Hi @SamStephens! Can you provide the cyclic dependency error that you're getting as well? Interested in understanding exactly what resources CDK thinks depend on each other. Usually with these dependencies there's some CDK magic under the hood that you never need to know about until you hit these dependency errors :), but I can dive into the source code for you and see whats up.

SamStephens commented 2 years ago

@kaizencc Sure thing; I'll put together a proper repro in a few days time when I'm back at work.

SamStephens commented 2 years ago

@kaizencc also I was interested in asking about broader design goals here. Is the infrastructure stack/code stack split pattern something that the CDK is explicitly interested in supporting?

Basically, my situation is that I'm currently exporting ARNs from my infrastructure stack and importing the resources using them into my code stack; this is the approach I'm seeing others to work around similar issues. Being able to directly share the resources between the two stacks would be cleaner; but I don't want to do it and then find myself with an issue that is considered by design and will not be resolved. I want to know that the CDK considers this is a usage pattern it supports before I move to it.

kaizencc commented 2 years ago

@SamStephens regarding your second question, I think you'd be better off opening a new guidance issue about it so that the question gets into the hands of the right people. My answer is that I'm not entirely sure. How are you planning on sharing resources between two stacks? I think if your plan is to use nested stacks to achieve this then that is definitely a CDK pattern we plan to support.

SamStephens commented 2 years ago

Thanks @kaizencc. I've updated the bug report to include a proper reproduction.

I'll open a separate guidance issue about the pattern, as you suggest. However, I am interested in using peer stacks as per the reproduction, not nested stacks. When I worked with Nested Stacks back when I was an AWS employee I found them quite clunky to work with, although this was a few years ago. I'd rather operate using peer stacks with exported using CfnOutput and imported using Fn.import_value, which is my current workaround for this issue.

SamStephens commented 2 years ago

@kaizencc I've got a proper repro here now, can you please remove the needs-reproduction tag?