Merge pull request #1149 from YunoHost/fix-dyndns-madness

Fix dyndns madness

data/actionsmap/yunohost.yml

@@ -1363,13 +1363,11 @@ dyndns:
 
         ### dyndns_installcron()
         installcron:
-            action_help: Install IP update cron
-            api: POST /dyndns/cron
+            deprecated: true
 
         ### dyndns_removecron()
         removecron:
-            action_help: Remove IP update cron
-            api: DELETE /dyndns/cron
+            deprecated: true
 
 
 #############################

data/hooks/backup/42-conf_ynh_dyndns

@@ -8,4 +8,3 @@ cd "$YNH_CWD"
 
 # Backup the configuration
 ynh_exec_warn_less ynh_backup --src_path="/etc/yunohost/dyndns" --not_mandatory
-ynh_exec_warn_less ynh_backup --src_path="/etc/cron.d/yunohost-dyndns" --not_mandatory

data/hooks/conf_regen/01-yunohost

@@ -86,6 +86,17 @@ SHELL=/bin/bash
 0 7,19 * * * root : YunoHost Automatic Diagnosis; sleep \$((RANDOM\\%1200)); yunohost diagnosis run --email > /dev/null 2>/dev/null || echo "Running the automatic diagnosis failed miserably"
 EOF
 
+  # If we subscribed to a dyndns domain, add the corresponding cron
+  # - delay between 0 and 60 secs to spread the check over a 1 min window
+  # - do not run the command if some process already has the lock, to avoid queuing hundreds of commands...
+  if ls -l /etc/yunohost/dyndns/K*.private 2>/dev/null
+  then
+    cat > $pending_dir/etc/cron.d/yunohost-dyndns << EOF
+SHELL=/bin/bash
+*/10 * * * * root : YunoHost DynDNS update; sleep \$((RANDOM\\%60)); test -e /var/run/moulinette_yunohost.lock || yunohost dyndns update >> /dev/null
+EOF
+  fi
+
   # legacy stuff to avoid yunohost reporting etckeeper as manually modified
   # (this make sure that the hash is null / file is flagged as to-delete)
   mkdir -p $pending_dir/etc/etckeeper

data/hooks/restore/42-conf_ynh_dyndns

@@ -7,4 +7,3 @@ cd "$YNH_CWD"
 
 # Restore file if exists
 ynh_restore_file --origin_path="/etc/yunohost/dyndns" --not_mandatory
-ynh_restore_file --origin_path="/etc/cron.d/yunohost-dyndns" --not_mandatory

debian/control

@@ -27,7 +27,7 @@ Depends: ${python3:Depends}, ${misc:Depends}
  , redis-server
  , metronome (>=3.14.0)
  , acl
- , git, curl, wget, cron, unzip, jq, bc
+ , git, curl, wget, cron, unzip, jq, bc, at
  , lsb-release, haveged, fake-hwclock, equivs, lsof, whois
 Recommends: yunohost-admin
  , ntp, inetutils-ping | iputils-ping

locales/en.json

@@ -291,9 +291,6 @@
     "dpkg_lock_not_available": "This command can't be run right now because another program seems to be using the lock of dpkg (the system package manager)",
     "dyndns_could_not_check_provide": "Could not check if {provider:s} can provide {domain:s}.",
     "dyndns_could_not_check_available": "Could not check if {domain:s} is available on {provider:s}.",
-    "dyndns_cron_installed": "DynDNS cron job created",
-    "dyndns_cron_remove_failed": "Could not remove the DynDNS cron job because: {error}",
-    "dyndns_cron_removed": "DynDNS cron job removed",
     "dyndns_ip_update_failed": "Could not update IP address to DynDNS",
     "dyndns_ip_updated": "Updated your IP on DynDNS",
     "dyndns_key_generating": "Generating DNS key... It may take a while.",

src/yunohost/domain.py

@@ -119,11 +119,11 @@ def domain_add(operation_logger, domain, dyndns=False):
     # DynDNS domain
     if dyndns:
 
-        # Do not allow to subscribe to multiple dyndns domains...
-        if os.path.exists("/etc/cron.d/yunohost-dyndns"):
-            raise YunohostError("domain_dyndns_already_subscribed")
+        from yunohost.dyndns import dyndns_subscribe, _dyndns_provides, _guess_current_dyndns_domain
 
-        from yunohost.dyndns import dyndns_subscribe, _dyndns_provides
+        # Do not allow to subscribe to multiple dyndns domains...
+        if _guess_current_dyndns_domain("dyndns.yunohost.org") != (None, None):
+            raise YunohostError('domain_dyndns_already_subscribed')
 
         # Check that this domain can effectively be provided by
         # dyndns.yunohost.org. (i.e. is it a nohost.me / noho.st)
