Pull request comments by Greg Neagle on what to fix: https://github.com/munki/munki/pull/685

IconImporter:
	- Not opening DMG and then mounting it anymore, directly mounting like how it was before
ManifestUtil:
	- Checking if the repo is mounted as well as if we (munki) mounted it. Only this will display prompt whether we want to unmount or not
FileRepo.py, Repo.py:
	- not hardcoding import path anymore
	- FileRepo - added 10.12 mounting fileshares code

code/client/iconimporter

@@ -40,8 +40,7 @@ from Foundation import CFPreferencesCopyAppValue
 def generate_png_from_copy_from_dmg_item(install_item, repo):
     '''Generate a PNG from a disk image containing an application'''
     dmgpath = repo.join('pkgs', install_item['installer_item_location'])
-    handle = repo.open(dmgpath, 'r')
-    mountpoints = munkicommon.mountdmg(handle.local_path)
+    mountpoints = munkicommon.mountdmg(dmgpath)
     if mountpoints:
         mountpoint = mountpoints[0]
         apps = [item for item in install_item.get('items_to_copy', [])
@@ -72,8 +71,8 @@ def generate_pngs_from_installer_pkg(install_item, repo):
     pkg_repo = None
     item_path = repo.join( u'pkgs', install_item['installer_item_location'])
     if munkicommon.hasValidDiskImageExt(item_path):
-        handle = repo.open(item_path, 'r')
-        mountpoints = munkicommon.mountdmg(handle.local_path)
+        dmg_path = item_path
+        mountpoints = munkicommon.mountdmg(dmg_path)
         if mountpoints:
             mountpoint = mountpoints[0]
             if install_item.get('package_path'):

code/client/manifestutil

@@ -424,7 +424,7 @@ def manifest_rename(source_manifest_name, dest_manifest_name,
 def cleanup_and_exit(exitcode):
     """Give the user the chance to unmount the repo when we exit"""
     result = 0
-    if repo.mounted:
+    if repo.mounted and WE_MOUNTED_THE_REPO:
         answer = raw_input('Unmount the repo fileshare? [y/n] ')
         if answer.lower().startswith('y'):
             result = repo.unmount()

code/client/munkiimport

@@ -441,7 +441,7 @@ def copy_pkginfo_to_repo(pkginfo, subdirectory=''):
     destination_path = repo.join('pkgsinfo', subdirectory)
     if not repo.exists(destination_path):
         try:
-            os.makedirs(destination_path)
+            repo.makedirs(destination_path)
         except OSError, errmsg:
             raise RepoCopyError('Could not create %s: %s'
                                 % (destination_path, errmsg))
@@ -769,7 +769,7 @@ def make_catalogs():
 def cleanup_and_exit(exitcode):
     """Unmounts the repo if we mounted it, then exits"""
     result = 0
-    if repo.mounted:
+    if repo.mounted and WE_MOUNTED_THE_REPO:
         if not NOINTERACTIVE:
             answer = raw_input('Unmount the repo fileshare? [y/n] ')
             if answer.lower().startswith('y'):

code/client/munkilib/FileRepo.py

@@ -23,13 +23,109 @@ a remote repo mounted via AFP, SMB, or NFS.
 """
 
 from collections import namedtuple
-import sys
-sys.path.append("/usr/local/munki/munkilib")
-import munkilib.munkicommon
-from munkicommon import listdir
+from munkilib.munkicommon import listdir
 import os
 import sys
 import subprocess
+import objc
+
+# NetFS share mounting code borrowed and liberally adapted from Michael Lynn's
+# work here: https://gist.github.com/pudquick/1362a8908be01e23041d
+try:
+    import errno
+    import getpass
+    import objc
+    from CoreFoundation import CFURLCreateWithString
+
+    class Attrdict(dict):
+        '''Custom dict class'''
+        __getattr__ = dict.__getitem__
+        __setattr__ = dict.__setitem__
+
+    NetFS = Attrdict()
+    # Can cheat and provide 'None' for the identifier, it'll just use
+    # frameworkPath instead
+    # scan_classes=False means only add the contents of this Framework
+    NetFS_bundle = objc.initFrameworkWrapper(
+        'NetFS', frameworkIdentifier=None,
+        frameworkPath=objc.pathForFramework('NetFS.framework'),
+        globals=NetFS, scan_classes=False)
+
+    # https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/
+    # ObjCRuntimeGuide/Articles/ocrtTypeEncodings.html
+    # Fix NetFSMountURLSync signature
+    del NetFS['NetFSMountURLSync']
+    objc.loadBundleFunctions(
+        NetFS_bundle, NetFS, [('NetFSMountURLSync', 'i@@@@@@o^@')])
+    NETFSMOUNTURLSYNC_AVAILABLE = True
+except (ImportError, KeyError):
+    NETFSMOUNTURLSYNC_AVAILABLE = False
+
+if NETFSMOUNTURLSYNC_AVAILABLE:
+    class ShareMountException(Exception):
+        '''An exception raised if share mounting failed'''
+        pass
+
+
+    class ShareAuthenticationNeededException(ShareMountException):
+        '''An exception raised if authentication is needed'''
+        pass
+
+
+    def mount_share(share_url):
+        '''Mounts a share at /Volumes, returns the mount point or raises an
+        error'''
+        sh_url = CFURLCreateWithString(None, share_url, None)
+        # Set UI to reduced interaction
+        open_options = {NetFS.kNAUIOptionKey: NetFS.kNAUIOptionNoUI}
+        # Allow mounting sub-directories of root shares
+        mount_options = {NetFS.kNetFSAllowSubMountsKey: True}
+        # Mount!
+        result, output = NetFS.NetFSMountURLSync(
+            sh_url, None, None, None, open_options, mount_options, None)
+        # Check if it worked
+        if result != 0:
+            if result in (errno.ENOTSUP, errno.EAUTH):
+                # errno.ENOTSUP is returned if an afp share needs a login
+                # errno.EAUTH is returned if authentication fails (SMB for sure)
+                raise ShareAuthenticationNeededException()
+            raise ShareMountException(
+                'Error mounting url "%s": %s, error %s'
+                % (share_url, os.strerror(result), result))
+        # Return the mountpath
+        return str(output[0])
+
+
+    def mount_share_with_credentials(share_url, username, password):
+        '''Mounts a share at /Volumes, returns the mount point or raises an
+        error. Include username and password as parameters, not in the
+        share_path URL'''
+        sh_url = CFURLCreateWithString(None, share_url, None)
+        # Set UI to reduced interaction
+        open_options = {NetFS.kNAUIOptionKey: NetFS.kNAUIOptionNoUI}
+        # Allow mounting sub-directories of root shares
+        mount_options = {NetFS.kNetFSAllowSubMountsKey: True}
+        # Mount!
+        result, output = NetFS.NetFSMountURLSync(
+            sh_url, None, username, password, open_options, mount_options, None)
+        # Check if it worked
+        if result != 0:
+            raise ShareMountException(
+                'Error mounting url "%s": %s, error %s'
+                % (share_url, os.strerror(result), result))
+        # Return the mountpath
+        return str(output[0])
+
+
+    def mount_share_url(share_url):
+        '''Mount a share url under /Volumes, prompting for password if needed
+        Raises ShareMountException if there's an error'''
+        try:
+            mount_share(share_url)
+        except ShareAuthenticationNeededException:
+            username = raw_input('Username: ')
+            password = getpass.getpass()
+            mount_share_with_credentials(share_url, username, password)
 
 class FileRepo:
     '''Repo implementation that access a local or locally-mounted repo.'''
@@ -126,24 +222,37 @@ class FileRepo:
 
     def mount(self):
         '''Mounts the repo locally.'''
+        global WE_MOUNTED_THE_REPO
         if os.path.exists(self.path):
             return
-        os.mkdir(self.path)
-        print self.url
-        print 'Attempting to mount fileshare %s:' % self.url
-        if self.url.startswith('afp:'):
-            cmd = ['/sbin/mount_afp', '-i', self.url, self.path]
-        elif self.url.startswith('smb:'):
-            cmd = ['/sbin/mount_smbfs', self.url[4:], self.path]
-        elif self.url.startswith('nfs://'):
-            cmd = ['/sbin/mount_nfs', self.url[6:], self.path]
+        if NETFSMOUNTURLSYNC_AVAILABLE:
+            try:
+                mount_share_url(self.url)
+            except ShareMountException, err:
+                print sys.stderr, err
+                return 
+            else:
+                WE_MOUNTED_THE_REPO = True
+                return 0
         else:
-            print >> sys.stderr, 'Unsupported filesystem URL!'
-            return
-        retcode = subprocess.call(cmd)
-        if retcode:
-            os.rmdir(self.path)
-        return retcode
+            os.mkdir(self.path)
+            print self.url
+            print 'Attempting to mount fileshare %s:' % self.url
+            if self.url.startswith('afp:'):
+                cmd = ['/sbin/mount_afp', '-i', self.url, self.path]
+            elif self.url.startswith('smb:'):
+                cmd = ['/sbin/mount_smbfs', self.url[4:], self.path]
+            elif self.url.startswith('nfs://'):
+                cmd = ['/sbin/mount_nfs', self.url[6:], self.path]
+            else:
+                print >> sys.stderr, 'Unsupported filesystem URL!'
+                return
+            retcode = subprocess.call(cmd)
+            if retcode:
+                os.rmdir(self.path)
+            else:
+                WE_MOUNTED_THE_REPO = True
+            return retcode
 
     def unmount(self):
         '''Unmounts the repo.'''
@@ -168,4 +277,4 @@ class FileRepo:
         os.chdir(path)
         for arg in args:
             pkgs += glob.glob(arg)
-        os.chdir(original_dir)
+        os.chdir(original_dir)

code/client/munkilib/Repo.py

@@ -29,11 +29,11 @@ def Open(path, url, plugin):
     #looks for plugin in /usr/local/munki/munkilib/plugins (installation of munki)
     if plugin == None or plugin == "":
         #default is FileRepo if no plugin is specified in configuration or options.
-        module = imp.load_source('FileRepo', '/usr/local/munki/munkilib/FileRepo.py')
+        module = imp.load_source('FileRepo', './munkilib/FileRepo.py')
         import_class = getattr(module, "FileRepo")
         parent = import_class
     else:
-        module = imp.load_source(plugin, '/usr/local/munki/munkilib/plugins/' + plugin + ".py")
+        module = imp.load_source(plugin, './munkilib/plugins/' + plugin + ".py")
         import_class = getattr(module, plugin)
         parent = import_class
 
@@ -58,4 +58,4 @@ def Open(path, url, plugin):
             # if we get this far, the repo path looks OK
             return True
 
-    return Repo(path, url)
+    return Repo(path, url)