sc-12670 add option to parse comments out to parameter descriptions (#19

)

Co-authored-by: Matthew Warren <[email protected]>

pyproject.toml

@@ -10,7 +10,7 @@ dependencies = [
     "python-dotenv @ git+https://github.com/cloudtruth/python-dotenv@feature/dump-dotenv",
     "python-hcl2",
     "python-liquid",
-    "pyyaml",
+    "ruamel.yaml[jinja2] < 0.19",
     "requests",
 ]
 requires-python = ">=3.10"
@@ -25,14 +25,17 @@ dev = [
     "pytest-mypy",
     "pytest-timeout",
     "types-requests",
-    "types-pyyaml",
 ]
 [project.scripts]
 cloudtruth-dynamic-importer = "dynamic_importer.main:import_config"
 
 [tool.mypy]
 packages = "dynamic_importer"
 
+[[tool.mypy.overrides]]
+module = ["ruamel"]
+ignore_missing_imports = "true"
+
 [tool.pytest.ini_options]
 addopts = ["--import-mode=importlib", "--cov=dynamic_importer", "--cov-report=term-missing", "--cov-report=xml", "--mypy"]
 minversion = 6.0

samples/advanced/app-config.yaml.hbs

@@ -9,39 +9,32 @@ backend:
   # Used for enabling authentication, secret is shared by all backend plugins
   # See https://backstage.io/docs/auth/service-to-service-auth for
   # information on the format
-  # auth:
-  #   keys:
-  #     - secret: ${BACKEND_SECRET}
+  auth:
+    keys:
+      - secret: ${BACKEND_SECRET}
   baseUrl: http://localhost:7007
   listen:
     port: 7007
-    # Uncomment the following host directive to bind to specific interfaces
-    # host: 127.0.0.1
   csp:
-    connect-src: ["'self'", 'http:', 'https:']
     # Content-Security-Policy directives follow the Helmet format: https://helmetjs.github.io/#reference
     # Default Helmet Content-Security-Policy values can be removed by setting the key to false
+    connect-src: ["'self'", 'http:', 'https:']
   cors:
     origin: http://localhost:3000
     methods: [GET, HEAD, PATCH, POST, PUT, DELETE]
     credentials: true
-  # This is for local development only, it is not recommended to use this in production
-  # The production database configuration is stored in app-config.production.yaml
   database:
+    # This is for local development only, it is not recommended to use this in production
+    # The production database configuration is stored in app-config.production.yaml
     client: better-sqlite3
     connection: ':memory:'
-  # workingDirectory: /tmp # Use this to configure a working directory for the scaffolder, defaults to the OS temp-dir
 
 integrations:
   github:
     - host: github.com
       # This is a Personal Access Token or PAT from GitHub. You can find out how to generate this token, and more information
       # about setting up the GitHub integration here: https://backstage.io/docs/integrations/github/locations#configuration
       token: ${GITHUB_TOKEN}
-    ### Example for how to add your GitHub Enterprise instance using the API:
-    # - host: ghe.example.net
-    #   apiBaseUrl: https://ghe.example.net/api/v3
-    #   token: ${GHE_TOKEN}
 
 proxy:
   ### Example for how to add a proxy endpoint for the frontend.
@@ -93,13 +86,3 @@ catalog:
       target: ../../examples/org.yaml
       rules:
         - allow: [User, Group]
-
-    ## Uncomment these lines to add more example data
-    # - type: url
-    #   target: https://github.com/backstage/backstage/blob/master/packages/catalog-model/examples/all.yaml
-
-    ## Uncomment these lines to add an example org
-    # - type: url
-    #   target: https://github.com/backstage/backstage/blob/master/packages/catalog-model/examples/acme-corp.yaml
-    #   rules:
-    #     - allow: [User, Group]

src/dynamic_importer/main.py

@@ -69,10 +69,17 @@ def import_config():
     default=".",
     required=False,
 )
+@click.option(
+    "--parse-descriptions",
+    help="Detect comments in the input file and use them for parameter descriptions",
+    is_flag=True,
+)
 @click.option(
     "-p", "--project", help="CloudTruth project to import data into", required=True
 )
-def process_configs(file_type, default_values, env_values, output_dir, project):
+def process_configs(
+    file_type, default_values, env_values, output_dir, parse_descriptions, project
+):
     if not default_values and not env_values:
         raise click.UsageError(
             "At least one of --default-values and --env-values must be provided"
@@ -91,7 +98,9 @@ def process_configs(file_type, default_values, env_values, output_dir, project):
         input_files[env] = file_path
     click.echo(f"Processing {file_type} files from: {', '.join(input_files)}")
     processing_class = get_processor_class(file_type)
-    processor: BaseProcessor = processing_class(input_files)
+    processor: BaseProcessor = processing_class(
+        input_files, should_parse_description=parse_descriptions
+    )
     template, config_data = processor.process()
 
     template_out_file = f"{output_dir}/{project}-{file_type}.cttemplate"
@@ -275,10 +284,17 @@ def _create_data(
     help="If specified, project hierarchy will be created based on directory hierarchy",
     is_flag=True,
 )
+@click.option(
+    "--parse-descriptions",
+    help="Detect comments in the input file and use them for parameter descriptions",
+    is_flag=True,
+)
 @click.option("-k", help="Ignore SSL certificate verification", is_flag=True)
 @click.option("-c", help="Create missing projects and enviroments", is_flag=True)
 @click.option("-u", help="Upsert values", is_flag=True)
-def walk_directories(config_dir, file_types, exclude_dirs, create_hierarchy, k, c, u):
+def walk_directories(
+    config_dir, file_types, exclude_dirs, create_hierarchy, parse_descriptions, k, c, u
+):
     """
     Walks a directory, constructs templates and config data, and uploads to CloudTruth.
     This is an interactive version of the process_configs and create_data commands. The
@@ -315,7 +331,9 @@ def walk_directories(config_dir, file_types, exclude_dirs, create_hierarchy, k,
 
             click.echo(f"Processing {project} files: {', '.join(env_paths.values())}")
             processing_class = get_processor_class(file_type)
-            processor: BaseProcessor = processing_class(env_paths)
+            processor: BaseProcessor = processing_class(
+                env_paths, should_parse_description=parse_descriptions
+            )
             template, config_data = processor.process()
 
             template_name = f"{project}-{file_type}.cttemplate"

src/dynamic_importer/processors/__init__.py

@@ -48,13 +48,16 @@ def get_supported_formats() -> List[str]:
 
 class BaseProcessor:
     default_values = None
-    parameters_and_values: Dict = {}
     parameters = None
+    parameters_and_values: Dict = {}
     raw_data: Dict = {}
-    values = None
+    should_parse_description = False
     template: Dict = {}
+    values = None
 
-    def __init__(self, env_values: Dict) -> None:
+    def __init__(
+        self, env_values: Dict, should_parse_description: bool = False
+    ) -> None:
         raise NotImplementedError("Subclasses must implement the __init__ method")
 
     def is_param_secret(self, param_name: str) -> bool:
@@ -111,6 +114,9 @@ def generate_template(self, hints: Optional[Dict] = None):
         hints = hints or self.parameters_and_values
         return self.encode_template_references(self.template, hints)
 
+    def _parse_description(self, obj: Union[List, Dict], value: Any) -> Optional[str]:
+        return None
+
     def _traverse_data(
         self,
         path: str,
@@ -126,18 +132,26 @@ def _traverse_data(
         params_and_values = {}
         if isinstance(obj, list):
             for i, subnode in enumerate(obj):
+                sub_path = path + f"[{i}]"
                 template_value, ct_data = self._traverse_data(
-                    path + f"[{i}]", subnode, env, hints=hints
+                    sub_path, subnode, env, hints=hints
                 )
                 obj[i] = template_value
+
+                if sub_path in ct_data and self.should_parse_description:
+                    ct_data[sub_path]["description"] = self._parse_description(obj, i)
                 params_and_values.update(ct_data)
             return obj, params_and_values
         elif isinstance(obj, dict):
             for k, v in obj.items():
+                sub_path = path + f"[{k}]"
                 template_value, ct_data = self._traverse_data(
-                    path + f"[{k}]", v, env, hints=hints
+                    sub_path, v, env, hints=hints
                 )
                 obj[k] = template_value
+
+                if sub_path in ct_data and self.should_parse_description:
+                    ct_data[sub_path]["description"] = self._parse_description(obj, k)
                 params_and_values.update(ct_data)
             return obj, params_and_values
         else:

src/dynamic_importer/processors/dotenv.py

@@ -16,7 +16,10 @@
 
 
 class DotEnvProcessor(BaseProcessor):
-    def __init__(self, env_values: Dict) -> None:
+    def __init__(
+        self, env_values: Dict, should_parse_description: bool = False
+    ) -> None:
+        self.should_parse_description = should_parse_description
         # Due to an unknown bug, self.parameters_and_values can persist between
         # Processor instances. Therefore, we reset it here.
         self.parameters_and_values: Dict = {}

src/dynamic_importer/processors/json.py

@@ -14,7 +14,10 @@
 
 
 class JSONProcessor(BaseProcessor):
-    def __init__(self, env_values: Dict) -> None:
+    def __init__(
+        self, env_values: Dict, should_parse_description: bool = False
+    ) -> None:
+        self.should_parse_description = should_parse_description
         # Due to an unknown bug, self.parameters_and_values can persist between
         # Processor instances. Therefore, we reset it here.
         self.parameters_and_values: Dict = {}

src/dynamic_importer/processors/tf.py

@@ -21,7 +21,10 @@
 class TFProcessor(BaseProcessor):
     data_keys = {"type", "default"}
 
-    def __init__(self, env_values: Dict) -> None:
+    def __init__(
+        self, env_values: Dict, should_parse_description: bool = False
+    ) -> None:
+        self.should_parse_description = should_parse_description
         # Due to an unknown bug, self.parameters_and_values can persist between
         # Processor instances. Therefore, we reset it here.
         self.parameters_and_values: Dict = {}

src/dynamic_importer/processors/tfvars.py

@@ -14,7 +14,10 @@
 
 
 class TFVarsProcessor(BaseProcessor):
-    def __init__(self, env_values: Dict) -> None:
+    def __init__(
+        self, env_values: Dict, should_parse_description: bool = False
+    ) -> None:
+        self.should_parse_description = should_parse_description
         # Due to an unknown bug, self.parameters_and_values can persist between
         # Processor instances. Therefore, we reset it here.
         self.parameters_and_values: Dict = {}

src/dynamic_importer/processors/yaml.py

@@ -6,24 +6,31 @@
 from __future__ import annotations
 
 from re import sub
+from typing import Any
 from typing import Dict
 from typing import Optional
 
-import yaml
 from dynamic_importer.processors import BaseProcessor
+from dynamic_importer.util import StringableYAML
 from liquid import Environment
+from ruamel.yaml import YAMLError
+
+yaml = StringableYAML()
 
 
 class YAMLProcessor(BaseProcessor):
-    def __init__(self, env_values: Dict) -> None:
+    def __init__(
+        self, env_values: Dict, should_parse_description: bool = False
+    ) -> None:
+        self.should_parse_description = should_parse_description
         # Due to an unknown bug, self.parameters_and_values can persist between
         # Processor instances. Therefore, we reset it here.
         self.parameters_and_values: Dict = {}
         for env, file_path in env_values.items():
             try:
                 with open(file_path, "r") as fp:
-                    self.raw_data[env] = yaml.safe_load(fp)
-            except yaml.YAMLError:
+                    self.raw_data[env] = yaml.load(fp)
+            except YAMLError:
                 raise ValueError(
                     f"Attempt to decode {file_path} as YAML failed. Is it valid YAML?"
                 )
@@ -41,10 +48,20 @@ def guess_type(self, value):
 
         return base_type
 
+    def _parse_description(self, obj: Any, value: Any) -> str | None:
+        comment_strings = []
+        if comments := obj.ca.items.get(value, None):
+            for c in comments:
+                if c and not isinstance(c, list) and c.value:
+                    comment_strings.append(c.value.lstrip().strip().lstrip("# "))
+            return "\n".join(comment_strings)
+
+        return None
+
     def encode_template_references(
         self, template: Dict, config_data: Optional[Dict]
     ) -> str:
-        template_body = yaml.safe_dump(template, width=1000)
+        template_body = yaml.dump(template, stream=None)
         if config_data:
             for _, data in config_data.items():
                 if data["type"] != "string":

src/dynamic_importer/util.py

@@ -9,6 +9,8 @@
 
 from click import Option
 from click import UsageError
+from ruamel.yaml import YAML
+from ruamel.yaml.compat import StringIO
 
 
 def validate_env_values(ctx, param, value):
@@ -65,3 +67,25 @@ def handle_parse_result(self, ctx, opts, args):
             )
 
         return super().handle_parse_result(ctx, opts, args)
+
+
+class StringableYAML(YAML):
+    """
+    ruamel.yaml has strong opinions about dumping to a string but provides
+    an example of how to do it "if you really need to have it (or think you do)"
+
+    Since we do some post-processing on the dumped string before writing it
+    to disk, we DO, in fact, need to have it.
+
+    See also:
+    https://yaml.readthedocs.io/en/latest/example/#output-of-dump-as-a-string
+    """
+
+    def dump(self, data, stream=None, **kw):
+        inefficient = False
+        if stream is None:
+            inefficient = True
+            stream = StringIO()
+        YAML.dump(self, data, stream, **kw)
+        if inefficient:
+            return stream.getvalue()

src/tests/test_cli.py

@@ -74,6 +74,43 @@ def test_regenerate_template_no_args(self):
             result.output,
         )
 
+    @mock.patch(
+        "dynamic_importer.main.CTClient",
+    )
+    @pytest.mark.timeout(30)
+    def test_walk_directories_parse_descriptions(self, mock_client):
+        mock_client = mock.MagicMock()  # noqa: F841
+        runner = CliRunner(
+            env={"CLOUDTRUTH_API_HOST": "localhost:8000", "CLOUDTRUTH_API_KEY": "test"}
+        )
+        current_dir = pathlib.Path(__file__).parent.resolve()
+
+        prompt_responses = [
+            "",
+            "myproj",
+            "default",
+        ]
+        result = runner.invoke(
+            import_config,
+            [
+                "walk-directories",
+                "-t",
+                "yaml",
+                "--config-dir",
+                f"{current_dir}/../../samples/advanced",
+                "-c",
+                "-u",
+                "--parse-descriptions",
+            ],
+            input="\n".join(prompt_responses),
+            catch_exceptions=False,
+        )
+        try:
+            assert result.exit_code == 0
+        except AssertionError as e:
+            print(result.output)
+            raise e
+
 
 @pytest.mark.usefixtures("tmp_path")
 def test_create_data_no_api_key(tmp_path):