aws-cdk icon indicating copy to clipboard operation
aws-cdk copied to clipboard

cli: #21513 breaks custom bootstrap use case

Open rabbittsoup opened this issue 2 years ago • 8 comments

Describe the bug

PR #21513 introduced asset prebuilding. This presumes that bootstrap resources have been deployed prior to and outside the scope of the currently deploying CDK project. However, we have restrictions on our accounts that prevent us from deploying the account wide default cdk bootstrap resources. Instead, we deploy project specific custom bootstrap resources within the currently deploying CDK project. The project specific custom bootstrap resource stack is deployed immediately before the project's main stacks, which allows us to bootstrap a specific project and meet our account requirements. This scenario means the bootstrap resources are not available before initial project deployment, and results in a "Error: Building Assets Failed" when the project includes assets, even though the bootstrap resources will be available by the time the assets are needed.

Expected Behavior

Bootstrap resources get used just-in-time.

Current Behavior

Bootstrap resources get used before project deployment.

Reproduction Steps

I'll work on an example...

Possible Solution

Rollback PR #21513?

Additional Information/Context

No response

CDK CLI Version

2.39.0

Framework Version

2.39.0

Node.js Version

14.16.0

OS

Amazon Linux 2

Language

Python

Language Version

3.8.13

Other information

No response

rabbittsoup avatar Sep 08 '22 13:09 rabbittsoup

Hey @rabbittsoup Thanks for reporting this. Interestingly enough, your exact use case came up in the discussions of the PR you mentioned (https://github.com/aws/aws-cdk/pull/21513#issuecomment-1213109551). So I'm mildly surprised if we accidentally made it harder for you.

To help you out, we'd really need a (minimal) reproduction example. Would you be able to provide this?

mrgrain avatar Sep 08 '22 13:09 mrgrain

Can you explain how this change breaks you? Only building has been pulled to the front, which shouldn't depend on the bootstrap resources being present.

Alternatively, you could start fresh deployments with:

cdk deploy -e MyCustomBootstrapStack

rix0rrr avatar Sep 08 '22 13:09 rix0rrr

@rix0rrr Looking at my deployment logs, I see that publishing occurs just before the individual stack. However, the failure I encountered appears to be because the prebuild is still looking for the bootstrap version parameter.

Building assets failed: Error: Building Assets Failed: Error: Stack: SSM parameter /Stack-cdk/version not found. Has the environment been bootstrapped? Please run 'cdk bootstrap' (see https://docs.aws.amazon.com/cdk/latest/guide/bootstrapping.html)
    at buildAllStackAssets (/app/cdk/node_modules/aws-cdk/lib/build.ts:21:11)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at CdkToolkit.deploy (/app/cdk/node_modules/aws-cdk/lib/cdk-toolkit.ts:174:7)
    at initCommandLine (/app/cdk/node_modules/aws-cdk/lib/cli.ts:349:12)

@mrgrain I'll try to put together a smallish example showcasing the project specific custom bootstrap pattern we use. Overall, the pattern has several advantages over the account wide shared bootstrap resources, which can create unmanageable resource use in large accounts.

rabbittsoup avatar Sep 08 '22 14:09 rabbittsoup

Let us know when you can put together that repro @rabbittsoup

peterwoodworth avatar Sep 14 '22 18:09 peterwoodworth

Sorry, I’m not going to get to this for a bit.

Very, very roughly I’m using a custom synthesizer extending the default synthesizer, which is tightly coupled to a couple separate bootstrapless stacks that are implicitly instantiated with the extended synthesizer as stage level siblings to the user’s stack. The bootstrapless stacks implement the bootstrap resources as needed.

rabbittsoup avatar Sep 15 '22 00:09 rabbittsoup

Can your custom synthesizer disable version checking on that SSM parameter?

rix0rrr avatar Sep 19 '22 13:09 rix0rrr

I could, I suppose. However, the pattern is library provided and the end user generally doesn't need to know much about it, so the versioning could still be beneficial. I will look into that further, though. I am able to temporarily workaround the issue at the moment by specifying the specific bootstrap stacks before then deploying all as you also suggested.

rabbittsoup avatar Sep 19 '22 16:09 rabbittsoup

@rix0rrr It looks like setting generateBootstrapVersionRule in the synthesizer does not workaround the problem. Although the setting does prevent the rule from being generated in the templates, the use of assets in future stacks still generates version requirements in the manifest which still causes the cli deploy to fail with the version error.

rabbittsoup avatar Sep 21 '22 16:09 rabbittsoup

I updated the original post with an example app.py.

rabbittsoup avatar Sep 23 '22 19:09 rabbittsoup

I gotcha. I think the pre-build doesn't need to check the parameter at all, that is just necessary for the publish.

rix0rrr avatar Oct 06 '22 13:10 rix0rrr

Oh no of course it does, because Docker builds are crazy and we need to assume roles and check ECR repositories so that Docker can load cache from the existing repo.

rix0rrr avatar Oct 07 '22 09:10 rix0rrr

@rix0rrr Any movement on accommodating this pattern? The issue still causes us grief.

rabbittsoup avatar Nov 04 '22 19:11 rabbittsoup

Sorry, dropped off my radar.

I think the simplest thing we can do is add a CLI flag to disable the prebuild. Would that suffice for you?

rix0rrr avatar Nov 08 '22 13:11 rix0rrr

I'm not familiar enough with the prebuild changes to understand what tradeoffs that would introduce? What might not work when using such a flag?

rabbittsoup avatar Nov 08 '22 16:11 rabbittsoup

The PR introducing the problem was trying to move all builds to the front of the deployment process, so that if the build fails potentially slow deploys are avoided.

It would be a flag to revert to the old behavior: do just-in-time building, interleaved in between deploys.

rix0rrr avatar Nov 09 '22 14:11 rix0rrr

Do you think it would be possible to do a flag-style decision for this at runtime, maybe passed into the App instance? For instance, suppose that maybe the app.py (or similar) top level cdk app code could check something, or in my scenario check if the bootstrap version parameter exists, and then choose pre-build or interleave-build option?

I suppose similar logic could also be done in a bash script if perhaps there was a way to check for bootstrap version parameter existence using the cdk credentials. I suppose this may be the same as aws cli commands and credentials 🤔

rabbittsoup avatar Nov 10 '22 20:11 rabbittsoup

⚠️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.

github-actions[bot] avatar Nov 16 '22 16:11 github-actions[bot]

@rix0rrr The behavior of all command line argument defaults overriding cdk.json and ~/.cdk.json appears to be getting in the way of using the newly added assetPrebuild option you added in #22930 to solve our prebuild pains. issue #21513 is not fixed without the option to use cdk.json or ~/.cdk.json. The cdk.json option problem is detailed in issue #3573

rabbittsoup avatar Nov 22 '22 19:11 rabbittsoup