/views/*/options: fix when used with e.g. tags

The issue is not now; it has always been broken in 6.x.
The model is that at most one view applies on any request.
If we need to do more things, they must happen as one meta-action.

test_view_insert_action(): dropped; can't see a suitable replacement

diff --git a/manager/knot_resolver_manager/datamodel/templates/macros/view_macros.lua.j2 b/manager/knot_resolver_manager/datamodel/templates/macros/view_macros.lua.j2
index b61cc3f03474f6a948721d8eec6908b57d32f284..2f1a796461b7f97646b0ebd2e661c1851ac1134d 100644 (file)
--- a/manager/knot_resolver_manager/datamodel/templates/macros/view_macros.lua.j2
+++ b/manager/knot_resolver_manager/datamodel/templates/macros/view_macros.lua.j2
@@ -5,11 +5,6 @@
 {%- endfor -%}
 {%- endmacro -%}
 
-{% macro view_insert_action(view, subnet, action) -%}
-assert(C.kr_view_insert_action('{{ subnet }}', '{{ view.dst_subnet or '' }}',
-       {{ get_proto_set(view.protocols) }}, {{ action }})==0)
-{%- endmacro %}
-
 {% macro view_flags(options) -%}
 {% if not options.minimize -%}
 "NO_MINIMIZE",

diff --git a/manager/knot_resolver_manager/datamodel/templates/views.lua.j2 b/manager/knot_resolver_manager/datamodel/templates/views.lua.j2
index d4357cbbe1ab5ad2a01a34c7c4b2b5e47a716ec1..81de8c7b78d28ef04ad0eb856d2094046c71ce37 100644 (file)
--- a/manager/knot_resolver_manager/datamodel/templates/views.lua.j2
+++ b/manager/knot_resolver_manager/datamodel/templates/views.lua.j2
@@ -1,21 +1,24 @@
 {% from 'macros/common_macros.lua.j2' import quotes %}
-{% from 'macros/view_macros.lua.j2' import view_insert_action, view_flags, view_answer %}
+{% from 'macros/view_macros.lua.j2' import get_proto_set, view_flags, view_answer %}
 {% from 'macros/policy_macros.lua.j2' import policy_flags, policy_tags_assign %}
 
 {% if cfg.views %}
 {% for view in cfg.views %}
 {% for subnet in view.subnets %}
 
-{% if view.tags -%}
-{{ view_insert_action(view, subnet, policy_tags_assign(view.tags)) }}
-{% elif view.answer %}
-{{ view_insert_action(view, subnet, view_answer(view.answer)) }}
+assert(C.kr_view_insert_action('{{ subnet }}', '{{ view.dst_subnet or '' }}',
+  {{ get_proto_set(view.protocols) }}, policy.COMBINE({
+{%- set flags = view_flags(view.options) -%}
+{% if flags %}
+       {{ quotes(policy_flags(flags)) }},
 {%- endif %}
 
-{%- set flags = view_flags(view.options) -%}
-{% if flags -%}
-{{ view_insert_action(view, subnet, quotes(policy_flags(flags))) }}
+{% if view.tags %}
+       {{ policy_tags_assign(view.tags) }},
+{% elif view.answer %}
+       {{ view_answer(view.answer) }},
 {%- endif %}
+  })) == 0)
 
 {% endfor %}
 {% endfor %}

diff --git a/manager/knot_resolver_manager/datamodel/view_schema.py b/manager/knot_resolver_manager/datamodel/view_schema.py
index 98d4a1a0dd506aa065659d71cbdf1563d170fb4d..ad44eb3b3db73c6fb877ee50c0670c6ddc935b69 100644 (file)
--- a/manager/knot_resolver_manager/datamodel/view_schema.py
+++ b/manager/knot_resolver_manager/datamodel/view_schema.py
@@ -42,4 +42,4 @@ class ViewSchema(ConfigSchema):
 
     def _validate(self) -> None:
         if bool(self.tags) == bool(self.answer):
-            raise ValueError("only one of 'tags' and 'answer' options must be configured")
+            raise ValueError("exactly one of 'tags' and 'answer' must be configured")

diff --git a/manager/tests/unit/datamodel/templates/test_view_macros.py b/manager/tests/unit/datamodel/templates/test_view_macros.py
index 3a3f35f93c0dae645851ccd9b823cacdffac4fe3..fbc62f49cb044d3d95356adf5822ed26c921a6c1 100644 (file)
--- a/manager/tests/unit/datamodel/templates/test_view_macros.py
+++ b/manager/tests/unit/datamodel/templates/test_view_macros.py
@@ -6,16 +6,6 @@ from knot_resolver_manager.datamodel.config_schema import template_from_str
 from knot_resolver_manager.datamodel.view_schema import ViewOptionsSchema, ViewSchema
 
 
-def test_view_insert_action():
-    subnet = "10.0.0.0/8"
-    action = "policy.DENY"
-    tmpl_str = """{% from 'macros/view_macros.lua.j2' import view_insert_action %}
-{{ view_insert_action(subnet, action) }}"""
-
-    tmpl = template_from_str(tmpl_str)
-    assert tmpl.render(subnet=subnet, action=action) == f"assert(C.kr_view_insert_action('{ subnet }',{ action })==0)"
-
-
 def test_view_flags():
     tmpl_str = """{% from 'macros/view_macros.lua.j2' import view_flags %}
 {{ view_flags(options) }}"""

diff --git a/modules/policy/policy.lua b/modules/policy/policy.lua
index 443fc0b03cb018606cbcdb199f95bed1794f09c2..60b034783aed70ecf6b43b95c425b5b896653ab9 100644 (file)
--- a/modules/policy/policy.lua
+++ b/modules/policy/policy.lua
@@ -856,6 +856,16 @@ function policy.TAGS_ASSIGN(names)
        return 'policy.tags_assign_bitmap(' .. tostring(bitmap) .. ')'
 end
 
+-- Perform a list of actions sequentially; meant for kr_view_insert_action().
+function policy.COMBINE(list)
+       if #list == 1 then return list[1] end
+       local r = 'function(state,req) '
+               for _, item in ipairs(list) do
+                       r = r .. item .. '(state,req); '
+               end
+       return r .. 'end'
+end
+
 --[[ Insert a forwarding rule, i.e. override upstream for one DNS subtree.
 
 Throws lua exceptions when detecting something fishy.