Remove curly brace properties, allow @@prop in lists (#516)

* JIRA WDT-377 Remove curly brace check for variable substitution

* JIRA WDT-377 Replace curly brace validation with @@prop:key@@ checks

* JIRA WDT-377 Updated copyrights

* JIRA WDT-377 Allow @@prop in model lists

* JIRA WDT-377 Log missing variables based on -method; revised system test to expect warning for missing variable file

* JIRA WDT-377 revised system test to expect warning for missing variable file

Author: rakillen

core/src/main/python/wlsdeploy/tool/validate/validation_utils.py

@@ -1,5 +1,5 @@
 """
-Copyright (c) 2017, 2019, Oracle Corporation and/or its affiliates.  All rights reserved.
+Copyright (c) 2017, 2020, Oracle Corporation and/or its affiliates.  All rights reserved.
 Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.
 """
 import re
@@ -10,8 +10,6 @@
 divider_string = '-----------------------------------------------'
 
 _class_name = "validation_utils"
-_variable_pattern = re.compile('\$\{[\w.-]+\}')
-_property_pattern = re.compile('@@PROP:[\w.-]+@@')
 _path_token_pattern = re.compile('@@[\w._]+@@')
 
 
@@ -107,27 +105,6 @@ def get_properties(value):
     return rtnval
 
 
-def extract_substitution_tokens(tokenized_value):
-    """
-    Returns a Python list containing the substitution variable expressions found in the
-    tokenized_value argument
-
-    :param tokenized_value:
-    :return:
-    """
-    tokens = re.findall(_property_pattern, str(tokenized_value))
-    if tokens is None:
-        # tokenized_value didn't contain any variable expressions, so
-        # return an empty list
-        return []
-    else:
-        # tokenized_value contained at least 1 variable expressions, so
-        # create a map of all the tokens and return the keys from that
-        map_entry = {}
-        map(map_entry.__setitem__, tokens, [])
-        return map_entry.keys()
-
-
 def is_compatible_data_type(expected_data_type, actual_data_type):
     """
     Returns boolean value indicating whether the actual_data_type argument is compatible

core/src/main/python/wlsdeploy/tool/validate/validator.py

@@ -1,5 +1,5 @@
 """
-Copyright (c) 2017, 2019, Oracle Corporation and/or its affiliates.  All rights reserved.
+Copyright (c) 2017, 2020, Oracle Corporation and/or its affiliates.  All rights reserved.
 Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.
 """
 import os
@@ -236,14 +236,8 @@ def __validate_model_file(self, model_dict, variables_map, archive_file_name):
         if self._model_file_name is not None:
             self._logger.info('WLSDPLY-05003', self._model_file_name, class_name=_class_name, method_name=_method_name)
 
-        try:
-            self._variable_properties = variables_map
-            variables.substitute(model_dict, self._variable_properties, self._model_context)
-        except VariableException, ve:
-            ex = exception_helper.create_validate_exception('WLSDPLY-20004', 'validateModel',
-                                                            ve.getLocalizedMessage(), error=ve)
-            self._logger.throwing(ex, class_name=_class_name, method_name=_method_name)
-            raise ex
+        self._variable_properties = variables_map
+        # don't substitute model here, it should be validated with variables intact
 
         if archive_file_name is not None:
             self._logger.info('WLSDPLY-05005', archive_file_name, class_name=_class_name, method_name=_method_name)
@@ -394,7 +388,7 @@ def __validate_model_section(self, model_section_key, model_dict, valid_section_
 
             model_folder_path = model_section_key + ":/"
 
-            if '${' in section_dict_key:
+            if variables.has_variables(section_dict_key):
                 self._report_unsupported_variable_usage(section_dict_key, model_folder_path)
 
             self._logger.finer('WLSDPLY-05011', section_dict_key, section_dict_value,
@@ -474,7 +468,7 @@ def __validate_section_folder(self, model_node, validation_location):
 
             for name in model_node:
                 expanded_name = name
-                if '${' in name:
+                if variables.has_variables(name):
                     expanded_name = self.__validate_variable_substitution(name, model_folder_path)
 
                 self._logger.finest('2 expanded_name={0}', expanded_name,
@@ -503,7 +497,7 @@ def __validate_section_folder(self, model_node, validation_location):
 
             for name in model_node:
                 expanded_name = name
-                if '${' in name:
+                if variables.has_variables(name):
                     self._report_unsupported_variable_usage(name, model_folder_path)
 
                 self._logger.finest('3 expanded_name={0}', expanded_name,
@@ -566,7 +560,7 @@ def __process_model_node(self, model_node, validation_location):
                             method_name=_method_name)
 
         for key, value in model_node.iteritems():
-            if '${' in key:
+            if variables.has_variables(key):
                 self._report_unsupported_variable_usage(key, model_folder_path)
 
             self._logger.finer('5 key={0}', key,
@@ -673,10 +667,10 @@ def __validate_attribute(self, attribute_name, attribute_value, valid_attr_infos
                               model_folder_path, str(validation_location),
                               class_name=_class_name, method_name=_method_name)
 
-        if '${' in attribute_name:
+        if variables.has_variables(attribute_name):
             self._report_unsupported_variable_usage(attribute_name, model_folder_path)
 
-        if '${' in str(attribute_value):
+        if variables.has_variables(str(attribute_value)):
             attribute_value = self.__validate_variable_substitution(attribute_value, model_folder_path)
 
         if attribute_name in valid_attr_infos:
@@ -720,10 +714,10 @@ def __validate_property(self, property_name, property_value, valid_prop_infos, m
         self._logger.entering(property_name, property_value, str(valid_prop_infos), model_folder_path,
                               class_name=_class_name, method_name=_method_name)
 
-        if '${' in property_name:
+        if variables.has_variables(property_name):
             property_name = self.__validate_variable_substitution(property_name, model_folder_path)
 
-        if '${' in str(property_value):
+        if variables.has_variables(str(property_value)):
             property_value = self.__validate_variable_substitution(property_value, model_folder_path)
 
         if property_name in valid_prop_infos:
@@ -750,9 +744,8 @@ def __validate_variable_substitution(self, tokenized_value, model_folder_path):
 
         if not isinstance(untokenized_value, dict):
             # Extract the variable substitution variables from tokenized_value
-            tokens = validation_utils.extract_substitution_tokens(tokenized_value)
-            for token in tokens:
-                property_name = token[2:len(token)-1]
+            matches = variables.get_variable_matches(tokenized_value)
+            for token, property_name in matches:
                 property_value = None
                 if property_name in self._variable_properties:
                     property_value = self._variable_properties[property_name]
@@ -761,13 +754,18 @@ def __validate_variable_substitution(self, tokenized_value, model_folder_path):
                 else:
                     # FIXME(mwooten) - the cla_utils should be fixing all windows paths to use forward slashes already
                     # assuming that the value is not None
+
+                    logger_method = self._logger.info
+                    if self._model_context.get_validation_method() == 'strict':
+                        logger_method = self._logger.warning
+
                     variables_file_name = self._model_context.get_variable_file()
                     if variables_file_name is None:
-                        self._logger.warning('WLSDPLY-05021', model_folder_path, property_name,
-                                             class_name=_class_name, method_name=_method_name)
+                        logger_method('WLSDPLY-05021', model_folder_path, property_name,
+                                      class_name=_class_name, method_name=_method_name)
                     else:
-                        self._logger.warning('WLSDPLY-05022', model_folder_path, property_name, variables_file_name,
-                                             class_name=_class_name, method_name=_method_name)
+                        logger_method('WLSDPLY-05022', model_folder_path, property_name, variables_file_name,
+                                      class_name=_class_name, method_name=_method_name)
 
         self._logger.exiting(class_name=_class_name, method_name=_method_name, result=untokenized_value)
         return untokenized_value
@@ -882,7 +880,7 @@ def __validate_server_group_targeting_limits(self, attribute_key, attribute_valu
                         self._logger.severe('WLSDPLY-05033', key, model_folder_path, str(type(key)),
                                             class_name=_class_name, method_name=__method_name)
                     else:
-                        if '${' in key:
+                        if variables.has_variables(key):
                             self._report_unsupported_variable_usage(key, model_folder_path)
 
                     if isinstance(value, basestring) and MODEL_LIST_DELIMITER in value:
@@ -904,7 +902,7 @@ def _validate_single_server_group_target_limits_value(self, key, value, model_fo
         _method_name = '_validate_single_server_group_target_limits_value'
 
         if type(value) is str:
-            if '${' in str(value):
+            if variables.has_variables(str(value)):
                 self._report_unsupported_variable_usage(str(value), model_folder_path)
         else:
             self._logger.severe('WLSDPLY-05035', key, str(value), model_folder_path, str(type(value)),
@@ -922,7 +920,7 @@ def __validate_wlsroles_section(self, attribute_value):
 
     def _report_unsupported_variable_usage(self, tokenized_value, model_folder_path):
         _method_name = '_report_unsupported_variable_usage'
-        tokens = validation_utils.extract_substitution_tokens(tokenized_value)
+        tokens = variables.get_variable_names(tokenized_value)
         for token in tokens:
             self._logger.severe('WLSDPLY-05030', model_folder_path, token,
                                 class_name=_class_name, method_name=_method_name)

core/src/main/python/wlsdeploy/util/variables.py

@@ -1,5 +1,5 @@
 """
-Copyright (c) 2017, 2019, Oracle Corporation and/or its affiliates.  All rights reserved.
+Copyright (c) 2017, 2020, Oracle Corporation and/or its affiliates.  All rights reserved.
 Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.
 """
 import os
@@ -23,9 +23,8 @@
 
 _class_name = "variables"
 _logger = platform_logger.PlatformLogger('wlsdeploy.variables')
-_variable_pattern = re.compile("\\$\\{[\w.-]+\\}")
 _file_variable_pattern = re.compile("@@FILE:[\w.\\\/:-]+@@")
-_property_pattern = re.compile("@@PROP:[\w.-]+@@")
+_property_pattern = re.compile("(@@PROP:([\\w.-]+)@@)")
 _file_nested_variable_pattern = re.compile("@@FILE:@@[\w]+@@[\w.\\\/:-]+@@")
 
 
@@ -153,17 +152,10 @@ def get_variable_names(text):
     :return: a list of variable names
     """
     names = []
-    if '${' in text:
-        tokens = _variable_pattern.findall(text)
-        if tokens is not None:
-            for token in tokens:
-                names.append(token[2:-1])
-
     if '@@' in text:
-        tokens = _property_pattern.findall(text)
-        if tokens:
-            for token in tokens:
-                names.append(token[7:-2])
+        matches = _property_pattern.findall(text)
+        for token, key in matches:
+            names.append(key)
 
     return names
 
@@ -201,6 +193,13 @@ def _process_node(nodes, variables, model_context):
 
         if isinstance(value, dict):
             _process_node(value, variables, model_context)
+
+        elif isinstance(value, list):
+            for member in value:
+                if type(member) in [str, unicode]:
+                    index = value.index(member)
+                    value[index] = _substitute(member, variables, model_context)
+
         elif type(value) in [str, unicode]:
             nodes[key] = _substitute(value, variables, model_context)
 
@@ -215,40 +214,25 @@ def _substitute(text, variables, model_context):
     """
     method_name = '_substitute'
 
-    if '${' in text:
-        tokens = _variable_pattern.findall(text)
-        if tokens:
-            for token in tokens:
-                key = token[2:-1]
-                # for ${key} variables, leave them in place if not defined.
-                # there are cases where WebLogic allows ${key} values, such as server templates.
-                # ${key} substitution is deprecated, so log if replacement occurs.
-                if key in variables:
-                    value = variables[key]
-                    text = text.replace(token, value)
-                    _logger.info('WLSDPLY-01735', token, key, method_name=method_name, class_name=_class_name)
-
     # skip lookups for text with no @@
     if '@@' in text:
 
         # do properties first, to cover the case @@FILE:/dir/@@PROP:name@@.txt@@
-        tokens = _property_pattern.findall(text)
-        if tokens:
-            for token in tokens:
-                key = token[7:-2]
-                # for @@PROP:key@@ variables, throw an exception if key is not found.
-                if key not in variables:
-                    if model_context.get_validation_method() == 'strict':
-                        _logger.severe('WLSDPLY-01732', key, class_name=_class_name, method_name=method_name)
-                        ex = exception_helper.create_variable_exception('WLSDPLY-01732', key)
-                        _logger.throwing(ex, class_name=_class_name, method_name=method_name)
-                        raise ex
-                    else:
-                        _logger.info('WLSDPLY-01732', key, class_name=_class_name, method_name=method_name)
-                        continue
-                            
-                value = variables[key]
-                text = text.replace(token, value)
+        matches = _property_pattern.findall(text)
+        for token, key in matches:
+            # log, or throw an exception if key is not found.
+            if key not in variables:
+                if model_context.get_validation_method() == 'strict':
+                    _logger.severe('WLSDPLY-01732', key, class_name=_class_name, method_name=method_name)
+                    ex = exception_helper.create_variable_exception('WLSDPLY-01732', key)
+                    _logger.throwing(ex, class_name=_class_name, method_name=method_name)
+                    raise ex
+                else:
+                    _logger.info('WLSDPLY-01732', key, class_name=_class_name, method_name=method_name)
+                    continue
+
+            value = variables[key]
+            text = text.replace(token, value)
 
         tokens = _file_variable_pattern.findall(text)
         if tokens:
@@ -306,16 +290,33 @@ def substitute_key(text, variables):
     """
     Substitute any @@PROP values in the text and return.
     If the corresponding variable is not found, leave the @@PROP value in place.
-    The deprecated ${} notation is not resolved.
     :param text: the text to be evaluated
     :param variables: the variable map
     :return: the substituted text value
     """
-    tokens = _property_pattern.findall(text)
-    if tokens:
-        for token in tokens:
-            key = token[7:-2]
-            if key in variables:
-                value = variables[key]
-                text = text.replace(token, value)
+    matches = _property_pattern.findall(text)
+    for token, key in matches:
+        if key in variables:
+            value = variables[key]
+            text = text.replace(token, value)
     return text
+
+
+def has_variables(text):
+    """
+    Determine if the specified text contains any variable references.
+    :param text: the text to be evaluated
+    :return: True if the text contains variable references, False otherwise
+    """
+    matches = _property_pattern.findall(text)
+    return len(matches) > 0
+
+
+def get_variable_matches(text):
+    """
+    Return a list containing a tuple for each property key in the specified text.
+    Each tuple contains the full expression (@@PROP:<key>@@) and just the key (<key>).
+    :param text: the text to be evaluated
+    :return: a list of tuples
+    """
+    return _property_pattern.findall(text)

core/src/test/python/variables_test.py

@@ -1,5 +1,5 @@
 """
-Copyright (c) 2017, 2019, Oracle Corporation and/or its affiliates.  All rights reserved.
+Copyright (c) 2017, 2020, Oracle Corporation and/or its affiliates.  All rights reserved.
 Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.
 """
 import unittest
@@ -47,17 +47,6 @@ def testSubstituteJson(self):
         self.assertEqual(True, 'myCluster' in model['topology']['Cluster'])
         self.assertEqual(True, 's3' in model['topology']['Server'])
 
-    def testVariableNotFound(self):
-        """
-        For ${key} substitution, no replacement is done, and no error is reported, if variable not found.
-        ${key} substitution is deprecated.
-        """
-        model = FileToPython(self._resources_dir + '/variables-test.json', self._use_ordering).parse()
-        model['topology']['Name'] = '${bad.variable}'
-        variable_map = variables.load_variables(self._variables_file)
-        variables.substitute(model, variable_map, self.model_context)
-        self.assertEqual(model['topology']['Name'], '${bad.variable}')
-
     def testPropertyNotFound(self):
         """
         For @@PROP:key@@ substitution, an exception is thrown if variable not found.
@@ -79,7 +68,7 @@ def testFileVariable(self):
         self.assertEqual(model['domainInfo']['AdminUserName'], 'file-variable-value')
 
     def testFileVariableWithVariable(self):
-        model = {'domainInfo': {'AdminUserName': '@@FILE:${variable_dir}/' + self._file_variable_name + '@@'}}
+        model = {'domainInfo': {'AdminUserName': '@@FILE:@@PROP:variable_dir@@/' + self._file_variable_name + '@@'}}
         variables.substitute(model, {'variable_dir': self._resources_dir}, self.model_context)
         self.assertEqual(model['domainInfo']['AdminUserName'], 'file-variable-value')