fix: auto-remap renamed vault types on load#422
Merged
Conversation
Add Azure Key Vault (`azure-kv`) as a vault provider using DefaultAzureCredential from the Azure SDK. Rename existing vault types for consistency: `aws` → `aws-sm`, `azure` → `azure-kv`. Key changes: - New AzureKvVaultProvider with secret name sanitization (/ and _ → -) - vault create gains --region (AWS) and --vault-url (Azure) flags with env var fallback (AWS_REGION, AZURE_KEYVAULT_URL) and logging - Runtime migration hints warn when old type names are used in config - Hidden `vault type list` alias for `vault type search` - AWS provider now requires explicit region (no silent default) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Azure SDK's core-rest-pipeline calls os.release() which requires the --allow-sys permission in Deno. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
After renaming vault types (aws → aws-sm, azure → azure-kv), existing users with old vault configs on disk would have their vaults fail to load. The YAML files contain type: aws and registerVault() rejects the old type name at the switch statement. Remap deprecated type names to their current equivalents before calling registerVault(), with a warning log so users know the name changed. Co-Authored-By: Pirmin Felber <pirminf@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
johnrwatson
force-pushed
the
feat/azure-kv-vault-provider
branch
from
b5c2436 to
c04dd0b
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Approved
This PR is well-implemented and ready for merge.
Review Summary
Code Quality:
- ✅ TypeScript strict mode compliance
- ✅ Named exports used throughout
- ✅ AGPLv3 copyright headers on new files
- ✅ Proper type definitions (
AzureKvVaultConfiginterface)
Domain-Driven Design:
- ✅
AzureKvVaultProvidercorrectly implementsVaultProviderinterface - ✅ Clean separation between domain service and infrastructure adapters
- ✅ Configuration types are proper value objects
Test Coverage:
- ✅ New tests in
azure_kv_vault_provider_test.ts(lives next to source) - ✅ Comprehensive auto-remapping tests in
vault_service_test.ts - ✅ All existing tests updated for renamed types
Security:
- ✅ Uses
DefaultAzureCredential(Azure SDK best practice) - ✅ No hardcoded credentials
- ✅ Required config validation with clear error messages
Backwards Compatibility:
- ✅ Auto-remaps deprecated types (
aws→aws-sm,azure→azure-kv) on load - ✅ Warning logs guide users to update configs
- ✅ Helpful error messages when using old type names directly
Minor Suggestions (non-blocking)
-
The double cast
as unknown as AzureKvVaultConfiginvault_service.ts:112could potentially use a type guard, but it's consistent with existing provider patterns. -
The
toAzureSecretNamecharacter replacement (/and_→-) can cause collisions—documented but worth noting for users storing secrets with both characters.
🤖 Generated with Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
aws→aws-sm,azure→azure-kv) in feat: Azure Key Vault vault provider #420, existing users with old vault configs on disk (type: awsin their YAML files) would have their vaults silently fail to load.registerVault()rejects the old type name at the switch statement, and the error was caught and logged at debug level — so vaults just disappeared with no clear indication of what went wrong.fromRepository()before callingregisterVault(), with a warning log so users know the name changed and can update their configs.aws→aws-smandazure→azure-kvremapping works when loading from a vault config repository.Test plan
deno fmt --checkpassesdeno lintpassesdeno run test— all 1817 tests passfromRepository()loads oldtype: awsconfigs asaws-smand oldtype: azureconfigs asazure-kv🤖 Generated with Claude Code