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

cdk.context.json: vpc lookup which depends on an ssm lookup fails #3654

Closed
1 task done
ccfife opened this issue Aug 14, 2019 · 8 comments · Fixed by #3772
Closed
1 task done

cdk.context.json: vpc lookup which depends on an ssm lookup fails #3654

ccfife opened this issue Aug 14, 2019 · 8 comments · Fixed by #3772
Assignees
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. package/tools Related to AWS CDK Tools or CLI

Comments

@ccfife
Copy link
Contributor

ccfife commented Aug 14, 2019

  • I'm submitting a ...

    • 🪲 bug report
  • What is the current behavior?
    If the current behavior is a 🪲bug🪲: Please provide the steps to reproduce
    In order to use the pre-existing VPC, as I understand it, I need to use aws_ec2.Vpc.from_lookup to find the VPC by an attribute (eg. its ID). For that to work, I need the VPC ID at synth time.

To do this, within my stack constructor method, I have:

self.vpc_id_lookup = aws_ssm.StringParameter.value_from_lookup(self, '/path/to/vpc_id')
 
self.vpc = aws_ec2.Vpc.from_lookup(
            self,
            'FoundVpc',
            vpc_id=self.vpc_id_lookup
        )
 
self.rds_cluster = aws_rds.DatabaseCluster(
            self,
            'AuroraCluster',
            cluster_identifier='test-aurora-cluster',
            engine=aws_rds.DatabaseClusterEngine.AURORA_POSTGRESQL,
            master_user=aws_rds.Login(username='master'),
            instance_props=aws_rds.InstanceProps(
                vpc=self.vpc,
                instance_type=aws_ec2.InstanceType(instance_type_identifier='m5.large'),
                vpc_subnets=aws_ec2.SubnetSelection(subnet_type=ec2.SubnetType.ISOLATED),
            )
        )

The problem is that when cdk synth is attempted on the above, it fails at the second action (vpc lookup) fails because the vpc ID doesn't already exist in cdk.context.json… the only way to get it written there is to comment out the second and third actions, run cdk synth, which adds the SSM parameter store value to the context. After that, it will succeed on step two, but fail on step 3, because the VPC information is also missing from the context. If I comment out the 3rd action, run cdk synth, then put the 3rd action back, it all works.

  • What is the expected behavior (or behavior of feature suggested)?

Something seems wrong here… it seems like the two lookup methods are not behaving idempotently. They should populate context if context is missing, but also always return the looked up value(s), so that this will work whether or not context is already populated. I do have the account and region specified in the stack 'env'.

@ccfife ccfife added needs-triage This issue or PR still needs to be triaged. package/tools Related to AWS CDK Tools or CLI service/vpc labels Aug 14, 2019
@bobobox
Copy link

bobobox commented Aug 14, 2019

Some environment info:

CDK CLI Version: 1.3.0 (build bba9914)
Module Version: 1.3.0 (aws-cdk.aws-ssm, aws-cdk.core)
OS: OS X 10.14.5
Language: Python

@eladb eladb changed the title cdk.context.json not populated with VPC lookup properly cdk.context.json: vpc lookup which depends on an ssm lookup fails Aug 14, 2019
@eladb
Copy link
Contributor

eladb commented Aug 14, 2019

This is a bug. The CLI has logic to handle dependencies between context provider lookups (it basically repeats synth until all context is resolved). For some reason, this mechanism fails in this instance.

@eladb eladb added the bug This issue is a bug. label Aug 14, 2019
@ghost
Copy link

ghost commented Aug 20, 2019

@eladb I am having the same issue but I have the vpc id hardcoded and not doing any ssm lookups.

The issue only exists when there are other constructs that try to use the vpc from lookup and there is no cdk.context.json file. To make it work we have to comment out everything apart from the vpc lookup, run cdk synth which then generates the cdk.context.json file. Then uncomment everything else, it's only then the other constructs can use the vpc from lookup.

using cdk 1.4.0

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 23, 2019

The issue is that we return a dummy value for the SSM lookup (instead of aborting execution), which encourages the VPC lookup provider to use the value and error out saying it couldn't find any.

Could not find any VPCs matching {"account":"xxxxxx","region":"us-east-1","filter":{"vpc-id":"dummy-value-for-/path/to/vpc_id"}}

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 23, 2019

The correct solution is for the toolkit not to error out, but propagate the error back to the CDK app which can then attach the error to the construct tree.

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 23, 2019

We need that behavior anyway to properly handle context lookups for multiple accounts in the same app.

rix0rrr added a commit that referenced this issue Aug 23, 2019
Instead of the toolkit failing and stopping when a context provider
lookup fails, the error is reported to the CDK app, which can then
decide what to do with it.

In practice, it will attach a (localized) error to the Construct tree.
This has the following advantages:

* Dependent context resolution can proceed in a stepwise fashion,
  without the toolkit stopping at the first failure. For example,
  a VPC lookup from a value found in SSM will work.
* Stacks in multiple accounts that need context lookup can be used
  in a single app with cold context. Without this change they couldn't,
  because you could only have credentials for a single account at a
  time available so lookup for one of the accounts was bound to fail.

Fixes #3654.
@rix0rrr rix0rrr added @aws-cdk/core Related to core CDK functionality and removed needs-triage This issue or PR still needs to be triaged. labels Aug 28, 2019
@tsykora-verimatrix
Copy link

The issue is that we return a dummy value for the SSM lookup (instead of aborting execution), which encourages the VPC lookup provider to use the value and error out saying it couldn't find any.

Could not find any VPCs matching {"account":"xxxxxx","region":"us-east-1","filter":{"vpc-id":"dummy-value-for-/path/to/vpc_id"}}

Does this mean that we shouldn't be using SSM to lookup values of parameters required for synthesis of other stacks?
What I was hoping to do is to store tenant information in a parameter store and then retrieve it using CDK to synthesise only those stacks that that tenant needs.

rix0rrr added a commit that referenced this issue Sep 20, 2019
Instead of the toolkit failing and stopping when a context provider
lookup fails, the error is reported to the CDK app, which can then
decide what to do with it.

In practice, it will attach a (localized) error to the Construct tree.
This has the following advantages:

* Dependent context resolution can proceed in a stepwise fashion,
  without the toolkit stopping at the first failure. For example,
  a VPC lookup from a value found in SSM will work.
* Stacks in multiple accounts that need context lookup can be used
  in a single app with cold context. Without this change they couldn't,
  because you could only have credentials for a single account at a
  time available so lookup for one of the accounts was bound to fail.

Fixes #3654.
eladb pushed a commit that referenced this issue Sep 23, 2019
Instead of the toolkit failing and stopping when a context provider
lookup fails, the error is reported to the CDK app, which can then
decide what to do with it.

In practice, it will attach a (localized) error to the Construct tree.
This has the following advantages:

* Dependent context resolution can proceed in a stepwise fashion,
  without the toolkit stopping at the first failure. For example,
  a VPC lookup from a value found in SSM will work.
* Stacks in multiple accounts that need context lookup can be used
  in a single app with cold context. Without this change they couldn't,
  because you could only have credentials for a single account at a
  time available so lookup for one of the accounts was bound to fail.

Fixes #3654.
@eciuca
Copy link

eciuca commented Dec 12, 2023

@ccfife Does the CDK pipeline fail at this step? My Java CDK pipeline, when it fails because to assume the lookup role still continues and returns a success result. I was curious if it is just me or maybe I should report it as a bug.

This is the output:

