Add Azure Managed Redis support alongside Azure Cache for Redis - #1438
Conversation
Azure Cache for Redis Basic, Standard, and Premium retire 2028-09-30, and Azure Managed Redis is the replacement. The two services listen on different TLS ports (10000 vs 6380), so the hardcoded 6380 in four call sites could only ever reach the retiring service. Application: - Add functions_redis_client.py as the single place that resolves service type, port, and credentials, and route session storage, the shared app cache, and the admin connection test through it. - Detect the service from the host name suffix, with optional redis_service_type and redis_port admin overrides for custom DNS and private endpoints. An unrecognized host keeps the previous port 6380 behavior, so existing Azure Cache for Redis deployments are unaffected. - Use the redis-entraid streaming credential provider so pooled connections re-AUTH before the Entra token expires, with one provider per long-lived client because the provider holds a single callback slot. Falls back to the in-repo provider when the package is absent so startup cannot break. - Align the admin connection test on the same token scope and factory the application uses; it previously used the legacy cacheinfra endpoint. - Report the resolved service and port in Redis Metrics. Deployer: - Provision Azure Managed Redis Balanced_B0 with high availability, the documented replacement for the Standard C0 previously deployed. - Set clusteringPolicy to NoCluster explicitly; the service default is OSSCluster, which requires a cluster-aware client SimpleChat does not use. - Keep a redisCacheKind switch for Azure Government and 21Vianet, where Azure Managed Redis is unavailable. - Grant data access through the redisEnterprise database access policy assignment, and retrieve keys with az redisenterprise. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Addresses two CodeQL findings introduced by the Azure Managed Redis change. py/stack-trace-exposure (route_backend_settings.py): moving Key Vault secret retrieval into the shared client factory meant the client-construction handler now catches credential errors, and it returned str(exc) to the browser. That regressed the original hardening, which deliberately logged Key Vault failures and replied with a generic message. Validation errors are now caught separately and still surfaced, while credential errors are logged under [REDIS_TEST] and answered generically. py/incomplete-url-substring-sanitization (postconfig.py): the fallback that infers the Redis offering from a host name used a substring check, so a host like evil.redis.azure.net.attacker.example.com would be treated as Azure Managed Redis and configured with port 10000. It now matches the full suffix, consistent with the application's own detection. Also removes a redundant function-level import in app_settings_cache.py that CodeQL flagged as py/import-and-import-from. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CodeQL flagged the Redis connection test as an information-exposure and clear-text-logging source: the validation handler returned the exception text to the browser, and both handlers interpolated the raw exception into log_event, which CodeQL traces into the shared Application Insights sinks. Both handlers now pass the exception through sanitize_log_message before logging and reply with fixed messages. The route already returns specific 400s for a missing host, key, or Key Vault secret name before reaching the factory, so no actionable detail is lost. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
REDIS_TOKEN_REFRESH_BUFFER_SECONDS and _get_redis_entra_token_scope had no callers outside functions_redis_client, so the back-compat re-export only produced CodeQL py/unused-import noise. REDIS_ENTRA_TOKEN_SCOPE and RedisManagedIdentityCredentialProvider are still re-exported because existing callers and tests reference them through app_settings_cache. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CodeQL traced the interpolated exception message from the Redis connection test into the shared Application Insights sinks as a clear-text-logging source, since the failing call chain resolves an access key or Key Vault secret. Both handlers now interpolate only the exception class name and pass exceptionTraceback=True, so Application Insights still captures the full exception and stack for diagnosis while no resolved credential material can reach the log message. This matches the _safe_error_summary pattern already used in functions_redis_monitoring. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CodeQL statusThe CodeQL check is red. Here is what I fixed and what is left, so you can make an informed call. Fixed in this PR (all genuinely introduced by this change)
I also stopped interpolating exception messages into Remaining 5 high alerts are pre-existing, in a file this PR does not touchAll five are
They surface as "new in code changed by this pull request" because Clearing them would mean refactoring The 4 remaining notes are Everything else is greenAll 13 other checks pass, including |
CodeQL py/unused-import: REDIS_ENTRA_TOKEN_SCOPE and RedisManagedIdentityCredentialProvider were re-exported for backward compatibility, but no production module imported them through app_settings_cache either before or after this change - on Development they were referenced only inside app_settings_cache itself and by one test. Both are dropped and the test now references them from functions_redis_client, where they are defined. The create_redis_managed_identity_client wrapper is kept, since app.py called it before this change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Security review comments resolvedAll 12 CodeQL review threads on this PR are now resolved. Final disposition, verified against the CodeQL run on Fixed
Both security-severity alerts were genuinely introduced by this PR, and both are gone. Explained and resolved3 × Pre-existing, not from this PR5 × Checks13 of 14 green. CodeQL remains red solely on the five pre-existing alerts above. |
2 participants
Why
Azure Cache for Redis Basic, Standard, and Premium retire on 2028-09-30 (retirement FAQ), and the Enterprise tiers retire 2027-03-31. Azure Managed Redis (AMR) is the replacement.
The deployer provisioned Azure Cache for Redis Standard C0, and the application hardcoded port 6380 in four places — so it could only ever reach the retiring service.
SKU:
Balanced_B0with high availabilityMicrosoft's official migration mapping puts Standard C0 →
Balanced_B0. That is 2× the memory (0.5 GB vs 0.25 GB) for roughly 40% less cost (~$23/mo vs ~$40/mo HA in East US), and it is the smallest AMR SKU.What changed
Application — both services, one code path
.redis.cache.windows.net.<region>.redis.azure.net.redis.cache.usgovcloudapi.net/.chinacloudapi.cnredis.azure.com/.default/oidclaimfunctions_redis_client.pyis the single place that resolves service type → port → credentials. Flask session storage, the shared application cache, and the admin connection test all route through it; the four hardcoded6380literals are gone.redis_service_typeandredis_portadmin settings override detection when a custom DNS name or private endpoint hides the suffix.db=0is safe on AMR: redis-py only emitsSELECTfor a non-zero index.INFOfield set, so absent counters render as "Not available".Entra token refresh on live connections
Managed identity auth now uses
redis-entraid, a redis-py streaming credential provider that renews the token in the background and re-issuesAUTHon already-open pooled connections — required for AMR. Previously credentials were supplied at connect time only.The app cache and session clients each get their own provider instance, because
EntraIdCredentialsProviderholds a single re-authentication callback slot.Requires
msal1.31→1.33 andazure-identity1.23→1.24. The import is defensive: if the package is missing, SimpleChat falls back to the previous in-repo provider rather than failing to start.Deployer
redisCache.bicepprovisionsMicrosoft.Cache/redisEnterprise+databases+accessPolicyAssignments.clusteringPolicyis set toNoClusterexplicitly. The service default when omitted isOSSCluster, which requires a cluster-aware client — SimpleChat uses a plainredis.Redisand hands the same client toflask-session.NoClusteris valid to 25 GB and is the only policy changeable in place later.redisCacheKindswitch retained (manageddefault,classicfallback) because AMR is unavailable in Azure Government and Azure operated by 21Vianet.defaultaccess policy on the database. The control-plane "Redis Cache Contributor" role is dropped for AMR — data access comes from the access policy, not RBAC.postconfig.pyretrieves keys withaz redisenterprise database list-keysand installs the required CLI extension first.Migration for existing deployments
Nothing to migrate. SimpleChat uses Redis as a look-aside cache and session store with a Cosmos DB fallback, which Microsoft explicitly sanctions as the "skip data migration" path (RDB export is Premium-only anyway). Cutover is a host name change in Admin Settings; the port follows automatically. Sessions do not survive it, so do it off-hours.
Review findings addressed
A review pass caught four real bugs, all fixed in this branch:
streaming_credentials=Falsestill built a streaming provider — leaked a thread + event loop per admin "Test" click.AUTH.az redisenterpriseneeds a CLI extension nothing installed —postconfigwould have hard-failed and written zero settings to Cosmos.postconfigwrote a definitiveredis_service_typeeven when no Redis was deployed, which could point an existing ACR instance at port 10000.Testing
functional_tests/test_redis_service_type_detection.py(8 tests) — suffix matrix incl. gov/21Vianet/Enterprise/unknown, overrides, invalid ports, monitoring payload.functional_tests/test_redis_client_factory.py(8 tests) — client construction per auth type × service,db=0, kwargs passthrough, validation,redis-entraidfallback, per-purpose providers.functional_tests/test_redis_entra_token_auth.py(7 tests) — streaming provider, sovereign-cloud authority.test_cosmos_wave1_cache_fallback.py, which was patching a symbol that no longer existed and therefore attempting real network calls instead of exercising the injected failure.main.jsonverified in sync, conditional resource guards verified mutually exclusive in the compiled ARM, no circular imports in any load order.Two
test_file_sync_capability.pyfailures are pre-existing on the base commit and unrelated.Versions
application/single_app/config.py:0.261.009→0.261.011deployers/version.txt:1.0.26→1.0.28Docs: new
AZURE_MANAGED_REDIS_SUPPORT.md, plus updates todocs/admin/scale.md, the manual provisioning guide, the deployer README, and release notes.Reviewer notes
AllMetricson the cluster,allLogson the database). Worth awhat-ifagainst a real subscription before merge.