-
Notifications
You must be signed in to change notification settings - Fork 96
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
Consul integration with serviceName prefix and ConsulClient library #313
base: master
Are you sure you want to change the base?
Changes from all commits
c71cd65
b3d41e6
9486732
68e34af
da22005
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
<Project Sdk="Microsoft.NET.Sdk.Web"> | ||
|
||
<PropertyGroup> | ||
<TargetFramework>net8.0</TargetFramework> | ||
<Nullable>enable</Nullable> | ||
<ImplicitUsings>enable</ImplicitUsings> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="8.0.4"/> | ||
<PackageReference Include="Swashbuckle.AspNetCore" Version="6.5.0" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<ProjectReference Include="..\..\..\src\MMLib.ServiceDiscovery.Consul\MMLib.ServiceDiscovery.Consul.csproj" /> | ||
</ItemGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
using MMLib.ServiceDiscovery.Consul.DependencyInjection; | ||
|
||
var builder = WebApplication.CreateBuilder(args); | ||
|
||
// Add services to the container. | ||
// Learn more about configuring Swagger/OpenAPI at https://aka.ms/aspnetcore/swashbuckle | ||
builder.Services.AddEndpointsApiExplorer(); | ||
builder.Services.AddSwaggerGen(); | ||
|
||
builder.AddConsulAutoServiceDiscovery("http://localhost:8500"); //This line added | ||
|
||
var app = builder.Build(); | ||
|
||
// Configure the HTTP request pipeline. | ||
if (app.Environment.IsDevelopment()) | ||
{ | ||
app.UseSwagger(); | ||
app.UseSwaggerUI(); | ||
} | ||
|
||
app.UseHttpsRedirection(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential mismatch between HTTPS redirection and application URL You have |
||
|
||
app.UseSwaggerForOcelotUI(); //This line added | ||
|
||
var summaries = new[] | ||
{ | ||
"Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching" | ||
}; | ||
|
||
app.MapGet("/weatherforecast", () => | ||
{ | ||
var forecast = Enumerable.Range(1, 5).Select(index => | ||
new WeatherForecast | ||
( | ||
DateOnly.FromDateTime(DateTime.Now.AddDays(index)), | ||
Random.Shared.Next(-20, 55), | ||
summaries[Random.Shared.Next(summaries.Length)] | ||
)) | ||
.ToArray(); | ||
return forecast; | ||
}) | ||
.WithName("GetWeatherForecast") | ||
.WithOpenApi(); | ||
|
||
app.Run("http://localhost:7002"); | ||
|
||
record WeatherForecast(DateOnly Date, int TemperatureC, string? Summary) | ||
{ | ||
public int TemperatureF => 32 + (int)(TemperatureC / 0.5556); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"Logging": { | ||
"LogLevel": { | ||
"Default": "Information", | ||
"Microsoft.AspNetCore": "Warning" | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{ | ||
"Logging": { | ||
"LogLevel": { | ||
"Default": "Information", | ||
"Microsoft.AspNetCore": "Warning" | ||
} | ||
}, | ||
"AllowedHosts": "*" | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,18 @@ | ||||||||||||||||||
<Project Sdk="Microsoft.NET.Sdk.Web"> | ||||||||||||||||||
|
||||||||||||||||||
<PropertyGroup> | ||||||||||||||||||
<TargetFramework>net8.0</TargetFramework> | ||||||||||||||||||
<Nullable>enable</Nullable> | ||||||||||||||||||
<ImplicitUsings>enable</ImplicitUsings> | ||||||||||||||||||
</PropertyGroup> | ||||||||||||||||||
|
||||||||||||||||||
<ItemGroup> | ||||||||||||||||||
<PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="8.0.4"/> | ||||||||||||||||||
<PackageReference Include="Swashbuckle.AspNetCore" Version="6.4.0"/> | ||||||||||||||||||
</ItemGroup> | ||||||||||||||||||
Comment on lines
+9
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Update Swashbuckle.AspNetCore to the latest version. While the Microsoft.AspNetCore.OpenApi package is up-to-date, the Swashbuckle.AspNetCore package (6.4.0) is not the latest version. Given the importance of Swagger in this project, it's recommended to use the most recent stable version. Update the Swashbuckle.AspNetCore package to the latest stable version: <ItemGroup>
<PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="8.0.4"/>
- <PackageReference Include="Swashbuckle.AspNetCore" Version="6.4.0"/>
+ <PackageReference Include="Swashbuckle.AspNetCore" Version="6.5.0"/>
</ItemGroup> This update ensures you have the latest features and bug fixes for Swagger integration. 📝 Committable suggestion
Suggested change
|
||||||||||||||||||
|
||||||||||||||||||
<ItemGroup> | ||||||||||||||||||
<ProjectReference Include="..\..\..\src\MMLib.SwaggerForOcelot\MMLib.SwaggerForOcelot.csproj" /> | ||||||||||||||||||
</ItemGroup> | ||||||||||||||||||
|
||||||||||||||||||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
using Ocelot.DependencyInjection; | ||
using Ocelot.Provider.Consul; | ||
using Ocelot.Middleware; | ||
|
||
var builder = WebApplication.CreateBuilder(args); | ||
builder.Services.AddEndpointsApiExplorer(); | ||
|
||
builder.Configuration | ||
.SetBasePath(builder.Environment.ContentRootPath) | ||
.AddJsonFile("ocelot.json", optional: false, reloadOnChange: true) | ||
.AddEnvironmentVariables(); | ||
|
||
builder.Services | ||
.AddOcelot(builder.Configuration) | ||
.AddConsul(); | ||
|
||
builder.Services.AddSwaggerForOcelot(builder.Configuration); | ||
|
||
var app = builder.Build(); | ||
|
||
app.UseSwaggerForOcelotUI(); | ||
await app.UseOcelot(); | ||
|
||
app.MapGet("/", () => "Hello World!").WithOpenApi(); | ||
app.Run("http://0.0.0.0:7001"); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"Logging": { | ||
"LogLevel": { | ||
"Default": "Information", | ||
"Microsoft.AspNetCore": "Warning" | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{ | ||
"Logging": { | ||
"LogLevel": { | ||
"Default": "Information", | ||
"Microsoft.AspNetCore": "Warning" | ||
} | ||
}, | ||
"AllowedHosts": "*" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
{ | ||
"Routes": [], | ||
"SwaggerEndPoints": [], | ||
"GlobalConfiguration": { | ||
"ServiceDiscoveryProvider": { | ||
"Scheme": "http", | ||
"Host": "localhost", | ||
"Port": 8500, | ||
"Type": "PollConsul", | ||
"PollingInterval": 100 | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,5 +13,9 @@ | |
<PackageReference Include="Swashbuckle.AspNetCore" Version="6.5.0" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<ProjectReference Include="..\..\src\MMLib.ServiceDiscovery.Consul\MMLib.ServiceDiscovery.Consul.csproj" /> | ||
</ItemGroup> | ||
Comment on lines
+16
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Potential Issue with Consul Integration While the project reference to Please ensure that:
🔗 Analysis chainLGTM! Verify Consul integration in OrderService. The project reference to To ensure proper integration, please verify that the OrderService is correctly using the Consul service discovery features. You may want to check the following:
Run the following script to check for Consul usage in the OrderService project: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for Consul usage in OrderService
# Test: Search for Consul-related imports and usages
rg --type csharp -i 'using.*consul|consul.*client|consul.*config' demo/OrderService
Length of output: 171 Script: #!/bin/bash
# Description: Verify Consul configuration and service setup in OrderService
# Check for Consul configurations in appsettings.json
rg 'consul' demo/OrderService/appsettings.json
# Check for Consul service setup in Program.cs and Startup.cs
rg --type csharp -i 'services\.(Add|Use).*Consul' demo/OrderService/Program.cs demo/OrderService/Startup.cs
Length of output: 156 |
||
|
||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,10 @@ | ||
using Microsoft.AspNetCore.Builder; | ||
using Microsoft.AspNetCore.Hosting; | ||
using Microsoft.AspNetCore.Mvc.ApiExplorer; | ||
using Microsoft.Extensions.Configuration; | ||
using Microsoft.Extensions.DependencyInjection; | ||
using Microsoft.Extensions.Options; | ||
using Microsoft.Extensions.PlatformAbstractions; | ||
using MMLib.ServiceDiscovery.Consul.DependencyInjection; | ||
using Swashbuckle.AspNetCore.SwaggerGen; | ||
using System.IO; | ||
using System.Reflection; | ||
|
@@ -67,6 +67,7 @@ public void ConfigureServices(IServiceCollection services) | |
// integrate xml comments | ||
options.IncludeXmlComments(XmlCommentsFilePath); | ||
}); | ||
|
||
} | ||
|
||
/// <summary> | ||
|
@@ -79,15 +80,34 @@ public void Configure(IApplicationBuilder app, IApiVersionDescriptionProvider pr | |
app.UseRouting(); | ||
app.UseEndpoints(endpoints => { endpoints.MapControllers(); }); | ||
app.UseSwagger(); | ||
_ = app.UseSwaggerUI( | ||
options => | ||
// _ = app.UseSwaggerUI( | ||
// options => | ||
// { | ||
// // build a swagger endpoint for each discovered API version | ||
// foreach (ApiVersionDescription description in provider.ApiVersionDescriptions) | ||
// { | ||
// options.SwaggerEndpoint($"/swagger/{description.GroupName}/swagger.json", description.GroupName.ToUpperInvariant()); | ||
// } | ||
// }); | ||
app.UseSwaggerUI(c => | ||
{ | ||
c.RoutePrefix = "swagger"; | ||
foreach (var description in provider.ApiVersionDescriptions) | ||
{ | ||
c.SwaggerEndpoint($"/swagger/{description.GroupName}/swagger.json", $"Version {description.GroupName}"); | ||
} | ||
}); | ||
|
||
// Separate endpoints that contain only one version | ||
foreach (var description in provider.ApiVersionDescriptions) | ||
{ | ||
app.UseSwaggerUI(c => | ||
{ | ||
// build a swagger endpoint for each discovered API version | ||
foreach (ApiVersionDescription description in provider.ApiVersionDescriptions) | ||
{ | ||
options.SwaggerEndpoint($"/swagger/{description.GroupName}/swagger.json", description.GroupName.ToUpperInvariant()); | ||
} | ||
c.RoutePrefix = $"swagger/{description.GroupName}"; | ||
c.SwaggerEndpoint($"/swagger/{description.GroupName}/swagger.json", $"Version {description.GroupName}"); | ||
}); | ||
} | ||
Comment on lines
+101
to
+109
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid multiple Calling Consider consolidating the configurations into a single - // Separate endpoints that contain only one version
- foreach (var description in provider.ApiVersionDescriptions)
- {
- app.UseSwaggerUI(c =>
- {
- c.RoutePrefix = $"swagger/{description.GroupName}";
- c.SwaggerEndpoint($"/swagger/{description.GroupName}/swagger.json", $"Version {description.GroupName}");
- });
- }
+ // Configure Swagger UI with multiple route prefixes for each version
+ app.UseSwaggerUI(c =>
+ {
+ c.RoutePrefix = string.Empty; // Allows for multiple route prefixes
+ foreach (var description in provider.ApiVersionDescriptions)
+ {
+ c.SwaggerEndpoint($"/swagger/{description.GroupName}/swagger.json", $"Version {description.GroupName}");
+ c.SwaggerEndpoint($"/swagger/{description.GroupName}/swagger.json", $"swagger/{description.GroupName}");
+ }
+ }); Alternatively, you can set up multiple endpoints within the same
|
||
|
||
} | ||
|
||
static string XmlCommentsFilePath | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
using Microsoft.AspNetCore.Builder; | ||
using Microsoft.Extensions.DependencyInjection; | ||
using Swashbuckle.AspNetCore.SwaggerUI; | ||
|
||
namespace MMLib.ServiceDiscovery.Consul.DependencyInjection; | ||
|
||
/// <summary> | ||
/// | ||
/// </summary> | ||
public static class ConfigureExtensions | ||
{ | ||
/// <summary> | ||
/// | ||
/// </summary> | ||
/// <param name="builder"></param> | ||
/// <returns></returns> | ||
public static IApplicationBuilder UseSwaggerForOcelotUI(this WebApplication builder) | ||
{ | ||
builder.ConfigureConsulConnection(); | ||
builder.MapHealthChecks("/api/health"); | ||
|
||
return builder; | ||
} | ||
Comment on lines
+17
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Review method name and implementation for Swagger UI. The method name
Ensure that the method aligns with its intended purpose and that its name accurately describes its functionality. |
||
|
||
/// <summary> | ||
/// | ||
/// </summary> | ||
/// <param name="builder"></param> | ||
/// <returns></returns> | ||
public static IApplicationBuilder ConfigureConsulConnection(this IApplicationBuilder builder) | ||
{ | ||
var consul = builder.ApplicationServices.GetService<IConsulConnectionService>(); | ||
consul?.Start(); | ||
|
||
return builder; | ||
} | ||
Comment on lines
+30
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance error handling and service resolution in ConfigureConsulConnection. Consider the following improvements to make the method more robust:
These changes will improve the method's reliability and provide better feedback if issues occur during Consul connection setup. |
||
} |
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.
🛠️ Refactor suggestion
Consider reading the Consul URL from configuration
The Consul service URL is hardcoded as
"http://localhost:8500"
. To enhance flexibility and maintainability, consider fetching this value from the application's configuration files (e.g.,appsettings.json
). This approach allows you to modify the Consul address without changing the code base when moving between environments.Apply this diff to update the code:
Remember to add the Consul address to your configuration file: