Merge branch 'salt' of github.com:c-po/vyos-1x into current

* 'salt' of github.com:c-po/vyos-1x:
  salt: T2384: migrate config options
  salt: T2385: XML: improve completion helpers on hash_type
  salt: T2384: always log to syslog
  Revert "salt: T2382: id and master nodes are mandatory - use in verify()"
  salt: T2382: ease config generation
  salt: T2382: migrate get_config() to list items
  salt: T2382: id and master nodes are mandatory - use in verify()
  salt: T2382: add missing verify()
  salt: T2382: XML: run as user nobody
  salt: T2382: XML: add proper valueHelp and validators for master
  salt: T2382: add missing dependency on salt-minion

5 files changed, 131 insertions, 110 deletions

diff --git a/data/templates/salt-minion/minion.tmpl b/data/templates/salt-minion/minion.tmpl
index 5e50d588c..9369573a4 100644
--- a/data/templates/salt-minion/minion.tmpl
+++ b/data/templates/salt-minion/minion.tmpl
@@ -12,7 +12,7 @@
 #
 # Prior to changing this value, the master should be stopped and all Salt
 # caches should be cleared.
-hash_type: {{ hash_type }}
+hash_type: {{ hash }}
 
 #####         Logging settings       #####
 ##########################################
@@ -21,11 +21,7 @@ hash_type: {{ hash_type }}
 # location. Remote logging works best when configured to use rsyslogd(8) (e.g.:
 # ``file:///dev/log``), with rsyslogd(8) configured for network logging. The URI
 # format is: <file|udp|tcp>://<host|socketpath>:<port-if-required>/<log-facility>
-#log_file: /var/log/salt/minion
-#log_file: file:///dev/log
-#log_file: udp://loghost:10514
-#
-log_file: {{ log_file }}
+log_file: file:///dev/log
 
 # The level of messages to send to the console.
 # One of 'garbage', 'trace', 'debug', info', 'warning', 'error', 'critical'.
@@ -58,6 +54,6 @@ id: {{ salt_id }}
 
 
 # The number of minutes between mine updates.
-mine_interval: {{ mine_interval }}
+mine_interval: {{ interval }}
 
 verify_master_pubkey_sign: {{ verify_master_pubkey_sign }}
diff --git a/debian/control b/debian/control
index 7b95b2c75..5c176f40a 100644
--- a/debian/control
+++ b/debian/control
@@ -89,6 +89,7 @@ Depends: python3,
   pmacct (>= 1.6.0),
   python3-certbot-nginx,
   pppoe,
+  salt-minion,
   ${shlibs:Depends},
   ${misc:Depends}
 Description: VyOS configuration scripts and data
diff --git a/interface-definitions/salt-minion.xml.in b/interface-definitions/salt-minion.xml.in
index 9aa60249a..d0e8e3e2b 100644
--- a/interface-definitions/salt-minion.xml.in
+++ b/interface-definitions/salt-minion.xml.in
@@ -1,5 +1,4 @@
 <?xml version="1.0"?>
-<!--Salt-minion configuration -->
 <interfaceDefinition>
   <node name="service">
     <children>
@@ -9,52 +8,33 @@
           <priority>500</priority>
         </properties>
         <children>
-          <leafNode name="hash_type">
+          <leafNode name="hash">
             <properties>
-              <help>The hash_type is the hash to use when discovering the hash of a file on the master server.</help>
+              <help>Hash used when discovering file on master server (default: sha256)</help>
+              <completionHelp>
+                <list>md5 sha1 sha224 sha256 sha384 sha512</list>
+              </completionHelp>
+              <constraint>
+                <regex>(md5|sha1|sha224|sha256|sha384|sha512)</regex>
+              </constraint>
             </properties>
           </leafNode>
-          <leafNode name="log_file">
-            <properties>
-              <help>The location of the minion log file.</help>
-            </properties>
-          </leafNode>
-          <leafNode name="log_level">
+          <leafNode name="master">
             <properties>
-              <help>Log level</help>
-              <valueHelp>
-                <format>garbage</format>
-                <description>log garbage info</description>
-              </valueHelp>
-              <valueHelp>
-                <format>trace</format>
-                <description>log trace info</description>
-              </valueHelp>
-              <valueHelp>
-                <format>debug</format>
-                <description>log debug info</description>
-              </valueHelp>
-              <valueHelp>
-                <format>info</format>
-                <description>log info</description>
-              </valueHelp>
-              <valueHelp>
-                <format>warning</format>
-                <description>log warning info</description>
-              </valueHelp>
+              <help>The hostname or IP address of the master.</help>
               <valueHelp>
-                <format>error</format>
-                <description>log error info</description>
+                <format>ipv4</format>
+                <description>Remote syslog server IPv4 address</description>
               </valueHelp>
               <valueHelp>
-                <format>critical</format>
-                <description>log critical info</description>
+                <format>hostname</format>
+                <description>Remote syslog server FQDN</description>
               </valueHelp>
-            </properties>
-          </leafNode>
-          <leafNode name="master">
-            <properties>
-              <help>The hostname or IP address of the master.</help>
+              <constraint>
+                <validator name="ip-address"/>
+                <validator name="fqdn"/>
+              </constraint>
+              <constraintErrorMessage>Invalid FQDN or IP address</constraintErrorMessage>
               <multi/>
             </properties>
           </leafNode>
@@ -63,12 +43,7 @@
               <help>Explicitly declare the id for this minion to use.</help>
             </properties>
           </leafNode>
-          <leafNode name="user">
-            <properties>
-              <help>The user to run the Salt processes.</help>
-            </properties>
-          </leafNode>
-          <leafNode name="mine_interval">
+          <leafNode name="interval">
             <properties>
               <help>The number of minutes between mine updates.</help>
             </properties>
diff --git a/src/conf_mode/salt-minion.py b/src/conf_mode/salt-minion.py
index 236480854..515019014 100755
--- a/src/conf_mode/salt-minion.py
+++ b/src/conf_mode/salt-minion.py
@@ -17,117 +17,108 @@
 import os
 
 from copy import deepcopy
-from pwd import getpwnam
 from socket import gethostname
 from sys import exit
 from urllib3 import PoolManager
 
 from vyos.config import Config
-from vyos import ConfigError
-from vyos.util import call
 from vyos.template import render
-
+from vyos.util import call, chown
+from vyos import ConfigError
 
 config_file = r'/etc/salt/minion'
+master_keyfile = r'/opt/vyatta/etc/config/salt/pki/minion/master_sign.pub'
 
 default_config_data = {
-    'hash_type': 'sha256',
-    'log_file': '/var/log/salt/minion',
+    'hash': 'sha256',
     'log_level': 'warning',
     'master' : 'salt',
-    'user': 'minion',
+    'user': 'nobody',
+    'group': 'nogroup',
     'salt_id': gethostname(),
     'mine_interval': '60',
-    'verify_master_pubkey_sign': 'false'
+    'verify_master_pubkey_sign': 'false',
+    'master_key': ''
 }
 
 def get_config():
     salt = deepcopy(default_config_data)
     conf = Config()
-    if not conf.exists('service salt-minion'):
+    base = ['service', 'salt-minion']
+
+    if not conf.exists(base):
         return None
     else:
-        conf.set_level('service salt-minion')
-
-    if conf.exists('hash_type'):
-        salt['hash_type'] = conf.return_value('hash_type')
+        conf.set_level(base)
 
-    if conf.exists('log_file'):
-        salt['log_file'] = conf.return_value('log_file')
+    if conf.exists(['hash']):
+        salt['hash'] = conf.return_value(['hash'])
 
-    if conf.exists('log_level'):
-        salt['log_level'] = conf.return_value('log_level')
+    if conf.exists(['master']):
+        salt['master'] = conf.return_values(['master'])
 
-    if conf.exists('master'):
-        master = conf.return_values('master')
-        salt['master'] = master
+    if conf.exists(['id']):
+        salt['salt_id'] = conf.return_value(['id'])
 
-    if conf.exists('id'):
-        salt['salt_id'] = conf.return_value('id')
+    if conf.exists(['user']):
+        salt['user'] = conf.return_value(['user'])
 
-    if conf.exists('user'):
-        salt['user'] = conf.return_value('user')
+    if conf.exists(['interval']):
+        salt['interval'] = conf.return_value(['interval'])
 
-    if conf.exists('mine_interval'):
-        salt['mine_interval'] = conf.return_value('mine_interval')
-
-    salt['master-key'] = None
-    if conf.exists('master-key'):
-        salt['master-key'] = conf.return_value('master-key')
+    if conf.exists(['master-key']):
+        salt['master_key'] = conf.return_value(['master-key'])
         salt['verify_master_pubkey_sign'] = 'true'
 
     return salt
 
-def generate(salt):
-    paths = ['/etc/salt/','/var/run/salt','/opt/vyatta/etc/config/salt/']
-    directory = '/opt/vyatta/etc/config/salt/pki/minion'
-    uid = getpwnam(salt['user']).pw_uid
-    http = PoolManager()
+def verify(salt):
+    return None
 
-    if salt is None:
+def generate(salt):
+    if not salt:
         return None
 
-    if not os.path.exists(directory):
-        os.makedirs(directory)
+    for file in [config_file, master_keyfile]:
+        dirname = os.path.dirname(file)
+        if not os.path.exists(dirname):
+            os.mkdir(dirname)
+            chown(dirname, salt['user'], salt['group'])
 
     render(config_file, 'salt-minion/minion.tmpl', salt)
+    chown(config_file, salt['user'], salt['group'])
 
-    path = "/etc/salt/"
-    for path in paths:
-      for root, dirs, files in os.walk(path):
-        for usgr in dirs:
-          os.chown(os.path.join(root, usgr), uid, 100)
-        for usgr in files:
-          os.chown(os.path.join(root, usgr), uid, 100)
-
-    if not os.path.exists('/opt/vyatta/etc/config/salt/pki/minion/master_sign.pub'):
-        if not salt['master-key'] is None:
-            r = http.request('GET', salt['master-key'], preload_content=False)
+    if not os.path.exists(master_keyfile):
+        if salt['master_key']:
+            req = PoolManager().request('GET', salt['master_key'], preload_content=False)
 
-            with open('/opt/vyatta/etc/config/salt/pki/minion/master_sign.pub', 'wb') as out:
+            with open(master_keyfile, 'wb') as f:
                 while True:
-                    data = r.read(1024)
+                    data = req.read(1024)
                     if not data:
                         break
-                    out.write(data)
+                    f.write(data)
 
-            r.release_conn()
+            req.release_conn()
+            chown(master_keyfile, salt['user'], salt['group'])
 
     return None
 
 def apply(salt):
-    if salt is not None:
-        call("sudo systemctl restart salt-minion")
+    if not salt:
+        # Salt removed from running config
+        call('systemctl stop salt-minion.service')
+        if os.path.exists(config_file):
+            os.unlink(config_file)
     else:
-        # Salt access is removed in the commit
-        call("sudo systemctl stop salt-minion")
-        os.unlink(config_file)
+        call('systemctl restart salt-minion.service')
 
     return None
 
 if __name__ == '__main__':
     try:
         c = get_config()
+        verify(c)
         generate(c)
         apply(c)
     except ConfigError as e:
diff --git a/src/migration-scripts/salt/0-to-1 b/src/migration-scripts/salt/0-to-1
new file mode 100755
index 000000000..79053c056
--- /dev/null
+++ b/src/migration-scripts/salt/0-to-1
@@ -0,0 +1,58 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2020 VyOS maintainers and contributors
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 or later as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Delete log_file, log_level and user nodes
+# rename hash_type to hash
+# rename mine_interval to interval
+
+from sys import argv,exit
+
+from vyos.configtree import ConfigTree
+
+if (len(argv) < 1):
+    print("Must specify file name!")
+    exit(1)
+
+file_name = argv[1]
+
+with open(file_name, 'r') as f:
+    config_file = f.read()
+
+config = ConfigTree(config_file)
+
+base = ['service', 'salt-minion']
+if not config.exists(base):
+    # Nothing to do
+    exit(0)
+else:
+
+    # delete nodes which are now populated with sane defaults
+    for node in ['log_file', 'log_level', 'user']:
+        if config.exists(base + [node]):
+            config.delete(base + [node])
+
+    if config.exists(base + ['hash_type']):
+        config.rename(base + ['hash_type'], 'hash')
+
+    if config.exists(base + ['mine_interval']):
+        config.rename(base + ['mine_interval'], 'interval')
+
+    try:
+        with open(file_name, 'w') as f:
+            f.write(config.to_string())
+    except OSError as e:
+        print("Failed to save the modified config: {}".format(e))
+        exit(1)