@@ -245,6 +245,9 @@ def domain_remove(operation_logger, domain, remove_apps=False, force=False):
 
     os.system("rm -rf /etc/yunohost/certs/%s" % domain)
 
+    # Delete dyndns keys for this domain (if any)
+    os.system('rm -rf /etc/yunohost/dyndns/K%s.+*' % domain)
+
     # Sometime we have weird issues with the regenconf where some files
     # appears as manually modified even though they weren't touched ...
     # There are a few ideas why this happens (like backup/restore nginx

src/yunohost/dyndns.py

@@ -40,6 +40,7 @@ from yunohost.utils.error import YunohostError
 from yunohost.domain import _get_maindomain, _build_dns_conf
 from yunohost.utils.network import get_public_ip, dig
 from yunohost.log import is_unit_operation
+from yunohost.regenconf import regen_conf
 
 logger = getActionLogger("yunohost.dyndns")
 
@@ -121,10 +122,9 @@ def dyndns_subscribe(
         subscribe_host -- Dynette HTTP API to subscribe to
 
     """
-    if len(glob.glob("/etc/yunohost/dyndns/*.key")) != 0 or os.path.exists(
-        "/etc/cron.d/yunohost-dyndns"
-    ):
-        raise YunohostError("domain_dyndns_already_subscribed")
+
+    if _guess_current_dyndns_domain(subscribe_host) != (None, None):
+        raise YunohostError('domain_dyndns_already_subscribed')
 
     if domain is None:
         domain = _get_maindomain()
@@ -169,7 +169,7 @@ def dyndns_subscribe(
     try:
         r = requests.post(
             "https://%s/key/%s?key_algo=hmac-sha512"
-            % (subscribe_host, base64.b64encode(key)),
+            % (subscribe_host, base64.b64encode(key.encode()).decode()),
             data={"subdomain": domain},
             timeout=30,
         )
@@ -186,9 +186,18 @@ def dyndns_subscribe(
             error = 'Server error, code: %s. (Message: "%s")' % (r.status_code, r.text)
         raise YunohostError("dyndns_registration_failed", error=error)
 
-    logger.success(m18n.n("dyndns_registered"))
+    # Yunohost regen conf will add the dyndns cron job if a private key exists
+    # in /etc/yunohost/dyndns
+    regen_conf(["yunohost"])
 
-    dyndns_installcron()
+    # Add some dyndns update in 2 and 4 minutes from now such that user should
+    # not have to wait 10ish minutes for the conf to propagate
+    cmd = "at -M now + {t} >/dev/null 2>&1 <<< \"/bin/bash -c 'yunohost dyndns update'\""
+    # For some reason subprocess doesn't like the redirections so we have to use bash -c explicity...
+    subprocess.check_call(["bash", "-c", cmd.format(t="2 min")])
+    subprocess.check_call(["bash", "-c", cmd.format(t="4 min")])
+
+    logger.success(m18n.n('dyndns_registered'))
 
 
 @is_unit_operation()
@@ -220,6 +229,10 @@ def dyndns_update(
     # If domain is not given, try to guess it from keys available...
     if domain is None:
         (domain, key) = _guess_current_dyndns_domain(dyn_host)
+
+    if domain is None:
+        raise YunohostError('dyndns_no_domain_registered')
+
     # If key is not given, pick the first file we find with the domain given
     else:
         if key is None:
@@ -361,29 +374,11 @@ def dyndns_update(
 
 
 def dyndns_installcron():
-    """
-    Install IP update cron
-
-
-    """
-    with open("/etc/cron.d/yunohost-dyndns", "w+") as f:
-        f.write("*/2 * * * * root yunohost dyndns update >> /dev/null\n")
-
-    logger.success(m18n.n("dyndns_cron_installed"))
+    logger.warning("This command is deprecated. The dyndns cron job should automatically be added/removed by the regenconf depending if there's a private key in /etc/yunohost/dyndns. You can run the regenconf yourself with 'yunohost tools regen-conf yunohost'.")
 
 
 def dyndns_removecron():
-    """
-    Remove IP update cron
-
-
-    """
-    try:
-        os.remove("/etc/cron.d/yunohost-dyndns")
-    except Exception as e:
-        raise YunohostError("dyndns_cron_remove_failed", error=e)
-
-    logger.success(m18n.n("dyndns_cron_removed"))
+    logger.warning("This command is deprecated. The dyndns cron job should automatically be added/removed by the regenconf depending if there's a private key in /etc/yunohost/dyndns. You can run the regenconf yourself with 'yunohost tools regen-conf yunohost'.")
 
 
 def _guess_current_dyndns_domain(dyn_host):
@@ -414,4 +409,4 @@ def _guess_current_dyndns_domain(dyn_host):
         else:
             return (_domain, path)
 
-    raise YunohostError("dyndns_no_domain_registered")
+    return (None, None)