-
Notifications
You must be signed in to change notification settings - Fork 1
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
Dashboard #763
Dashboard #763
Conversation
📝 WalkthroughWalkthroughThis pull request introduces new infrastructure files and configurations. It adds Bicep files to deploy an Azure Container App dashboard and its associated parameter file, a Docker ignore file, and a GitHub Actions workflow for deployment. It also updates the solution file with two new projects and introduces several new files to support an ASP.NET Core dashboard application, including a Dockerfile, launch settings, configuration files, and supporting C# code. Additionally, a minor removal of an unused using directive is included. Changes
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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: 4
🧹 Nitpick comments (4)
tools/Altinn.Correspondence.Dashboard/Program.cs (2)
7-24
: Consider enhancing application configuration with additional reliability featuresThe service configuration is good for basic functionality, but could benefit from adding:
- Health checks for monitoring application status
- Explicit error handling for database connection failures
- More comprehensive logging configuration
var builder = WebApplication.CreateBuilder(args); builder.Configuration .AddJsonFile("appsettings.json", true, true) .AddJsonFile($"appsettings.{builder.Environment.EnvironmentName}.json", true, true) .AddJsonFile("appsettings.local.json", true, true); builder.Services.AddPersistence(builder.Configuration); builder.Services.AddApplicationInsightsTelemetry(new ApplicationInsightsServiceOptions() { EnableAdaptiveSampling = false }); builder.Services.AddSingleton<IConnectionFactory, HangfireConnectionFactory>(); +builder.Services.AddHealthChecks() + .AddNpgSql(name: "Hangfire-Postgres", tags: new[] { "ready" }); builder.Services.AddHangfire((provider, config) => { config.UsePostgreSqlStorage( c => c.UseConnectionFactory(provider.GetService<IConnectionFactory>()) ); config.UseSerilogLogProvider(); });
28-35
: Add health check endpoints and improve middleware configurationConsider enhancing the application's middleware pipeline to include health checks and potentially improve security.
app.UseHttpsRedirection(); +// Configure health checks +app.MapHealthChecks("/health/ready", new HealthCheckOptions +{ + Predicate = (check) => check.Tags.Contains("ready") +}); +app.MapHealthChecks("/health/live", new HealthCheckOptions +{ + Predicate = (_) => false +}); app.UseHangfireDashboard("/hangfire", new DashboardOptions() { Authorization = [new HangfireDashboardAuthorizationFilter()] }); app.MapGet("/", () => Results.Redirect("/hangfire"));tools/Altinn.Correspondence.Dashboard/Dockerfile (1)
10-20
: Consider optimizing the Docker build processThe build stage copies project files individually before restoring dependencies. Consider consolidating COPY commands where possible to reduce the number of layers.
# This stage is used to build the service project FROM mcr.microsoft.com/dotnet/sdk:9.0 AS build WORKDIR /src -COPY ["tools/Altinn.Correspondence.Dashboard/Altinn.Correspondence.Dashboard.csproj", "tools/Altinn.Correspondence.Dashboard/"] -COPY ["src/Altinn.Correspondence.Persistence/Altinn.Correspondence.Persistence.csproj", "src/Altinn.Correspondence.Persistence/"] -COPY ["src/Altinn.Correspondence.Integrations/Altinn.Correspondence.Integrations.csproj", "src/Altinn.Correspondence.Integrations/"] +COPY ["tools/Altinn.Correspondence.Dashboard/*.csproj", "tools/Altinn.Correspondence.Dashboard/"] +COPY ["src/Altinn.Correspondence.Persistence/*.csproj", "src/Altinn.Correspondence.Persistence/"] +COPY ["src/Altinn.Correspondence.Integrations/*.csproj", "src/Altinn.Correspondence.Integrations/"] RUN dotnet restore "tools/Altinn.Correspondence.Dashboard/Altinn.Correspondence.Dashboard.csproj" COPY . . WORKDIR "/src/tools/Altinn.Correspondence.Dashboard" RUN dotnet build "Altinn.Correspondence.Dashboard.csproj" -c Release -o /app/build.github/workflows/deploy-dashboard.yml (1)
49-55
: Update Azure login action versionThe static analysis indicates that the runner version for the Azure login action may be outdated.
Consider updating to a newer version:
- uses: azure/login@v1 + uses: azure/login@v2🧰 Tools
🪛 actionlint (1.7.4)
50-50: the runner of "azure/login@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.azure/applications/dashboard/main.bicep
(1 hunks).azure/applications/dashboard/main.bicepparam
(1 hunks).dockerignore
(1 hunks).github/workflows/deploy-dashboard.yml
(1 hunks)Altinn.Correspondence.sln
(2 hunks)src/Altinn.Correspondence.Integrations/Hangfire/DependencyInjection.cs
(1 hunks)tools/Altinn.Correspondence.Dashboard/Altinn.Correspondence.Dashboard.csproj
(1 hunks)tools/Altinn.Correspondence.Dashboard/Dockerfile
(1 hunks)tools/Altinn.Correspondence.Dashboard/HangfireDashboardAuthorizationFilter.cs
(1 hunks)tools/Altinn.Correspondence.Dashboard/Program.cs
(1 hunks)tools/Altinn.Correspondence.Dashboard/Properties/launchSettings.json
(1 hunks)tools/Altinn.Correspondence.Dashboard/appsettings.Development.json
(1 hunks)tools/Altinn.Correspondence.Dashboard/appsettings.json
(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- tools/Altinn.Correspondence.Dashboard/appsettings.Development.json
- tools/Altinn.Correspondence.Dashboard/appsettings.json
- .dockerignore
- tools/Altinn.Correspondence.Dashboard/Properties/launchSettings.json
- src/Altinn.Correspondence.Integrations/Hangfire/DependencyInjection.cs
- tools/Altinn.Correspondence.Dashboard/Altinn.Correspondence.Dashboard.csproj
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/deploy-dashboard.yml
50-50: the runner of "azure/login@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
.github/workflows/deploy-dashboard.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
🔇 Additional comments (11)
Altinn.Correspondence.sln (3)
24-27
: New project structure added correctlyThe solution has been properly extended with a new "tools" folder containing the Dashboard project, following good organizational practices by separating utility applications from the core codebase.
66-69
: Project configuration properly definedConfiguration settings for both Debug and Release builds have been correctly added for the Dashboard project.
77-77
: Nested project structure configured correctlyThe Dashboard project has been properly nested under the "tools" folder, maintaining a clean solution organization.
.github/workflows/deploy-dashboard.yml (4)
3-18
: Well-structured deployment workflow triggerThe workflow trigger configuration using
workflow_dispatch
with environment selection is well-designed, allowing for flexible deployment across multiple environments.
19-23
: Appropriate permissions definedThe permissions are correctly configured with the minimum necessary access levels for the workflow operations.
32-48
: Container build and push properly configuredThe Docker image build and push steps are well-structured, with appropriate tagging using both the commit SHA and 'latest' for versioning.
62-69
: Security credentials properly passed as parametersThe workflow correctly passes necessary security credentials and configuration from GitHub secrets to the Bicep deployment.
.azure/applications/dashboard/main.bicep (4)
1-8
: Well-defined parametersAll necessary parameters for the deployment are properly defined, including security credentials and configuration values.
14-30
: Key Vault access policy properly configuredThe access policy is correctly set up to grant the container app the minimum necessary permissions to retrieve secrets from the Key Vault.
32-77
: Container App configuration well-structuredThe Container App resource is properly defined with appropriate identity, ingress settings, and secrets configuration. The scale configuration limits the app to a single replica, which is appropriate for a dashboard application.
79-113
: Robust authentication configurationThe Azure AD authentication for the Container App is comprehensively configured with proper client credentials, issuer, and authorization policy that restricts access to a specific group.
tools/Altinn.Correspondence.Dashboard/HangfireDashboardAuthorizationFilter.cs
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Dashboard * This leads to double messages * Restored old functionality, simplified ours instead * Got no swagger * Fixed deploy script * Fix dockerfile * Use port 2526 for dashboard * Github action and biceps * Updated dockerfile * Removed unused .http file * Fixed bicep to include the necessary setup * FIx * Clean-up * Add authentication to bicep * Delete old script as we do bicep now * Update .github/workflows/deploy-dashboard.yml Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Fixed auth config --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Description
This is a simple app that bootstraps Hangfire dashboard in an Azure Container App. This container app is secured by Azure AD authentication through the integrated container apps solution.
Related Issue(s)
Verification
Documentation
Summary by CodeRabbit