-
Notifications
You must be signed in to change notification settings - Fork 3
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
chore(deps): update localstack/localstack and Spring Boot #807
Conversation
WalkthroughThe pull request includes updates to multiple files within the Changes
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 3
🧹 Outside diff range and nitpick comments (3)
aws-sqs-project/src/test/java/com/learning/awspring/utils/FakeObjectCreator.java (1)
12-13
: Good security improvement using secure random generation!The switch to
RandomStringUtils.secure()
is a positive change as it usesSecureRandom
internally, which is cryptographically strong compared to the previous implementation.Consider adding a comment explaining why secure random generation was chosen, even for test data:
public static SQSMessagePayload createMessage() { + // Using secure random generation to maintain consistency with production security practices return new SQSMessagePayload( RandomStringUtils.secure().nextNumeric(3), RandomStringUtils.secure().nextAlphanumeric(100)); }
aws-sqs-project/docker/docker-compose.yml (1)
Line range hint
19-28
: Consider adding health check for LocalStack serviceWhile PostgreSQL service has health checks configured, LocalStack service doesn't. This could lead to race conditions where dependent services might start before LocalStack is ready.
Consider adding health check configuration:
localstack: image: localstack/localstack:4.0.3 environment: - AWS_DEFAULT_REGION=us-east-1 - EDGE_PORT=4566 - SERVICES=sqs ports: - '4566:4566' + healthcheck: + test: ["CMD", "curl", "-f", "http://localhost:4566/_localstack/health"] + interval: 10s + timeout: 5s + retries: 5 volumes: - "${LOCALSTACK_VOLUME_DIR:-./volume}:/var/lib/localstack" - "/var/run/docker.sock:/var/run/docker.sock" - '../.localstack:/docker-entrypoint-initaws.d'aws-sqs-project/src/test/java/com/learning/awspring/config/LocalStackTestContainers.java (1)
14-18
: Consider adding explicit service initializationLocalStack 4.x may require explicit service initialization. Consider adding required services using
.withServices()
to ensure all needed AWS services are properly initialized.Example enhancement:
LocalStackContainer localstackContainer() { return new LocalStackContainer( DockerImageName.parse("localstack/localstack").withTag("4.0.3")) + .withServices(LocalStackContainer.Service.SQS) // Add other services as needed .withReuse(true); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
aws-sqs-project/docker/docker-compose.yml
(1 hunks)aws-sqs-project/pom.xml
(1 hunks)aws-sqs-project/src/test/java/com/learning/awspring/config/LocalStackTestContainers.java
(1 hunks)aws-sqs-project/src/test/java/com/learning/awspring/config/SQLTestcontainersConfig.java
(1 hunks)aws-sqs-project/src/test/java/com/learning/awspring/repositories/SchemaValidationTest.java
(1 hunks)aws-sqs-project/src/test/java/com/learning/awspring/utils/FakeObjectCreator.java
(1 hunks)aws-sqs-project/src/test/java/com/learning/awspring/web/controllers/CustomerControllerTest.java
(1 hunks)aws-sqs-project/src/test/java/com/learning/awspring/web/controllers/InboundLogControllerTest.java
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- aws-sqs-project/src/test/java/com/learning/awspring/config/SQLTestcontainersConfig.java
- aws-sqs-project/pom.xml
🔇 Additional comments (4)
aws-sqs-project/src/test/java/com/learning/awspring/web/controllers/InboundLogControllerTest.java (1)
32-32
: LGTM! Import change aligns with annotation update.
The import change correctly reflects the move to using @MockitoBean
from the Spring Test Context Framework.
aws-sqs-project/docker/docker-compose.yml (1)
19-19
: Verify LocalStack major version upgrade compatibility
The update from 3.8.1 to 4.0.3 is a major version upgrade that might include breaking changes.
Let's verify the changes in LocalStack 4.x that might affect the current setup:
✅ Verification successful
Based on the search results, I can now make an informed decision about the LocalStack version upgrade. Let me analyze the key findings:
-
The project uses LocalStack in two contexts:
- Docker Compose for local development (being updated to 4.0.3)
- Test containers for integration tests (already updated to 4.0.3 in
LocalStackTestContainers.java
)
-
The LocalStack usage is consistent across the codebase:
- Standard port mapping (4566)
- Basic AWS service configurations
- No custom or deprecated LocalStack-specific features
-
The configuration patterns are simple and standard:
- Volume mounts for initialization scripts
- Basic environment variables
- AWS endpoint configurations
LocalStack version 4.0.3 upgrade is compatible with current usage
The update from 3.8.1 to 4.0.3 is safe because:
- The codebase uses standard AWS SDK interfaces and basic LocalStack features
- Test containers are already using 4.0.3 successfully
- No deprecated features or breaking changes are being used
- The configuration patterns remain unchanged in 4.x
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check LocalStack release notes and current usage
# Check if there are any direct LocalStack API calls that might be affected
# Search for LocalStack related configurations and usage
rg -g '!docker-compose.yml' -g '*.{java,yml,properties}' -i 'localstack|aws.endpoint'
# Check test files specifically as they often contain LocalStack configuration
rg -g '*Test.java' -g '*IT.java' 'LocalStack'
Length of output: 28092
aws-sqs-project/src/test/java/com/learning/awspring/repositories/SchemaValidationTest.java (1)
10-10
: LGTM!
The import is correctly added to support the new @AutoConfigureTestDatabase
annotation.
aws-sqs-project/src/test/java/com/learning/awspring/config/LocalStackTestContainers.java (1)
16-16
: Heads up: Major version upgrade from LocalStack 3.x to 4.x
The upgrade from LocalStack 3.7.2 to 4.0.3 is a major version jump that might introduce breaking changes. While the code change itself is correct, please ensure you've reviewed the LocalStack migration guide for any necessary adjustments in your test suite.
Let's verify the AWS services used in the tests:
✅ Verification successful
LocalStack 4.0.3 upgrade is compatible with current SQS test usage
Based on the verification results, the upgrade to LocalStack 4.0.3 is safe for this codebase:
- The project only uses SQS service, as evidenced by the integration tests
- The test configuration is using Spring Boot's
@ServiceConnection
which handles service initialization - The SQS integration tests use AWS SDK v2 (software.amazon.awssdk) which is fully supported in LocalStack 4.x
- No custom LocalStack configuration is required as the tests use standard SQS operations (createQueue, sendMessage, receiveMessage)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find AWS services used in tests to verify compatibility
# Look for AWS service client instantiations and LocalStack service enablement
# Search for AWS client usage in tests
echo "AWS Services used in tests:"
rg -t java "AmazonSQS|AmazonS3|AmazonDynamoDB|AmazonKinesis|AmazonSNS" aws-sqs-project/src/test/
Length of output: 163
Script:
#!/bin/bash
# Let's try a different approach to find AWS service usage
# 1. Look for AWS SDK dependencies in pom.xml
echo "AWS SDK Dependencies:"
rg -A 2 "aws-java-sdk|software.amazon" pom.xml
# 2. Search for AWS service-related annotations and configurations
echo -e "\nAWS Service Usage in Tests:"
rg -t java "(@SqsListener|@QueueListener|@S3|@DynamoDB|@Kinesis|@SNS)" aws-sqs-project/src/test/
# 3. Search for AWS service-related imports
echo -e "\nAWS Service Imports:"
rg -t java "^import.*aws.*" aws-sqs-project/src/test/
# 4. Look for LocalStack container configuration
echo -e "\nLocalStack Container Configuration:"
rg -t java "LocalStackContainer|enableServices|getEndpoint" aws-sqs-project/src/test/
Length of output: 6284
Script:
#!/bin/bash
# Let's examine the SQS configuration and usage more closely
# 1. Check the main application.yml/properties for SQS configuration
echo "Application Properties SQS Configuration:"
rg -g "application*.{yml,properties}" "sqs|aws" aws-sqs-project/src/
# 2. Look for SQS-specific code in the test files
echo -e "\nSQS Configuration in Tests:"
cat aws-sqs-project/src/test/java/com/learning/awspring/config/LocalStackTestContainers.java
# 3. Check SQS integration test
echo -e "\nSQS Integration Test Content:"
cat aws-sqs-project/src/test/java/com/learning/awspring/SqsListenerIntegrationTest.java
Length of output: 5697
...qs-project/src/test/java/com/learning/awspring/web/controllers/InboundLogControllerTest.java
Show resolved
Hide resolved
aws-sqs-project/src/test/java/com/learning/awspring/web/controllers/CustomerControllerTest.java
Show resolved
Hide resolved
aws-sqs-project/src/test/java/com/learning/awspring/repositories/SchemaValidationTest.java
Show resolved
Hide resolved
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: 1
🧹 Outside diff range and nitpick comments (3)
aws-sns-project/src/test/java/com/learning/awssns/common/ContainersConfig.java (1)
Line range hint
11-17
: Consider adding explicit service initializationWhile the current configuration is functional, consider explicitly enabling required AWS services using
withServices()
to make the dependencies clear and improve startup time.Example enhancement:
@Bean @ServiceConnection LocalStackContainer localstackContainer() { return new LocalStackContainer( - DockerImageName.parse("localstack/localstack").withTag("4.0.3")); + DockerImageName.parse("localstack/localstack").withTag("4.0.3")) + .withServices(LocalStackContainer.Service.SNS); }aws-sns-project/src/main/java/com/learning/awssns/config/WebMvcConfig.java (2)
19-22
: Consider adding parameter validation in the constructorWhile the explicit constructor is clearer than using Lombok, it should validate its parameters since they're critical dependencies.
Consider adding validation:
public WebMvcConfig(ApplicationProperties properties, SnsClient snsClient) { + if (properties == null) { + throw new IllegalArgumentException("ApplicationProperties must not be null"); + } + if (snsClient == null) { + throw new IllegalArgumentException("SnsClient must not be null"); + } this.properties = properties; this.snsClient = snsClient; }
Security Warning: Overly Permissive CORS Configuration
The CORS configuration in
ApplicationProperties.java
uses wildcard (*) values for all major CORS settings with credentials allowed, which is a security risk:
allowedMethods = "*"
: Allows all HTTP methodsallowedHeaders = "*"
: Allows all headersallowedOriginPatterns = "*"
: Allows requests from any originallowCredentials = true
: Allows credentials with wildcard originsThis combination violates CORS security best practices. When
allowCredentials
is true, wildcard origins should not be used as it exposes the application to cross-site attacks.Recommendations:
- Explicitly specify allowed HTTP methods (e.g., "GET", "POST", "PUT")
- Define specific allowed headers
- List specific allowed origins instead of using wildcards
- Consider setting
allowCredentials
to false if wide access is needed🔗 Analysis chain
Line range hint
25-31
: LGTM! Verify CORS configuration for securityThe @nonnull addition is appropriate. However, since this is a security-related configuration:
Let's verify the CORS settings:
The previous search might have failed because:
- The specific "cors:" pattern was too strict
- We need to check both yaml/properties and Java files
- We should locate the ApplicationProperties class to see how CORS is configured
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check CORS configuration values # Look for CORS-related properties in application.yml/properties files rg -l "cors:" . | xargs catLength of output: 27
Script:
#!/bin/bash # Let's try different approaches to find CORS configuration # 1. Search for CORS-related properties in yaml/properties files rg -t yaml -t properties "cors" . # 2. Search for CORS configuration in Java files rg "cors" -t java . # 3. Look specifically for ApplicationProperties class that was referenced fd -e java "ApplicationProperties"Length of output: 2595
Script:
#!/bin/bash # Let's examine the specific CORS configuration in the SNS project's ApplicationProperties cat aws-sns-project/src/main/java/com/learning/awssns/config/ApplicationProperties.java # Also look for any application.yml/properties in the SNS project fd -e yml -e yaml -e properties . aws-sns-project/src/main/resources/Length of output: 939
Script:
#!/bin/bash # Let's check the application properties files for any CORS overrides cat aws-sns-project/src/main/resources/application.properties cat aws-sns-project/src/main/resources/application-local.propertiesLength of output: 893
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
aws-sns-project/docker/docker-compose.yml
(1 hunks)aws-sns-project/pom.xml
(1 hunks)aws-sns-project/src/main/java/com/learning/awssns/config/WebMvcConfig.java
(2 hunks)aws-sns-project/src/test/java/com/learning/awssns/TestSNSApplication.java
(1 hunks)aws-sns-project/src/test/java/com/learning/awssns/common/ContainersConfig.java
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- aws-sns-project/docker/docker-compose.yml
- aws-sns-project/pom.xml
🔇 Additional comments (3)
aws-sns-project/src/test/java/com/learning/awssns/TestSNSApplication.java (1)
4-4
: LGTM! Good improvement in profile configuration.
The change from hardcoded string to AppConstants.PROFILE_LOCAL
and the use of withAdditionalProfiles()
is a better approach as it:
- Eliminates magic strings
- Uses Spring Boot's preferred configuration method
- Maintains consistency across the codebase
Let's verify the constant value matches the previous configuration:
Also applies to: 12-12
✅ Verification successful
✓ Constant value correctly matches the previous configuration
The verification confirms that AppConstants.PROFILE_LOCAL
is defined as "local" in the constants file, which matches the previous hardcoded string value. The constant is properly used in TestSNSApplication.java
and there are no inconsistencies in profile configurations within the SNS project.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the PROFILE_LOCAL constant value matches the previous "local" configuration
# and check for any other profile-related constants or configurations
# Check the constant definition
rg -l "PROFILE_LOCAL\s*=\s*\"local\"" --type java
# Check for any other profile configurations that might need similar updates
rg "spring\.profiles\.active" --type java
Length of output: 547
Script:
#!/bin/bash
# Let's verify the actual value of PROFILE_LOCAL constant in the AppConstants class
ast-grep --pattern 'PROFILE_LOCAL = "$_"' aws-sns-project/src/main/java/com/learning/awssns/utils/AppConstants.java
# Also check if there are any other usages of this constant in the SNS project
rg "PROFILE_LOCAL" --type java aws-sns-project/
Length of output: 437
aws-sns-project/src/main/java/com/learning/awssns/config/WebMvcConfig.java (2)
7-7
: LGTM! Appropriate use of Spring's @nonnull annotation
Using Spring's own nullability annotations is the correct choice for a Spring application.
34-36
: LGTM! Verify SNS configuration
The @nonnull addition and SNS configuration look correct.
Let's verify the SNS configuration:
✅ Verification successful
Let me gather more information about the SNS configuration by checking the code.
Let me check the actual SNS configuration implementation and usage.
Let me check one final aspect - the SNS configuration class and beans.
SNS configuration is properly implemented and integrated
The verification shows that:
- SNS client is correctly autowired in WebMvcConfig
- NotificationHandlerMethodArgumentResolver is properly configured using the official AWS Spring Cloud utility
- Integration tests demonstrate the SNS setup with both SnsTemplate and SnsSmsTemplate
- LocalStack is configured in docker-compose for SNS/SQS services
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check SNS configuration and usage
# Look for SNS-related configuration
rg -A 5 "sns" --glob '*.{yml,properties}'
Length of output: 1827
Script:
#!/bin/bash
# Check SNS client configuration and related code
ast-grep --pattern 'class $_ {
$$$
snsClient($$$) {
$$$
}
$$$
}'
# Also check for any SNS related properties or configuration
rg -l "aws.sns" --type java
rg -l "SnsClient" --type java
Length of output: 509
Script:
#!/bin/bash
# Check the SNS client configuration implementation
rg -A 10 "SnsClient" --type java
# Check for AWS SNS related beans and configuration
ast-grep --pattern 'class $_ {
$$$
@Bean
$$$
SnsClient $$$($$$) {
$$$
}
$$$
}'
Length of output: 4888
Script:
#!/bin/bash
# Look for AWS configuration class and SNS related beans
rg -A 15 "@Configuration.*AWS" --type java
rg -A 15 "SnsAutoConfiguration" --type java
# Check for NotificationHandlerMethodArgumentResolver usage
rg -A 5 "NotificationHandlerMethodArgumentResolver" --type java
Length of output: 1285
@@ -13,6 +13,6 @@ public class ContainersConfig { | |||
@ServiceConnection | |||
LocalStackContainer localstackContainer() { | |||
return new LocalStackContainer( | |||
DockerImageName.parse("localstack/localstack").withTag("3.7.2")); | |||
DockerImageName.parse("localstack/localstack").withTag("4.0.3")); |
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.
💡 Codebase verification
Version upgrade requires project-wide synchronization
The analysis reveals significant version inconsistencies that need attention:
- Only aws-sns-project and aws-sqs-project use LocalStack 4.0.3
- Other projects use older versions (3.7.1, 3.7.2, 3.8.1)
- Configuration differences exist between versions (e.g., environment variables, port mappings)
Key findings:
- 4.0.3 configurations use
EDGE_PORT
instead of legacy port mappings - 3.x versions have additional port ranges (4510-4559) in some projects
- Different environment variable patterns between versions
🔗 Analysis chain
Verify LocalStack 4.0.3 compatibility
The upgrade from 3.x to 4.x is a major version change that might include breaking changes.
Let's verify the LocalStack version consistency across the project and check for any known issues:
Version inconsistency detected across LocalStack configurations
The codebase has mixed LocalStack versions that need to be aligned:
- Java test files use versions: 3.7.1, 3.7.2, and 4.0.3
- Docker compose files use versions: 3.8.1 and 4.0.3
Let's verify if there are any breaking changes or configuration differences between these versions:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check LocalStack version consistency and configuration requirements
# Check LocalStack versions across all files
echo "Checking LocalStack versions in docker-compose files:"
fd docker-compose.yml --exec grep -l "localstack/localstack"
echo -e "\nChecking LocalStack versions in Java test files:"
rg "localstack/localstack" --type java
# Check if there are any custom LocalStack configurations that might need updates
echo -e "\nChecking for LocalStack environment variables and configurations:"
fd docker-compose.yml --exec grep -A 5 "localstack:"
Length of output: 5784
Script:
#!/bin/bash
# Check for configuration differences between versions
echo "Checking 4.0.3 configurations:"
rg -B 2 -A 5 "localstack/localstack:4\.0\.3" --type yaml
echo -e "\nChecking 3.x configurations:"
rg -B 2 -A 5 "localstack/localstack:3\." --type yaml
# Check for any version-specific environment variables or settings in test files
echo -e "\nChecking LocalStack container configurations in test files:"
rg "LocalStackContainer\(" -A 5 --type java
Length of output: 16447
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Tests
Chores