feat(bridge): accept wrapped secrets as passwords #11896
Conversation
apps/emqx_ldap/src/emqx_ldap.erl
Outdated
| @@ -142,7 +140,7 @@ on_start( | |||
| ?SLOG(info, #{ | |||
| msg => "starting_ldap_connector", | |||
| connector => InstId, | |||
| config => emqx_utils:redact(Config) | |||
There was a problem hiding this comment.
In the particular case of LDAP schema, for example, there seems to be this bind_password field that could be lurking around in the config.
I haven't checked if it's really the case here, but I think it's safer to keep those redact calls wherever we log configurations, in general.
There was a problem hiding this comment.
Besides, it'll format passwords as ****** rather than #Fun<mod.43.3316493>.
keynslug
force-pushed
the
ft/EMQX-10808/opt-file-secret-bridges
branch
4 times, most recently
from
62ab4ca
to
cb69604
Compare
| @@ -280,7 +280,7 @@ handle_backend_update_result({error, Reason}, _) -> | |||
|
|
|||
| to_json(Data) -> | |||
| emqx_utils_maps:jsonable_map( | |||
| Data, | |||
| emqx_utils:redact(Data), | |||
There was a problem hiding this comment.
Nit: mb. rename the fun to to_redacted_json to avoid misuse 🤔
| @@ -140,7 +140,7 @@ mongo_fields() -> | |||
| {srv_record, fun srv_record/1}, | |||
| {pool_size, fun emqx_connector_schema_lib:pool_size/1}, | |||
| {username, fun emqx_connector_schema_lib:username/1}, | |||
| {password, fun emqx_connector_schema_lib:password/1}, | |||
| {password, emqx_connector_schema_lib:password_field()}, | |||
There was a problem hiding this comment.
Nit: may be a bit confusing lack of uniformness of API, mod:fun1() vs fun mod:fun2/1
There was a problem hiding this comment.
Agree here, that's kinda unfortunate. I added a field suffix which follows a module-level convention of naming field schemas in literal map() form, but it's indeed suboptimal. I honestly currently have no idea what to do with this without making a lot of changes across a lot of apps, once more.
apps/emqx_bridge_sqlserver/src/emqx_bridge_sqlserver_connector.erl
Outdated
Show resolved
Hide resolved
apps/emqx_bridge_influxdb/src/emqx_bridge_influxdb_connector.erl
Outdated
Show resolved
Hide resolved
| "\n resource_opts {" | ||
| "\n health_check_interval = 10s" | ||
| "\n }" | ||
| "\n }". |
There was a problem hiding this comment.
Nit: previous version was easier to read. 😅
There was a problem hiding this comment.
Mostly agree. One of the issues I had is that former formatting, in addition to requiring erlfmt pragmas, was messing my editor badly for some reason. Another one is that it was kinda misleading: it doesn't work the way people usually expect when they see """ (at least it won't work until Erlang/OTP 27).
That are coming from `emqx_schema_secret`. Also adapt pgsql-related connectors.
Also test authorization with mongo in bridge / auth test suites.
Co-authored-by: Thales Macedo Garitezi <thalesmg@gmail.com>
keynslug
force-pushed
the
ft/EMQX-10808/opt-file-secret-bridges
branch
from
c2b84fc
to
d1c3b1c
Compare
Fixes EMQX-10808.
Followup to #11809.
Documentation in emqx/emqx-docs#2215.
Progress
Summary
🤖 Generated by Copilot at e215451
This pull request improves the password handling and authentication support for various bridge plugins, such as MongoDB, Kafka, ClickHouse, and PostgreSQL. It also enhances the test modules and docker-compose files for these plugins, and updates some version numbers and code formatting.
PR Checklist
Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:
changes/(ce|ee)/(feat|perf|fix|breaking)-<PR-id>.en.mdfiles