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

(integ-tests): awsApiCall Response object is too long #22059

Closed
dontirun opened this issue Sep 15, 2022 · 4 comments · Fixed by #23102
Closed

(integ-tests): awsApiCall Response object is too long #22059

dontirun opened this issue Sep 15, 2022 · 4 comments · Fixed by #23102
Assignees
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@dontirun
Copy link
Contributor

Describe the bug

When using assertions.awsApiCall my response object was too large and caused the integration test stack to fail deployment.

Expected Behavior

I can limit the outputPath of the api call like done here ##2825

Current Behavior

I can't limit the output and get around the 4k response limit for CloudFormation

Reproduction Steps

integ.reproduction.ts

import * as ec2 from '@aws-cdk/aws-ec2';
import { App, Stack, StackProps } from '@aws-cdk/core';
import * as integ from '@aws-cdk/integ-tests';
import { Construct } from 'constructs';
import * as redshift from '../lib';

class RedshiftEnv extends Stack {
  readonly cluster: redshift.Cluster

  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const vpc = new ec2.Vpc(this, 'VPC');

    this.cluster = new redshift.Cluster(this, 'Cluster', {
      vpc: vpc,
      vpcSubnets: {
        subnetType: ec2.SubnetType.PUBLIC,
      },
      masterUser: {
        masterUsername: 'admin',
      },
    });
  }
}

const app = new App();
const stack = new RedshiftEnv(app, 'redshift-integ');
const test = new integ.IntegTest(app, 'LoggingBucketInteg', {
  testCases: [stack],
});

test.assertions.awsApiCall('Redshift', 'describeClusters', {
  ClusterIdentifier: stack.cluster.clusterName,
});

app.synth();

Possible Solution

Implement an outputPath feature like done here ##2825 to limit the response object

Additional Information/Context

No response

CDK CLI Version

2.41.0

Framework Version

No response

Node.js Version

v16.16.0

OS

OsX

Language

Typescript

Language Version

No response

Other information

No response

@dontirun dontirun added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 15, 2022
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Sep 15, 2022
@corymhall
Copy link
Contributor

I'm working on an update where the assertions are done in the same handler instead of passing the data to another resource. Hopefully that will fix this issue.

@dontirun
Copy link
Contributor Author

@corymhall I saw that #22238 was merged, but it didn't seem to fix the issue. Perhaps an easy way to fix the issue would be to use the bootstrap assets bucket as a proxy.

  1. Make API Call
  2. Upload Output of API Call JSON to the CDK Assets Bucket
  3. Output S3 object Key
  4. Assertion reads in and downloads the JSON from the Bucket
  5. Nice to have: Another resource deletes all the uploaded objects after the tests

@corymhall
Copy link
Contributor

Currently awsApiCall can serve two roles.

  1. Make an API call in order to return a value that can be referenced elsewhere in the template
  2. Make an API call and assert the results (don't need to return anything).

In order to accommodate the second we first flatten the results (if we need to) and then perform assertions and then return everything. What I want to do is change this around a little. We should perform assertions on the response (without flattening) and then if we also need to return a value we can flatten if needed and then only return the value that is needed.

@mergify mergify bot closed this as completed in #23102 Nov 28, 2022
mergify bot pushed a commit that referenced this issue Nov 28, 2022
fixes #22059

follows precedent set in #14041

also added filtering to assertAtPath to reduce duplication and reduce the chance of hitting the error for developers using the library

also added test for assertAtPath

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https:/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https:/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https:/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

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

brennanho pushed a commit to brennanho/aws-cdk that referenced this issue Dec 9, 2022
fixes aws#22059

follows precedent set in aws#14041

also added filtering to assertAtPath to reduce duplication and reduce the chance of hitting the error for developers using the library

also added test for assertAtPath

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https:/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https:/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https:/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
brennanho pushed a commit to brennanho/aws-cdk that referenced this issue Jan 20, 2023
fixes aws#22059

follows precedent set in aws#14041

also added filtering to assertAtPath to reduce duplication and reduce the chance of hitting the error for developers using the library

also added test for assertAtPath

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https:/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https:/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https:/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
mergify bot pushed a commit that referenced this issue Feb 17, 2023
…#22063)

Closes #22009

Currently waiting on #22055  and #22059 for the assertions in the integration test to successfully run

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https:/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https:/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [X] Have you added the new feature to an [integration test](https:/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [X] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
brennanho pushed a commit to brennanho/aws-cdk that referenced this issue Feb 22, 2023
fixes aws#22059

follows precedent set in aws#14041

also added filtering to assertAtPath to reduce duplication and reduce the chance of hitting the error for developers using the library

also added test for assertAtPath

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https:/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https:/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https:/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
2 participants