oscerd commented on PR #1900:
URL: 
https://github.com/apache/camel-spring-boot/pull/1900#issuecomment-5434518618

   Thanks @davsclaus — I've addressed the two substantive points. Head is now 
`801da0c94f1` (rebased onto current `main`, which is why it's a force update).
   
   **4. "from the vault" wording in `SpringBootCloudConfigPropertiesParser` — 
fixed.**
   
   You're right, that was a wording defect I introduced by applying the same 
text uniformly across all seven parsers. Spring Cloud Config isn't a vault. Its 
messages now say "from Spring Cloud Config", and the comment above the flag 
says "effective configuration value" rather than "effective secret".
   
   **3. `camel.vault.ignore-resolution-failures` not in config metadata — 
fixed.**
   
   Good catch. The flag is read from the `Environment` at 
`ApplicationEnvironmentPreparedEvent` time, before any bean exists, so it can't 
hang off a `@ConfigurationProperties` bean the way the per-vault options do. 
I've declared it in `core/camel-spring-boot`'s 
`additional-spring-configuration-metadata.json` instead, which is the mechanism 
already used there for exactly this case (`management.info.camel.enabled`, 
`management.server.accesslog.enabled`). It regenerates into `spring-boot.json` 
alongside the other `camel.vault.*` entries, so it now shows up in IDE 
completion.
   
   **1. Test coverage across the other six parsers — noted, not done here.**
   
   Agreed it's worth doing, and you're right that the pattern needs no Docker. 
I've left it out of this PR to keep the changeset reviewable rather than adding 
six near-identical test classes to a security-relevant change that's already 
touching seven modules. Happy to follow up with a separate PR, or to add them 
here if you'd prefer them to land together — your call.
   
   **2. Upgrade-guide entry — still outstanding, deliberately.**
   
   The companion `apache/camel` PR for `camel-4x-upgrade-guide-4_23.adoc` isn't 
open yet. I'd rather write it against the wording that actually lands here than 
pre-empt review. It will be opened before this merges.
   
   Verification after the changes: `EarlyResolutionFailureTest` 2/2 green; full 
reactor build 394 modules SUCCESS. The one reactor failure is 
`camel-toon-starter`, which is unrelated to this PR — it was added to `main` in 
`978a820dd46` and depends on `org.apache.camel:camel-toon:4.23.0-SNAPSHOT`, 
which isn't published to apache.snapshots yet; it fails the same way on a clean 
`main`.
   
   Conversations left open for you to resolve.


-- 
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]

Reply via email to