KeyVault JCA: lazy-load certificate material and add alias regex filtering - #49774
KeyVault JCA: lazy-load certificate material and add alias regex filtering#49774rujche wants to merge 39 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances azure-security-keyvault-jca to reduce unnecessary Key Vault reads by (1) allowing users to configure a subset of certificate aliases to consider and (2) lazily loading certificate details only when a specific alias is requested—addressing the scenario described in #39487 (iterating/fetching all aliases when only one is configured).
Changes:
- Added
azure.keyvault.jca.certificatessystem property support to filter Key Vault certificate aliases to a configured subset. - Implemented lazy loading of Key Vault certificate key/certificate/chain data per alias (instead of eagerly loading all details on refresh).
- Updated tests and documentation (README + CHANGELOG) to cover and describe the new behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/keyvault/azure-security-keyvault-jca/src/main/java/com/azure/security/keyvault/jca/KeyVaultKeyStore.java | Wires configured alias filtering into keystore initialization and routes Key Vault lookups through lazy-loading accessors. |
| sdk/keyvault/azure-security-keyvault-jca/src/main/java/com/azure/security/keyvault/jca/implementation/certificates/KeyVaultCertificates.java | Implements configured-alias filtering and lazy loading of certificate details per alias. |
| sdk/keyvault/azure-security-keyvault-jca/src/test/java/com/azure/security/keyvault/jca/KeyVaultKeyStoreUnitTest.java | Adds unit coverage for parsing configured aliases and verifying they are passed into KeyVaultCertificates. |
| sdk/keyvault/azure-security-keyvault-jca/src/test/java/com/azure/security/keyvault/jca/implementation/certificates/KeyVaultCertificatesTest.java | Adds unit coverage ensuring alias listing is not eager and that only requested/configured aliases trigger Key Vault reads. |
| sdk/keyvault/azure-security-keyvault-jca/README.md | Documents the new azure.keyvault.jca.certificates configuration option. |
| sdk/keyvault/azure-security-keyvault-jca/CHANGELOG.md | Records the new filtering + lazy-loading features for the upcoming release. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
sdk/keyvault/azure-security-keyvault-jca/src/main/java/com/azure/security/keyvault/jca/implementation/certificates/KeyVaultCertificates.java:300
- The inline comment says this outer check avoids acquiring the lock, but certificatesNeedRefresh() is synchronized and always acquires the instance monitor. This is misleading when reasoning about contention; either adjust the comment or change the locking strategy.
private void refreshCertificatesIfNeeded() {
if (certificatesNeedRefresh()) { // Avoid acquiring the lock as much as possible.
synchronized (this) {
if (certificatesNeedRefresh()) { // After obtaining the lock, avoid doing too many operations.
refreshCertificates();
}
Listing aliases ran outside the instance lock, so two concurrent refreshes could apply their results in completion order rather than start order. A slow refresh could overwrite a newer alias list and still stamp lastRefreshTime, pinning the stale list for the whole refresh interval. Every caller also issued its own list request while only one result was kept. Move the listing into the existing synchronized block and hoist the double check ahead of it. The stale-client identity guard is no longer reachable there because updateKeyVaultClient holds the same lock, so it is removed. The per-alias lazy loaders keep their guards and still run their remote calls outside the lock.
The comma separated azure.keyvault.jca.certificate-alias-filter-patterns
property could not carry arbitrary regexes: a comma is valid regex
syntax, so a bounded quantifier such as \d{1,5} was split into \d{1 and
5} and failed to compile. No delimiter is safe here, because a regex can
contain any printable character.
Replace it with azure.keyvault.jca.certificate-alias-filter-pattern,
optionally suffixed to configure more than one filter. The suffix only
keeps property names unique and does not affect evaluation, since
include patterns are matched with anyMatch and exclude patterns with
noneMatch. The ! prefix for exclude patterns is unchanged.
The replaced property was introduced in this unreleased version, so it
is removed rather than deprecated.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (1)
sdk/keyvault/azure-security-keyvault-jca/src/main/java/com/azure/security/keyvault/jca/KeyVaultKeyStore.java:64
- The configured system property name for alias filtering is hard-coded here as
azure.keyvault.jca.certificate-alias-filter-pattern, but the PR description/notes indicate the property was renamed toazure.keyvault.jca.certificate-alias-filter-patterns(plural). This inconsistency will confuse users and makes it unclear which property is actually supported. Please align the property name across code, README, CHANGELOG, and tests (or explicitly support both names for backward/forward compatibility).
static final String CERTIFICATE_ALIAS_FILTER_PATTERN_PROPERTY
= "azure.keyvault.jca.certificate-alias-filter-pattern";
Description
This PR completes #39487 in
azure-security-keyvault-jcaand also finalizes the follow-up lazy-loading/thread-safety refinements requested during review.What changed
sdk/keyvault/azure-security-keyvault-jca/checkstyle-suppressions.xmlupdates are script-generated viaeng/scripts/linting_suppression_generator.py(not manually edited).KeyVaultCertificatesfor certificate, key, and certificate chain by alias.azure.keyvault.jca.certificate-alias-filter-patternconfigures a single filter.azure.keyvault.jca.certificate-alias-filter-pattern.1orazure.keyvault.jca.certificate-alias-filter-pattern.prod.!.Pattern.matcher(alias).matches()).KeyVaultCertificates:KeyVaultKeyStorelookup pathing and filter-pattern collection.README.md,CHANGELOG.md).KeyVaultKeyStoreUnitTestKeyVaultCertificatesTestWhy one property per filter
Each property value is a regex, so no delimiter is safe: a comma is valid regex syntax, and a bounded quantifier such as
\d{1,5}would be split into\d{1and5}and fail to compile. Escaping does not help either, because\is already the regex escape character, so\,cannot be distinguished from a legitimate regex escape. Giving each filter its own property removes the delimiter entirely, so a pattern may contain any character.A suffix only keeps property names unique and does not affect evaluation, because include patterns are matched with
anyMatchand exclude patterns withnoneMatch. Property names are case-sensitive, so.prodand.PRODare two distinct filters.Quote the value as required by your shell. On
cmd.exein particular,-D...='^prod-.*'arrives as'prod-.*', because single quotes are not stripped and^is the escape character; the result still compiles as a regex but matches nothing. The README documents the correct quoting per shell.Validation
mvn -f sdk/keyvault/azure-security-keyvault-jca/pom.xml clean test: 115 tests pass.mvn -f sdk/keyvault/azure-security-keyvault-jca/pom.xml checkstyle:check spotbugs:check: 0 Checkstyle violations, 0 SpotBugs findings.Notes
azure.keyvault.jca.certificate-alias-filter-patternstoazure.keyvault.jca.certificate-alias-filter-patternis not a breaking change, and the old name is removed rather than deprecated.mainhas been merged in after the 2.12.0 release, so the release notes now target2.13.0-beta.1.All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines