oscerd opened a new pull request, #28105: URL: https://github.com/apache/camel/pull/28105
Fixes [CAMEL-25801](https://issues.apache.org/jira/browse/CAMEL-25801) — four of the seven items on that umbrella issue. The remaining three are deliberately deferred; see the bottom. ## 1. `updateKeyMetadata` re-uploaded the private key AWS, Azure and Vault all did: ```java public void updateKeyMetadata(String keyId, KeyMetadata metadata) throws Exception { KeyPair keyPair = getKey(keyId); storeKey(keyId, keyPair, metadata); } ``` So every `expireKey`, `revokeKey` and rotation-deprecation re-uploaded the PKCS#8 private key. On a Vault KV v2 mount with the default `max_versions = 10`, **ten metadata edits purge every historical version of the private key**. It also meant `revokeKey` threw from `getKey` for a key whose private secret was already gone — exactly when revoking matters most. All three now write only the metadata entry, which `FileBasedKeyLifecycleManager` already did. ## 2. `FileBasedKeyLifecycleManager.listKeys()` read a boot-time cache ```java public List<KeyMetadata> listKeys() throws Exception { return new ArrayList<>(metadataCache.values()); } ``` `metadataCache` is filled once by `loadExistingKeys()` in the constructor. A key file written afterwards was invisible for the life of the process, though `getKey` would read it happily. `KeyRotationScheduler.checkAndRotate()` drives entirely off `listKeys()`, so a file-backed deployment silently stopped rotating anything it had not seen at boot. It now scans the directory, as the other three query their store. ## 3. `deleteKeyState` reported success having deleted nothing ```java KeyLifecycleManager klm = getConfiguration().getKeyLifecycleManager(); if (klm != null) { klm.deleteKey(keyId); } ``` Every other lifecycle operation uses `getRequiredKeyLifecycleManager()` and fails loudly. For a stateful key, "I deleted the exhausted state" when nothing was deleted is the worst available failure mode. ## 4. A combined PEM export could not be imported back `exportKeyPair(PEM, includePrivate = true)` emits both a `PUBLIC KEY` and a `PRIVATE KEY` block, but the parser stripped only one pair of armour lines and handed the rest — still containing `-----BEGIN PUBLIC KEY-----` — to `Base64.getDecoder()`, which rejects `-`. Against `main` the new test fails with `java.lang.IllegalArgumentException: Illegal base64 character 2d`. Blocks are now located by their BEGIN/END lines, so a document with several yields the requested one. **The export format is unchanged**, so documents written by earlier versions still read. PEM handling is also made explicitly UTF-8 rather than the platform default charset. ## Tests - `KeyLifecycleParityTest` — `listKeys` sees a key written by a second manager instance; non-metadata files are ignored; combined and public-only PEM round-trips; a metadata update does not touch the private key file. - `PQCDeleteKeyStateTest` — fails without a manager, deletes with one, rejects a missing key id. Against `main`: `expected: <1> but was: <0>`, `Illegal base64 character 2d`, and the silent delete. Full build green: 247 tests, 0 failures. ### Honest gap Items 1 and 3 are proven against the file-based and in-memory managers. **The AWS, Azure and Vault `updateKeyMetadata` changes have no unit test** — those managers only have `*IT` tests needing live services, which is itself one of the findings on the umbrella issue. The change is the same three-line shape in each, and `KeyLifecycleParityTest` pins the contract via the file-based manager, but a reviewer should eyeball those three diffs. ## Deferred from the umbrella issue Not in this PR, to keep behaviour fixes separate from refactoring: - **AWS/Azure deletion semantics** — AWS uses `forceDeleteWithoutRecovery(true)`, Azure discards the `SyncPoller` from `beginDeleteSecret`. Opposite durability behind one interface; picking either is a semantics decision worth its own discussion. - **AWS and Vault duplicating `KeyAlgorithmSupport`** (~40 lines each) — pure refactor. - **AWS `listKeys` has no server-side name filter** — an optimisation. ## Note: autowiring keeps masking these Three times now a test passed for the wrong reason because `keyPair`, `classicalKeyPair` and `keyLifecycleManager` are all `@Metadata(autowired = true)` — a single registry bean reaches endpoints meant to have none. Both new tests set `setAutowiredEnabled(false)`. Flagged on #28091 as well; it likely wants its own issue. --- _Reported, analysed and fixed by Claude Code (AI agent) on behalf of @oscerd._ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
