feat: add API versioning to Docker build and Swagger documentation #43
Conversation
WalkthroughThe changes made in this pull request involve updates to the GitHub Actions workflow, Dockerfile, and the Program.cs file of the Bmb.Payment.Api project. A new build argument Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant Docker
participant API
Developer->>GitHub Actions: Push changes
GitHub Actions->>Docker: Build and push Docker image with API_VERSION
Docker->>API: Publish API with dynamic version
API->>GitHub Actions: Notify build completion
GitHub Actions->>Developer: Notify success
Warning Rate limit exceeded@italopessoa has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (3)src/Bmb.Payment.Api/Bmb.Payment.Api.csproj (3)
The version and product properties look good, but let's ensure the version matches across Docker configurations and workflow files. ✅ Verification successfulVersion configuration is properly integrated across the build system The version consistency is well-maintained:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check version consistency across Docker and workflow files
# Look for version references in Dockerfile and workflow files
echo "Checking version references in Dockerfile:"
rg -A 2 'ARG API_VERSION' Dockerfile
echo "Checking version references in workflow:"
rg -A 2 'API_VERSION' .github/workflows/
Length of output: 576 Script: #!/bin/bash
# Check for Dockerfile in the correct location and its content
echo "Checking for Dockerfile in src/Bmb.Payment.Api:"
fd Dockerfile src/Bmb.Payment.Api
echo "Content of Dockerfile if found:"
cat src/Bmb.Payment.Api/Dockerfile 2>/dev/null
echo "Checking version in workflow file:"
cat .github/workflows/build-and-deploy.yaml
Length of output: 9758
The replacement of ✅ Verification successfulLet me gather more information about authentication-related code and the new Bmb.Tools package usage. Authentication functionality is properly migrated to Bmb.Tools The codebase shows that the authentication functionality has been successfully migrated from
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for authentication-related code that might need updates
echo "Checking for auth-related code:"
rg -l 'using.*Bmb.Auth'
echo "Checking for potential auth implementations:"
ast-grep --pattern 'class $_ : $AuthHandler { $$$ }'
Length of output: 252 Script: #!/bin/bash
# Check for any authentication/authorization related code and Bmb.Tools usage
echo "Checking for authentication/authorization related code:"
rg -i "auth|authenticate|authorize" -t cs -A 3
echo -e "\nChecking for Bmb.Tools usage:"
rg "using.*Bmb.Tools" -t cs -A 1
echo -e "\nChecking Program.cs and Startup.cs for auth configuration:"
fd -e cs -E "*Test*" "Program.cs|Startup.cs" --exec cat {}
Length of output: 23880
The major version update of Swashbuckle.AspNetCore (6.4.0 -> 7.0.0) may introduce breaking changes. The addition of Filters package is good for API versioning support. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
src/Bmb.Payment.Api/Dockerfile (1)
18-18: Consider adding documentation and default value for API_VERSION argumentWhile the argument is correctly placed, it would be beneficial to:
- Add a comment explaining its purpose
- Provide a default value to prevent build failures when not specified
-ARG API_VERSION +# Version to be embedded in the published assembly +ARG API_VERSION=1.0.0src/Bmb.Payment.Api/Program.cs (2)
26-26: Consider extracting version retrieval to avoid duplicationThe assembly version is retrieved twice (here and at line 114). Consider extracting this into a single variable at the start of the method to improve maintainability and handle potential null values.
public static void Main(string[] args) { var builder = WebApplication.CreateBuilder(args); ILogger<Program>? logger = null; try { + var version = Assembly.GetExecutingAssembly().GetName().Version + ?? throw new InvalidOperationException("Assembly version is not set"); - var version = Assembly.GetExecutingAssembly().GetName().Version;
Line range hint
26-116: Architecture Review: Changes align with Clean Architecture principlesThe versioning implementation:
- Maintains separation of concerns
- Follows single responsibility principle
- Properly integrates with the existing configuration setup
- Provides good error handling and logging
The changes successfully achieve the PR objective of adding API versioning while maintaining the architectural integrity of the application.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
.github/workflows/build-and-deploy.yaml(1 hunks)src/Bmb.Payment.Api/Dockerfile(1 hunks)src/Bmb.Payment.Api/Program.cs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/Bmb.Payment.Api/Program.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
🔇 Additional comments (4)
src/Bmb.Payment.Api/Dockerfile (1)
19-19: Improve command readability and verify version propagation
The publish command is becoming lengthy. Consider improving readability by splitting parameters across multiple lines.
-RUN dotnet publish "src/Bmb.Payment.Api/Bmb.Payment.Api.csproj" -c $BUILD_CONFIGURATION -o /app/publish /p:UseAppHost=false /p:Version=$API_VERSION
+RUN dotnet publish "src/Bmb.Payment.Api/Bmb.Payment.Api.csproj" \
+ -c $BUILD_CONFIGURATION \
+ -o /app/publish \
+ /p:UseAppHost=false \
+ /p:Version=$API_VERSIONLet's verify how the version parameter affects the assemblies:
✅ Verification successful
Command readability improvement is valid, version parameter is properly propagated
The suggested command readability improvement is beneficial. Regarding version propagation:
- The API project references three other projects (Masstransit, Controllers, and DI)
- No AssemblyInfo files were found in the referenced projects that would override versioning
- No explicit version-related properties were found in the referenced projects' csproj files
- The
/p:Versionparameter in the publish command will properly cascade to all referenced projects
The original suggestion to improve command readability stands, and the version parameter will work as intended.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the project references other projects that might need version synchronization
# Look for project references in the API project
rg -A 5 "ProjectReference" "src/Bmb.Payment.Api/Bmb.Payment.Api.csproj"
# Check if there are any AssemblyInfo files that might override versioning
fd -e cs "AssemblyInfo.cs" src/
Length of output: 528
Script:
#!/bin/bash
# Check the content of AssemblyInfo.cs to see if it overrides version
cat "src/Bmb.Payment.Api/Properties/AssemblyInfo.cs"
# Check if referenced projects have their own AssemblyInfo files
fd -e cs "AssemblyInfo.cs" "src/Bmb.Payment.Masstransit" "src/Bmb.Payment.Controllers" "src/Bmb.Payment.DI"
# Check if referenced projects have version-related properties in their csproj files
rg -g "*.csproj" "Version|AssemblyVersion|FileVersion|InformationalVersion" "src/Bmb.Payment.Masstransit" "src/Bmb.Payment.Controllers" "src/Bmb.Payment.DI"
Length of output: 1126
src/Bmb.Payment.Api/Program.cs (1)
59-61: Verify Swagger endpoint format and version display
The configuration looks good, but there are two points to consider:
- The version format follows semantic versioning (major.minor.build) which is good practice.
- The endpoint at line 115 uses
.yamlextension - verify if this is intentional as.jsonis more commonly used.
.github/workflows/build-and-deploy.yaml (2)
Line range hint 192-205: LGTM! Environment variables are properly configured.
The API_IMAGE_TAG is correctly referenced from the build-docker-image job output and properly used to set the api_docker_image Terraform variable.
170-171: LGTM! Verify version propagation.
The addition of the API_VERSION build argument is well-implemented and aligns with the PR objective of adding API versioning to the Docker build.
Let's verify that the version is correctly propagated through the workflow:
✅ Verification successful
Version propagation verified successfully
The Dockerfile correctly accepts and utilizes the API_VERSION build argument:
- ARG API_VERSION is properly declared
- The version is correctly passed to the dotnet publish command via /p:Version=$API_VERSION
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the Dockerfile accepts and uses the API_VERSION build arg
# Check if Dockerfile accepts the build arg
rg -A 5 'ARG API_VERSION' src/Bmb.Payment.Api/Dockerfile
# Check if the version is used in the dotnet publish command
rg -A 5 '/p:Version=\$API_VERSION' src/Bmb.Payment.Api/Dockerfile
Length of output: 541
src/Bmb.Payment.Api/Program.cs
Outdated
| app.UseSwaggerUI(options => | ||
| { | ||
| var version = Assembly.GetExecutingAssembly().GetName().Version.Major; | ||
| options.SwaggerEndpoint($"/swagger/v{version}/swagger.yaml", $"v{version}"); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor Swagger UI configuration to reuse version
The Swagger UI configuration can be improved:
- Reuse the version variable from line 26 instead of retrieving it again.
- Consider using
.jsonformat for consistency unless YAML is specifically required.
- app.UseSwaggerUI(options =>
- {
- var version = Assembly.GetExecutingAssembly().GetName().Version.Major;
- options.SwaggerEndpoint($"/swagger/v{version}/swagger.yaml", $"v{version}");
- });
+ app.UseSwaggerUI(options =>
+ {
+ options.SwaggerEndpoint($"/swagger/v{version.Major}/swagger.json", $"v{version.Major}");
+ });Committable suggestion skipped: line range outside the PR's diff.
|
No description provided.