-
Notifications
You must be signed in to change notification settings - Fork 98
Add Configuration Management System with Azure App Configuration Integration #1586
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
base: main
Are you sure you want to change the base?
Conversation
|
/integration sha=5e7e06b |
|
⌛ Integration tests are running... Check their status here 👈 |
|
✅ Integration tests have finished successfully! |
|
/integration sha=bb40086 |
|
⌛ Integration tests are running... Check their status here 👈 |
|
✅ Integration tests have finished successfully! |
|
/integration sha=42e28c6 |
|
⌛ Integration tests are running... Check their status here 👈 |
|
✅ Integration tests have finished successfully! |
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.
Pull Request Overview
This PR introduces a comprehensive configuration management system for Booster applications with Azure App Configuration integration. The system enables multi-tier configuration resolution, automatic infrastructure provisioning, and dynamic configuration updates without requiring code changes or redeployments.
- Implements a flexible configuration provider system with priority-based resolution hierarchy
- Adds seamless Azure App Configuration integration with automatic infrastructure provisioning
- Provides configuration APIs for simple value resolution and source tracking
Reviewed Changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| website/sidebars.js | Adds configuration documentation to the sidebar |
| website/docs/10_going-deeper/infrastructure-providers.mdx | Documents Azure App Configuration integration and usage |
| website/docs/10_going-deeper/environment-configuration.mdx | Adds reference to configuration management documentation |
| website/docs/03_features/03_configuration.mdx | Comprehensive configuration management documentation |
| packages/framework-types/src/config.ts | Adds configuration provider interfaces and Azure App Configuration support |
| packages/framework-types/src/configuration-resolver.ts | Implements configuration resolution logic and default providers |
| packages/framework-types/src/index.ts | Exports new configuration types |
| packages/framework-types/test/configuration-resolver.test.ts | Tests for configuration resolution functionality |
| packages/framework-core/src/services/configuration-service.ts | Main configuration service implementation |
| packages/framework-core/src/index.ts | Exports configuration service functions |
| packages/framework-core/test/services/configuration-service.test.ts | Tests for configuration service |
| packages/framework-provider-azure/src/library/configuration-adapter.ts | Azure App Configuration provider implementation |
| packages/framework-provider-azure/src/index.ts | Integrates Azure configuration adapter |
| packages/framework-provider-azure/src/constants.ts | Adds Azure App Configuration environment variables |
| packages/framework-provider-azure/package.json | Adds Azure App Configuration dependency |
| packages/framework-provider-azure/test/library/configuration-adapter.test.ts | Tests for Azure configuration adapter |
| packages/framework-provider-azure-infrastructure/src/infrastructure/synth/terraform-app-configuration.ts | Terraform resource for Azure App Configuration |
| packages/framework-provider-azure-infrastructure/src/infrastructure/synth/application-synth.ts | Integrates App Configuration into infrastructure synthesis |
| packages/framework-provider-azure-infrastructure/src/infrastructure/synth/terraform-function-app-settings.ts | Adds App Configuration environment variables |
| packages/framework-provider-azure-infrastructure/src/infrastructure/helper/utils.ts | Helper function for building connection strings |
| packages/framework-provider-azure-infrastructure/src/infrastructure/application-builder.ts | Populates infrastructure environment variables |
| packages/framework-provider-azure-infrastructure/src/infrastructure/types/application-synth-stack.ts | Adds App Configuration to infrastructure stack |
| common/changes/@boostercloud/framework-core/azure_app_configuration_2025-08-06-21-35.json | Change log entry |
Files not reviewed (1)
- common/config/rush/pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| import { WebSiteManagementClient as WebSiteManagement } from '@azure/arm-appservice' | ||
| import { ResourceManagementClient } from '@azure/arm-resources' | ||
| import { TokenCredential, ClientSecretCredential } from '@azure/identity' | ||
| import { ClientSecretCredential, TokenCredential } from '@azure/identity' |
Copilot
AI
Aug 12, 2025
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.
[nitpick] The reordering of imports appears to be unnecessary and doesn't follow a clear pattern. Consider maintaining the original alphabetical order or using a consistent ordering strategy across the codebase.
| import { environmentVarNames } from '@boostercloud/framework-provider-azure' | ||
| import { ApplicationSynthStack } from '../types/application-synth-stack' | ||
| import { toTerraformName } from '../helper/utils' | ||
| import { buildAzureAppConfigConnectionString, toTerraformName } from '../helper/utils' |
Copilot
AI
Aug 12, 2025
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.
The import of toTerraformName is added but never used in this file. Consider removing unused imports to keep the code clean.
| connectionString?: string | ||
| endpoint?: string | ||
| labelFilter?: string | ||
| }): void { |
Copilot
AI
Aug 12, 2025
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.
[nitpick] The method signature uses an inline options object type. Consider defining a separate interface for the options parameter to improve type reusability and maintainability.
| }): void { | |
| public enableAzureAppConfiguration(options?: AzureAppConfigurationOptions): void { |
| `Failed to initialize Azure App Configuration client: ${originalError.message}` | ||
| ) | ||
| // Preserve the original error as a property for debugging | ||
| ;(this.initializationError as any).originalError = originalError |
Copilot
AI
Aug 12, 2025
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.
Using as any type assertion to add a property is not type-safe. Consider creating a custom error class or using a more structured approach to preserve the original error.
| ;(this.initializationError as any).originalError = originalError | |
| this.initializationError = new InitializationError( | |
| `Failed to initialize Azure App Configuration client: ${originalError.message}`, | |
| originalError | |
| ) |
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.
I would try to avoid the use of any
|
|
||
| Booster.configure('production', (config: BoosterConfig): void => { | ||
| config.appName = 'my-app' | ||
| config.providerPackage = '@boostercloud/framework-provider-azure' |
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.
is this line necessary if we are using an Config Custom provider?
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.
Are you referring to the line with the config.providerPackage? A provider package is always required depending on where your Booster app is running. Using a custom provider gives you the freedom to use your own implementation of the ConfigurationProvider interface even if your Booster app is running on Azure. Does this answer your question?
| this.appConfiguration = new appConfiguration.AppConfiguration(this, 'AppConfiguration', { | ||
| name, | ||
| resourceGroupName: resourceGroup.name, | ||
| location: resourceGroup.location, | ||
| sku: 'free', // Use free tier by default. For more information, see https://azure.microsoft.com/en-us/pricing/details/app-configuration/ | ||
| tags: { | ||
| Application: config.appName, | ||
| Environment: config.environmentName, | ||
| BoosterManaged: 'true', | ||
| }, | ||
| // Enable managed identity for secure access | ||
| identity: { | ||
| type: 'SystemAssigned', | ||
| }, | ||
| // Configure public network access | ||
| publicNetworkAccess: 'Enabled', | ||
| // Configure local authentication | ||
| localAuthEnabled: true, | ||
| }) | ||
| } |
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.
(1) regarding the config param public_network_access - (Optional) The Public Network Access setting of the App Configuration. Possible values are Enabled and Disabled.
Is this flag allowing access from external connections or only from the Azure resources present in the resource group?
(2) Are we intending to store only config variables defining behaviors of the application or also to store secrets? If this is intended for secrets I would consider including an encryption mechanism. Here there is an example with encryption https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/app_configuration.html that can be use as a reference, relying also in Azure Key Vault
| describe('getValue', () => { | ||
| it('should return undefined when not available', async () => { | ||
| const adapter = new ConfigurationAdapter() | ||
| const value = await adapter.getValue('test-key') | ||
| expect(value).to.be.undefined | ||
| }) | ||
|
|
||
| it('should return undefined when client fails', async () => { | ||
| const adapter = new ConfigurationAdapter('invalid-connection-string') | ||
| const value = await adapter.getValue('test-key') | ||
| expect(value).to.be.undefined | ||
| }) | ||
| }) |
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.
I miss a test using getValue with an existing entry
| /** | ||
| * Configuration options for Azure App Configuration integration | ||
| * Used to store configuration without circular dependencies | ||
| */ | ||
| export interface AzureAppConfigurationOptions { | ||
| /** Connection string for Azure App Configuration (alternative to endpoint + managed identity) */ | ||
| connectionString?: string | ||
|
|
||
| /** Endpoint URL for Azure App Configuration (used with managed identity) */ | ||
| endpoint?: string | ||
|
|
||
| /** Optional label filter to target specific configuration values */ | ||
| labelFilter?: string | ||
|
|
||
| /** Whether to enable Azure App Configuration (default: false) */ | ||
| enabled?: boolean | ||
| } |
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.
Maybe this interface can be declared in a Azure-specific 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.
I have finished my review. Apart from the comments, I also miss an integration test on the sample application using a config variable stored in Azure App Store. Maybe you can implement a Command that reads the config variable and returns it to the client and call it from the integration test.
Description
This PR introduces a flexible configuration management system for Booster applications, supporting multi-tier configuration resolution and external configuration providers. The system enables applications to retrieve configuration values from multiple sources with automatic fallback, allowing for dynamic configuration updates without requiring code changes or redeployments.
The implementation includes seamless integration with Azure App Configuration, automatic infrastructure provisioning, label-based environment isolation, and comprehensive documentation.
Changes
Core Framework:
Azure Provider:
ConfigurationAdapterfor Azure App Configuration integrationConfiguration API:
resolveConfigurationValue()- Simple value resolutionresolveConfigurationWithSource()- Resolution with source trackingconfig.enableAzureAppConfiguration()- Enable Azure integrationconfig.addConfigurationProvider()- Add custom providersInfrastructure:
Documentation:
Priority Hierarchy:
Checks