Don't configure Asterisk channels whose domain name cannot be resolved

app/models/channel.rb

@@ -40,6 +40,10 @@ class Channel < ActiveRecord::Base
 
   broker_cached
 
+  def name=(value)
+    super(value.try(:strip))
+  end
+
   def config
     self[:config] ||= {}
   end

app/models/channels/sip.rb

@@ -18,12 +18,12 @@
 class Channels::Sip < Channel
   validate :server_username_uniqueness
 
-  config_accessor :username
-  config_accessor :password
-  config_accessor :domain
+  config_accessor :username, strip: true
+  config_accessor :password, strip: true
+  config_accessor :domain, strip: true
   config_accessor :direction
   config_accessor :register
-  config_accessor :number
+  config_accessor :number, strip: true
 
   def register?
     register == true || register == "1"

broker/src/asterisk/asterisk_channel_srv.erl

@@ -21,7 +21,11 @@
 % channel_status: dict(channel_id, {channel_id, registration_ok, error_message})
 %   Registration statuses
 %
--record(state, {channels, dynamic_channels, registry, channel_status, config_job_state = idle, status_job_state = idle}).
+% config_channel_status: dict(channel_id, {channel_id, registration_ok, error_message})
+%   Channel configuration errors; if a channel fails to configure correctly,
+%   there will be an entry for it here
+%
+-record(state, {channels, dynamic_channels, registry, channel_status, config_channel_status, config_job_state = idle, status_job_state = idle}).
 
 start_link() ->
   gen_server:start_link({local, ?SERVER}, ?MODULE, {}, []).
@@ -83,49 +87,49 @@ handle_call({register_channel, ChannelId, PeerIp}, _From, State) ->
   {reply, ok, NewState};
 
 handle_call({get_channel_status, ChannelIds}, _From, State) ->
-  case State#state.channel_status of
-    undefined -> {reply, [], State};
-    Status ->
-      Result = lists:foldl(fun(ChannelId, R) ->
-        case dict:find(ChannelId, Status) of
-          {ok, ChannelStatus} -> [ChannelStatus | R];
-          _ -> R
-        end
-      end, [], ChannelIds),
-      {reply, Result, State}
-  end;
+  Result = lists:foldl(fun(ChannelId, R) ->
+    case find_channel_status(ChannelId, State) of
+      undefined -> R;
+      ChannelStatus -> [ChannelStatus | R]
+    end
+  end, [], ChannelIds),
+  {reply, Result, State};
 
 handle_call(_Request, _From, State) ->
   {reply, {error, unknown_call}, State}.
 
