Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

app-staging-synthesizer: Deletion of DefaultStagingStack fails when it does not contains any s3 asset #27834

Open
tmokmss opened this issue Nov 4, 2023 · 0 comments
Labels
@aws-cdk/app-staging-synthesizer-alpha Related to the @aws-cdk/app-staging-synthesizer-alpha package bug This issue is a bug. effort/medium Medium work item – several days of effort p2

Comments

@tmokmss
Copy link
Contributor

tmokmss commented Nov 4, 2023

Describe the bug

When using AppStagingSynthesizer.defaultResources and we don't use any asset in stacks in the app, CFn DependsOn properties are not rendered properly on DefaultStagingStack.

This at least makes deleting the staging stack fail because a bucket policy sometimes gets deleted before autoDeleteObject custom resource ignoring its dependency.

Expected Behavior

DependsOn properties are rendered.

Current Behavior

DependsOn properties are not rendered.

Reproduction Steps

Use the following code for cdk app.

// bin/cdk.ts
import 'source-map-support/register';
import * as cdk from 'aws-cdk-lib';
import { AppStagingSynthesizer, DeploymentIdentities } from '@aws-cdk/app-staging-synthesizer-alpha';

const app = new cdk.App({
  defaultStackSynthesizer: AppStagingSynthesizer.defaultResources({
    appId: 'my-app-id',
    deploymentIdentities: DeploymentIdentities.defaultBootstrapRoles({ bootstrapRegion: 'us-east-1' }),
  }),
});

// stack without any asset
new cdk.Stack(app, 'StagingSynthesizerStack');

Run cdk synth and check the output template (StagingStack-my-app-id2-ACCOUNT-REGION.template.json).

You can find this resource:

  "CdkStagingBucketAutoDeleteObjectsCustomResource800E998D": {
   "Type": "Custom::S3AutoDeleteObjects",
   "Properties": {
    "ServiceToken": {
     "Fn::GetAtt": [
      "CustomS3AutoDeleteObjectsCustomResourceProviderHandler9D90184F",
      "Arn"
     ]
    },
    "BucketName": {
     "Ref": "CdkStagingBucket1636058C"
    }
   },
   "UpdateReplacePolicy": "Delete",
   "DeletionPolicy": "Delete"
  },

Where it should be:

  "CdkStagingBucketAutoDeleteObjectsCustomResource800E998D": {
   "Type": "Custom::S3AutoDeleteObjects",
   "Properties": {
    "ServiceToken": {
     "Fn::GetAtt": [
      "CustomS3AutoDeleteObjectsCustomResourceProviderHandler9D90184F",
      "Arn"
     ]
    },
    "BucketName": {
     "Ref": "CdkStagingBucket1636058C"
    }
   },
+   "DependsOn": [
+    "CdkStagingBucketPolicy42BD1F92"
+   ],
   "UpdateReplacePolicy": "Delete",
   "DeletionPolicy": "Delete"
  },

Possible Solution

Here's my observation:

The root cause for this issue is that we add resources to an CDK app AFTER cdk runs prepareApp function.

Bucket resource can be added even after prepareApp here (it's added on the first time addFileAsset is called) :

const templateAsset = this.addFileAsset(templateAssetSource);

When we use any (s3) assets in the app (e.g. Lambda code asset), addFileAsset is called before prepareApp, so in that case it works as expected.

In prepareApp function, we search for all the dependencies between nodes and register them as a CFn resource dependencies. That's why dependencies added after prepareApp are not rendered at all.

export function prepareApp(root: IConstruct) {
// apply dependencies between resources in depending subtrees
for (const dependency of findTransitiveDeps(root)) {
const targetCfnResources = findCfnResources(dependency.target);
const sourceCfnResources = findCfnResources(dependency.source);
for (const target of targetCfnResources) {
for (const source of sourceCfnResources) {
source.addDependency(target);
}
}
}

So the possible solution can be to always create a staging bucket first to avoid adding it after prepareApp. To do that, for example, we can add this.getCreateBucket(); to the stack constructor here:

constructor(scope: App, id: string, private readonly props: DefaultStagingStackProps) {
super(scope, id, {
...props,
synthesizer: new BootstraplessSynthesizer(),
analyticsReporting: false, // removing AWS::CDK::Metadata construct saves ~3KB
});

I'm not quite sure if there is more thorough solution (given that there are possibly other resources other than a staging bucket that can be added after prepareApp). Afaik it seems only a staging bucket is causing this issue because the bucket is used to upload CFn templates in the app (that's why it can be added after prepareApp).

Additional Information/Context

On the other hand, DependsOn of CustomResourceProviderHandler is rendered properly.

  "CustomS3AutoDeleteObjectsCustomResourceProviderHandler9D90184F": {
   "Type": "AWS::Lambda::Function",
   "Properties": { 
   },
   "DependsOn": [
    "CustomS3AutoDeleteObjectsCustomResourceProviderRole3B1BD092"
   ]
  }

This is because custom resource provider uses CfnResource.addDependency directly instead of Node.addDependency.

CDK CLI Version

2.104.0

Framework Version

2.104.0

Node.js Version

18

OS

macOS

Language

TypeScript

Language Version

No response

Other information

No response

@tmokmss tmokmss added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 4, 2023
@github-actions github-actions bot added the @aws-cdk/app-staging-synthesizer-alpha Related to the @aws-cdk/app-staging-synthesizer-alpha package label Nov 4, 2023
@pahud pahud added p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Nov 10, 2023
@tmokmss tmokmss changed the title app-staging-synthesizer: DependsOn is not rendered on DefaultStagingStack when no asset is used app-staging-synthesizer: Deletion of DefaultStagingStack fails when it does not contains any s3 asset Feb 20, 2024
@pahud pahud added p2 and removed p1 labels Jun 11, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
@aws-cdk/app-staging-synthesizer-alpha Related to the @aws-cdk/app-staging-synthesizer-alpha package bug This issue is a bug. effort/medium Medium work item – several days of effort p2
Projects
None yet
Development

No branches or pull requests

2 participants