-
Notifications
You must be signed in to change notification settings - Fork 1
feat(lambda): add BDD testing support for Lambda functions #194
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
Conversation
- Add LambdaAsserter interface and implementation with assertions for: - Function existence and configuration (runtime, handler, timeout, memory) - Environment variables - Versions and aliases - Function URLs and auth types - Layers and event source mappings - Add Lambda helper in awshelpers for SDK client creation - Add Gherkin step definitions for all Lambda assertions - Add Terraform examples: - basic-function: Simple Lambda with configurable runtime/handler - function-with-alias: Lambda with published version and alias - function-url: Lambda with public/IAM-authenticated URL - Add feature files for testing Lambda scenarios 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Pull Request Review: Lambda BDD Testing SupportThis PR adds comprehensive Lambda testing support with well-structured assertions, step definitions, and Terraform examples. The implementation follows the established patterns in the codebase. Below is my detailed review: ✅ Strengths
🐛 Issues to Fix1. Incorrect Error Type Checking in
|
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Pull Request Review: Lambda BDD Testing SupportThis is a solid implementation that adds comprehensive Lambda testing support to InfraSpec. The code follows established patterns and integrates well with the existing architecture. Here's my detailed feedback: ✅ Strengths1. Excellent Code Organization
2. Comprehensive Test Coverage
3. Virtual Cloud Integration
4. Consistent API Design
🐛 Issues & SuggestionsCritical: Unused Variable in AssertFunctionNotExistsLocation: // Check if the error is a ResourceNotFoundException
var notFoundErr *types.ResourceNotFoundException
if !strings.Contains(err.Error(), "ResourceNotFoundException") {
// If it's a different error, return it
return fmt.Errorf("unexpected error checking Lambda function %s: %w", functionName, err)
}
_ = notFoundErr // Silence unused variable warningProblem: The code declares Fix: Use proper error type checking: // Check if the error is a ResourceNotFoundException
var notFoundErr *types.ResourceNotFoundException
if !errors.As(err, ¬FoundErr) {
// If it's a different error, return it
return fmt.Errorf("unexpected error checking Lambda function %s: %w", functionName, err)
}Don't forget to add Code Quality: Unused ImportLocation: // Silence unused import warning for strconv
var _ = strconv.ItoaProblem: Fix: Remove the Enhancement: Client Reuse PatternCurrent Pattern: Many methods create a new client for each call: func (a *AWSAsserter) AssertFunctionExists(functionName string) error {
client, err := awshelpers.NewLambdaClientWithDefaultRegion()
// ...
}
func (a *AWSAsserter) AssertFunctionVersionExists(functionName, version string) error {
client, err := awshelpers.NewLambdaClientWithDefaultRegion()
// ...
}Observation: Unlike S3 which has Suggestion: Consider adding a helper method for consistency: func (a *AWSAsserter) createLambdaClient() (*lambda.Client, error) {
return awshelpers.NewLambdaClientWithDefaultRegion()
}Then use Documentation: Missing Comments on Interface MethodsThe Example: type LambdaAsserter interface {
// AssertFunctionExists verifies that a Lambda function exists in AWS
// Returns an error if the function doesn't exist or cannot be accessed
AssertFunctionExists(functionName string) error
// AssertFunctionNotExists verifies that a Lambda function does NOT exist
// Returns an error if the function exists or on unexpected errors
AssertFunctionNotExists(functionName string) error
// ... etc
}🔒 Security ConsiderationsGood Security Practices:
Terraform Example Security:The function-url example has a good security pattern: resource "aws_lambda_permission" "function_url" {
count = var.authorization_type == "NONE" ? 1 : 0
# Only grants public access when explicitly configured
}This prevents accidental public exposure. Well done! 🚀 Performance ConsiderationsAPI Call Efficiency:
Potential Optimization:If multiple assertions run against the same function in a single test, they'll each call 🧪 Test CoverageWhat's Tested:
What's Missing:
Recommendation: Either add feature tests for layers and event source mappings, or remove those methods in this PR and add them when needed. Following YAGNI principle from CLAUDE.md. 📝 Conventional CommitsThe commit message follows the Conventional Commits spec: Perfect use of scope and type! ✅ 🎯 RecommendationsHigh Priority:
Medium Priority:
Low Priority:
✨ SummaryThis is a high-quality PR that follows InfraSpec's established patterns and conventions. The implementation is clean, well-organized, and production-ready. The critical issues are minor and easily fixed. The Terraform examples are particularly well-crafted and provide good real-world patterns. Approval Status: ✅ Approved with minor changes requested Great work on maintaining consistency with the existing codebase! The Lambda implementation will be a valuable addition to InfraSpec's testing capabilities. |
Emulation is now the default behavior - no flag needed. Use --live flag when testing against real AWS infrastructure. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Pull Request Review - Lambda BDD Testing SupportSummaryThis PR adds comprehensive Lambda testing support to InfraSpec with BDD-style Gherkin tests. The implementation is well-structured and follows established patterns in the codebase. ✅ StrengthsCode Quality
Terraform Examples
Gherkin Feature Files
🐛 Issues Found1. Improper Error Type Checking (Bug - High Priority)Location: pkg/assertions/aws/lambda.go:78-84 Current code: var notFoundErr *types.ResourceNotFoundException
if !strings.Contains(err.Error(), "ResourceNotFoundException") {
return fmt.Errorf("unexpected error checking Lambda function %s: %w", functionName, err)
}
_ = notFoundErr // Silence unused variable warningProblems:
Recommended Fix: var notFoundErr *types.ResourceNotFoundException
if !errors.As(err, ¬FoundErr) {
return fmt.Errorf("unexpected error checking Lambda function %s: %w", functionName, err)
}Don't forget to add "errors" import. 2. Unused Import (Code Quality - Low Priority)Location: pkg/steps/aws/lambda.go:321-322 // Silence unused import warning for strconv
var _ = strconv.ItoaIssue: The strconv import appears unnecessary. Check if it can be removed entirely. If needed, it should be used naturally rather than with a dummy assignment. 3. Missing Unit Tests (Test Coverage - High Priority)Issue: No unit tests found for:
Recommendation: According to CLAUDE.md testing guidelines, add unit tests for at least:
4. Lambda Service Endpoint Configuration (Integration - Medium Priority)Question: Has the Lambda service endpoint been added to the serviceMap in pkg/steps/terraform/terraform.go? According to CLAUDE.md Virtual Cloud Integration section, ensure there's an entry like: "LAMBDA": "lambda",This is critical for routing Lambda API calls to the embedded emulator. 🎯 RecommendationsPerformance
Security
Code Organization
Documentation
📋 Action Items Before MergeCritical
Nice to Have
🎉 Overall AssessmentStrong implementation that demonstrates good understanding of the InfraSpec architecture. The code quality is high, follows established patterns, and the Terraform examples are practical and well-designed. The main concerns are:
Recommendation: Request changes for the critical items, then approve once addressed. 📚 References
|
Add pre-defined AWS managed policy support to fix Lambda BDD tests that use AWSLambdaBasicExecutionRole and other managed policies. - Add aws_managed_policies.go with 25 common AWS managed policies - Include realistic policy documents for Lambda, S3, DynamoDB, EC2, RDS, IAM, SQS, CloudWatch, and SSM services - Add fallback stub policy for unrecognized managed policy ARNs - Update attachRolePolicy/detachRolePolicy to handle AWS managed policies - Update getPolicy/getPolicyVersion to return managed policy data 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Pull Request Review: Lambda BDD Testing SupportSummaryThis PR adds comprehensive BDD testing infrastructure for AWS Lambda functions, including 15+ assertion methods, 25+ Gherkin step definitions, and three well-designed Terraform examples. The implementation follows the project's established patterns and integrates cleanly with the Virtual Cloud emulator. Strengths1. Excellent Code Organization
2. Comprehensive Feature CoverageAssertions cover all major Lambda functionality: existence checks, configuration (runtime, handler, timeout, memory), environment variables, versions/aliases, function URLs, layers, and event source mappings. 3. Well-Designed Terraform ExamplesThree examples demonstrate: basic functions with configurable parameters, versioning/aliasing, and function URLs with both public and IAM auth. All include proper IAM roles and validation. 4. Virtual Cloud IntegrationProper endpoint configuration following established patterns, CI workflow updated. Issues and ConcernsCRITICAL: Missing Unit Tests (Severity: High)Per CLAUDE.md: Every new AWS operation MUST have unit tests. Missing test files:
This is a blocking issue per project guidelines. Code Quality Issues1. Unused Variable Warning Suppression (pkg/assertions/aws/lambda.go:84) Current code declares notFoundErr but never uses it, then suppresses the warning. Use proper type assertion with errors.As instead of strings.Contains to validate error type. 2. Unnecessary Import Suppression (pkg/steps/aws/lambda.go:322) strconv is imported but never used. Remove the import instead of suppressing the warning. 3. Context.TODO() Usage Multiple assertions use context.TODO() instead of accepting and propagating context from callers. This prevents proper timeout/cancellation propagation and distributed tracing. Recommendation: Refactor assertion methods to accept context.Context parameter. Additional ObservationsPositive:
Suggestions:
Security ReviewNo concerns identified. Proper IAM policies, no credential logging, function URLs correctly configured. PerformanceGood practices: single API calls, helper methods prevent redundancy, Virtual Cloud enables fast testing. Test Plan Status
RecommendationsBefore Merge (MUST):
Before Merge (SHOULD):
Nice to Have:
Overall AssessmentScore: 7.5/10 High-quality work that adds significant value. Main blocker is missing unit tests (mandatory per guidelines). Architecture is sound, follows conventions, comprehensive coverage. Effort to fix: approximately 4-6 hours for unit tests + 30 min for code quality fixes. Great job on the feature design and implementation! |
- Use errors.As() instead of strings.Contains() for proper AWS SDK error type checking in AssertFunctionNotExists - Add null safety check for empty runtime in AssertFunctionRuntime - Remove unused strconv import and associated hack 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Pull Request Review - Lambda BDD Testing SupportI've completed a thorough review of this PR. Overall, this is excellent work that adds comprehensive Lambda testing support following the project's established patterns. Here's my detailed feedback: ✅ Strengths1. Excellent Code Organization & Consistency
2. Comprehensive Coverage
3. Strong Error Handling// Example from lambda.go:79-83
var notFoundErr *types.ResourceNotFoundException
if \!errors.As(err, ¬FoundErr) {
return fmt.Errorf("unexpected error checking Lambda function %s: %w", functionName, err)
}
4. Virtual Cloud Integration
5. Terraform Examples Quality
🔍 Issues & Concerns1.
|
Summary
Adds full BDD testing infrastructure for AWS Lambda, enabling Gherkin-based tests for Lambda functions with the Virtual Cloud emulator.
New Files
Core Implementation
pkg/awshelpers/lambda.go- Lambda SDK client helper with Virtual Cloud endpoint supportpkg/assertions/aws/lambda.go- LambdaAsserter interface with 15+ assertion methodspkg/steps/aws/lambda.go- 25+ Gherkin step definitionsTerraform Examples
examples/aws/lambda/basic-function/- Basic Lambda with configurable runtime, handler, timeout, memoryexamples/aws/lambda/function-with-alias/- Lambda with published version and aliasexamples/aws/lambda/function-url/- Lambda with public or IAM-authenticated function URLFeature Files
features/aws/lambda/lambda_function.feature- Basic function creation and configuration testsfeatures/aws/lambda/lambda_alias.feature- Version and alias testsfeatures/aws/lambda/lambda_function_url.feature- Function URL testsSupported Assertions
AssertFunctionExists,AssertFunctionNotExistsTest plan
go build ./...passesmake go-test-coverpasses./bin/infraspec features/aws/lambda/ --virtual-cloud🤖 Generated with Claude Code