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

(assertions): argument to skip bundling of assets #18125

Open
2 tasks
joel-aws opened this issue Dec 21, 2021 · 14 comments
Open
2 tasks

(assertions): argument to skip bundling of assets #18125

joel-aws opened this issue Dec 21, 2021 · 14 comments
Labels
@aws-cdk/assertions Related to the @aws-cdk/assertv2 package @aws-cdk/core Related to core CDK functionality effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1

Comments

@joel-aws
Copy link
Contributor

Description

When running assertions.Template.from_stack(stack), one cannot skip the bundling of assets during synthesis/generation of the CloudFormation template.

Use Case

When writing unit-tests that leverage the assertions library, assets do not always have to be created to check the synthesized template. Depending on how many assets are created (and their bundling strategy), the process can be lengthy. This feature-request is for an argument which can disable bundling of assets.

Proposed Solution

assertions.Template.from_stack(stack, bundle_assets=False)

Other information

I tried doing something like below, to no avail:

@jsii.implements(IAspect)
class MockAssetStaging:
    def __init__(self, fake_stage: AssetStaging):
        self.fake_stage = fake_stage

    def visit(self, node):
        if isinstance(node, AssetStaging):
            node = self.fake_stage

app = App()
core_stack = CoreStack(...)

fake_staged_asset = AssetStaging(core_stack, "fake", source_path=f"./hello_world.py")
Aspects.of(app).add(MockAssetStaging(fake_staged_asset))

Acknowledge

  • I may be able to implement this feature request
  • This feature might incur a breaking change
@joel-aws joel-aws added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Dec 21, 2021
@github-actions github-actions bot added the @aws-cdk/assertions Related to the @aws-cdk/assertv2 package label Dec 21, 2021
@kaizencc kaizencc added effort/medium Medium work item – several days of effort p2 feature-request A feature should be added or improved. and removed feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Dec 29, 2021
@kaizencc kaizencc removed their assignment Dec 29, 2021
@ChrisSargent
Copy link

ChrisSargent commented Jan 20, 2022

there is a bundlingStacks option in the context, in our tests we do something along the lines of:

import baseContext from './cdk.json'

const context = {
  ...baseContext,
  'aws:cdk:bundling-stacks': [],
};

@joel-aws
Copy link
Contributor Author

joel-aws commented Jan 20, 2022

That's a great idea, @ChrisSargent! I couldn't get it to work, but am going to keep trying iterations of it.

I searched around a bit and saw #18290 (comment), which makes me think this wasn't possible?

@joel-aws
Copy link
Contributor Author

@ChrisSargent -- I've hit a wall. Would you mind sharing more of your code (or a link to a repo) so I could see how you set bundlingStacks for your tests?

@ChrisSargent
Copy link

ChrisSargent commented Apr 14, 2022

Hi @joel-aws - sorry, I didn't see this comment until now. Not much more to share but here it is:

const context = {
  ...baseContext,
  'aws:cdk:bundling-stacks': [],
};
const app = new cdk.App({
  context,
});

If definitely works for us.

@rally25rs
Copy link

rally25rs commented Nov 22, 2022

the aws:cdk:bundling-stacks does not seem to work, nor does it seem to be a documented context option that I could find.
I was poking through the source, and the issue seems to be that the constructor for new NodejsFunction() will unconditionally call Bundling.bundle(), which ends up always trying to execute some docker commands as a child process (unless you pass it a custom DockerImage of your own) and then later Code.bundle executes the bundling in docker.
It seems like the BundlingOptions passed to new NodejsFunction({ bundling: { /* add disable flag here? */ }}) could benefit from some custom bundling command overrides, or a flag to disable bundling.


Best I was able to come up with was a Jest mock that overrides the NodejsFunction constructor and doesn't call the stuff that triggers bundling.

jest.mock('aws-cdk-lib/aws-lambda-nodejs', () => {
  const originalModule = jest.requireActual('aws-cdk-lib/aws-lambda-nodejs');
  const { Function } = jest.requireActual('aws-cdk-lib/aws-lambda');

  class MockNodejsFunction extends Function {
    constructor(scope: any, id: any, props: any = {}) {
      const handler = props.handler ?? 'handler';
      const runtime = props.runtime ?? 'nodejs14.x';
      super(scope, id, {
        ...props,
        runtime,
        // the normal CDK SDK unconditionally uses Docker to make a container with esbuild in it, and bundles your function code with it.
        // this override of `code` and the `bind()` function avoids that, since compiling the TS code is not needed for testing and  generating
        // the CloudFormation template.
        code: {
          bind() {
            return {
              s3Location: {
                bucketName: '',
                objectKey: '',
              },
            };
          },
          bindToResource() {},
        },
        handler: `index.${handler}`,
      });
    }
  }

  return {
    ...originalModule,
    __esModule: true,
    NodejsFunction: MockNodejsFunction,
  };
});

I don't particularly like this approach, but I couldn't find a better thing to mock out or any conditionals to pass through to disable bundling.

@LarsFronius
Copy link
Contributor

Hi @joel-aws - sorry, I didn't see this comment until now. Not much more to share but here it is:

const context = {
  ...baseContext,
  'aws:cdk:bundling-stacks': [],
};
const app = new cdk.App({
  context,
});

If definitely works for us.

Thanks, this works well for us!
@rally25rs The feature flag is in

export const BUNDLING_STACKS = 'aws:cdk:bundling-stacks';
and is then evaluated here
public get bundlingRequired() {
/
skip = !Stack.of(this).bundlingRequired;

@rally25rs
Copy link

@LarsFronius thanks for the reply. It looks like there is a difference between what I was running into vs the original post.
I think the code you linked and the aws:cdk:bundling-stacks config probably works with a Stack as used in the original issue;

app = App()
core_stack = CoreStack(...)

However in my case we were using some NodejsFunctions, and constructor for NodejsFunction doesn't seem to use that config or asset-staging code.

@bestickley
Copy link

bestickley commented Aug 8, 2023

@rally25rs and @joel-aws , should NodejsFunction use aws:cdk:bundling-stacks or Stack.of(this).bundlingRequiredto not bundle if cdk deploy ... --exclusively was called with a stack the current function isn't in? I would think so.

@onhate
Copy link
Contributor

onhate commented Oct 30, 2023

'aws:cdk:bundling-stacks': [],

does not seems to be working anymore on latest version

@castleadmin
Copy link

castleadmin commented Nov 28, 2023

I agree that an asset creation parameter would decrease the test execution time significantly. Furthermore, it would help to create apps where the infrastructure definition lives right beside the application code.

The following dirty piece of code can be used as a workaround:

import {CodeConfig} from "aws-cdk-lib/aws-lambda";

jest.mock( "aws-cdk-lib/aws-lambda", () => {
  const module = jest.requireActual<typeof import("aws-cdk-lib/aws-lambda")>( "aws-cdk-lib/aws-lambda");
  module.Code.fromAsset = (path: string): any => ({
    path,
    isInline: false,
    bind: (): CodeConfig => {
      return {
        s3Location: {
          bucketName: 'bucketName',
          objectKey: 'objectKey',
        },
      };
    },
    bindToResource: () => {}
  });

  return module;
});

Copy link

This issue has received a significant amount of attention so we are automatically upgrading its priority. A member of the community will see the re-prioritization and provide an update on the issue.

@pahud pahud added the @aws-cdk/core Related to core CDK functionality label Apr 17, 2024
@CaptainDriftwood
Copy link

Any chance we could see some progress made on this issue? Would be very nice to be able to test constructs without having to bundle them.

@jusdino
Copy link
Contributor

jusdino commented Sep 18, 2024

Any chance we could see some progress made on this issue? Would be very nice to be able to test constructs without having to bundle them.

@CaptainDriftwood, for what it's worth, there is a method identified to suppress bundling via context, as mentioned above

I've adopted this in my test methods to look something like:

    def test_example(self):
        with open('cdk.json', 'r') as f:
            context = json.load(f)['context']

        # Suppresses lambda bundling for tests
        context['aws:cdk:bundling-stacks'] = []

        app = ExampleApp(context=context)

@CaptainDriftwood
Copy link

CaptainDriftwood commented Sep 18, 2024

Any chance we could see some progress made on this issue? Would be very nice to be able to test constructs without having to bundle them.

@CaptainDriftwood, for what it's worth, there is a method identified to suppress bundling via context, as mentioned above

I've adopted this in my test methods to look something like:

    def test_example(self):
        with open('cdk.json', 'r') as f:
            context = json.load(f)['context']

        # Suppresses lambda bundling for tests
        context['aws:cdk:bundling-stacks'] = []

        app = ExampleApp(context=context)

@jusdino Yes, I saw the work around as mentioned above (and implemented it in my project with success as a pytest fixture), I was just unsure if this functionality is going to be formally adopted in the Template class as mentioned in the Proposed Solution, and if so, when could we expect this issue to be marked in progress. It's been open for a couple years so far.

Nonetheless, shout out to @ChrisSargent for the work around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/assertions Related to the @aws-cdk/assertv2 package @aws-cdk/core Related to core CDK functionality effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

No branches or pull requests