Alon Bar-Lev has uploaded a new change for review.

Change subject: packaging: setup: support lexical parsing of configuration
......................................................................

packaging: setup: support lexical parsing of configuration

the configuration file format is shell like format, it should be parsed
by shell, python and java.

current implementation does not parse the value using lexical method, so
escape of characters is impossible, and there may be other side effects
like comments that should remain within quotes.

new implementation uses primitive but sufficient parsing of literals.

sync implementation between java and python and sync implementation of
service parser to match this of setup.

Change-Id: I945c8ddbe5de512ab330153c27802bb71964b76f
Signed-off-by: Alon Bar-Lev <alo...@redhat.com>
---
M 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/LocalConfig.java
A 
backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/LocalConfigTest.java
A backend/manager/modules/utils/src/test/resources/localconfig.conf
A backend/manager/modules/utils/src/test/resources/localconfig.conf.ref
M packaging/services/ovirt-engine-logging.properties.in
M packaging/services/ovirt-engine-notifier.py
M packaging/services/ovirt-engine.py
M packaging/services/ovirt-engine.xml.in
M packaging/services/service.py
M packaging/setup/ovirt_engine_setup/util.py
M packaging/setup/plugins/ovirt-engine-setup/config/ca.py
M packaging/setup/plugins/ovirt-engine-setup/config/database.py
12 files changed, 403 insertions(+), 247 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/42/15142/1

diff --git 
a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/LocalConfig.java
 
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/LocalConfig.java
index 5863df7..be90438 100644
--- 
a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/LocalConfig.java
+++ 
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/LocalConfig.java
@@ -29,10 +29,8 @@
     private static final Logger log = Logger.getLogger(LocalConfig.class);
 
     // Compile regular expressions:
-    private static final Pattern COMMENT_EXPR = Pattern.compile("\\s*#.*$");
-    private static final Pattern BLANK_EXPR = Pattern.compile("^\\s*$");
-    private static final Pattern VALUE_EXPR = 
Pattern.compile("^\\s*(\\w+)\\s*=\\s*(.*?)\\s*$");
-    private static final Pattern REF_EXPR = Pattern.compile("\\$\\{(\\w+)\\}");
+    private static final Pattern EMPTY_LINE = Pattern.compile("^\\s*(#.*)?$");
+    private static final Pattern KEY_VALUE_EXPRESSION = 
Pattern.compile("^\\s*(\\w+)=(.*)$");
 
     // The properties object storing the current values of the parameters:
     private Map<String, String> values = new HashMap<String, String>();
@@ -127,17 +125,25 @@
 
         // Load the file:
         BufferedReader reader = null;
+        int index = 0;
         try {
             reader = new BufferedReader(new FileReader(file));
             String line = null;
             while ((line = reader.readLine()) != null) {
+                index++;
                 loadLine(line);
             }
             log.info("Loaded file \"" + file.getAbsolutePath() + "\".");
         }
-        catch (IOException exception) {
-            log.error("Can't load file \"" + file.getAbsolutePath() + "\".", 
exception);
-            throw exception;
+        catch (Exception e) {
+            String msg = String.format(
+                "Can't load file '%s' line %d: %s",
+                file.getAbsolutePath(),
+                index,
+                e
+            );
+            log.error(msg, e);
+            throw new RuntimeException(msg, e);
         }
         finally {
             if (reader != null) {
@@ -158,20 +164,57 @@
      * @param value String.
      */
     public String expandString(String value) {
-        for (;;) {
-            Matcher refMatch = REF_EXPR.matcher(value);
-            if (!refMatch.find()) {
-                break;
+        StringBuilder ret = new StringBuilder();
+
+        boolean escape = false;
+        boolean inQuotes = false;
+        int index = 0;
+        while (index < value.length()) {
+            char c = value.charAt(index++);
+            if (escape) {
+                escape = false;
+                ret.append(c);
             }
-            String refKey = refMatch.group(1);
-            String refValue = values.get(refKey);
-            if (refValue == null) {
-                break;
+            else {
+                switch(c) {
+                    case '\\':
+                        escape = true;
+                    break;
+                    case '$':
+                        if (value.charAt(index++) != '{') {
+                            throw new RuntimeException("Malformed variable 
assignement");
+                        }
+                        int i = value.indexOf('}', index);
+                        if (i == -1) {
+                            throw new RuntimeException("Malformed variable 
assignement");
+                        }
+                        String name = value.substring(index, i);
+                        index = i+1;
+                        String v = values.get(name);
+                        if (v != null) {
+                            ret.append(v);
+                        }
+                    break;
+                    case '"':
+                        inQuotes = !inQuotes;
+                    break;
+                    case ' ':
+                    case '#':
+                        if (inQuotes) {
+                            ret.append(c);
+                        }
+                        else {
+                            index = value.length();
+                        }
+                    break;
+                    default:
+                        ret.append(c);
+                    break;
+                }
             }
-            value = value.substring(0, refMatch.start()) + refValue + 
value.substring(refMatch.end());
         }
 
-        return value;
+        return ret.toString();
     }
 
     /**
@@ -181,36 +224,18 @@
      * @param line the line from the properties file
      */
     private void loadLine(String line) throws IOException {
-        // Remove comments:
-        Matcher commentMatch = COMMENT_EXPR.matcher(line);
-        if (commentMatch.find()) {
-            line = line.substring(0, commentMatch.start()) + 
line.substring(commentMatch.end());
+        Matcher blankMatch = EMPTY_LINE.matcher(line);
+        if (!blankMatch.find()) {
+            Matcher keyValueMatch = KEY_VALUE_EXPRESSION.matcher(line);
+            if (!keyValueMatch.find()) {
+                throw new RuntimeException("Invalid line");
+            }
+
+            values.put(
+                keyValueMatch.group(1),
+                expandString(keyValueMatch.group(2))
+            );
         }
-
-        // Skip empty lines:
-        Matcher blankMatch = BLANK_EXPR.matcher(line);
-        if (blankMatch.find()) {
-            return;
-        }
-
-        // Separate name from value:
-        Matcher keyValueMatch = VALUE_EXPR.matcher(line);
-        if (!keyValueMatch.find()) {
-            return;
-        }
-        String key = keyValueMatch.group(1);
-        String value = keyValueMatch.group(2);
-
-        // Strip quotes from value:
-        if (value.length() >= 2 && value.startsWith("\"") && 
value.endsWith("\"")) {
-            value = value.substring(1, value.length() - 1);
-        }
-
-        // Expand nested variables
-        value = expandString(value);
-
-        // Update the values:
-        values.put(key, value);
     }
 
     /**
diff --git 
a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/LocalConfigTest.java
 
b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/LocalConfigTest.java
new file mode 100644
index 0000000..5268bc6
--- /dev/null
+++ 
b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/LocalConfigTest.java
@@ -0,0 +1,49 @@
+package org.ovirt.engine.core.utils;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URLDecoder;
+import java.nio.charset.Charset;
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.Test;
+
+public class LocalConfigTest {
+
+    @Test
+    public void testValid() throws Exception {
+        LocalConfig config = new LocalConfig();
+        config.loadConfig(
+            
URLDecoder.decode(ClassLoader.getSystemResource("localconfig.conf").getPath(), 
"UTF-8"),
+            "/dev/null"
+        );
+        List<String> res = new LinkedList<String>();
+        for (Map.Entry<String, String> e : config.getProperties().entrySet()) {
+            res.add(String.format("%s=%s", e.getKey(), e.getValue()));
+        }
+        Collections.sort(res);
+
+        String reference;
+        InputStream in = null;
+        try {
+            in = new 
FileInputStream(URLDecoder.decode(ClassLoader.getSystemResource("localconfig.conf.ref").getPath(),
 "UTF-8"));
+            byte buffer[] = new byte[2048];
+            int size = in.read(buffer);
+            reference = new String(buffer, 0, size, Charset.forName("UTF-8"));
+        }
+        finally {
+            try {
+                in.close();
+            }
+            catch (IOException e) {}
+        }
+
+        assertEquals(reference.split("\n"), res.toArray());
+    }
+}
diff --git a/backend/manager/modules/utils/src/test/resources/localconfig.conf 
b/backend/manager/modules/utils/src/test/resources/localconfig.conf
new file mode 100644
index 0000000..3fd4698
--- /dev/null
+++ b/backend/manager/modules/utils/src/test/resources/localconfig.conf
@@ -0,0 +1,20 @@
+# comment
+# comment # comment
+# blank line:
+
+# blank line with spaces:
+     
+# blank line with spaces and comment:
+         #           comment 
+
+key01=
+key02=value2
+key03=value31 value32
+key04="value41 value42"
+key05="value51\"value52\"value53"
+key06="value61#value62"
+key07="value71#value72"# comment
+key08="value81#value82" # comment
+key09=#comment
+key10="prefix ${key01} ${key02} ${unknown} ${key03} ${key04} suffix"
+key11="\${key02}"
diff --git 
a/backend/manager/modules/utils/src/test/resources/localconfig.conf.ref 
b/backend/manager/modules/utils/src/test/resources/localconfig.conf.ref
new file mode 100644
index 0000000..d033e17
--- /dev/null
+++ b/backend/manager/modules/utils/src/test/resources/localconfig.conf.ref
@@ -0,0 +1,12 @@
+SENSITIVE_KEYS=
+key01=
+key02=value2
+key03=value31
+key04=value41 value42
+key05=value51"value52"value53
+key06=value61#value62
+key07=value71#value72
+key08=value81#value82
+key09=
+key10=prefix  value2  value31 value41 value42 suffix
+key11=${key02}
diff --git a/packaging/services/ovirt-engine-logging.properties.in 
b/packaging/services/ovirt-engine-logging.properties.in
index 754c097..e3651dc 100644
--- a/packaging/services/ovirt-engine-logging.properties.in
+++ b/packaging/services/ovirt-engine-logging.properties.in
@@ -5,7 +5,7 @@
 handler.FILE.level=DEBUG
 handler.FILE.properties=autoFlush,fileName
 handler.FILE.autoFlush=true
-handler.FILE.fileName=$getString('ENGINE_LOG')/boot.log
+handler.FILE.fileName=$getstring('ENGINE_LOG')/boot.log
 handler.FILE.formatter=PATTERN
 
 formatter.PATTERN=org.jboss.logmanager.formatters.PatternFormatter
diff --git a/packaging/services/ovirt-engine-notifier.py 
b/packaging/services/ovirt-engine-notifier.py
index 824542c..cb695e2 100755
--- a/packaging/services/ovirt-engine-notifier.py
+++ b/packaging/services/ovirt-engine-notifier.py
@@ -38,7 +38,7 @@
         # Check that the Java home directory exists and that it contais at
         # least the java executable:
         self.check(
-            name=self._config.getString('JAVA_HOME'),
+            name=self._config.get('JAVA_HOME'),
             directory=True,
         )
         self.check(
@@ -48,7 +48,7 @@
 
         # Check the required JBoss directories and files:
         self.check(
-            name=self._config.getString('JBOSS_HOME'),
+            name=self._config.get('JBOSS_HOME'),
             directory=True,
         )
         self.check(
@@ -58,14 +58,14 @@
         # Check the required engine directories and files:
         self.check(
             os.path.join(
-                self._config.getString('ENGINE_USR'),
+                self._config.get('ENGINE_USR'),
                 'services',
             ),
             directory=True,
         )
         self.check(
             os.path.join(
-                self._config.getString('ENGINE_LOG'),
+                self._config.get('ENGINE_LOG'),
                 'notifier',
             ),
             directory=True,
@@ -74,7 +74,7 @@
         for log in ('notifier.log', 'console.log'):
             self.check(
                 name=os.path.join(
-                    self._config.getString("ENGINE_LOG"),
+                    self._config.get("ENGINE_LOG"),
                     'notifier',
                     log,
                 ),
@@ -113,11 +113,11 @@
         )
 
         jbossModulesJar = os.path.join(
-            self._config.getString('JBOSS_HOME'),
+            self._config.get('JBOSS_HOME'),
             'jboss-modules.jar',
         )
         java = os.path.join(
-            self._config.getString('JAVA_HOME'),
+            self._config.get('JAVA_HOME'),
             'bin',
             'java',
         )
@@ -133,7 +133,7 @@
         self._engineArgs = [
             'ovirt-engine-notifier',
             '-Dlog4j.configuration=file://%s/notifier/log4j.xml' % (
-                self._config.getString('ENGINE_ETC'),
+                self._config.get('ENGINE_ETC'),
             ),
             '-Djboss.modules.write-indexes=false',
             '-jar', jbossModulesJar,
@@ -149,11 +149,11 @@
             'CLASSPATH': '',
             'JAVA_MODULEPATH': ':'.join([
                 os.path.join(
-                    self._config.getString('ENGINE_USR'),
+                    self._config.get('ENGINE_USR'),
                     'modules',
                 ),
                 os.path.join(
-                    self._config.getString('JBOSS_HOME'),
+                    self._config.get('JBOSS_HOME'),
                     'modules',
                 ),
             ]),
@@ -166,7 +166,7 @@
     def daemonStdHandles(self):
         consoleLog = open(
             os.path.join(
-                self._config.getString('ENGINE_LOG'),
+                self._config.get('ENGINE_LOG'),
                 'notifier',
                 'console.log'
             ),
@@ -179,10 +179,10 @@
             executable=self._executable,
             args=self._engineArgs,
             env=self._engineEnv,
-            stopTime=self._config.getInteger(
+            stopTime=self._config.getinteger(
                 'NOTIFIER_STOP_TIME'
             ),
-            stopInterval=self._config.getInteger(
+            stopInterval=self._config.getinteger(
                 'NOTIFIER_STOP_INTERVAL'
             ),
         )
diff --git a/packaging/services/ovirt-engine.py 
b/packaging/services/ovirt-engine.py
index 9c411ae..bfb1d12 100755
--- a/packaging/services/ovirt-engine.py
+++ b/packaging/services/ovirt-engine.py
@@ -49,7 +49,7 @@
         """Link all the JBoss modules into a temporary directory"""
 
         modulesTmpDir = os.path.join(
-            self._config.getString('ENGINE_TMP'),
+            self._config.get('ENGINE_TMP'),
             'modules',
         )
 
@@ -78,7 +78,7 @@
         # Check that the Java home directory exists and that it contais at
         # least the java executable:
         self.check(
-            name=self._config.getString('JAVA_HOME'),
+            name=self._config.get('JAVA_HOME'),
             directory=True,
         )
         self.check(
@@ -88,7 +88,7 @@
 
         # Check the required JBoss directories and files:
         self.check(
-            name=self._config.getString('JBOSS_HOME'),
+            name=self._config.get('JBOSS_HOME'),
             directory=True,
         )
         self.check(
@@ -98,18 +98,18 @@
         # Check the required engine directories and files:
         self.check(
             os.path.join(
-                self._config.getString('ENGINE_USR'),
+                self._config.get('ENGINE_USR'),
                 'services',
             ),
             directory=True,
         )
         self.check(
-            self._config.getString('ENGINE_CACHE'),
+            self._config.get('ENGINE_CACHE'),
             directory=True,
             writable=True,
         )
         self.check(
-            self._config.getString('ENGINE_TMP'),
+            self._config.get('ENGINE_TMP'),
             directory=True,
             writable=True,
             mustExist=False,
@@ -117,20 +117,20 @@
         for dir in ('.', 'content', 'deployments'):
             self.check(
                 os.path.join(
-                    self._config.getString('ENGINE_VAR'),
+                    self._config.get('ENGINE_VAR'),
                     dir
                 ),
                 directory=True,
                 writable=True,
             )
         self.check(
-            self._config.getString('ENGINE_LOG'),
+            self._config.get('ENGINE_LOG'),
             directory=True,
             writable=True,
         )
         self.check(
             name=os.path.join(
-                self._config.getString("ENGINE_LOG"),
+                self._config.get("ENGINE_LOG"),
                 'host-deploy',
             ),
             directory=True,
@@ -138,7 +138,7 @@
         )
         for log in ('engine.log', 'console.log', 'server.log'):
             self.check(
-                name=os.path.join(self._config.getString("ENGINE_LOG"), log),
+                name=os.path.join(self._config.get("ENGINE_LOG"), log),
                 mustExist=False,
                 writable=True,
             )
@@ -152,10 +152,10 @@
     def _setupEngineApps(self):
 
         # The list of applications to be deployed:
-        for engineApp in self._config.getString('ENGINE_APPS').split():
+        for engineApp in self._config.get('ENGINE_APPS').split():
             # Do nothing if the application is not available:
             engineAppDir = os.path.join(
-                self._config.getString('ENGINE_USR'),
+                self._config.get('ENGINE_USR'),
                 engineApp,
             )
             if not os.path.exists(engineAppDir):
@@ -173,7 +173,7 @@
             # Make sure the application is linked in the deployments
             # directory, if not link it now:
             engineAppLink = os.path.join(
-                self._config.getString('ENGINE_VAR'),
+                self._config.get('ENGINE_VAR'),
                 'deployments',
                 engineApp,
             )
@@ -251,11 +251,11 @@
         )
 
         jbossModulesJar = os.path.join(
-            self._config.getString('JBOSS_HOME'),
+            self._config.get('JBOSS_HOME'),
             'jboss-modules.jar',
         )
         java = os.path.join(
-            self._config.getString('JAVA_HOME'),
+            self._config.get('JAVA_HOME'),
             'bin',
             'java',
         )
@@ -266,24 +266,24 @@
             java=java,
         )
 
-        self._tempDir = service.TempDir(self._config.getString('ENGINE_TMP'))
+        self._tempDir = service.TempDir(self._config.get('ENGINE_TMP'))
         self._tempDir.create()
 
         self._setupEngineApps()
 
         jbossTempDir = os.path.join(
-            self._config.getString('ENGINE_TMP'),
+            self._config.get('ENGINE_TMP'),
             'tmp',
         )
 
         jbossConfigDir = os.path.join(
-            self._config.getString('ENGINE_TMP'),
+            self._config.get('ENGINE_TMP'),
             'config',
         )
 
         jbossModulesTmpDir = self._linkModules(
             os.path.join(
-                self._config.getString('JBOSS_HOME'),
+                self._config.get('JBOSS_HOME'),
                 'modules',
             ),
         )
@@ -294,7 +294,7 @@
 
         jbossBootLoggingFile = self._processTemplate(
             template=os.path.join(
-                self._config.getString('ENGINE_USR'),
+                self._config.get('ENGINE_USR'),
                 'services',
                 'ovirt-engine-logging.properties.in'
             ),
@@ -303,7 +303,7 @@
 
         jbossConfigFile = self._processTemplate(
             template=os.path.join(
-                self._config.getString('ENGINE_USR'),
+                self._config.get('ENGINE_USR'),
                 'services',
                 'ovirt-engine.xml.in',
             ),
@@ -324,10 +324,10 @@
             # Virtual machine options:
             '-server',
             '-XX:+TieredCompilation',
-            '-Xms%s' % self._config.getString('ENGINE_HEAP_MIN'),
-            '-Xmx%s' % self._config.getString('ENGINE_HEAP_MAX'),
-            '-XX:PermSize=%s' % self._config.getString('ENGINE_PERM_MIN'),
-            '-XX:MaxPermSize=%s' % self._config.getString(
+            '-Xms%s' % self._config.get('ENGINE_HEAP_MIN'),
+            '-Xmx%s' % self._config.get('ENGINE_HEAP_MAX'),
+            '-XX:PermSize=%s' % self._config.get('ENGINE_PERM_MIN'),
+            '-XX:MaxPermSize=%s' % self._config.get(
                 'ENGINE_PERM_MAX'
             ),
             '-Djava.net.preferIPv4Stack=true',
@@ -337,14 +337,14 @@
         ])
 
         # Add extra system properties provided in the configuration:
-        engineProperties = self._config.getString('ENGINE_PROPERTIES')
+        engineProperties = self._config.get('ENGINE_PROPERTIES')
         for engineProperty in engineProperties.split():
             if not engineProperty.startswith('-D'):
                 engineProperty = '-D' + engineProperty
             self._engineArgs.append(engineProperty)
 
         # Add arguments for remote debugging of the java virtual machine:
-        engineDebugAddress = self._config.getString('ENGINE_DEBUG_ADDRESS')
+        engineDebugAddress = self._config.get('ENGINE_DEBUG_ADDRESS')
         if engineDebugAddress:
             self._engineArgs.append(
                 (
@@ -356,7 +356,7 @@
             )
 
         # Enable verbose garbage collection if required:
-        if self._config.getBoolean('ENGINE_VERBOSE_GC'):
+        if self._config.getboolean('ENGINE_VERBOSE_GC'):
             self._engineArgs.extend([
                 '-verbose:gc',
                 '-XX:+PrintGCTimeStamps',
@@ -371,16 +371,16 @@
             '-Djboss.modules.system.pkgs=org.jboss.byteman',
             '-Djboss.modules.write-indexes=false',
             '-Djboss.server.default.config=ovirt-engine',
-            '-Djboss.home.dir=%s' % self._config.getString(
+            '-Djboss.home.dir=%s' % self._config.get(
                 'JBOSS_HOME'
             ),
-            '-Djboss.server.base.dir=%s' % self._config.getString(
+            '-Djboss.server.base.dir=%s' % self._config.get(
                 'ENGINE_USR'
             ),
-            '-Djboss.server.data.dir=%s' % self._config.getString(
+            '-Djboss.server.data.dir=%s' % self._config.get(
                 'ENGINE_VAR'
             ),
-            '-Djboss.server.log.dir=%s' % self._config.getString(
+            '-Djboss.server.log.dir=%s' % self._config.get(
                 'ENGINE_LOG'
             ),
             '-Djboss.server.config.dir=%s' % jbossConfigDir,
@@ -393,7 +393,7 @@
             # application server if needed:
             '-mp', "%s:%s" % (
                 os.path.join(
-                    self._config.getString('ENGINE_USR'),
+                    self._config.get('ENGINE_USR'),
                     'modules',
                 ),
                 jbossModulesTmpDir,
@@ -411,18 +411,18 @@
             'LC_ALL': 'en_US.UTF-8',
             'ENGINE_DEFAULTS': config.ENGINE_DEFAULT_FILE,
             'ENGINE_VARS': config.ENGINE_VARS,
-            'ENGINE_ETC': self._config.getString('ENGINE_ETC'),
-            'ENGINE_LOG': self._config.getString('ENGINE_LOG'),
-            'ENGINE_TMP': self._config.getString('ENGINE_TMP'),
-            'ENGINE_USR': self._config.getString('ENGINE_USR'),
-            'ENGINE_VAR': self._config.getString('ENGINE_VAR'),
-            'ENGINE_CACHE': self._config.getString('ENGINE_CACHE'),
+            'ENGINE_ETC': self._config.get('ENGINE_ETC'),
+            'ENGINE_LOG': self._config.get('ENGINE_LOG'),
+            'ENGINE_TMP': self._config.get('ENGINE_TMP'),
+            'ENGINE_USR': self._config.get('ENGINE_USR'),
+            'ENGINE_VAR': self._config.get('ENGINE_VAR'),
+            'ENGINE_CACHE': self._config.get('ENGINE_CACHE'),
         })
 
     def daemonStdHandles(self):
         consoleLog = open(
             os.path.join(
-                self._config.getString('ENGINE_LOG'),
+                self._config.get('ENGINE_LOG'),
                 'console.log'
             ),
             'w+',
@@ -434,10 +434,10 @@
             executable=self._executable,
             args=self._engineArgs,
             env=self._engineEnv,
-            stopTime=self._config.getInteger(
+            stopTime=self._config.getinteger(
                 'ENGINE_STOP_TIME'
             ),
-            stopInterval=self._config.getInteger(
+            stopInterval=self._config.getinteger(
                 'ENGINE_STOP_INTERVAL'
             ),
         )
diff --git a/packaging/services/ovirt-engine.xml.in 
b/packaging/services/ovirt-engine.xml.in
index eb39d2d..aece3eb 100644
--- a/packaging/services/ovirt-engine.xml.in
+++ b/packaging/services/ovirt-engine.xml.in
@@ -44,7 +44,7 @@
         <formatter>
           <pattern-formatter pattern="%d %-5p [%c] (%t) %s%E%n"/>
         </formatter>
-        <file path="$getString('ENGINE_LOG')/server.log"/>
+        <file path="$getstring('ENGINE_LOG')/server.log"/>
         <rotate-size value="10M"/>
         <max-backup-index value="30"/>
         <append value="true"/>
@@ -56,7 +56,7 @@
         <formatter>
           <pattern-formatter pattern="%d %-5p [%c] (%t) %s%E%n"/>
         </formatter>
-        <file path="$getString('ENGINE_LOG')/engine.log"/>
+        <file path="$getstring('ENGINE_LOG')/engine.log"/>
         <rotate-size value="10M"/>
         <max-backup-index value="30"/>
         <append value="true"/>
@@ -104,17 +104,17 @@
       <datasources>
 
         <datasource jndi-name="java:/ENGINEDataSource" 
pool-name="ENGINEDataSource" enabled="true">
-          
<connection-url><![CDATA[$getString('ENGINE_DB_URL')]]></connection-url>
+          
<connection-url><![CDATA[$getstring('ENGINE_DB_URL')]]></connection-url>
           <driver>postgresql</driver>
           
<transaction-isolation>TRANSACTION_READ_COMMITTED</transaction-isolation>
           <pool>
-            
<min-pool-size>$getInteger('ENGINE_DB_MIN_CONNECTIONS')</min-pool-size>
-            
<max-pool-size>$getInteger('ENGINE_DB_MAX_CONNECTIONS')</max-pool-size>
+            
<min-pool-size>$getinteger('ENGINE_DB_MIN_CONNECTIONS')</min-pool-size>
+            
<max-pool-size>$getinteger('ENGINE_DB_MAX_CONNECTIONS')</max-pool-size>
             <prefill>true</prefill>
           </pool>
           <security>
-            <user-name><![CDATA[$getString('ENGINE_DB_USER')]]></user-name>
-            <password><![CDATA[$getString('ENGINE_DB_PASSWORD')]]></password>
+            <user-name><![CDATA[$getstring('ENGINE_DB_USER')]]></user-name>
+            <password><![CDATA[$getstring('ENGINE_DB_PASSWORD')]]></password>
           </security>
           <statement>
             <prepared-statement-cache-size>100</prepared-statement-cache-size>
@@ -135,7 +135,7 @@
     </subsystem>
 
     <subsystem xmlns="urn:jboss:domain:deployment-scanner:1.1">
-      <deployment-scanner scan-interval="5000" 
path="$getString('ENGINE_VAR')/deployments"/>
+      <deployment-scanner scan-interval="5000" 
path="$getstring('ENGINE_VAR')/deployments"/>
     </subsystem>
 
     <subsystem xmlns="urn:jboss:domain:ee:1.0"/>
@@ -163,7 +163,7 @@
       </caches>
       <async thread-pool-name="default"/>
       <timer-service thread-pool-name="default">
-        <data-store path="$getString('ENGINE_VAR')/timer-service-data"/>
+        <data-store path="$getstring('ENGINE_VAR')/timer-service-data"/>
       </timer-service>
       <remote connector-ref="remoting-connector" thread-pool-name="default"/>
       <thread-pools>
@@ -259,16 +259,16 @@
     <subsystem xmlns="urn:jboss:domain:threads:1.1"/>
 
     <subsystem xmlns="urn:jboss:domain:web:1.1" native="false" 
default-virtual-server="default-host">
-      #if $getBoolean('ENGINE_HTTP_ENABLED')
-        <connector name="http" protocol="HTTP/1.1" scheme="http" 
socket-binding="http" redirect-port="$getInteger('ENGINE_HTTPS_PORT')"/>
+      #if $getboolean('ENGINE_HTTP_ENABLED')
+        <connector name="http" protocol="HTTP/1.1" scheme="http" 
socket-binding="http" redirect-port="$getinteger('ENGINE_HTTPS_PORT')"/>
       #end if
-      #if $getBoolean('ENGINE_HTTPS_ENABLED')
+      #if $getboolean('ENGINE_HTTPS_ENABLED')
         <connector name="https" protocol="HTTP/1.1" scheme="https" 
socket-binding="https" secure="true">
-          <ssl name="ssl" password="mypass" 
certificate-key-file="$getString('ENGINE_PKI')/keys/jboss.p12" 
keystore-type="PKCS12" key-alias="1" 
protocol="$getString('ENGINE_HTTPS_PROTOCOLS')" verify-client="false"/>
+          <ssl name="ssl" password="mypass" 
certificate-key-file="$getstring('ENGINE_PKI')/keys/jboss.p12" 
keystore-type="PKCS12" key-alias="1" 
protocol="$getstring('ENGINE_HTTPS_PROTOCOLS')" verify-client="false"/>
         </connector>
       #end if
-      #if $getBoolean('ENGINE_AJP_ENABLED')
-        <connector name="ajp" protocol="AJP/1.3" scheme="http" 
socket-binding="ajp" redirect-port="$getInteger('ENGINE_PROXY_HTTPS_PORT')"/>
+      #if $getboolean('ENGINE_AJP_ENABLED')
+        <connector name="ajp" protocol="AJP/1.3" scheme="http" 
socket-binding="ajp" redirect-port="$getinteger('ENGINE_PROXY_HTTPS_PORT')"/>
       #end if
       <virtual-server name="default-host" enable-welcome-root="false">
         <alias name="localhost"/>
@@ -288,14 +288,14 @@
   </interfaces>
 
   <socket-binding-group name="standard-sockets" default-interface="loopback">
-    #if $getBoolean('ENGINE_HTTP_ENABLED')
-      <socket-binding name="http" port="$getInteger('ENGINE_HTTP_PORT')" 
interface="public"/>
+    #if $getboolean('ENGINE_HTTP_ENABLED')
+      <socket-binding name="http" port="$getinteger('ENGINE_HTTP_PORT')" 
interface="public"/>
     #end if
-    #if $getBoolean('ENGINE_HTTPS_ENABLED')
-      <socket-binding name="https" port="$getInteger('ENGINE_HTTPS_PORT')" 
interface="public"/>
+    #if $getboolean('ENGINE_HTTPS_ENABLED')
+      <socket-binding name="https" port="$getinteger('ENGINE_HTTPS_PORT')" 
interface="public"/>
     #end if
-    #if $getBoolean('ENGINE_AJP_ENABLED')
-      <socket-binding name="ajp" port="$getInteger('ENGINE_AJP_PORT')"/>
+    #if $getboolean('ENGINE_AJP_ENABLED')
+      <socket-binding name="ajp" port="$getinteger('ENGINE_AJP_PORT')"/>
     #end if
     <socket-binding name="remoting" port="8703"/>
     <socket-binding name="txn-recovery-environment" port="8704"/>
diff --git a/packaging/services/service.py b/packaging/services/service.py
index 5adb632..3337918 100755
--- a/packaging/services/service.py
+++ b/packaging/services/service.py
@@ -82,31 +82,29 @@
 
 class ConfigFile(Base):
     """
-    Helper class to simplify getting values from the configuration, specially
-    from the template used to generate the application server configuration
-    file
+    Parsing of shell style config file.
+    Follow closly the java LocalConfig implementaiton.
     """
 
-    # Compile regular expressions:
-    COMMENT_EXPR = re.compile(r'\s*#.*$')
-    BLANK_EXPR = re.compile(r'^\s*$')
-    VALUE_EXPR = re.compile(r'^\s*(?P<key>\w+)\s*=\s*(?P<value>.*?)\s*$')
-    REF_EXPR = re.compile(r'\$\{(?P<ref>\w+)\}')
+    _EMPTY_LINE = re.compile(r'^\s*(#.*|)$')
+    _KEY_VALUE_EXPRESSION = re.compile(r'^\s*(?P<key>\w+)=(?P<value>.*)$')
 
-    def __init__(self, files):
+    def _loadLine(self, line):
+        emptyMatch = self._EMPTY_LINE.search(line)
+        if emptyMatch is None:
+            keyValueMatch = self._KEY_VALUE_EXPRESSION.search(line)
+            if keyValueMatch is None:
+                raise RuntimeError(_('Invalid sytax'))
+            self._values[keyValueMatch.group('key')] = self.expandString(
+                keyValueMatch.group('value')
+            )
+
+    def __init__(self, files=[]):
         super(ConfigFile, self).__init__()
 
-        self._dir = dir
-        # Save the list of files:
-        self.files = files
+        self._values = {}
 
-        # Start with an empty set of values:
-        self.values = {}
-
-        # Merge all the given configuration files, in the same order
-        # given, so that the values in one file are overriden by values
-        # in files appearing later in the list:
-        for file in self.files:
+        for file in files:
             self.loadFile(file)
             for filed in sorted(
                 glob.glob(
@@ -121,77 +119,85 @@
     def loadFile(self, file):
         if os.path.exists(file):
             self._logger.debug("loading config '%s'", file)
-            with open(file, 'r') as f:
-                for line in f:
-                    self.loadLine(line)
+            index = 0
+            try:
+                with open(file, 'r') as f:
+                    for line in f:
+                        index += 1
+                        self._loadLine(line)
+            except Exception as e:
+                self._logger(
+                    "File '%s' index %d error" % (file, index),
+                    exc_info=True,
+                )
+                raise RuntimeError(
+                    _(
+                        "Cannot parse configuration file "
+                        "'{file}' line {line}: {error}"
+                    ).format(
+                        file=file,
+                        line=index,
+                        error=e
+                    )
+                )
 
-    def loadLine(self, line):
-        # Remove comments:
-        commentMatch = self.COMMENT_EXPR.search(line)
-        if commentMatch is not None:
-            line = line[:commentMatch.start()] + line[commentMatch.end():]
+    def expandString(self, value):
+        ret = ""
 
-        # Skip empty lines:
-        emptyMatch = self.BLANK_EXPR.search(line)
-        if emptyMatch is not None:
-            return
+        escape = False
+        inQuotes = False
+        index = 0
+        while (index < len(value)):
+            c = value[index]
+            index += 1
+            if escape:
+                escape = False
+                ret += c
+            else:
+                if c == '\\':
+                    escape = True
+                elif c == '$':
+                    if value[index] != '{':
+                        raise RuntimeError('Malformed variable assignment')
+                    index += 1
+                    i = value.find('}', index)
+                    if i == -1:
+                        raise RuntimeError('Malformed variable assignment')
+                    name = value[index:i]
+                    index = i + 1
+                    ret += self._values.get(name, "")
+                elif c == '"':
+                    inQuotes = not inQuotes
+                elif c in (' ', '#'):
+                    if inQuotes:
+                        ret += c
+                    else:
+                        index = len(value)
+                else:
+                    ret += c
 
-        # Separate name from value:
-        keyValueMatch = self.VALUE_EXPR.search(line)
-        if keyValueMatch is None:
-            return
-        key = keyValueMatch.group('key')
-        value = keyValueMatch.group('value')
+        return ret
 
-        # Strip quotes from value:
-        if len(value) >= 2 and value[0] == '"' and value[-1] == '"':
-            value = value[1:-1]
+    def getstring(self, name, default=None):
+        "alias to get as cheetah.template cannot call get"
+        return self.get(name, default)
 
-        # Expand references to other parameters:
-        while True:
-            refMatch = self.REF_EXPR.search(value)
-            if refMatch is None:
-                break
-            refKey = refMatch.group('ref')
-            refValue = self.values.get(refKey)
-            if refValue is None:
-                break
-            value = '%s%s%s' % (
-                value[:refMatch.start()],
-                refValue,
-                value[refMatch.end():],
-            )
+    def get(self, name, default=None):
+        return self._values.get(name, default)
 
-        # Update the values:
-        self.values[key] = value
-
-    def getString(self, name):
-        text = self.values.get(name)
+    def getboolean(self, name, default=None):
+        text = self.get(name)
         if text is None:
-            raise RuntimeError(
-                _("The parameter '{name}' does not have a value").format(
-                    name=name,
-                )
-            )
-        return text
+            return default
+        else:
+            return text.lower() in ('t', 'true', 'y', 'yes', '1')
 
-    def getBoolean(self, name):
-        return self.getString(name) in ('t', 'true', 'y', 'yes', '1')
-
-    def getInteger(self, name):
-        value = self.getString(name)
-        try:
+    def getinteger(self, name, default=None):
+        value = self.get(name)
+        if value is None:
+            return default
+        else:
             return int(value)
-        except ValueError:
-            raise RuntimeError(
-                _(
-                    "The value '{value}' of parameter '{name}' "
-                    "is not a valid integer"
-                ).format(
-                    name,
-                    value,
-                )
-            )
 
 
 class TempDir(Base):
diff --git a/packaging/setup/ovirt_engine_setup/util.py 
b/packaging/setup/ovirt_engine_setup/util.py
index ec5816d..5b014e2 100644
--- a/packaging/setup/ovirt_engine_setup/util.py
+++ b/packaging/setup/ovirt_engine_setup/util.py
@@ -33,46 +33,23 @@
 
 
 class ConfigFile(base.Base):
-    _COMMENT_EXPR = re.compile(r'\s*#.*$')
-    _BLANK_EXPR = re.compile(r'^\s*$')
-    _VALUE_EXPR = re.compile(r'^\s*(?P<key>\w+)\s*=\s*(?P<value>.*?)\s*$')
-    _REF_EXPR = re.compile(r'\$\{(?P<ref>\w+)\}')
+    """
+    Parsing of shell style config file.
+    Follow closly the java LocalConfig implementaiton.
+    """
+
+    _EMPTY_LINE = re.compile(r'^\s*(#.*|)$')
+    _KEY_VALUE_EXPRESSION = re.compile(r'^\s*(?P<key>\w+)=(?P<value>.*)$')
 
     def _loadLine(self, line):
-        # Remove comments:
-        commentMatch = self._COMMENT_EXPR.search(line)
-        if commentMatch is not None:
-            line = line[:commentMatch.start()] + line[commentMatch.end():]
-
-        # Skip empty lines:
-        emptyMatch = self._BLANK_EXPR.search(line)
+        emptyMatch = self._EMPTY_LINE.search(line)
         if emptyMatch is None:
-            # Separate name from value:
-            keyValueMatch = self._VALUE_EXPR.search(line)
-            if keyValueMatch is not None:
-                key = keyValueMatch.group('key')
-                value = keyValueMatch.group('value')
-
-                # Strip quotes from value:
-                if len(value) >= 2 and value[0] == '"' and value[-1] == '"':
-                    value = value[1:-1]
-
-                # Expand references to other parameters:
-                while True:
-                    refMatch = self._REF_EXPR.search(value)
-                    if refMatch is None:
-                        break
-                    refKey = refMatch.group('ref')
-                    refValue = self._values.get(refKey)
-                    if refValue is None:
-                        break
-                    value = '%s%s%s' % (
-                        value[:refMatch.start()],
-                        refValue,
-                        value[refMatch.end():],
-                    )
-
-                self._values[key] = value
+            keyValueMatch = self._KEY_VALUE_EXPRESSION.search(line)
+            if keyValueMatch is None:
+                raise RuntimeError(_('Invalid sytax'))
+            self._values[keyValueMatch.group('key')] = self.expandString(
+                keyValueMatch.group('value')
+            )
 
     def __init__(self, files=[]):
         super(ConfigFile, self).__init__()
@@ -94,9 +71,68 @@
     def loadFile(self, file):
         if os.path.exists(file):
             self.logger.debug("loading config '%s'", file)
-            with open(file, 'r') as f:
-                for line in f:
-                    self._loadLine(line)
+            index = 0
+            try:
+                with open(file, 'r') as f:
+                    for line in f:
+                        index += 1
+                        self._loadLine(line)
+            except Exception as e:
+                self.logger(
+                    "File '%s' index %d error" % (file, index),
+                    exc_info=True,
+                )
+                raise RuntimeError(
+                    _(
+                        "Cannot parse configuration file "
+                        "'{file}' line {line}: {error}"
+                    ).format(
+                        file=file,
+                        line=index,
+                        error=e
+                    )
+                )
+
+    def expandString(self, value):
+        ret = ""
+
+        escape = False
+        inQuotes = False
+        index = 0
+        while (index < len(value)):
+            c = value[index]
+            index += 1
+            if escape:
+                escape = False
+                ret += c
+            else:
+                if c == '\\':
+                    escape = True
+                elif c == '$':
+                    if value[index] != '{':
+                        raise RuntimeError('Malformed variable assignment')
+                    index += 1
+                    i = value.find('}', index)
+                    if i == -1:
+                        raise RuntimeError('Malformed variable assignment')
+                    name = value[index:i]
+                    index = i + 1
+                    ret += self._values.get(name, "")
+                elif c == '"':
+                    inQuotes = not inQuotes
+                elif c in (' ', '#'):
+                    if inQuotes:
+                        ret += c
+                    else:
+                        index = len(value)
+                else:
+                    ret += c
+
+        return ret
+
+    def getstring(self, name, default=None):
+        "alias to get as cheetah.template cannot call get"
+        return self.get(name, default)
 
     def get(self, name, default=None):
         return self._values.get(name, default)
diff --git a/packaging/setup/plugins/ovirt-engine-setup/config/ca.py 
b/packaging/setup/plugins/ovirt-engine-setup/config/ca.py
index d4633b2..f2fb7cf 100644
--- a/packaging/setup/plugins/ovirt-engine-setup/config/ca.py
+++ b/packaging/setup/plugins/ovirt-engine-setup/config/ca.py
@@ -19,6 +19,7 @@
 """CA plugin."""
 
 
+import re
 import gettext
 _ = lambda m: gettext.dgettext(message=m, domain='ovirt-engine-setup')
 
@@ -99,12 +100,16 @@
                         osetupcons.FileLocations.
                         OVIRT_ENGINE_PKI_ENGINE_TRUST_STORE
                     ),
-                    trust_store_password=osetupcons.Const.PKI_PASSWORD,
+                    trust_store_password=re.escape(
+                        osetupcons.Const.PKI_PASSWORD
+                    ),
                     engine_store=(
                         osetupcons.FileLocations.
                         OVIRT_ENGINE_PKI_ENGINE_STORE
                     ),
-                    engine_store_password=osetupcons.Const.PKI_PASSWORD,
+                    engine_store_password=re.escape(
+                        osetupcons.Const.PKI_PASSWORD
+                    ),
                     engine_store_alias='1',
                 ),
                 modifiedList=uninstall_files,
diff --git a/packaging/setup/plugins/ovirt-engine-setup/config/database.py 
b/packaging/setup/plugins/ovirt-engine-setup/config/database.py
index a6b6384..6d871eb 100644
--- a/packaging/setup/plugins/ovirt-engine-setup/config/database.py
+++ b/packaging/setup/plugins/ovirt-engine-setup/config/database.py
@@ -19,6 +19,7 @@
 """Database plugin."""
 
 
+import re
 import gettext
 _ = lambda m: gettext.dgettext(message=m, domain='ovirt-engine-setup')
 
@@ -77,7 +78,9 @@
                     host=self.environment[osetupcons.DBEnv.HOST],
                     port=self.environment[osetupcons.DBEnv.PORT],
                     user=self.environment[osetupcons.DBEnv.USER],
-                    password=self.environment[osetupcons.DBEnv.PASSWORD],
+                    password=re.escape(
+                        self.environment[osetupcons.DBEnv.PASSWORD]
+                    ),
                     db=self.environment[osetupcons.DBEnv.DATABASE],
                     secured=self.environment[osetupcons.DBEnv.SECURED],
                     securedValidation=self.environment[


--
To view, visit http://gerrit.ovirt.org/15142
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I945c8ddbe5de512ab330153c27802bb71964b76f
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev <alo...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to