From 85a8eff90bf89ac3c8bd4f99fba2c7dacd132acc Mon Sep 17 00:00:00 2001
From: "Zaiming (Stone) Shi" <zmstone@gmail.com>
Date: Tue, 10 Jan 2023 21:58:48 +0100
Subject: [PATCH] fix(emqx_resource_manager): do not start when disabled

---
 apps/emqx_bridge/src/emqx_bridge_resource.erl | 23 ++++++++++---------
 .../src/emqx_resource_manager.erl             |  4 ++--
 changes/v5.0.14/fix-9717.en.md                |  1 +
 changes/v5.0.14/fix-9717.zh.md                |  1 +
 4 files changed, 16 insertions(+), 13 deletions(-)
 create mode 100644 changes/v5.0.14/fix-9717.en.md
 create mode 100644 changes/v5.0.14/fix-9717.zh.md

diff --git a/apps/emqx_bridge/src/emqx_bridge_resource.erl b/apps/emqx_bridge/src/emqx_bridge_resource.erl
index b28b891a8..18ce354f1 100644
--- a/apps/emqx_bridge/src/emqx_bridge_resource.erl
+++ b/apps/emqx_bridge/src/emqx_bridge_resource.erl
@@ -132,13 +132,14 @@ create(BridgeId, Conf) ->
 create(Type, Name, Conf) ->
     create(Type, Name, Conf, #{}).
 
-create(Type, Name, Conf, Opts) ->
+create(Type, Name, Conf, Opts0) ->
     ?SLOG(info, #{
         msg => "create bridge",
         type => Type,
         name => Name,
         config => Conf
     }),
+    Opts = override_start_after_created(Conf, Opts0),
     {ok, _Data} = emqx_resource:create_local(
         resource_id(Type, Name),
         <<"emqx_bridge">>,
@@ -146,7 +147,7 @@ create(Type, Name, Conf, Opts) ->
         parse_confs(bin(Type), Name, Conf),
         Opts
     ),
-    maybe_disable_bridge(Type, Name, Conf).
+    ok.
 
 update(BridgeId, {OldConf, Conf}) ->
     {BridgeType, BridgeName} = parse_bridge_id(BridgeId),
@@ -155,7 +156,7 @@ update(BridgeId, {OldConf, Conf}) ->
 update(Type, Name, {OldConf, Conf}) ->
     update(Type, Name, {OldConf, Conf}, #{}).
 
-update(Type, Name, {OldConf, Conf}, Opts) ->
+update(Type, Name, {OldConf, Conf}, Opts0) ->
     %% TODO: sometimes its not necessary to restart the bridge connection.
     %%
     %% - if the connection related configs like `servers` is updated, we should restart/start
@@ -164,6 +165,7 @@ update(Type, Name, {OldConf, Conf}, Opts) ->
     %% the `method` or `headers` of a WebHook is changed, then the bridge can be updated
     %% without restarting the bridge.
     %%
+    Opts = override_start_after_created(Conf, Opts0),
     case emqx_map_lib:if_only_to_toggle_enable(OldConf, Conf) of
         false ->
             ?SLOG(info, #{
@@ -174,10 +176,10 @@ update(Type, Name, {OldConf, Conf}, Opts) ->
             }),
             case recreate(Type, Name, Conf, Opts) of
                 {ok, _} ->
-                    maybe_disable_bridge(Type, Name, Conf);
+                    ok;
                 {error, not_found} ->
                     ?SLOG(warning, #{
-                        msg => "updating_a_non-exist_bridge_need_create_a_new_one",
+                        msg => "updating_a_non_existing_bridge",
                         type => Type,
                         name => Name,
                         config => Conf
@@ -242,12 +244,6 @@ remove(Type, Name, _Conf, _Opts) ->
         {error, Reason} -> {error, Reason}
     end.
 
-maybe_disable_bridge(Type, Name, Conf) ->
-    case maps:get(enable, Conf, true) of
-        false -> stop(Type, Name);
-        true -> ok
-    end.
-
 maybe_clear_certs(TmpPath, #{ssl := SslConf} = Conf) ->
     %% don't remove the cert files if they are in use
     case is_tmp_path_conf(TmpPath, SslConf) of
@@ -321,3 +317,8 @@ str(Str) when is_list(Str) -> Str.
 bin(Bin) when is_binary(Bin) -> Bin;
 bin(Str) when is_list(Str) -> list_to_binary(Str);
 bin(Atom) when is_atom(Atom) -> atom_to_binary(Atom, utf8).
+
+override_start_after_created(Config, Opts) ->
+    Enabled = maps:get(enable, Config, true),
+    StartAfterCreated = Enabled andalso maps:get(start_after_created, Opts, Enabled),
+    Opts#{start_after_created => StartAfterCreated}.
diff --git a/apps/emqx_resource/src/emqx_resource_manager.erl b/apps/emqx_resource/src/emqx_resource_manager.erl
index 8ad3fdd80..8531f1641 100644
--- a/apps/emqx_resource/src/emqx_resource_manager.erl
+++ b/apps/emqx_resource/src/emqx_resource_manager.erl
@@ -112,7 +112,7 @@ recreate(ResId, ResourceType, NewConfig, Opts) ->
     end.
 
 create_and_return_data(MgrId, ResId, Group, ResourceType, Config, Opts) ->
-    create(MgrId, ResId, Group, ResourceType, Config, Opts),
+    _ = create(MgrId, ResId, Group, ResourceType, Config, Opts),
     {ok, _Group, Data} = lookup(ResId),
     {ok, Data}.
 
@@ -304,7 +304,7 @@ init({Data, Opts}) ->
     process_flag(trap_exit, true),
     %% init the cache so that lookup/1 will always return something
     insert_cache(Data#data.id, Data#data.group, Data),
-    case maps:get(start_after_created, Opts, true) of
+    case maps:get(start_after_created, Opts, ?START_AFTER_CREATED) of
         true -> {ok, connecting, Data, {next_event, internal, start_resource}};
         false -> {ok, stopped, Data}
     end.
diff --git a/changes/v5.0.14/fix-9717.en.md b/changes/v5.0.14/fix-9717.en.md
new file mode 100644
index 000000000..9a3b29157
--- /dev/null
+++ b/changes/v5.0.14/fix-9717.en.md
@@ -0,0 +1 @@
+Prior to this fix, if it always times out when trying to connect a bridge server, it's not possible to change other configs even when the bridge is disabled.
diff --git a/changes/v5.0.14/fix-9717.zh.md b/changes/v5.0.14/fix-9717.zh.md
new file mode 100644
index 000000000..859d7806f
--- /dev/null
+++ b/changes/v5.0.14/fix-9717.zh.md
@@ -0,0 +1 @@
+修复已禁用的桥接资源服务器连接超时的情况下不能修改其他配置参数的问题。