+
 %% @private
 handle_cast(regenerate_config, State = #state{config_job_state = JobState}) ->
   NewState = case JobState of
     idle ->
       spawn_link(fun() ->
         {ok, BaseConfigPath} = application:get_env(asterisk_config_dir),
         ConfigFilePath = filename:join(BaseConfigPath, "pjsip_verboice.conf"),
-        {ChannelIndex, RegistryIndex} = asterisk_config:generate(ConfigFilePath),
+        {ChannelIndex, RegistryIndex, ConfigErrors} = asterisk_config:generate(ConfigFilePath),
         ami_client:sip_reload(),
-        gen_server:cast(?MODULE, {set_channels, ChannelIndex, RegistryIndex})
+        gen_server:cast(?MODULE, {set_channels, ChannelIndex, RegistryIndex, ConfigErrors})
       end),
-      State#state{config_job_state = working};
+      State#state{config_job_state = working, config_channel_status = undefined};
     working ->
       State#state{config_job_state = must_regenerate};
     must_regenerate ->
       State
   end,
   {noreply, NewState};
 
-handle_cast({set_channels, ChannelIndex, RegistryIndex}, State = #state{config_job_state = JobState}) ->
-  lager:info("Updated Asterisk Channel registry: ~B IP-number and ~B SIP registrations",
-             [dict:size(ChannelIndex), dict:size(RegistryIndex)]),
-  lager:debug("Asterisk Channel registry: ~n~p~n~p~n", [ChannelIndex, RegistryIndex]),
+handle_cast({set_channels, ChannelIndex, RegistryIndex, ConfigErrors}, State = #state{config_job_state = JobState}) ->
+  lager:info("Updated Asterisk Channel registry: ~B IP-number and ~B SIP registrations, ~B errors",
+             [dict:size(ChannelIndex), dict:size(RegistryIndex), length(ConfigErrors)]),
+  lager:debug("Asterisk Registrations: ~p", [RegistryIndex]),
+  lager:debug("Asterisk Channels: ~p", [ChannelIndex]),
+  lager:debug("Asterisk Configuration Errors: ~p", [ConfigErrors]),
   case JobState of
     must_regenerate -> regenerate_config();
     _ -> ok
   end,
-  {noreply, State#state{channels = ChannelIndex, registry = RegistryIndex, config_job_state = idle}};
+  ConfigChannelStatus = dict:from_list(lists:map(fun(Value = {ChannelId, _, _}) -> {ChannelId, Value} end, ConfigErrors)),
+  {noreply, State#state{channels = ChannelIndex, registry = RegistryIndex, config_channel_status = ConfigChannelStatus, config_job_state = idle}};
 
 handle_cast({set_channel_status, Status}, State = #state{channel_status = PrevStatus}) ->
   channel:log_broken_channels(PrevStatus, Status),
@@ -174,3 +178,20 @@ find_registered_channel(ChannelIds, ChannelStatus) ->
         Result
     end, undefined, ChannelIds).
 
+find_channel_status(ChannelId, #state{channel_status = ChannelStatus, config_channel_status = ConfigChannelStatus}) ->
+  case find_channel_status(ChannelId, ChannelStatus) of
+    undefined ->
+      find_channel_status(ChannelId, ConfigChannelStatus);
+    Status ->
+      Status
+  end;
+
+find_channel_status(_, undefined) ->
+  undefined;
+find_channel_status(ChannelId, StatusDict) ->
+  case dict:find(ChannelId, StatusDict) of
+    {ok, Status} ->
+      Status;
+    _ ->
+      undefined
+  end.

broker/src/asterisk/asterisk_config.erl

@@ -23,17 +23,18 @@ generate(ConfigFilePath) ->
 % user.
 generate_config(ConfigFile) ->
   Channels = channel:find_all_sip(),
-  generate_config(Channels, ConfigFile, dict:new(), dict:new(), dict:new()).
+  generate_config(Channels, ConfigFile, dict:new(), dict:new(), dict:new(), []).
 
-generate_config([], _, _, ChannelIndex, RegistryIndex) -> {ChannelIndex, RegistryIndex};
-generate_config([Channel | Rest], ConfigFile, ResolvCache, ChannelIndex, RegistryIndex) ->
+generate_config([], _, _, ChannelIndex, RegistryIndex, ConfigErrors) -> {ChannelIndex, RegistryIndex, ConfigErrors};
+generate_config([Channel | Rest], ConfigFile, ResolvCache, ChannelIndex, RegistryIndex, ConfigErrors) ->
   HeaderLines = binary:split(erlang:iolist_to_binary(pretty_print(Channel)), <<"\n">>, [global]),
   lists:foreach(fun(Line) ->
     file:write(ConfigFile, ["; ", Line, "\n"])
   end, HeaderLines),
   file:write(ConfigFile, "\n"),
 
-  Section = ["verboice_", integer_to_list(Channel#channel.id)],
+  ChannelId = Channel#channel.id,
+  Section = ["verboice_", integer_to_list(ChannelId)],
   Username = channel:username(Channel),
   Password = channel:password(Channel),
   Domain = channel:domain(Channel),
@@ -72,93 +73,114 @@ generate_config([Channel | Rest], ConfigFile, ResolvCache, ChannelIndex, Registr
       file:write(ConfigFile, "remove_existing=yes\n"),
       file:write(ConfigFile, "\n"),
 
-      generate_config(Rest, ConfigFile, ResolvCache, ChannelIndex, RegistryIndex);
+      generate_config(Rest, ConfigFile, ResolvCache, ChannelIndex, RegistryIndex, ConfigErrors);
 
     _ ->
-      HasAuth = length(Username) > 0 andalso length(Password) > 0,
-      UserOrNumber = case length(Username) > 0 of
-        true -> Username;
-        _ -> Number
-      end,
-
-      % Registration
-      NewRegistryIndex = case channel:register(Channel) of
+      {Expanded, NewCache} = expand_domain(Domain, ResolvCache),
+      % TODO: if we consider the channel's direction (ie. inbound/outbound) we
+      % may relax this condition since we only need a set of resolved IP
+      % addresses to match incoming calls. Although, the domain needs to be
+      % resolvable also if registration is enabled.
+      case is_resolved(Expanded) of
         true ->
+          HasAuth = length(Username) > 0 andalso length(Password) > 0,
+          UserOrNumber = case length(Username) > 0 of
+            true -> Username;
+            _ -> Number
+          end,
+
+          % Registration
+          NewRegistryIndex = case channel:register(Channel) of
+            true ->
+              file:write(ConfigFile, ["[", Section, "]\n"]),
+              file:write(ConfigFile, "type=registration\n"),
+              if HasAuth ->
+                file:write(ConfigFile, ["outbound_auth=", Section, "\n"]);
+                true -> ok
+              end,
+              file:write(ConfigFile, ["server_uri=sip:", Domain, "\n"]),
+              file:write(ConfigFile, ["client_uri=sip:", UserOrNumber, "@", Domain, "\n"]),
+              %% When Asterisk Version > 13.1...
+              % file:write(ConfigFile, "line=yes\n"),
+              % file:write(ConfigFile, ["endpoint=", Section, "\n"]),
+              file:write(ConfigFile, "\n"),
+              dict:store({Username, Domain}, Channel#channel.id, RegistryIndex);
+
+            _ -> RegistryIndex
+          end,
+
+          % Endpoint
           file:write(ConfigFile, ["[", Section, "]\n"]),
-          file:write(ConfigFile, "type=registration\n"),
+          file:write(ConfigFile, "type=endpoint\n"),
+          file:write(ConfigFile, "context=verboice\n"),
+          file:write(ConfigFile, ["aors=", Section, "\n"]),
           if HasAuth ->
             file:write(ConfigFile, ["outbound_auth=", Section, "\n"]);
             true -> ok
           end,
-          file:write(ConfigFile, ["server_uri=sip:", Domain, "\n"]),
-          file:write(ConfigFile, ["client_uri=sip:", UserOrNumber, "@", Domain, "\n"]),
-          %% When Asterisk Version > 13.1...
-          % file:write(ConfigFile, "line=yes\n"),
-          % file:write(ConfigFile, ["endpoint=", Section, "\n"]),
+          file:write(ConfigFile, ["from_user=", UserOrNumber, "\n"]),
+          file:write(ConfigFile, ["from_domain=", Domain, "\n"]),
+          file:write(ConfigFile, "disallow=all\n"),
+          file:write(ConfigFile, "allow=ulaw\n"),
+          file:write(ConfigFile, "allow=gsm\n"),
           file:write(ConfigFile, "\n"),
-          dict:store({Username, Domain}, Channel#channel.id, RegistryIndex);
-
-        _ -> RegistryIndex
-      end,
-
-      % Endpoint
-      file:write(ConfigFile, ["[", Section, "]\n"]),
-      file:write(ConfigFile, "type=endpoint\n"),
-      file:write(ConfigFile, "context=verboice\n"),
-      file:write(ConfigFile, ["aors=", Section, "\n"]),
-      if HasAuth ->
-        file:write(ConfigFile, ["outbound_auth=", Section, "\n"]);
-        true -> ok
-      end,
-      file:write(ConfigFile, ["from_user=", UserOrNumber, "\n"]),
-      file:write(ConfigFile, ["from_domain=", Domain, "\n"]),
-      file:write(ConfigFile, "disallow=all\n"),
-      file:write(ConfigFile, "allow=ulaw\n"),
-      file:write(ConfigFile, "allow=gsm\n"),
-      file:write(ConfigFile, "\n"),
-
-      % Auth
-      if HasAuth ->
-        file:write(ConfigFile, ["[", Section, "]\n"]),
-        file:write(ConfigFile, "type=auth\n"),
-        file:write(ConfigFile, "auth_type=userpass\n"),
-        file:write(ConfigFile, ["username=", Username, "\n"]),
-        file:write(ConfigFile, ["password=", Password, "\n"]),
-        file:write(ConfigFile, "\n");
-        true -> ok
-      end,
 
-      % AOR
-      file:write(ConfigFile, ["[", Section, "]\n"]),
-      file:write(ConfigFile, "type=aor\n"),
-      file:write(ConfigFile, "qualify_frequency=60\n"),
-      file:write(ConfigFile, ["contact=sip:", Domain, "\n"]),
-      file:write(ConfigFile, "\n"),
+          % Auth
+          if HasAuth ->
+            file:write(ConfigFile, ["[", Section, "]\n"]),
+            file:write(ConfigFile, "type=auth\n"),
+            file:write(ConfigFile, "auth_type=userpass\n"),
+            file:write(ConfigFile, ["username=", Username, "\n"]),
+            file:write(ConfigFile, ["password=", Password, "\n"]),
+            file:write(ConfigFile, "\n");
+            true -> ok
+          end,
 
-      % Identify
-      file:write(ConfigFile, ["[", Section, "]\n"]),
-      file:write(ConfigFile, "type=identify\n"),
-      file:write(ConfigFile, ["endpoint=", Section, "\n"]),
+          % AOR
+          file:write(ConfigFile, ["[", Section, "]\n"]),
+          file:write(ConfigFile, "type=aor\n"),
+          file:write(ConfigFile, "qualify_frequency=60\n"),
+          file:write(ConfigFile, ["contact=sip:", Domain, "\n"]),
+          file:write(ConfigFile, "\n"),
 
-      {Expanded, NewCache} = expand_domain(Domain, ResolvCache),
-      {NewChannelIndex, _} = lists:foldl(fun ({_Host, IPs, Port}, {R1, I}) ->
-        case Port of
-          undefined -> ok;
-          _ -> ok
-            % file:write(ConfigFile, ["port=", integer_to_list(Port), "\n"])
-        end,
+          % Identify
+          file:write(ConfigFile, ["[", Section, "]\n"]),
+          file:write(ConfigFile, "type=identify\n"),
+          file:write(ConfigFile, ["endpoint=", Section, "\n"]),
+
+          {NewChannelIndex, _} = lists:foldl(fun ({_Host, IPs, Port}, {R1, I}) ->
+            case Port of
+              undefined -> ok;
+              _ -> ok
+                % file:write(ConfigFile, ["port=", integer_to_list(Port), "\n"])
+            end,
+
+            R3 = lists:foldl(fun (IP, R2) ->
+              file:write(ConfigFile, ["match=", IP, "\n"]),
+              dict:append({util:to_string(IP), Number}, Channel#channel.id, R2)
+            end, R1, IPs),
+            {R3, I + 1}
+          end, {ChannelIndex, 0}, Expanded),
+          file:write(ConfigFile, "\n"),
 
-        R3 = lists:foldl(fun (IP, R2) ->
-          file:write(ConfigFile, ["match=", IP, "\n"]),
-          dict:append({util:to_string(IP), Number}, Channel#channel.id, R2)
-        end, R1, IPs),
-        {R3, I + 1}
-      end, {ChannelIndex, 0}, Expanded),
-      file:write(ConfigFile, "\n"),
+          generate_config(Rest, ConfigFile, NewCache, NewChannelIndex, NewRegistryIndex, ConfigErrors);
 
-      generate_config(Rest, ConfigFile, NewCache, NewChannelIndex, NewRegistryIndex)
+        false ->
+          % Domain could not be resolved, we cannot generate the endpoint
+          % information for Asterisk
+          Message = iolist_to_binary(["Error: could not resolve domain ", Domain]),
+          NewConfigErrors = [{ChannelId, false, [Message]}|ConfigErrors],
+          generate_config(Rest, ConfigFile, NewCache, ChannelIndex, RegistryIndex, NewConfigErrors)
+      end
   end.
 
+is_resolved([]) ->
+  false;
+is_resolved([{_Host, [_Ip|_], _Port}|_]) ->
+  true;
+is_resolved([_|Rest]) ->
+  is_resolved(Rest).
+
 expand_domain(<<>>, ResolvCache) -> {[], ResolvCache};
 expand_domain(Domain, ResolvCache) ->
   case dict:find(Domain, ResolvCache) of
@@ -191,7 +213,7 @@ expand_domain(Domain) ->
         {ok, #hostent{h_addr_list = IpList}} ->
           IPs = map_ips(IpList),
           [{Domain, IPs, undefined}];
-        _ -> [{Domain, [Domain], undefined}]
+        _ -> [{Domain, [], undefined}]
       end
   end.
 

config/initializers/active_record.rb

@@ -19,6 +19,7 @@ class ActiveRecord::Base
   def self.config_accessor(*names)
     options = names.extract_options!
     default = options[:default]
+    strip = options[:strip]
 
     names.each do |name|
       self.config_attrs << name
@@ -31,6 +32,7 @@ def self.config_accessor(*names)
         respond_to?(:encrypted_config_will_change!) ? encrypted_config_will_change! : config_will_change!
 
         self.config ||= {}
+        value = value.try(:strip) if strip
         self.config[name.to_s] = value
 
         # this is needed so attr_encrypted encrypts the value correctly