search

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.68k stars 3.93k forks source link

(aws-docdb): should not try to validate list of subnets if they are unresolved #14262

Open tomSpark opened 3 years ago

tomSpark commented 3 years ago

When importing a VPC information from CloudFormation Export (using fn.import) the cluster constructor does not work.

The VPC.fromVPCAttribute reference the subnets using one "ImportedSubnet". When the DocumentCluster is constructed there is a check for the number of subnets (>=2). However in this case, the array of subnets contains only one object.

tried to comment the test in cluster.js and the CF template is properly created

Reproduction Steps

Assuming there is a stack exporting the following

Networking-VPC-ID => "vpc-XXXX" Networking-PrivateSubnets-IDs => "subnet-XXX,subnet-YYY,subnet-ZZZZ"

` vpc=Vpc.fromVpcAttributes(this.scope, "ImportedVPC", { vpcId: Fn.importValue("Networking-VPC-ID"), availabilityZones: Fn.getAzs(Aws.REGION), privateSubnetIds: Fn.split(",", Fn.importValue("Networking-PrivateSubnets-IDs")), });

cluster = new DatabaseCluster(this, DatabaseCluster, { instanceType: InstanceType.of(InstanceClass.T3, InstanceSize.MEDIUM), engineVersion: "4.0.0", masterUser: { username: 'masterUser' // NOTE: 'admin' is reserved by DocumentDB }, vpc: vpc, vpcSubnets: { subnetType: SubnetType.PRIVATE } }); `

Generate the following error: Error: Cluster requires at least 2 subnets, got 1 .....

What did you expect to happen?

I was expecting to create a stack with a SubnetGroup as follow "Type": "AWS::DocDB::DBSubnetGroup", "Properties": { "DBSubnetGroupDescription": "Subnet group for DocumentDBConstruct-Database", "SubnetIds": { "Fn::Split": [ ",", { "Fn::ImportValue": "Networking-PrivateSubnets-IDs" } ] } },

What actually happened?

Does not generate the stack. The test in cluster.js if (subnetIds.length < 2) { throw new Error(Cluster requires at least 2 subnets, got ${subnetIds.length}); }

stop the synthetization of the stack

Environment

Other

Issue is not environement specific. I believe the test in cluster.js to prevent creating a cluster on a VPC with only one Subnet kinda make sense. However it does not take into account the case when the vpc is imported from a CloudFormation Import (Standard practice in CF)

This is :bug: Bug Report

rix0rrr commented 3 years ago

The problem is that the construct cannot tell how many elements there are in the array because it's imported.

importListValue will help you out.

tomSpark commented 3 years ago

@rix0rrr Agree. The construct should check if it's unresolved or not. In the affirmative, assume it's ok otherwise do check.

Quite painful now as need to create all using CfNs :-(

iliapolo commented 3 years ago

We are unassigning and marking this issue as p2, which means that we are unable to work on this immediately.
We use +1s to help prioritize our work, and are happy to revaluate this issue based on community feedback. You can reach out to the cdk.dev community on Slack to solicit support for reprioritization.

mmassenzio commented 2 years ago

Not sure whether this is a related issue, but when using a VPC "looked up" (via tags) the builder for DocDB fails; while the very same code for RDS (Postgres) succeeds - in the sense that CDK runs it twice: the first time it looks up the VPC (and creates the cdk.context.json file) and the second time round it uses it.

The funny thing is, if I comment out the DocDB code and make the CDK run once with synth it resolves the VPC (and creates the context) if I then uncomment the DocDB code, and make it run again, it success (because it finds it in the context).

I cannot share the entire repo, as it's private, but here is the condensed version:

1) the VPC code lookup (this works, in several places, no problem):

  public IVpc findVpc(Construct scope, String vpcName) {
    return Vpc.fromLookup(scope, "somethingelse", VpcLookupOptions.builder()
        .tags(Map.of(
            VPC_NAME, vpcName,
            ENVIRONMENT, p.getDeploymentEnv()
        ))
        .build());
  }

