Skip to content

Conversation

@adityathebe
Copy link
Member

@adityathebe adityathebe requested a review from Copilot June 3, 2025 16:18
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds health checks for Azure App Registration client secrets (and certificates) by implementing a new GetAzureHealth function, wiring it into the main health router, and providing fixtures and tests to validate healthy, expiring, and expired states.

  • Implement GetAzureHealth with default and configurable expiry grace period
  • Integrate Azure health into GetHealthByConfigType and allow overriding the grace period
  • Add YAML fixtures and table-driven tests covering healthy, expiring, and expired client secrets

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

File Description
pkg/health/testdata/azure-client-secret-*.yaml New test fixtures for Azure client secret health states
pkg/health/health_azure.go New Azure health logic and expiry handling
pkg/health/health_azure_test.go Table-driven tests for the new health logic
pkg/health/health.go Register Azure in the main health router and load configurable grace period
Comments suppressed due to low confidence (2)

pkg/health/health_azure.go:16

  • The variable azureClientSecretExpiry represents a grace period rather than an absolute expiry; consider renaming to azureClientSecretExpiryGracePeriod for clarity.
azureClientSecretExpiry = defaultAzureClientSecretExpiry

pkg/health/health_azure_test.go:54

  • The certificate case (Azure::AppRegistration::Certificate) is handled in production code but not covered by tests; add a fixture and test case for certificate expiry/healthy states.
{

@adityathebe adityathebe requested a review from moshloop June 3, 2025 16:22
@moshloop moshloop merged commit 31b672a into main Jun 4, 2025
5 checks passed
@moshloop moshloop deleted the feat/azure-health branch June 4, 2025 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants