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

feat: lambda logging rules #1677

Closed

Conversation

Kevinwochan
Copy link

Serverless Nag Pack - lambda logging rules

First rule of many to check if this is in-line with the team's standards

Copy link
Collaborator

@dontirun dontirun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add this to the Additional Rules section in the RULES.mdfile.

That section lists rules that are not currently part of any included NagPacks

test/rules/Lambda.test.ts Outdated Show resolved Hide resolved
test/rules/Lambda.test.ts Outdated Show resolved Hide resolved
test/rules/Lambda.test.ts Outdated Show resolved Hide resolved
src/rules/lambda/LambdaLogging.ts Outdated Show resolved Hide resolved
Kevinwochan and others added 3 commits May 9, 2024 10:20
Copy link
Collaborator

@dontirun dontirun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • All the rules that are being added need tests.
  • The Serverless Pack needs its own section in the RULES file
  • The Serverless Pack needs a test
  • Rule info needs to be declarative
  • Explanations needs to describe the benefit of doing what the rule tells you to do

RULES.md Outdated Show resolved Hide resolved
RULES.md Outdated Show resolved Hide resolved
src/rules/appsync/AppSyncTracing.ts Outdated Show resolved Hide resolved
src/packs/serverless.ts Outdated Show resolved Hide resolved
src/packs/serverless.ts Outdated Show resolved Hide resolved
@Kevinwochan Kevinwochan force-pushed the feat/serverless-nag-pack branch from e9351f7 to bc1d61c Compare June 17, 2024 03:29
@Kevinwochan
Copy link
Author

I've written up rules for the rest of the pack.
Just thought i'd get this Lambda Log rule through first so i can make sure the rest of the serverless pack implementation aligns

RULES.md Outdated
@@ -702,6 +702,7 @@ A collection of community rules that are not currently included in any of the pr
| Rule ID | Cause | Explanation |
| --------------------- | ------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| LambdaFunctionUrlAuth | The Lambda Function URL allows for public, unauthenticated access. | AWS Lambda Function URLs allow you to invoke your function via a HTTPS end-point, setting the authentication to NONE allows anyone on the internet to invoke your function. |
| LambdaLogLevel | The Lambda Function does not define an explicit Log Level | For cost optimization purposes, you should explicitly define the required log level for cost effective storage of Lambda Logs. |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for the wrong rule

@@ -351,4 +357,63 @@ describe('AWS Lambda', () => {
validateStack(stack, ruleId, TestType.VALIDATION_FAILURE);
});
});

describe('LambdaLogLevel: Lambda functions have a explicit log level', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like these tests are for the wrong rule

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants