Merge pull request #205 from Vitor-Avila/main

fix: Escaping Jinja logical statements from assets

README.rst

@@ -301,6 +301,45 @@ The function can then be called from any template the following way:
     params:
       ...
 
+Disabling Jinja Templating
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Both the CLI and Superset support Jinja templating. To prevent the CLI from loading Superset Jinja syntax, the export operation automatically escapes Jinja syntax from YAML files. As a consequence, this query:
+
+.. code-block:: yaml
+
+    sql: 'SELECT action, count(*) as times
+        FROM logs
+        {% if filter_values(''action_type'')|length %}
+            WHERE action is null
+            {% for action in filter_values(''action_type'') %}
+                or action = ''{{ action }}''
+            {% endfor %}
+        {% endif %}
+        GROUP BY action'
+
+Becomes this:
+
+.. code-block:: yaml
+
+    sql: 'SELECT action, count(*) as times
+        FROM logs
+        {{ '{% if' }} filter_values(''action_type'')|length {{ '%}' }}
+            WHERE action is null
+            {{ '{% for' }} action in filter_values(''action_type'') {{ '%}' }}
+                or action = ''{{ '{{' }} action {{ '}}' }}''
+            {{ '{% endfor %}' }}
+        {{ '{% endif %}' }}
+        GROUP BY action'
+
+When performing the import, the CLI would load any templating syntax that isn't escaped, and remove escaping. However, this escaping syntax isn't compatible with UI imports. 
+To avoid issues when running migrations using both the CLI and the UI, you can use:
+
+- ``--disable-jinja-escaping`` flag with the ``export-assets`` command to disable the escaping (so that exported assets can be imported via the UI)
+- ``--disable-jinja-templating`` flag with the ``sync native`` command to disable jinja templating (so that assets exported via the UI can be imported via the CLI)
+
+Note that using these flags would remove the ability to dynamically modify the content through the CLI. 
+
 Synchronizing to and from dbt
 -----------------------------
 

src/preset_cli/cli/superset/export.py

@@ -2,6 +2,7 @@
 A command to export Superset resources into a directory.
 """
 
+import re
 from collections import defaultdict
 from pathlib import Path
 from typing import List, Set, Tuple
@@ -27,6 +28,12 @@
     default=False,
     help="Overwrite existing resources",
 )
+@click.option(
+    "--disable-jinja-escaping",
+    is_flag=True,
+    default=False,
+    help="Disable Jinja template escaping",
+)
 @click.option(
     "--asset-type",
     help="Asset type",
@@ -62,6 +69,7 @@ def export_assets(  # pylint: disable=too-many-locals, too-many-arguments
     chart_ids: List[str],
     dashboard_ids: List[str],
     overwrite: bool = False,
+    disable_jinja_escaping: bool = False,
 ) -> None:
     """
     Export DBs/datasets/charts/dashboards to a directory.
@@ -89,16 +97,18 @@ def export_assets(  # pylint: disable=too-many-locals, too-many-arguments
                 root,
                 client,
                 overwrite,
+                disable_jinja_escaping,
                 skip_related=not ids_requested,
             )
 
 
-def export_resource(  # pylint: disable=too-many-arguments
+def export_resource(  # pylint: disable=too-many-arguments, too-many-locals
     resource_name: str,
     requested_ids: Set[int],
     root: Path,
     client: SupersetClient,
     overwrite: bool,
+    disable_jinja_escaping: bool,
     skip_related: bool = True,
 ) -> None:
     """
@@ -131,21 +141,48 @@ def export_resource(  # pylint: disable=too-many-arguments
             target.parent.mkdir(parents=True, exist_ok=True)
 
         # escape any pre-existing Jinja2 templates
-        file_contents = file_contents.replace(
-            "{{",
-            f"{JINJA2_OPEN_MARKER} '{{{{' {JINJA2_CLOSE_MARKER}",
-        )
-        file_contents = file_contents.replace(
-            "}}",
-            f"{JINJA2_OPEN_MARKER} '}}}}' {JINJA2_CLOSE_MARKER}",
-        )
-        file_contents = file_contents.replace(JINJA2_OPEN_MARKER, "{{")
-        file_contents = file_contents.replace(JINJA2_CLOSE_MARKER, "}}")
+        if not disable_jinja_escaping:
+            file_contents = jinja_escaper(file_contents)
 
         with open(target, "w", encoding="utf-8") as output:
             output.write(file_contents)
 
 
+def jinja_escaper(value: str) -> str:
+    """
+    Escape Jinja macros and logical statements that shouldn't be handled by CLI
+    """
+    logical_statements_patterns = [
+        r"(\{%-?\s*if)",  # {%if || {% if || {%-if || {%- if
+        r"(\{%-?\s*elif)",  # {%elif || {% elif || {%-elif || {%- elif
+        r"(\{%-?\s*else)",  # {%else || {% else || {%-else || {%- else
+        r"(\{%-?\s*endif)",  # {%endif || {% endif || {%-endif || {%- endif
+        r"(\{%-?\s*for)",  # {%for || {% for || {%-for || {%- for
+        r"(\{%-?\s*endfor)",  # {%endfor || {% endfor || {%-endfor || {%- endfor
+        r"(%})",  # %}
+        r"(-%})",  # -%}
+    ]
+
+    for syntax in logical_statements_patterns:
+        replacement = JINJA2_OPEN_MARKER + " '" + r"\1" + "' " + JINJA2_CLOSE_MARKER
+        value = re.sub(syntax, replacement, value)
+
+    # escaping macros
+    value = value.replace(
+        "{{",
+        f"{JINJA2_OPEN_MARKER} '{{{{' {JINJA2_CLOSE_MARKER}",
+    )
+    value = value.replace(
+        "}}",
+        f"{JINJA2_OPEN_MARKER} '}}}}' {JINJA2_CLOSE_MARKER}",
+    )
+    value = value.replace(JINJA2_OPEN_MARKER, "{{")
+    value = value.replace(JINJA2_CLOSE_MARKER, "}}")
+    value = re.sub(r"' }} {{ '", " ", value)
+
+    return value
+
+
 @click.command()
 @click.argument(
     "path",

src/preset_cli/cli/superset/sync/native/command.py

@@ -80,6 +80,16 @@ def is_yaml_config(path: Path) -> bool:
     )
 
 
+def load_yaml(path: Path) -> Dict[str, Any]:
+    """
+    Load a YAML file and returns it as a dictionary.
+    """
+    with open(path, encoding="utf-8") as input_:
+        content = input_.read()
+
+    return yaml.load(content, Loader=yaml.SafeLoader)
+
+
 def render_yaml(path: Path, env: Dict[str, Any]) -> Dict[str, Any]:
     """
     Load a YAML file as a template, render it, and deserialize it.
@@ -108,6 +118,12 @@ def render_yaml(path: Path, env: Dict[str, Any]) -> Dict[str, Any]:
     multiple=True,
     help="Custom values for templates (eg, country=BR)",
 )
+@click.option(
+    "--disable-jinja-templating",
+    is_flag=True,
+    default=False,
+    help="By default, the CLI supports Jinja templating. This flag disables it",
+)
 @click.option(
     "--disallow-edits",
     is_flag=True,
@@ -130,11 +146,12 @@ def render_yaml(path: Path, env: Dict[str, Any]) -> Dict[str, Any]:
     help="Split imports into individual assets",
 )
 @click.pass_context
-def native(  # pylint: disable=too-many-locals, too-many-arguments
+def native(  # pylint: disable=too-many-locals, too-many-arguments, too-many-branches
     ctx: click.core.Context,
     directory: str,
     option: Tuple[str, ...],
     overwrite: bool = False,
+    disable_jinja_templating: bool = False,
     disallow_edits: bool = True,  # pylint: disable=unused-argument
     external_url_prefix: str = "",
     load_env: bool = False,
@@ -171,11 +188,19 @@ def native(  # pylint: disable=too-many-locals, too-many-arguments
         if path_name.is_dir() and not path_name.stem.startswith("."):
             queue.extend(path_name.glob("*"))
         elif is_yaml_config(relative_path):
-            config = render_yaml(path_name, env)
+            config = (
+                load_yaml(path_name)
+                if disable_jinja_templating
+                else render_yaml(path_name, env)
+            )
 
             overrides_path = path_name.with_suffix(".overrides" + path_name.suffix)
             if overrides_path.exists():
-                overrides = render_yaml(overrides_path, env)
+                overrides = (
+                    load_yaml(overrides_path)
+                    if disable_jinja_templating
+                    else render_yaml(overrides_path, env)
+                )
                 dict_merge(config, overrides)
 
             config["is_managed_externally"] = disallow_edits