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

DefaultStackSynthesizer: Using a StackSynthesizer and Sharing Assets will only Publish One Asset and Not the Other #25927

Closed
ObOne5524 opened this issue Jun 9, 2023 · 5 comments · Fixed by #26910
Assignees
Labels
@aws-cdk/assets Related to the @aws-cdk/assets package bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@ObOne5524
Copy link

Describe the bug

If a Synthesizer is used and the an Asset is shared between two Stacks on the same App, one of the Stacks will have a published asset to reference while the other Stack will not.

Expected Behavior

If a Synthesizer is used and the prefixes are different, the Assets should be able accessible to both Stacks.

Current Behavior

One Stack will be able to access the asset, while the other will return a Resource handler returned message: "Error occurred while GetObject. S3 Error Code: NoSuchKey. S3 Error Message: The specified key does not exist. due to a missing asset.

Reproduction Steps

  1. Define a bin or app.py file that creates two stacks
  2. Define a DefaultStackSynthesizer
  3. Define a Lambda Function resource with a logRetention prop

Possible Solution

Since this is unique to version 2.8X.X, the Asset removal might need to be reviewed

Additional Information/Context

No response

CDK CLI Version

2.81.0

Framework Version

No response

Node.js Version

18.16.0

OS

macOS 12.6.6

Language

Typescript, Python, .NET, Java, Go

Language Version

5.0.4

Other information

No response

@ObOne5524 ObOne5524 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 9, 2023
@github-actions github-actions bot added the @aws-cdk/assets Related to the @aws-cdk/assets package label Jun 9, 2023
@pahud
Copy link
Contributor

pahud commented Jun 12, 2023

Thank you. Are you able to provide a minimal code sample that I can copy/paste to reproduce this bug?

@pahud pahud added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jun 12, 2023
@ObOne5524
Copy link
Author

Bin File:

#!/usr/bin/env node
import * as cdk from 'aws-cdk-lib';
import { SharedAssetStack } from '../lib/shared_asset-stack';
import { SharedAssetStack2 } from '../lib/shared_asset-stack2';

const app = new cdk.App();

const stack1Synth = new cdk.DefaultStackSynthesizer({
    bucketPrefix: "newtest/"
})

const stack2Synth = new cdk.DefaultStackSynthesizer({
    bucketPrefix: "newtest2/"
})

new SharedAssetStack(app, 'SharedAssetStack', {synthesizer: stack1Synth});
new SharedAssetStack2(app, 'SharedAssetStack2', {synthesizer: stack2Synth});

Lib File:
Stack 1:

import { Duration, Stack, StackProps } from 'aws-cdk-lib';
import * as sns from 'aws-cdk-lib/aws-sns';
import * as lambda from 'aws-cdk-lib/aws-lambda'
import * as subs from 'aws-cdk-lib/aws-sns-subscriptions';
import * as sqs from 'aws-cdk-lib/aws-sqs';
import * as logs from 'aws-cdk-lib/aws-logs'
import { Construct } from 'constructs';
import * as path from 'path';

export class SharedAssetStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    new lambda.Function(this, 'Function', {
        runtime: lambda.Runtime.NODEJS_18_X,
        handler: 'index.handler',
        code: lambda.Code.fromAsset(path.join(__dirname, 'lambda-handler')),
        logRetention: logs.RetentionDays.ONE_WEEK
      });
    }
}

Stack 2:

import { Duration, Stack, StackProps } from 'aws-cdk-lib';
import * as sns from 'aws-cdk-lib/aws-sns';
import * as lambda from 'aws-cdk-lib/aws-lambda'
import * as subs from 'aws-cdk-lib/aws-sns-subscriptions';
import * as sqs from 'aws-cdk-lib/aws-sqs';
import * as logs from 'aws-cdk-lib/aws-logs'
import { Construct } from 'constructs';
import * as path from 'path';

export class SharedAssetStack2 extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    new lambda.Function(this, 'Function', {
        runtime: lambda.Runtime.NODEJS_18_X,
        handler: 'index.handler',
        code: lambda.Code.fromAsset(path.join(__dirname, 'lambda-handler')),
        logRetention: logs.RetentionDays.ONE_WEEK
      });
    }
}

Index Handler:

import { Context, APIGatewayProxyCallback, APIGatewayEvent } from 'aws-lambda';

export const handler = (event: APIGatewayEvent, context: Context, callback: APIGatewayProxyCallback): void => {
    console.log(`Event: ${JSON.stringify(event, null, 2)}`);
    console.log(`Context: ${JSON.stringify(context, null, 2)}`);
    callback(null, {
        statusCode: 200,
        body: JSON.stringify({
            message: 'hello world!',
        }),
    });
};

@ObOne5524
Copy link
Author

If you run cdk deploy --all the Resource handler returned message: "Error occurred while GetObject. S3 Error Code: NoSuchKey. S3 Error Message: The specified key does not exist. will be returned by one of the Stacks

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jun 13, 2023
@rix0rrr rix0rrr added p1 and removed p2 labels Aug 24, 2023
@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 24, 2023

Thanks for the report!

@rix0rrr rix0rrr self-assigned this Aug 25, 2023
rix0rrr added a commit that referenced this issue Aug 28, 2023
If the same asset is used in 2 stacks that use different synthesizer
configurations for publishing (for example, by using a different prefix)
the asset will only be uploaded once instead of twice.

We used to make the assumption that it was okay to use the destination
ID as token of uniqueness. This is true inside a single manifest, but
does not hold when there is more than stack that each have a manifest:
both may have the destination ID `current_account:current_region`, but
have different parameters for each destination.

Instead, we calculate a content hash over the destination definition
itself. That way, if the definitions are different we will create
different nodes for each of them.

Fixes #25927.
@mergify mergify bot closed this as completed in #26910 Aug 29, 2023
mergify bot pushed a commit that referenced this issue Aug 29, 2023
If the same asset is used in 2 stacks that use different synthesizer configurations for publishing (for example, by using a different prefix) the asset will only be uploaded once instead of twice.

We used to make the assumption that it was okay to use the destination ID as token of uniqueness. This is true inside a single manifest, but does not hold when there is more than stack that each have a manifest: both may have the destination ID `current_account:current_region`, but have different parameters for each destination.

Instead, we calculate a content hash over the destination definition itself. That way, if the definitions are different we will create different nodes for each of them.

Fixes #25927.

----

*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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/assets Related to the @aws-cdk/assets package bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants