Fix validate config missed errors, when recursion is detected, add tests

E unable to resolv env.foo = {foo}, error: recursion exceeds 20
E unable to resolv env.afoo = a {afoo}, error: recursion exceeds 20
E unable to resolv env.foobar = {barfoo}, error: recursion exceeds 20
E unable to resolv env.barfoo = {foobar}, error: recursion exceeds 20
E unable to resolv env.repeat_unref_exceed = {qq} {qq} {qq} {qq} {qq} {qq}, error: recursion exceeds 20
E unable to resolv env.repeat_unref_exceed_x_4 = {qq} {qq} {qq} {qq} {qq} {qq} {qq} {qq} {qq} {qq} {qq} {qq} {qq} {qq} {qq} {qq} {qq} {qq} {qq} {qq}, error: recursion exceeds 20
E unable to resolv env.repeat_unref_exceed_a_b = {a}{b} {a}{b} {a}{b}, error: recursion exceeds 20

Maximum allowed unresolved ref names is 5
=> env.bar = {foo} {foo} {foo} {foo} {foo}
   evaluated to
   {foo} {foo} {foo} {foo} {foo}

=> env.bar = {foo} {foo} {foo} {foo} {foo} {foo}
   config validate is not accepted, eval is refused

opensvc/core/extconfig.py

@@ -1368,6 +1368,20 @@ def validate_default_options(ret):
                     ret["errors"] += check_known_option(key, "DEFAULT", option)
             return ret
 
+        def validate_env_references(ret):
+            """
+            Validate env section options.
+            """
+            section = "env"
+            for option in cd.get(section, {}):
+                try:
+                    self.conf_get(section, option, stack=[])
+                except ex.Error as error:
+                    value = cd.get(section, {}).get(option)
+                    self.log.error('unable to resolv %s.%s = %s, error: %s', section, option, value, str(error)[:20])
+                    ret["errors"] += 1
+            return ret
+
         def validate_resources_options(ret):
             """
             Validate resource sections options.
@@ -1437,6 +1451,7 @@ def validate_build(ret):
 
         ret = validate_build(ret)
         ret = validate_default_options(ret)
+        ret = validate_env_references(ret)
         ret = validate_resources_options(ret)
 
         return ret

opensvc/tests/commands/svc/test_references.py

@@ -5,12 +5,19 @@
 import pytest
 
 from commands.svc import Mgr
+from core.extconfig import MAX_RECURSION
 from env import Env
 
 NODENAME = Env.nodename
 ID = str(uuid.uuid4())
 SVCNAME = "svc-ref"
 
+
+class ErrorValueContains(object):
+    def __init__(self, message):
+        self.message = message
+
+
 REFS = [  # (name, value, expected_evaluated_value),
     ("ref0", "a b 33", "a b 33"),
     ("string_index0", "{env.ref0[0]}", "a"),
@@ -38,7 +45,6 @@
     ("ref_fqdn", "{fqdn}", "%s.root.svc.default" % SVCNAME),
     ("ref_id", "{id}", ID),
     ("ref_clustername", "{clustername}", "default"),
-    ("ref_clustername", "{clustername}", "default"),
     ("ref_dns", "{dns}", ""),  # no setup yet
     ("ref_dnsnodes", "{dnsnodes}", ""),  # no setup yet
     ("ref_dnsuxsock", "{dnsuxsock}", str(Env.paths.dnsuxsock)),
@@ -58,14 +64,41 @@
     ("mod_upper", "{upper:clustername}", "DEFAULT"),
     ("mod_capitalize", "{capitalize:clustername}", "Default"),
     ("mod_swapcase", "{swapcase:clustername}", "DEFAULT"),
-    ("mod_upper", "{upper:clustername}", "DEFAULT"),
     ("mod_title", "{title:clustername}", "Default"),
 
     # defers
     ("exposed_must_be_null", "{disk#slv1.exposed_devs[0]}", None),  # not "None"
     ("exposed_must_be_empty", "must be null: {disk#slv1.exposed_devs[0]}", None),
+
+    # recursion
+    ("foo", "{foo}", ErrorValueContains("recursion")),
+    ("afoo", "a {afoo}", ErrorValueContains("recursion")),
+    ("foobar", "{barfoo}", ErrorValueContains("recursion")),
+    ("barfoo", "{foobar}", ErrorValueContains("recursion")),
+    ("repeat_unref_exceed", " ".join(["{qq}"] * int(1 + MAX_RECURSION / 4)), ErrorValueContains("recursion exceeds")),
+    ("repeat_unref_exceed_x_4", " ".join(["{qq}"] * MAX_RECURSION), ErrorValueContains("recursion exceeds")),
+    ("repeat_unref_exceed_a_b", " ".join(["{a}{b}"] * (1 + int(MAX_RECURSION / 8))),
+     ErrorValueContains("recursion exceeds")),
+
+    ("repeat_unref_2", "{qq} {qq}", "{qq} {qq}"),
+    ("repeat_unref_3", "{qq} {qq} {qq}", "{qq} {qq} {qq}"),
+
+    # max repeated unresolved names is MAX_RECURSION / 4
+    ("repeat_unref_max", " ".join(["{qq}"] * (int(MAX_RECURSION / 4))), " ".join(["{qq}"] * (int(MAX_RECURSION / 4)))),
+    ("repeat_unref_max2", " ".join(["{a}{b}"] * int(MAX_RECURSION / 8)), " ".join(["{a}{b}"] * int(MAX_RECURSION / 8))),
+
+    ("repeat_ref_2", "{clustername} {clustername}", "default default"),
+    ("repeat_ref_100", " ".join(["{clustername}"] * MAX_RECURSION * 2), " ".join(["default"] * MAX_RECURSION * 2)),
+    ("baz", "abc", "abc"),
+    ("bar", "{baz}ref", "abcref"),
+    ("foo_bar_ref", "a={bar} b={bar} c={bar}",  "a=abcref b=abcref c=abcref"),
+    ("foo_bar_ref_max", " ".join(["{bar}"] * int(MAX_RECURSION / 4)), " ".join(["abcref"] * int(MAX_RECURSION / 4))),
 ]
 
+ref_names = [key[0] for key in REFS]
+dup_ref_names = set([k for k in ref_names if ref_names.count(k) > 1])
+assert len(dup_ref_names) == 0, "duplicated name detected in REFS: %s" % ", ".join(dup_ref_names)
+
 
 @pytest.fixture(scope='function')
 def has_svc_with_ref(has_cluster_config):
@@ -84,14 +117,37 @@ def has_svc_with_ref(has_cluster_config):
         svc_conf.write("\n".join(config_lines))
 
 
+@pytest.fixture(scope='function')
+def has_svc_with_valid_ref(has_cluster_config):
+    with open(os.path.join(Env.paths.pathetc, "%s.conf" % SVCNAME), "w") as svc_conf:
+        config_lines = [
+                           '[DEFAULT]',
+                           'id = %s' % ID,
+                           "nodes = %s" % NODENAME,
+                           "[disk#slv1]",
+                           "type = drbd",
+                           "res = {fqdn}1",
+                           "disk = /tmp/{fqdn}1",
+                           "standby = true",
+                           '[env]'
+                       ] + ["%s = %s" % (name, value) for name, value, expected in REFS
+                            if isinstance(expected, str)]
+        svc_conf.write("\n".join(config_lines))
+
+
 @pytest.mark.ci
-@pytest.mark.usefixtures("has_svc_with_ref")
 @pytest.mark.usefixtures("has_euid_0")
-class TestReferencesConfig(object):
+class TestReferencesConfigValidate(object):
     @staticmethod
-    def test_validate_config():
+    @pytest.mark.usefixtures("has_svc_with_valid_ref")
+    def test_validate_config_result_is_0_when_confif_has_no_errors():
         assert Mgr()(argv=["-s", SVCNAME, "validate", "config"]) == 0
 
+    @staticmethod
+    @pytest.mark.usefixtures("has_svc_with_ref")
+    def test_validate_config_result_is_not_0_when_corrupt():
+        assert Mgr()(argv=["-s", SVCNAME, "validate", "config"]) > 0
+
 
 @pytest.mark.ci
 @pytest.mark.usefixtures("has_svc_with_ref")
@@ -106,26 +162,37 @@ def test_can_get_native_value(capsys, name, value):
     @staticmethod
     @pytest.mark.parametrize("name, expected_eval", [[name, expected_eval] for name, _, expected_eval in REFS])
     def test_can_get_evaluated_value(capsys, osvc_path_tests, name, expected_eval):
-        assert Mgr()(argv=["-s", SVCNAME, "get", "--eval", "--kw", "env.%s" % name]) == 0
         if isinstance(expected_eval, str) and "OSVC_PATH_TESTS" in expected_eval:
             expected_eval = expected_eval.replace("OSVC_PATH_TESTS",
                                                   str(osvc_path_tests))
-        assert capsys.readouterr().out.strip() == str(expected_eval)
+        if isinstance(expected_eval, ErrorValueContains):
+            assert Mgr()(argv=["-s", SVCNAME, "get", "--eval", "--kw", "env.%s" % name]) == 1
+            assert expected_eval.message in capsys.readouterr().err
+        else:
+            assert Mgr()(argv=["-s", SVCNAME, "get", "--eval", "--kw", "env.%s" % name]) == 0
+            assert capsys.readouterr().out.strip() == str(expected_eval)
 
 
 @pytest.mark.ci
-@pytest.mark.usefixtures("has_svc_with_ref")
 @pytest.mark.usefixtures("has_euid_0")
 class TestReferencesPrintConfig(object):
     @staticmethod
     @pytest.mark.parametrize("name, value", [[name, value] for name, value, _ in REFS])
-    def test_has_native_value(capsys, name, value):
+    def test_has_native_value(capsys, has_svc_with_ref, name, value):
         assert Mgr()(argv=["-s", SVCNAME, "print", "config", "--format", "json"]) == 0
         assert json.loads(capsys.readouterr().out)["env"][name] == value
 
     @staticmethod
-    @pytest.mark.parametrize("name, expected_eval", [[name, expected_eval] for name, _, expected_eval in REFS])
-    def test_has_expected_value_when_eval_is_asked(capsys, osvc_path_tests, name, expected_eval):
+    @pytest.mark.parametrize("name, expected_eval",
+                             [[name, expected_eval] for name, _, expected_eval in REFS
+                              if isinstance(expected_eval, str)
+                              ])
+    def test_has_expected_value_when_eval_is_asked(
+            capsys,
+            osvc_path_tests,
+            has_svc_with_valid_ref,
+            name,
+            expected_eval):
         assert Mgr()(argv=["-s", SVCNAME, "print", "config", "--eval", "--format", "json"]) == 0
         if isinstance(expected_eval, str) and "OSVC_PATH_TESTS" in expected_eval:
             expected_eval = expected_eval.replace("OSVC_PATH_TESTS",