DanielLeens commented on issue #10309: URL: https://github.com/apache/seatunnel/issues/10309#issuecomment-4414923642
Thanks @ryanvflannery for picking this up. I rechecked the current config path before replying. The important point is that SeaTunnel already has a concrete extension point here: `ConfigShade` + `ConfigShadeUtils`. Today the current behavior is still parse-time decryption, not worker-only late resolution: - CLI / file loading goes through `ConfigBuilder`, which calls `ConfigShadeUtils.decryptConfig(...)` while loading the config. - REST submission also calls the same `decryptConfig(...)` path on the server before job submission. So for a first implementation, I strongly recommend keeping the scope aligned with the current contract instead of introducing a new generic runtime secret-resolution model in the same PR. A good first scope would be: 1. implement one provider via `ConfigShade`; 2. use `shade.identifier` + `shade.properties` for provider selection and provider-specific settings; 3. keep decryption limited to the current sensitive-option model (`password`, `token`, etc., plus explicit `shade.options` when needed); 4. keep it opt-in and backward-compatible. A few boundaries are worth keeping explicit: - server-local / worker-local key material outside the submitted job config is not part of the current `ConfigShade` contract today, so that should be treated as a follow-up design topic rather than bundled into the first PR; - a built-in AES implementation in core and external key-vault integration are related, but they are not the same scope. I would avoid mixing both in the first patch; - if we eventually want "client does not need vault credentials, only server/workers do", that is a larger architectural change than the current SPI path, because the config is decrypted where it is parsed today. For this particular issue, I would keep the PR centered on an external provider path first. If you want to add a built-in AES implementation in core later, please treat that as a separate follow-up issue/PR so the key-management discussion stays clear. If you open a focused draft PR around the existing `ConfigShade` SPI, that should be a good starting point. Happy to review it. -- 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]