[19:33:00] Some context information is missing. Fetching...
[19:33:00] Retrieved account ID {devops-account} from disk cache
[19:33:00] Assuming role 'arn:aws:iam::{target-account1}:role/cdk-hnb659fds-lookup-role-{target-account1}-eu-west-1'.
[19:33:00] Assuming role failed: User: arn:aws:sts::{account2}:assumed-role/csaee-pipeline-PipelineUpdatePipelineSelfMutationRo-7SwuJzpM46I3/AWSCodeBuild-2e6f3310-3644-451e-bbda-218a5baa8e93 is not authorized to perform: sts:AssumeRole on resource: arn:aws:iam::{target-account1}:role/cdk-hnb659fds-lookup-role-{target-account1}-eu-west-1
[19:33:00] Setting "vpc-provider:account={target-account1}:filter.owner-id={target-account1}:filter.tag:Name=c1m-cbdev-jobs-VPC:region=eu-west-1:returnAsymmetricSubnets=true" context to {"$providerError":"Could not assume role in target account using current credentials (which are for account {account2}) User: arn:aws:sts::{account2}:assumed-role/csaee-pipeline-PipelineUpdatePipelineSelfMutationRo-7SwuJzpM46I3/AWSCodeBuild-2e6f3310-3644-451e-bbda-218a5baa8e93 is not authorized to perform: sts:AssumeRole on resource: arn:aws:iam::{target-account1}:role/cdk-hnb659fds-lookup-role-{target-account1}-eu-west-1 . Please make sure that this role exists in the account. If it doesn't exist, (re)-bootstrap the environment with the right '--trust', using the latest version of the CDK CLI.","$dontSaveContext":true}
[19:33:00] Retrieved account ID {account2} from disk cache
[19:33:00] Assuming role 'arn:aws:iam::{target-account2}:role/cdk-hnb659fds-lookup-role-{target-account2}-eu-west-1'.
[19:33:00] Assuming role failed: User: arn:aws:sts::{account2}:assumed-role/csaee-pipeline-PipelineUpdatePipelineSelfMutationRo-7SwuJzpM46I3/AWSCodeBuild-2e6f3310-3644-451e-bbda-218a5baa8e93 is not authorized to perform: sts:AssumeRole on resource: arn:aws:iam::{target-account2}:role/cdk-hnb659fds-lookup-role-{target-account2}-eu-west-1
[19:33:00] Setting "vpc-provider:account={target-account2}:filter.owner-id={target-account2}:filter.tag:Name=c1m-cbtst-jobs-VPC:region=eu-west-1:returnAsymmetricSubnets=true" context to {"$providerError":"Could not assume role in target account using current credentials (which are for account {account2}) User: arn:aws:sts::{account2}:assumed-role/csaee-pipeline-PipelineUpdatePipelineSelfMutationRo-7SwuJzpM46I3/AWSCodeBuild-2e6f3310-3644-451e-bbda-218a5baa8e93 is not authorized to perform: sts:AssumeRole on resource: arn:aws:iam::{target-account2}:role/cdk-hnb659fds-lookup-role-{target-account2}-eu-west-1 . Please make sure that this role exists in the account. If it doesn't exist, (re)-bootstrap the environment with the right '--trust', using the latest version of the CDK CLI.","$dontSaveContext":true}
[19:33:00] Setting "CDK_DEFAULT_REGION" environment variable to eu-west-1
[19:33:00] Setting "CDK_DEFAULT_ACCOUNT" environment variable to {account2}
[19:33:00] context: {
  'vpc-provider:account={target-account1}:filter.owner-id={target-account1}:filter.tag:Name=c1m-cbdev-jobs-VPC:region=eu-west-1:returnAsymmetricSubnets=true': {
    '$providerError': "Could not assume role in target account using current credentials (which are for account {account2}) User: arn:aws:sts::{account2}:assumed-role/csaee-pipeline-PipelineUpdatePipelineSelfMutationRo-7SwuJzpM46I3/AWSCodeBuild-2e6f3310-3644-451e-bbda-218a5baa8e93 is not authorized to perform: sts:AssumeRole on resource: arn:aws:iam::{target-account1}:role/cdk-hnb659fds-lookup-role-{target-account1}-eu-west-1 . Please make sure that this role exists in the account. If it doesn't exist, (re)-bootstrap the environment with the right '--trust', using the latest version of the CDK CLI.",
    '$dontSaveContext': true
  },
  'vpc-provider:account={target-account2}:filter.owner-id={target-account2}:filter.tag:Name=c1m-cbtst-jobs-VPC:region=eu-west-1:returnAsymmetricSubnets=true': {
    '$providerError': "Could not assume role in target account using current credentials (which are for account {account2}) User: arn:aws:sts::{account2}:assumed-role/csaee-pipeline-PipelineUpdatePipelineSelfMutationRo-7SwuJzpM46I3/AWSCodeBuild-2e6f3310-3644-451e-bbda-218a5baa8e93 is not authorized to perform: sts:AssumeRole on resource: arn:aws:iam::{target-account2}:role/cdk-hnb659fds-lookup-role-{target-account2}-eu-west-1 . Please make sure that this role exists in the account. If it doesn't exist, (re)-bootstrap the environment with the right '--trust', using the latest version of the CDK CLI.",
    '$dontSaveContext': true
  },
  'aws:cdk:enable-path-metadata': true,
  'aws:cdk:enable-asset-metadata': true,
  'aws:cdk:version-reporting': true,
  'aws:cdk:bundling-stacks': [ '**' ]
}
[19:33:00] --app points to a cloud assembly, so we bypass synth
[19:33:00] Not making progress trying to resolve environmental context. Giving up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants