-
Notifications
You must be signed in to change notification settings - Fork 9
chore: before pushing to prod #166
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
base: develop
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces comprehensive post-deployment setup documentation and infrastructure enhancements. Key changes include GitHub Actions workflow version upgrades, DynamoDB table initialization setup, IAM policy additions to Lambda functions, enhanced JWT logging with defensive null checks, and a post-deployment guide for configuration and verification. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Lambda as Lambda Functions
participant DynamoDB as DynamoDB<br/>(featureFlag,<br/>featureFlagUserMapping,<br/>requestLimit)
participant SSM as Parameter Store<br/>(Public Keys)
User->>Lambda: 1. Deploy application
Lambda->>DynamoDB: 2. Create tables<br/>(setup-dynamodb-tables.sh)
DynamoDB-->>Lambda: Tables created & initialized
User->>SSM: 3. Verify/Create SSM<br/>parameters (PROD/STAGING<br/>public keys)
SSM-->>User: Parameters ready
Lambda->>SSM: 4. Fetch JWT public key<br/>(with logging & defensive checks)
SSM-->>Lambda: Return key (trimmed)
Lambda->>Lambda: 5. Parse & verify key<br/>(PEM → PKIX → RSA)
Lambda-->>Lambda: JWT utils initialized
User->>Lambda: 6. Test API endpoints<br/>(health check,<br/>feature-flag operations)
Lambda->>DynamoDB: 7. Execute operations<br/>(read/write requests<br/>with IAM permissions)
DynamoDB-->>Lambda: Return results
Lambda-->>User: Success response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
template.yaml (1)
190-410: Remove unused DynamoDB table permissions from functions.Functions are over-provisioned with permissions to tables they don't access:
getFeatureFlagByIdandgetAllFeatureFlags: RemoveDynamoDBCrudPolicyforfeatureFlagUserMappinggetUserFeatureFlagandgetUserFeatureFlags: RemoveDynamoDBCrudPolicyforfeatureFlagcreateFeatureFlagandupdateFeatureFlag: RemoveDynamoDBCrudPolicyforfeatureFlagUserMappingcreateUserFeatureFlagandupdateUserFeatureFlag: RemoveDynamoDBCrudPolicyforfeatureFlagAll functions require write access to
requestLimit(not read-only) becauseCheckRequestAllowedactively decrements the rate limit counter withPutItemoperations.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/pipeline.yamlPOST_DEPLOYMENT_SETUP.mdlayer/jwt/jwt.gosetup-dynamodb-tables.shtemplate.yaml
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/pipeline.yaml
22-22: shellcheck reported issue in this script: SC2086:info:1:42: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 LanguageTool
POST_DEPLOYMENT_SETUP.md
[style] ~63-~63: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...t` Table After creating the table, you need to add an initial value: ```bash aws dyna...
(REP_NEED_TO_VB)
🪛 markdownlint-cli2 (0.18.1)
POST_DEPLOYMENT_SETUP.md
184-184: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
🪛 Shellcheck (0.11.0)
setup-dynamodb-tables.sh
[info] 22-22: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 39-39: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 54-54: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 64-64: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 68-68: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 72-72: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 85-85: Double quote to prevent globbing and word splitting.
(SC2086)
🔇 Additional comments (8)
.github/workflows/pipeline.yaml (3)
17-25: LGTM! Modern GitHub Actions syntax.The upgrade to use
$GITHUB_OUTPUTinstead of the deprecatedset-outputcommand is correct and aligns with current GitHub Actions best practices. The action version upgrades are also appropriate.
55-129: LGTM! Comprehensive GitHub Actions upgrades.The workflow upgrades are well-coordinated:
- AWS credentials action upgraded to v4 for improved security
- Artifact upload/download actions upgraded to v6 (consistent versions)
- Python setup upgraded to v6 with Python 3.11
- SAM CLI setup upgraded to v2 with installer approach
These changes modernize the CI/CD pipeline appropriately.
34-36: No issues found. Go version 1.25.1 is a valid, released version as of January 2026 (the latest patch in the 1.25 series is 1.25.5). The specified version in the workflow is acceptable.Likely an incorrect or invalid review comment.
layer/jwt/jwt.go (3)
88-117: Excellent defensive coding and observability improvements!The enhancements significantly improve debuggability:
- Comprehensive logging at each initialization step aids troubleshooting
TrimSpaceon line 94 addresses SSM parameter formatting issues (trailing whitespace)- Safe substring using
min()on line 98 prevents panics when logging truncated keys- Defensive nil checks ensure robust error handling
- Security is maintained by only logging sanitized/truncated information
These changes directly address the production JWT authentication issues mentioned in the PR objectives.
119-148: Strong defensive programming with SSM parameter handling.The nil checks on lines 141-144 are essential defensive measures:
- Handles edge case where
ParameterorValuemight be nil- Provides clear error message for debugging
TrimSpaceon line 146 ensures consistency with public key trimmingThe enhanced logging throughout this function will significantly aid in diagnosing SSM-related issues in production.
150-155: LGTM! Simple and effective helper.The
min()helper prevents potential panics when taking substrings for logging. The implementation is straightforward and appropriate for integer comparison.POST_DEPLOYMENT_SETUP.md (1)
1-91: Comprehensive and well-structured documentation!The post-deployment guide provides excellent coverage:
- Dual approach (script-based and manual) accommodates different user preferences
- Clear explanations of required DynamoDB tables and their schemas
- Verification commands help users confirm successful setup
- Aligns perfectly with the
setup-dynamodb-tables.shscripttemplate.yaml (1)
1-416: Strong infrastructure-as-code migration!Moving IAM policies from manual configuration into
template.yamlsignificantly improves:
- Infrastructure reproducibility and version control
- Consistency across environments (PROD/DEV)
- Alignment with AWS best practices
The policy structure is well-organized and parameter names correctly match those referenced in
layer/jwt/jwt.goandPOST_DEPLOYMENT_SETUP.md.
Date: 02 Jan 2026
Developer Name: @lakshayman
Description
This PR includes several improvements and fixes to the feature flag backend:
JWT Authentication Improvements: Enhanced JWT token validation with better error logging and handling of public key whitespace issues. Added detailed logging to help debug SSM parameter retrieval and public key parsing failures.
Infrastructure Setup: Added DynamoDB table setup script (
setup-dynamodb-tables.sh) and comprehensive post-deployment setup documentation (POST_DEPLOYMENT_SETUP.md) to streamline deployment and testing processes.IAM Policy Fixes: Moved IAM policies from manual configuration to template.yaml for better infrastructure-as-code practices and easier management.
Pipeline Fixes: Resolved CI/CD pipeline issues to ensure smooth deployments.
Code Refactoring: Migrated shared code to AWS Lambda Layer architecture (from previous PR refactor: migrate shared code to AWS Lambda Layer #165), improving code reusability and maintainability.
Key Changes:
Documentation Updated?
Added
POST_DEPLOYMENT_SETUP.mdwith comprehensive deployment and testing instructions. Updated test scripts and documentation.Under Feature Flag
These are infrastructure and authentication improvements, not feature changes.
Database Changes
Added setup script for DynamoDB tables (
setup-dynamodb-tables.sh). No schema changes to existing tables.Breaking Changes
No breaking changes. All changes are backward compatible.
Development Tested?
Tested locally and in production:
Screenshots
JWT Authentication Success
Test Coverage
API Testing Results
All endpoints tested and verified:
Additional Notes
JWT Authentication: Fixed issues with public key retrieval from SSM Parameter Store by adding whitespace trimming and comprehensive error logging. This helps identify issues during initialization.
Error Logging: Added detailed logging at each step of JWT initialization to help debug authentication failures:
Setup Scripts: The
setup-dynamodb-tables.shscript automates DynamoDB table creation for local development and testing.Documentation:
POST_DEPLOYMENT_SETUP.mdprovides step-by-step instructions for:Testing: All changes have been tested in production environment with successful JWT authentication and feature flag operations.