From fdfa3213ccf8dc6a65990dd64b541d6e56fccba7 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Thu, 23 Nov 2023 10:25:36 -0300 Subject: [PATCH] fix(bridge_api): don't mangle configs, use correct type as argument Fixes https://emqx.atlassian.net/browse/EMQX-11412 - The wrong type was being used in a list lookup function, resulting in the automatic transformation being called erroneously and mangling the config. - There was a left-over workaround still around which could still mangle the config. --- apps/emqx_bridge/src/emqx_action_info.erl | 11 +++-- apps/emqx_bridge/src/emqx_bridge.erl | 1 - apps/emqx_bridge/src/emqx_bridge_v2.erl | 49 ++++++++++--------- apps/emqx_bridge/test/emqx_bridge_testlib.erl | 16 ++++++ .../test/emqx_bridge_v2_testlib.erl | 1 + .../emqx_bridge_azure_event_hub_v2_SUITE.erl | 9 ++++ .../emqx_bridge_v2_kafka_producer_SUITE.erl | 14 ++++++ 7 files changed, 74 insertions(+), 27 deletions(-) diff --git a/apps/emqx_bridge/src/emqx_action_info.erl b/apps/emqx_bridge/src/emqx_action_info.erl index 44f871d53..12988b163 100644 --- a/apps/emqx_bridge/src/emqx_action_info.erl +++ b/apps/emqx_bridge/src/emqx_action_info.erl @@ -120,12 +120,15 @@ action_type_to_bridge_v1_type(ActionType, Conf) -> ActionInfoMap = info_map(), ActionTypeToBridgeV1Type = maps:get(action_type_to_bridge_v1_type, ActionInfoMap), case maps:get(ActionType, ActionTypeToBridgeV1Type, undefined) of - undefined -> ActionType; - BridgeV1TypeFun when is_function(BridgeV1TypeFun) -> BridgeV1TypeFun(get_confs(Conf)); - BridgeV1Type -> BridgeV1Type + undefined -> + ActionType; + BridgeV1TypeFun when is_function(BridgeV1TypeFun) -> + BridgeV1TypeFun(get_confs(ActionType, Conf)); + BridgeV1Type -> + BridgeV1Type end. -get_confs(#{connector := ConnectorName, type := ActionType} = ActionConfig) -> +get_confs(ActionType, #{<<"connector">> := ConnectorName} = ActionConfig) -> ConnectorType = action_type_to_connector_type(ActionType), ConnectorConfig = emqx_conf:get_raw([connectors, ConnectorType, ConnectorName]), {ActionConfig, ConnectorConfig}. diff --git a/apps/emqx_bridge/src/emqx_bridge.erl b/apps/emqx_bridge/src/emqx_bridge.erl index 44945e22f..0e116589b 100644 --- a/apps/emqx_bridge/src/emqx_bridge.erl +++ b/apps/emqx_bridge/src/emqx_bridge.erl @@ -314,7 +314,6 @@ list() -> BridgeV2Bridges = emqx_bridge_v2:bridge_v1_list_and_transform(), BridgeV1Bridges ++ BridgeV2Bridges. -%%BridgeV2Bridges = emqx_bridge_v2:list(). lookup(Id) -> {Type, Name} = emqx_bridge_resource:parse_bridge_id(Id), diff --git a/apps/emqx_bridge/src/emqx_bridge_v2.erl b/apps/emqx_bridge/src/emqx_bridge_v2.erl index e02783de4..6f296c63c 100644 --- a/apps/emqx_bridge/src/emqx_bridge_v2.erl +++ b/apps/emqx_bridge/src/emqx_bridge_v2.erl @@ -1063,33 +1063,38 @@ bridge_v1_list_and_transform() -> Bridges = list_with_lookup_fun(fun bridge_v1_lookup_and_transform/2), [B || B <- Bridges, B =/= not_bridge_v1_compatible_error()]. -bridge_v1_lookup_and_transform(BridgeV1Type, Name) -> - case ?MODULE:bridge_v1_is_valid(BridgeV1Type, Name) of - true -> - Type = ?MODULE:bridge_v1_type_to_bridge_v2_type(BridgeV1Type), - case lookup(Type, Name) of - {ok, #{raw_config := #{<<"connector">> := ConnectorName}} = BridgeV2} -> - ConnectorType = connector_type(Type), +bridge_v1_lookup_and_transform(ActionType, Name) -> + case lookup(ActionType, Name) of + {ok, #{raw_config := #{<<"connector">> := ConnectorName}} = ActionConfig} -> + BridgeV1Type = ?MODULE:bridge_v2_type_to_bridge_v1_type(ActionType, ActionConfig), + case ?MODULE:bridge_v1_is_valid(BridgeV1Type, Name) of + true -> + ConnectorType = connector_type(ActionType), case emqx_connector:lookup(ConnectorType, ConnectorName) of {ok, Connector} -> bridge_v1_lookup_and_transform_helper( - BridgeV1Type, Name, Type, BridgeV2, ConnectorType, Connector + BridgeV1Type, + Name, + ActionType, + ActionConfig, + ConnectorType, + Connector ); Error -> Error end; - Error -> - Error + false -> + not_bridge_v1_compatible_error() end; - false -> - not_bridge_v1_compatible_error() + Error -> + Error end. not_bridge_v1_compatible_error() -> {error, not_bridge_v1_compatible}. bridge_v1_lookup_and_transform_helper( - BridgeV1Type, BridgeName, BridgeV2Type, BridgeV2, ConnectorType, Connector + BridgeV1Type, BridgeName, ActionType, Action, ConnectorType, Connector ) -> ConnectorRawConfig1 = maps:get(raw_config, Connector), ConnectorRawConfig2 = fill_defaults( @@ -1098,10 +1103,10 @@ bridge_v1_lookup_and_transform_helper( <<"connectors">>, emqx_connector_schema ), - BridgeV2RawConfig1 = maps:get(raw_config, BridgeV2), - BridgeV2RawConfig2 = fill_defaults( - BridgeV2Type, - BridgeV2RawConfig1, + ActionRawConfig1 = maps:get(raw_config, Action), + ActionRawConfig2 = fill_defaults( + ActionType, + ActionRawConfig1, <<"actions">>, emqx_bridge_v2_schema ), @@ -1110,7 +1115,7 @@ bridge_v1_lookup_and_transform_helper( emqx_action_info:has_custom_connector_action_config_to_bridge_v1_config(BridgeV1Type) of false -> - BridgeV1Config1 = maps:remove(<<"connector">>, BridgeV2RawConfig2), + BridgeV1Config1 = maps:remove(<<"connector">>, ActionRawConfig2), %% Move parameters to the top level ParametersMap = maps:get(<<"parameters">>, BridgeV1Config1, #{}), BridgeV1Config2 = maps:remove(<<"parameters">>, BridgeV1Config1), @@ -1118,13 +1123,13 @@ bridge_v1_lookup_and_transform_helper( emqx_utils_maps:deep_merge(ConnectorRawConfig2, BridgeV1Config3); true -> emqx_action_info:connector_action_config_to_bridge_v1_config( - BridgeV1Type, ConnectorRawConfig2, BridgeV2RawConfig2 + BridgeV1Type, ConnectorRawConfig2, ActionRawConfig2 ) end, - BridgeV1Tmp = maps:put(raw_config, BridgeV1ConfigFinal, BridgeV2), + BridgeV1Tmp = maps:put(raw_config, BridgeV1ConfigFinal, Action), BridgeV1 = maps:remove(status, BridgeV1Tmp), - BridgeV2Status = maps:get(status, BridgeV2, undefined), - BridgeV2Error = maps:get(error, BridgeV2, undefined), + BridgeV2Status = maps:get(status, Action, undefined), + BridgeV2Error = maps:get(error, Action, undefined), ResourceData1 = maps:get(resource_data, BridgeV1, #{}), %% Replace id in resouce data BridgeV1Id = <<"bridge:", (bin(BridgeV1Type))/binary, ":", (bin(BridgeName))/binary>>, diff --git a/apps/emqx_bridge/test/emqx_bridge_testlib.erl b/apps/emqx_bridge/test/emqx_bridge_testlib.erl index df404d9b0..f486e5d64 100644 --- a/apps/emqx_bridge/test/emqx_bridge_testlib.erl +++ b/apps/emqx_bridge/test/emqx_bridge_testlib.erl @@ -120,6 +120,22 @@ create_bridge(Config, Overrides) -> ct:pal("creating bridge with config: ~p", [BridgeConfig]), emqx_bridge:create(BridgeType, BridgeName, BridgeConfig). +list_bridges_api() -> + Params = [], + Path = emqx_mgmt_api_test_util:api_path(["bridges"]), + AuthHeader = emqx_mgmt_api_test_util:auth_header_(), + Opts = #{return_all => true}, + ct:pal("listing bridges (via http)"), + Res = + case emqx_mgmt_api_test_util:request_api(get, Path, "", AuthHeader, Params, Opts) of + {ok, {Status, Headers, Body0}} -> + {ok, {Status, Headers, emqx_utils_json:decode(Body0, [return_maps])}}; + Error -> + Error + end, + ct:pal("list bridge result: ~p", [Res]), + Res. + create_bridge_api(Config) -> create_bridge_api(Config, _Overrides = #{}). diff --git a/apps/emqx_bridge/test/emqx_bridge_v2_testlib.erl b/apps/emqx_bridge/test/emqx_bridge_v2_testlib.erl index 6c48f5663..5cb9b043f 100644 --- a/apps/emqx_bridge/test/emqx_bridge_v2_testlib.erl +++ b/apps/emqx_bridge/test/emqx_bridge_v2_testlib.erl @@ -139,6 +139,7 @@ create_bridge(Config, Overrides) -> ConnectorName = ?config(connector_name, Config), ConnectorType = ?config(connector_type, Config), ConnectorConfig = ?config(connector_config, Config), + ct:pal("creating connector with config: ~p", [ConnectorConfig]), {ok, _} = emqx_connector:create(ConnectorType, ConnectorName, ConnectorConfig), diff --git a/apps/emqx_bridge_azure_event_hub/test/emqx_bridge_azure_event_hub_v2_SUITE.erl b/apps/emqx_bridge_azure_event_hub/test/emqx_bridge_azure_event_hub_v2_SUITE.erl index 4d441ea0b..9661004d0 100644 --- a/apps/emqx_bridge_azure_event_hub/test/emqx_bridge_azure_event_hub_v2_SUITE.erl +++ b/apps/emqx_bridge_azure_event_hub/test/emqx_bridge_azure_event_hub_v2_SUITE.erl @@ -368,3 +368,12 @@ t_parameters_key_api_spec(_Config) -> ?assert(is_map_key(<<"parameters">>, ActionProps), #{action_props => ActionProps}), ok. + +t_http_api_get(Config) -> + ?assertMatch({ok, _}, emqx_bridge_v2_testlib:create_bridge(Config)), + %% v1 api; no mangling of configs; has `kafka' top level config key + ?assertMatch( + {ok, {{_, 200, _}, _, [#{<<"kafka">> := _}]}}, + emqx_bridge_testlib:list_bridges_api() + ), + ok. diff --git a/apps/emqx_bridge_kafka/test/emqx_bridge_v2_kafka_producer_SUITE.erl b/apps/emqx_bridge_kafka/test/emqx_bridge_v2_kafka_producer_SUITE.erl index 8ce3b7f6b..2ad0504b4 100644 --- a/apps/emqx_bridge_kafka/test/emqx_bridge_v2_kafka_producer_SUITE.erl +++ b/apps/emqx_bridge_kafka/test/emqx_bridge_v2_kafka_producer_SUITE.erl @@ -369,3 +369,17 @@ t_parameters_key_api_spec(_Config) -> ?assert(is_map_key(<<"parameters">>, ActionProps), #{action_props => ActionProps}), ok. + +t_http_api_get(_Config) -> + ConnectorName = <<"test_connector">>, + ActionName = <<"test_action">>, + ActionConfig = bridge_v2_config(<<"test_connector">>), + ConnectorConfig = connector_config(), + ?assertMatch({ok, _}, create_connector(ConnectorName, ConnectorConfig)), + ?assertMatch({ok, _}, create_action(ActionName, ActionConfig)), + %% v1 api; no mangling of configs; has `kafka' top level config key + ?assertMatch( + {ok, {{_, 200, _}, _, [#{<<"kafka">> := _}]}}, + emqx_bridge_testlib:list_bridges_api() + ), + ok.