From 28a922de51caec3c6ea44848eb928f404d4abd02 Mon Sep 17 00:00:00 2001
From: Alexandre Aubin <alex.aubin@mailoo.org>
Date: Thu, 28 May 2020 21:37:15 +0200
Subject: [PATCH] Get rid of old hashes of file that 'are expected to be
 removed' and are indeed 'already removed'

---
 src/yunohost/regenconf.py            |  6 +++
 src/yunohost/tests/test_regenconf.py | 55 +++++++++++++++++++++++++---
 2 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/src/yunohost/regenconf.py b/src/yunohost/regenconf.py
index 5bfc5df77..ddaf7fe30 100644
--- a/src/yunohost/regenconf.py
+++ b/src/yunohost/regenconf.py
@@ -503,6 +503,12 @@ def _update_conf_hashes(category, hashes):
     if category_conf is None:
         category_conf = {}
 
+    # If a file shall be removed and is indeed removed, forget entirely about
+    # that path.
+    # It avoid keeping weird old entries like
+    # /etc/nginx/conf.d/some.domain.that.got.removed.conf
+    hashes = {path: hash_ for path, hash_ in hashes.items() if hash_ is not None or os.path.exists(path)}
+
     category_conf['conffiles'] = hashes
     categories[category] = category_conf
     _save_regenconf_infos(categories)
diff --git a/src/yunohost/tests/test_regenconf.py b/src/yunohost/tests/test_regenconf.py
index f618278f3..0e2d7a418 100644
--- a/src/yunohost/tests/test_regenconf.py
+++ b/src/yunohost/tests/test_regenconf.py
@@ -1,8 +1,4 @@
-import glob
 import os
-import pytest
-import shutil
-import requests
 
 from conftest import message, raiseYunohostError
 
@@ -14,7 +10,8 @@ from yunohost.utils.error import YunohostError
 from yunohost.regenconf import manually_modified_files, regen_conf, _get_conf_hashes, _force_clear_hashes
 
 TEST_DOMAIN = "secondarydomain.test"
-TEST_DOMAIN_NGINX_CONFIG = "/etc/nginx/conf.d/secondarydomain.test.conf"
+TEST_DOMAIN_NGINX_CONFIG = "/etc/nginx/conf.d/%s.conf" % TEST_DOMAIN
+TEST_DOMAIN_DNSMASQ_CONFIG = "/etc/dnsmasq.d/%s" % TEST_DOMAIN
 SSHD_CONFIG = "/etc/ssh/sshd_config"
 
 def setup_function(function):
@@ -22,11 +19,13 @@ def setup_function(function):
     _force_clear_hashes([TEST_DOMAIN_NGINX_CONFIG])
     clean()
 
+
 def teardown_function(function):
 
     clean()
     _force_clear_hashes([TEST_DOMAIN_NGINX_CONFIG])
 
+
 def clean():
 
     assert os.system("pgrep slapd >/dev/null") == 0
@@ -116,3 +115,49 @@ def test_ssh_conf_unmanaged_and_manually_modified(mocker):
 
     assert SSHD_CONFIG in _get_conf_hashes("ssh")
     assert SSHD_CONFIG not in manually_modified_files()
+
+
+def test_stale_hashes_get_removed_if_empty():
+    """
+    This is intended to test that if a file gets removed and is indeed removed,
+    we don't keep a useless empty hash corresponding to an old file.
+    In this case, we test this using the dnsmasq conf file (we don't do this
+    using the nginx conf file because it's already force-removed during
+    domain_remove())
+    """
+
+    domain_add(TEST_DOMAIN)
+
+    assert os.path.exists(TEST_DOMAIN_DNSMASQ_CONFIG)
+    assert TEST_DOMAIN_DNSMASQ_CONFIG in _get_conf_hashes("dnsmasq")
+
+    domain_remove(TEST_DOMAIN)
+
+    assert not os.path.exists(TEST_DOMAIN_DNSMASQ_CONFIG)
+    assert TEST_DOMAIN_DNSMASQ_CONFIG not in _get_conf_hashes("dnsmasq")
+
+
+def test_stale_hashes_if_file_manually_deleted():
+    """
+    Same as other test, but manually delete the file in between and check
+    behavior
+    """
+
+    domain_add(TEST_DOMAIN)
+
+    assert os.path.exists(TEST_DOMAIN_DNSMASQ_CONFIG)
+    assert TEST_DOMAIN_DNSMASQ_CONFIG in _get_conf_hashes("dnsmasq")
+
+    os.remove(TEST_DOMAIN_DNSMASQ_CONFIG)
+
+    assert not os.path.exists(TEST_DOMAIN_DNSMASQ_CONFIG)
+
+    regen_conf(names=["dnsmasq"])
+
+    assert not os.path.exists(TEST_DOMAIN_DNSMASQ_CONFIG)
+    assert TEST_DOMAIN_DNSMASQ_CONFIG in _get_conf_hashes("dnsmasq")
+
+    domain_remove(TEST_DOMAIN)
+
+    assert not os.path.exists(TEST_DOMAIN_DNSMASQ_CONFIG)
+    assert TEST_DOMAIN_DNSMASQ_CONFIG not in _get_conf_hashes("dnsmasq")