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

[pipelines] Metadata errors should fail synthesis (on stacks deployed in the pipeline) #9226

Closed
J11522 opened this issue Jul 23, 2020 · 20 comments · Fixed by #11461
Closed
Assignees
Labels
@aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@J11522
Copy link
Contributor

J11522 commented Jul 23, 2020

This issue is similar to #5594 however, occurs without nested stacks.
Instead, I have a cross-account CdkPipeline setup according to the minimal example from the documentation.
My Stack consists only of a simple VPC.

When deploying, the CloudFormation template in the target account still contains the dummy1a, dummy1b etc. AZs.
Finally, these are reported as invalid and my stack is rolled back.

Reproduction Steps

pipeline.addApplicationStage(new MyApplication(this, 'Dev', {
      env: {
        account: '11111111111',
        region: 'eu-west-1',
      }
    }));

Where MyApplication consist of a single stack with only new VPC(this, 'VPCName')

Error Log

Value (dummy1a) for parameter availabilityZone is invalid. Subnets can currently only be created in the following availability zones: eu-west-1a, eu-west-1b, eu-west-1c. (Service: AmazonEC2; Status Code: 400; Error Code: InvalidParameterValue

Environment

  • CLI Version : 1.54.0
  • Framework Version: 1.54.0
  • Node.js Version: 14.4.0
  • OS :
  • Language (Version): TypeScript (3.8.3)

Other


This is 🐛 Bug Report

@J11522 J11522 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 23, 2020
@github-actions github-actions bot added the @aws-cdk/pipelines CDK Pipelines library label Jul 23, 2020
@ericzbeard ericzbeard changed the title [pipelines] availabilityZones with account and region specified not working in pipeline deployment availabilityZones with account and region specified not working in pipeline deployment Jul 23, 2020
@ericzbeard
Copy link
Contributor

Pipelines does not currently support lookups.

(From the blog)

Developer preview limitations
CDK Pipelines is currently in developer preview. AWS is working to lift some limitations, but you should keep the following in mind:

No context queries – Context queries are not supported. That means that Vpc.fromLookup() and other functions like it don’t work.

@ericzbeard ericzbeard removed the needs-triage This issue or PR still needs to be triaged. label Jul 23, 2020
@J11522
Copy link
Contributor Author

J11522 commented Jul 24, 2020

Thanks for the pointer.
In case someone else encounters this, have a look a the context section: here
An alternative could be to do a dynamic lookup in the build/synth phase and populate the context there.

Do you propose to close this issue as this is a know limitation?

@ericzbeard ericzbeard added feature-request A feature should be added or improved. and removed bug This issue is a bug. labels Jul 24, 2020
@ericzbeard ericzbeard changed the title availabilityZones with account and region specified not working in pipeline deployment Support context lookups in pipelines Jul 24, 2020
@ericzbeard
Copy link
Contributor

I changed the title and made it a feature request.

@njdancer
Copy link

@J11522 I might be missing something. Does that page outline a workaround or just additional info around context and how to get the available AZ's out of band. The only way I seem to be able to workaround this is by overriding the availabilityZones member inside my Stack subclass.

@J11522
Copy link
Contributor Author

J11522 commented Jul 30, 2020

@njdancer You are right, this is not a workaround but rather an offline solution.
An alternative would be to perform this lookup yourself via the AWS SDK, but then you could run into issues with the cross account setup.

@ericzbeard ericzbeard removed their assignment Aug 3, 2020
@rix0rrr rix0rrr added effort/large Large work item – several weeks of effort p1 labels Aug 4, 2020
@rix0rrr rix0rrr added this to the [CDK Pipelines] Soon milestone Aug 12, 2020
@moshir
Copy link

moshir commented Aug 21, 2020

What's the current/recommended way to create a VPC in a pipeline ?

@moshir
Copy link

moshir commented Aug 21, 2020

@J11522 I might be missing something. Does that page outline a workaround or just additional info around context and how to get the available AZ's out of band. The only way I seem to be able to workaround this is by overriding the availabilityZones member inside my Stack subclass.

This does not seem to work, even when i override the Stack #.availability_zones(self), it ends up with dummy values :

class TheStack(core.Stack):
    def availability_zones(self) -> typing.List[str]:
        return ['eu-west-1a', 'eu-west-1b']

    def __init__(self, scope, id, **kwargs):
        super().__init__(
            scope,
            id,
            **kwargs
        )
            vpc = ec2.Vpc(self, "VPC",
                           max_azs=2,
                           cidr="172.31.0.0/16",
                           subnet_configuration=[ec2.SubnetConfiguration(
                               subnet_type=ec2.SubnetType.PUBLIC,
                               name="Public",

                               cidr_mask=20
                           ), ec2.SubnetConfiguration(
                               subnet_type=ec2.SubnetType.PRIVATE,
                               name="Private",
                               cidr_mask=24
                           )
                           ],
                           nat_gateways=1,
                           )

The output stack will still have dummy1a in the subnet AvailabilityZone:

    "VPCPublicSubnet1SubnetB4246D30": {
      "Type": "AWS::EC2::Subnet",
      "Properties": {
        "CidrBlock": "172.31.0.0/20",
        "VpcId": {
          "Ref": "VPCB9E5F0B4"
        },
        "AvailabilityZone": "dummy1a",

@J11522
Copy link
Contributor Author

J11522 commented Aug 24, 2020

@moshir Try setting the AZs in your context.
You can do this through the context file very easily:

{
  "availability-zones:account=<your account id>:region=eu-west-1": [
    "eu-west-1a",
    "eu-west-1b",
    "eu-west-1c"
  ]
}

@arpowers
Copy link

There needs to be more clarity around how cdk.context.json works and what values are needed if we have to edit it directly.

How did you figure out what to add to that file? @J11522

@J11522
Copy link
Contributor Author

J11522 commented Oct 22, 2020

@arpowers To be honest, I can't recall how I figured this out. I guess looking at code and documentation helped in the end.

@jguice
Copy link

jguice commented Oct 28, 2020

If you run cdk deploy from your local, it should populate the cdk.context.json file with AZ info you can use as a template for additional regions, etc.

@victoravr
Copy link

victoravr commented Oct 30, 2020

Pipelines does not currently support lookups.

(From the blog)

Developer preview limitations
CDK Pipelines is currently in developer preview. AWS is working to lift some limitations, but you should keep the following in mind:

No context queries – Context queries are not supported. That means that Vpc.fromLookup() and other functions like it don’t work.

Are there any plans to support lookups like @aws-cdk/aws-ec2.Vpc.fromLookup() in @aws-cdk/pipelines anytime soon?

@SoManyHs
Copy link
Contributor

Thanks @J11522 for the tip, adding that totally worked for me! https://github.com/SoManyHs/cdkpipelines-demo/blob/main/cdk.json#L8-L11

@rix0rrr rix0rrr changed the title Support context lookups in pipelines Missing context keys in subassemblies not propagated to top level assembly Nov 11, 2020
@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort and removed effort/large Large work item – several weeks of effort labels Nov 11, 2020
rix0rrr added a commit that referenced this issue Nov 13, 2020
Missing context in Stages was reported at the inner-assembly
level. Since the CLI only inspects the top-level assembly for
missing context, it would never detect this and not query for it.

Propagate the missing context up to the top-level assembly.

Fixes #9226.
@mergify mergify bot closed this as completed in #11461 Nov 16, 2020
mergify bot pushed a commit that referenced this issue Nov 16, 2020
Missing context in Stages was reported at the inner-assembly
level. Since the CLI only inspects the top-level assembly for
missing context, it would never detect this and not query for it.

Propagate the missing context up to the top-level assembly.

Fixes #9226.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

michael-dickinson-sainsburys added a commit to michael-dickinson-sainsburys/continuous-audit that referenced this issue Nov 22, 2020
michael-dickinson-sainsburys added a commit to michael-dickinson-sainsburys/continuous-audit that referenced this issue Nov 22, 2020
michael-dickinson-sainsburys added a commit to michael-dickinson-sainsburys/continuous-audit that referenced this issue Nov 22, 2020
@rix0rrr rix0rrr reopened this Dec 4, 2020
@rix0rrr rix0rrr added bug This issue is a bug. and removed feature-request A feature should be added or improved. labels Dec 4, 2020
@rix0rrr rix0rrr changed the title Missing context keys in subassemblies not propagated to top level assembly New VPC in subassembly has incorrect AZ names Dec 4, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Dec 4, 2020

synth needs to be able check the manifests of selected stacks for errors. Normally deploy would do that, but since we're not using that, we don't see the errors right now.

@rix0rrr
Copy link
Contributor

rix0rrr commented Dec 4, 2020

What's happening is something has failed during synthesis, but the synthesis CodeBuild step is not failing and that's why you're seeing dummy AZs in your template

@rix0rrr
Copy link
Contributor

rix0rrr commented Dec 5, 2020

For example, if I run cdk synth -v I will see something like this in the log:

Setting "availability-zones:account=111111111:region=us-east-1" context to {"$providerError":"Need to perform AWS calls for account 111111111, but no credentials have been configured.","$dontSaveContext":true}

The mistake here is that this error is not failing the Pipelines synthesis, which it probably should.

The error itself is due to some kind of misconfiguration.

@rix0rrr rix0rrr changed the title New VPC in subassembly has incorrect AZ names [pipelines] Metadata errors should fail synthesis Dec 5, 2020
@rix0rrr rix0rrr changed the title [pipelines] Metadata errors should fail synthesis [pipelines] Metadata errors should fail synthesis (on stacks deployed in the pipeline) Dec 5, 2020
@lengebre
Copy link

lengebre commented Jan 24, 2021

@J11522 I might be missing something. Does that page outline a workaround or just additional info around context and how to get the available AZ's out of band. The only way I seem to be able to workaround this is by overriding the availabilityZones member inside my Stack subclass.

This does not seem to work, even when i override the Stack #.availability_zones(self), it ends up with dummy values :

class TheStack(core.Stack):
    def availability_zones(self) -> typing.List[str]:
        return ['eu-west-1a', 'eu-west-1b']

    def __init__(self, scope, id, **kwargs):
        super().__init__(
            scope,
            id,
            **kwargs
        )
            vpc = ec2.Vpc(self, "VPC",
                           max_azs=2,
                           cidr="172.31.0.0/16",
                           subnet_configuration=[ec2.SubnetConfiguration(
                               subnet_type=ec2.SubnetType.PUBLIC,
                               name="Public",

                               cidr_mask=20
                           ), ec2.SubnetConfiguration(
                               subnet_type=ec2.SubnetType.PRIVATE,
                               name="Private",
                               cidr_mask=24
                           )
                           ],
                           nat_gateways=1,
                           )

The output stack will still have dummy1a in the subnet AvailabilityZone:

    "VPCPublicSubnet1SubnetB4246D30": {
      "Type": "AWS::EC2::Subnet",
      "Properties": {
        "CidrBlock": "172.31.0.0/20",
        "VpcId": {
          "Ref": "VPCB9E5F0B4"
        },
        "AvailabilityZone": "dummy1a",

This will allow you to override the default availability zones.

@property
def availability_zones(self):
    return ['us-east-1d', 'us-east-1e']

def __init__(self, scope: core.Construct, construct_id: str, **kwargs) -> None:
    super().__init__(scope, construct_id, **kwargs)

    # The code that defines your stack goes here
    vpc = ec2.Vpc(
        self, "TestVpc",
        max_azs=2,
        cidr="10.0.0.0/16",
    )

@rix0rrr
Copy link
Contributor

rix0rrr commented Apr 15, 2021

Should probably be an extension to the cdk metadata command?

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

michael-dickinson-sainsburys added a commit to michael-dickinson-sainsburys/continuous-audit that referenced this issue Mar 30, 2022
michael-dickinson-sainsburys added a commit to michael-dickinson-sainsburys/continuous-audit that referenced this issue Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.