Merge pull request #99 from Karandash8/98-incorrect-value-for-variabl…

…e-scopes-merging-when-variable-references-are-used

Fix variable scopes merging when referencing another variable

README.md

@@ -98,7 +98,6 @@ envs:
       <application_name>:
         app_deployer: <bootstrap_application>  ## application that will deploy this application
         app_deployer_env: <environment_name>  ## (OPTIONAL) for multi-environments with single ArgoCD deployment
-        non_k8s_files_to_render: [<filename>]  ## (OPTIONAL) list of files to render that are not Kubernetes resources (e.g., values.yml)
 vars:
   argocd:
     namespace: <argocd_namespace>  ## (OPTIONAL) namespace for ArgoCD `Application` resource, default: argocd
@@ -137,18 +136,34 @@ envs:
           <variable_name>: <variable_value>
     vars:
       <variable_name>: <variable_value>
-
 vars:
   <variable_name>: <variable_value>
 ```
 
+In order to unset a key of a dictionary variable that is set at a higher level, use the `null` value:
+```
+envs:
+  <environment_name>:
+    vars:
+      <variable_name>:
+        <key>: null
+vars:
+  <variable_name>:
+    <key>: <value>
+```
+
 ### Variables in `config.yml`
 Variables can be referenced in the configuration file using the following syntax:
 ```${var_name}``` and  ```${var_name[dict_key][...]}```.
 
 Variables can also be used as substring values:
 ```prefix-${var_name}-suffix```.
 
+The following variable resolution rules apply:
+- Variable referenced in global scope is resolved using global variables.
+- Variable referenced in per-environment scope is resolved using global and per-environment variables.
+- Variable referenced in per-application scope is resolved using global, per-environment, and per-application variables.
+
 ### Jinja2 templates
 To include file content in the current Jinja2 template, use the following block:
 
@@ -196,6 +211,16 @@ When kustomization overlays are used, kustomization base directory shall be name
 Example:
 ```tests/manual/source/app_2```
 
+When Helm `values.yml` file is used, the file shall be named `values.yml` and reside in the application directory.
+On top of that the file shall be explicitly set for rendering in the configuration file:
+```
+envs:
+  <environment_name>:
+    apps:
+      <application_name>:
+        non_k8s_files_to_render: [<filename>]  ## (OPTIONAL) list of files to render that are not Kubernetes resources (e.g., values.yml)
+```
+
 ## Caveats
 - Comments are not rendered in the final output manifests.
 

make_argocd_fly/application.py

@@ -79,17 +79,26 @@ async def process(self) -> None:
     await self.find_apps_step.run()
 
     for (app_name, env_name) in self.find_apps_step.get_apps():
-      template_vars = VarsResolver.resolve_all(merge_dicts(self.config.get_vars(), self.config.get_env_vars(env_name),
-                                                           self.config.get_app_vars(env_name, app_name), {
-                                                           '__application': {
-                                                             'application_name': '-'.join([os.path.basename(app_name), env_name]).replace('_', '-'),
-                                                             'path': os.path.join(os.path.basename(self.config.get_output_dir()),
-                                                                                  env_name, app_name
-                                                                                  )
-                                                           },
-                                                           'env_name': env_name,
-                                                           'app_name': app_name}),
-                                               var_identifier=self.cli_args.get_var_identifier())
+      global_vars_resolved = VarsResolver.resolve_all(self.config.get_vars(),
+                                                      self.config.get_vars(),
+                                                      var_identifier=self.cli_args.get_var_identifier())
+      env_vars_resolved = merge_dicts(global_vars_resolved,
+                                      VarsResolver.resolve_all(self.config.get_env_vars(env_name),
+                                                               global_vars_resolved,
+                                                               var_identifier=self.cli_args.get_var_identifier()))
+      app_vars_resolved = merge_dicts(env_vars_resolved,
+                                      VarsResolver.resolve_all(self.config.get_app_vars(env_name, app_name),
+                                                               env_vars_resolved,
+                                                               var_identifier=self.cli_args.get_var_identifier()))
+      template_vars = merge_dicts(app_vars_resolved,
+                                  {
+                                    '__application': {
+                                      'application_name': '-'.join([os.path.basename(app_name), env_name]).replace('_', '-'),
+                                      'path': os.path.join(os.path.basename(self.config.get_output_dir()), env_name, app_name)
+                                    },
+                                    'env_name': env_name,
+                                    'app_name': app_name
+                                  })
 
       self.render_jinja_step.configure(textwrap.dedent(self.APPLICATION_RESOUCE_TEMPLATE), self.app_name, self.env_name, template_vars)
       await self.render_jinja_step.run()
@@ -113,10 +122,23 @@ def __init__(self, app_name: str, env_name: str, app_viewer: ResourceViewer = No
   async def process(self) -> None:
     log.debug('Starting to process application {} in environment {}'.format(self.app_name, self.env_name))
 
-    template_vars = VarsResolver.resolve_all(merge_dicts(self.config.get_vars(), self.config.get_env_vars(self.env_name),
-                                                         self.config.get_app_vars(self.env_name, self.app_name),
-                                                         {'env_name': self.env_name, 'app_name': self.app_name}),
-                                             var_identifier=self.cli_args.get_var_identifier())
+    global_vars_resolved = VarsResolver.resolve_all(self.config.get_vars(),
+                                                    self.config.get_vars(),
+                                                    var_identifier=self.cli_args.get_var_identifier())
+    env_vars_resolved = merge_dicts(global_vars_resolved,
+                                    VarsResolver.resolve_all(self.config.get_env_vars(self.env_name),
+                                                             global_vars_resolved,
+                                                             var_identifier=self.cli_args.get_var_identifier()))
+    app_vars_resolved = merge_dicts(env_vars_resolved,
+                                    VarsResolver.resolve_all(self.config.get_app_vars(self.env_name, self.app_name),
+                                                             env_vars_resolved,
+                                                             var_identifier=self.cli_args.get_var_identifier()))
+    template_vars = merge_dicts(app_vars_resolved,
+                                {
+                                  'env_name': self.env_name,
+                                  'app_name': self.app_name
+                                })
+
     if self.cli_args.get_print_vars():
       log.info('Variables for application {} in environment {}:\n{}'.format(self.app_name, self.env_name, pformat(template_vars)))
 
@@ -151,11 +173,23 @@ def __init__(self, app_name: str, env_name: str, app_viewer: ResourceViewer = No
   async def process(self) -> None:
     log.debug('Starting to process application {} in environment {}'.format(self.app_name, self.env_name))
 
-    template_vars = VarsResolver.resolve_all(merge_dicts(self.config.get_vars(), self.config.get_env_vars(self.env_name),
-                                                         self.config.get_app_vars(self.env_name, self.app_name),
-                                                         {'env_name': self.env_name, 'app_name': self.app_name}),
-                                             var_identifier=self.cli_args.get_var_identifier()
-                                             )
+    global_vars_resolved = VarsResolver.resolve_all(self.config.get_vars(),
+                                                    self.config.get_vars(),
+                                                    var_identifier=self.cli_args.get_var_identifier())
+    env_vars_resolved = merge_dicts(global_vars_resolved,
+                                    VarsResolver.resolve_all(self.config.get_env_vars(self.env_name),
+                                                             global_vars_resolved,
+                                                             var_identifier=self.cli_args.get_var_identifier()))
+    app_vars_resolved = merge_dicts(env_vars_resolved,
+                                    VarsResolver.resolve_all(self.config.get_app_vars(self.env_name, self.app_name),
+                                                             env_vars_resolved,
+                                                             var_identifier=self.cli_args.get_var_identifier()))
+    template_vars = merge_dicts(app_vars_resolved,
+                                {
+                                  'env_name': self.env_name,
+                                  'app_name': self.app_name
+                                })
+
     if self.cli_args.get_print_vars():
       log.info('Variables for application {} in environment {}:\n{}'.format(self.app_name, self.env_name, pformat(template_vars)))
 

make_argocd_fly/utils.py

@@ -29,7 +29,7 @@ def _find_var_position(self, value: str, start: int = 0) -> tuple[int, int]:
 
     return (var_start + 1, var_end)
 
-  def _resolve_value(self, vars: dict, value: str) -> str:
+  def _resolve_value(self, value: str, source: dict) -> str:
     resolved_value = ''
     try:
       start = 0
@@ -42,7 +42,7 @@ def _resolve_value(self, vars: dict, value: str) -> str:
         if (var_start - 1) > start:
           resolved_value += value[start:var_start - 1]
 
-        resolved_value += value[var_start:var_end + 1].format(**vars)
+        resolved_value += value[var_start:var_end + 1].format(**source)
         self.resolution_counter += 1
         start = var_end + 1
 
@@ -60,33 +60,33 @@ def _resolve_value(self, vars: dict, value: str) -> str:
       log.error('Variable {} not found in vars'.format(value[var_start - 1:var_end + 1]))
       raise
 
-  def _iterate(self, vars: dict, value=None, initial=True):
-    value = value or vars if initial else value
+  def _iterate(self, to_resolve: dict, source: dict, value=None, initial=True):
+    value = value or to_resolve if initial else value
     if isinstance(value, dict):
       for k, v in value.items():
-        value[k] = self._iterate(vars, v, False)
+        value[k] = self._iterate(to_resolve, source, v, False)
     elif isinstance(value, list):
       for idx, i in enumerate(value):
-        value[idx] = self._iterate(vars, i, False)
+        value[idx] = self._iterate(to_resolve, source, i, False)
     elif isinstance(value, str):
-      value = self._resolve_value(vars, value)
+      value = self._resolve_value(value, source)
     return value
 
   def get_resolutions(self) -> int:
     return self.resolution_counter
 
-  def resolve(self, vars: dict) -> dict:
+  def resolve(self, to_resolve: dict, source: dict) -> dict:
     self.resolution_counter = 0
 
-    return self._iterate(copy.deepcopy(vars))
+    return self._iterate(copy.deepcopy(to_resolve), source)
 
   @staticmethod
-  def resolve_all(vars: dict, var_identifier: str = '$') -> dict:
+  def resolve_all(to_resolve: dict, source: dict, var_identifier: str = '$') -> dict:
       resolver = VarsResolver(var_identifier)
 
-      resolved_vars = resolver.resolve(vars)
+      resolved_vars = resolver.resolve(to_resolve, source)
       while resolver.get_resolutions() > 0:
-        resolved_vars = resolver.resolve(resolved_vars)
+        resolved_vars = resolver.resolve(resolved_vars, source)
 
       return resolved_vars
 
@@ -147,8 +147,8 @@ def merge_dicts(*dicts):
         merged[key] = merge_dicts(merged[key], value)
       elif isinstance(value, dict):
         merged[key] = merge_dicts({}, value)
-      elif value is None:
-        # If the value on the right is None, delete the key on the left
+      elif value is None and key in merged:
+        # If the value on the right is None and key exists on the left, delete the key on the left
         merged.pop(key, None)
       else:
         merged[key] = value

tests/manual/config.yml

@@ -2,7 +2,7 @@ envs:
   management:
     apps:
       bootstrap: {}
-      core_deployer: {app_deployer: bootstrap, vars: {argocd: {project: default, destination: {namespace: kube-system}}}}
+      core_deployer: {app_deployer: bootstrap, vars: {argocd: "${argocd_app_presets[default]}"}}
       service_deployer: {app_deployer: bootstrap, vars: {argocd: {project: default, destination: {namespace: kube-system}}}}
       app_1:
         app_deployer: core_deployer
@@ -85,6 +85,11 @@ vars:
       # https://www.arthurkoziel.com/fixing-argocd-crd-too-long-error/
       syncOptions:
         - ServerSideApply=true
+  argocd_app_presets:
+    default:
+      project: default
+      destination:
+        namespace: ${namespace}
   namespace: kube-default
   version: 0.1.0
   double_reference_version: ${reference_version}

tests/manual/output/management/bootstrap/application_core-deployer-management.yml

@@ -14,7 +14,7 @@ spec:
     path: output/management/core_deployer
   destination:
     server: management-api-server
-    namespace: kube-system
+    namespace: kube-default
   syncPolicy:
     syncOptions:
       - ServerSideApply=true