(incidentally we use the VPC_NAME tag instead of the vpcName() method, because the actual name gets mangled with the stack name, and instead we want to preserve the actual name we configure -- it doesn't really matter, this code works just fine)

The VPC is deployed across 2 AZs and has only ISOLATED subnets (yes, I know they are deprecated, but no one bothered to adjust the Java version of CDK, so that's all there is right now)

Unfortunately, this fails:

    SubnetSelection subnetSelection = SubnetSelection.builder()
        .subnetType(SubnetType.ISOLATED)
        .build();

    software.amazon.awscdk.services.docdb.DatabaseCluster dbCluster =
        DatabaseCluster.Builder.create(scope, "ApiDocDb")
            .dbClusterName(mongo.getDbName())
            .masterUser(Login.builder()
                .username(mongo.getAdminUser())
                .kmsKey(passwordKey)
                .password(SecretValue.secretsManager(mongo.getSsmSecretName()))
                .build())
            .vpc(vpc)
            .vpcSubnets(subnetSelection)
            .securityGroup(dbSecurityGroup)
            .instanceType(InstanceType.of(InstanceClass.MEMORY5, InstanceSize.LARGE))
            .instances(mongo.getReplicas())
            .storageEncrypted(true)
            .build();

with this error:

21-12-17 Fri 21:01:51 [WARN ] Exception encountered during context initialization - cancelling refresh attempt: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'main': Invocation of init method failed; nested exception is software.amazon.jsii.JsiiException: Cluster requires at least 2 subnets, got 0
Error: Cluster requires at least 2 subnets, got 0
    at new DatabaseCluster (/tmp/jsii-kernel-3Hii0j/node_modules/@aws-cdk/aws-docdb/lib/cluster.js:61:19)
    at /tmp/jsii-java-runtime17765149654098498693/lib/program.js:8432:58
    at Kernel._wrapSandboxCode (/tmp/jsii-java-runtime17765149654098498693/lib/program.js:8860:24)
    at Kernel._create (/tmp/jsii-java-runtime17765149654098498693/lib/program.js:8432:34)
    at Kernel.create (/tmp/jsii-java-runtime17765149654098498693/lib/program.js:8173:29)
    at KernelHost.processRequest (/tmp/jsii-java-runtime17765149654098498693/lib/program.js:9757:36)
    at KernelHost.run (/tmp/jsii-java-runtime17765149654098498693/lib/program.js:9720:22)
    at Immediate._onImmediate (/tmp/jsii-java-runtime17765149654098498693/lib/program.js:9721:46)
    at processImmediate (internal/timers.js:464:21)

When we run the exact same code (in the same class, pretty much all the same) but for RDS:

    software.amazon.awscdk.services.rds.DatabaseInstance dbInstance =
        software.amazon.awscdk.services.rds.DatabaseInstance.Builder.create(scope,
                "PostgresDB")
            .engine(engine)
            .instanceIdentifier(postgres.getDbName())
            .vpc(vpc)
            .vpcSubnets(SubnetSelection.builder()
                .subnetType(SubnetType.ISOLATED)
                .build())
            .securityGroups(List.of(dbSecurityGroup))
            .instanceType(InstanceType.of(
                InstanceClass.BURSTABLE3,
                InstanceSize.SMALL))
            .credentials(Credentials.fromSecret(masterPassword))
            .backupRetention(Duration.days(RETENTION_DAYS))
            .storageEncrypted(true)
            .storageEncryptionKey(pgKey)
            .deleteAutomatedBackups(true)
            .performanceInsightRetention(PerformanceInsightRetention.LONG_TERM)
            .publiclyAccessible(false)
            .build();

this works just fine.

Not really sure whether this is a bug, or "works as intended" and I'm missing how to work around it. Looking pretty much everywhere on the intertubes provided no clue - any suggestion appreciated.

We're using CDK and Solutions versions 1.129.0 (this is the most recent for Solutions; and yes, I know 2.0 is out, but right now we need this to work)

Thanks!

mmassenzio commented 2 years ago

UPDATE

So this is definitely a bug, somewhere. If I try to deploy this very same code to a VPC with 2 PRIVATE subnets, it still fails, but commenting out the selection:

//            .vpcSubnets(subnetSelection)

then this works. It seems to me that somehow the selection of ISOLATED subnets is ignored (when it shouldn't - ie, before the VPC lookup) but is then honored when it's too late (at actual creation, after the lookup)

Or something like that - it's really hard to figure out what's going on where.

Bottom line, AFAICT, one cannot deploy a DocDB using CDK in a VPC with only ISOLATED subnets (well, it is possible, by doing some very hacky workaround to get the ckd.context.json created first).

If this is a known/desired limitation, it should be documented somewhere, though. Thanks.

falkvonohlen commented 1 year ago

The issue also occurs, in the following setup:

const vpc = Vpc.fromLookup(this, "LookedUpVpc", { vpcName: "my-vpc"})

const cluster = new DatabaseCluster(this, "MetaDatabase", {
  vpc: vpc,
  vpcSubnets: {
      subnetGroupName: "app"
  },
  ...
})

As there is an incompleteSubnetDefinition in the LookedUpVpc. In the SubnetSelection this property is used to make the check for the number of subnets only if this property is false. See: https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-ec2/lib/vpc.ts#L601C39-L601C65

I expect changing the conditional check here accordingly would solve the issue.