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): Should Annotations impact unit tests? #29047

Open
akash1810 opened this issue Feb 9, 2024 · 2 comments
Open

(assertions): Should Annotations impact unit tests? #29047

akash1810 opened this issue Feb 9, 2024 · 2 comments
Labels
@aws-cdk/assertions Related to the @aws-cdk/assertv2 package @aws-cdk/aws-s3 Related to Amazon S3 bug This issue is a bug. effort/medium Medium work item – several days of effort needs-review p2

Comments

@akash1810
Copy link
Contributor

akash1810 commented Feb 9, 2024

Describe the bug

With a Stack of:

// cdk_test-stack.ts
import { Aspects, Stack, StackProps } from 'aws-cdk-lib';
import { Construct } from 'constructs';
import { Bucket } from 'aws-cdk-lib/aws-s3';
import { AwsSolutionsChecks } from "cdk-nag";

export class CdkTestStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);
    const bucket = new Bucket(this, 'Bucket');
    Aspects.of(this).add(new AwsSolutionsChecks({ verbose: true }));
  }
}

We get errors via Annotations at synth time:

npx cdk synth
[Error at /CdkTestStack/Bucket/Resource] AwsSolutions-S1: The S3 Bucket has server access logs disabled. The bucket should have server access logging enabled to provide detailed records for the requests that are made to the bucket.

[Error at /CdkTestStack/Bucket/Resource] AwsSolutions-S10: The S3 Bucket or bucket policy does not require requests to use SSL. You can use HTTPS (TLS) to help prevent potential attackers from eavesdropping on or manipulating network traffic using person-in-the-middle or similar attacks. You should allow only encrypted connections over HTTPS (TLS) using the aws:SecureTransport condition on Amazon S3 bucket policies.


Found errors

However, with a unit test of:

// cdk_test-stack.test.ts
import {App} from "aws-cdk-lib";
import {Template} from "aws-cdk-lib/assertions";
import {CdkTestStack} from "./cdk_test-stack";

describe('The test stack', () => {
 it('matches the snapshot', () => {
  const app = new App();
  const stack = new CdkTestStack(app, 'cdk-test-stack');
  expect(Template.fromStack(stack).toJSON()).toMatchSnapshot();
 });
});

npm test does not observe the error Annotations. More specifically, Template.fromStack does not observe Annotations.

IIUC Template.fromStack is in the synth step1, so it's curious that error Annotations are not observed. Is this correct? Should an error Annotation prevent a stack from being synthed in all scenarios?

Expected Behavior

Error Annotations should prevent template synthesis in all scenarios.

Current Behavior

Error Annotations do not cause errors in unit tests.

Reproduction Steps

See above.

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.126.0 (build fb74c41)

Framework Version

No response

Node.js Version

v20.10.0

OS

macOS 14.3

Language

TypeScript

Language Version

TypeScript 5.3.3

Other information

Code examples are taken from https://aws.amazon.com/blogs/devops/manage-application-security-and-compliance-with-the-aws-cloud-development-kit-and-cdk-nag/.

I don't think this is an issue with cdk-nag, but with Annotations. Hence raising this issue here. Let me know if this is incorrect though.

Footnotes

  1. https://docs.aws.amazon.com/cdk/v2/guide/apps.html#lifecycle

@akash1810 akash1810 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 9, 2024
@github-actions github-actions bot added the @aws-cdk/aws-s3 Related to Amazon S3 label Feb 9, 2024
@akash1810 akash1810 changed the title (annotations): Should Annotations impact unit tests? (annotations): Should Annotations impact unit tests? Feb 9, 2024
@akash1810 akash1810 changed the title (annotations): Should Annotations impact unit tests? (assertions): Should Annotations impact unit tests? Feb 9, 2024
@github-actions github-actions bot added the @aws-cdk/assertions Related to the @aws-cdk/assertv2 package label Feb 9, 2024
@pahud
Copy link
Contributor

pahud commented Feb 9, 2024

Error Annotations should prevent template synthesis in all scenarios.

Generally agree with you but we'll need more inputs from the maintainers. Thank you for the report.

@pahud pahud added needs-review p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Feb 9, 2024
@kaspar-p
Copy link

kaspar-p commented Sep 4, 2024

Agree, this would especially make it easier to test the implementations of Aspects themselves, to make sure they fail in the right scenarios, and don't in the wrong ones.

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/aws-s3 Related to Amazon S3 bug This issue is a bug. effort/medium Medium work item – several days of effort needs-review p2
Projects
None yet
Development

No branches or pull requests

3 participants