Merge pull request #23 from homebysix/1.2.0

v1.2.0
homebysix · Jun 27, 2019 · b322bd2 · b322bd2
2 parents e0be1aa + 187926a
commit b322bd2
Show file tree

Hide file tree

Showing 6 changed files with 189 additions and 107 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -16,3 +16,4 @@ repos:
   rev: 19.3b0
   hooks:
   - id: black
+    language_version: python3.7
diff --git a/README.md b/README.md
@@ -13,7 +13,7 @@ For any hook in this repo you wish to use, add the following to your pre-commit
 
 ```
 -   repo: https://github.com/homebysix/pre-commit-macadmin
-    rev: v1.1.4
+    rev: v1.2.0
     hooks:
     -   id: check-plists
     # -   id: ...
@@ -105,7 +105,7 @@ When combining arguments that take lists (for example: `--required-keys`, `--cat
 
 ```
 -   repo: https://github.com/homebysix/pre-commit-macadmin
-    rev: v1.1.4
+    rev: v1.2.0
     hooks:
     -   id: check-munki-pkgsinfo
         args: ['--catalogs', 'testing', 'stable', '--']
@@ -115,7 +115,7 @@ But if you also use the `--categories` argument, you would move the trailing `--
 
 ```
 -   repo: https://github.com/homebysix/pre-commit-macadmin
-    rev: v1.1.4
+    rev: v1.2.0
     hooks:
     -   id: check-munki-pkgsinfo
         args: ['--catalogs', 'testing', 'stable', '--categories', 'Design', 'Engineering', 'Web Browsers', '--']
@@ -127,7 +127,7 @@ If it looks better to your eye, feel free to use a multi-line list for long argu
 
 ```
 -   repo: https://github.com/homebysix/pre-commit-macadmin
-    rev: v1.1.4
+    rev: v1.2.0
     hooks:
     -   id: check-munki-pkgsinfo
         args: [

diff --git a/pre_commit_hooks/check_autopkg_recipes.py b/pre_commit_hooks/check_autopkg_recipes.py
@@ -7,58 +7,10 @@
 
 import argparse
 import plistlib
-from xml.parsers.expat import ExpatError
 from distutils.version import LooseVersion
+from xml.parsers.expat import ExpatError
 
-from pre_commit_hooks.util import validate_pkginfo_key_types
-
-# Processors for which a minimum version of AutoPkg is required.
-PROC_MIN_VERSIONS = {
-    "AppPkgCreator": "1.0.0",
-    "BrewCaskInfoProvider": "0.2.5",
-    "CodeSignatureVerifier": "0.3.1",
-    "CURLDownloader": "0.5.1",
-    "CURLTextSearcher": "0.5.1",
-    "DeprecationWarning": "1.1.0",
-    "EndOfCheckPhase": "0.1.0",
-    "FileFinder": "0.2.3",
-    "FileMover": "0.2.9",
-    "FlatPkgPacker": "0.2.4",
-    "FlatPkgUnpacker": "0.1.0",
-    "GitHubReleasesInfoProvider": "0.5.0",
-    "Installer": "0.4.0",
-    "InstallFromDMG": "0.4.0",
-    "MunkiCatalogBuilder": "0.1.0",
-    "MunkiImporter": "0.1.0",
-    "MunkiInstallsItemsCreator": "0.1.0",
-    "MunkiPkginfoMerger": "0.1.0",
-    "MunkiSetDefaultCatalog": "0.4.2",
-    "PackageRequired": "0.5.1",
-    "PathDeleter": "0.1.0",
-    "PkgCopier": "0.1.0",
-    "PkgExtractor": "0.1.0",
-    "PkgPayloadUnpacker": "0.1.0",
-    "PlistEditor": "0.1.0",
-    "PlistReader": "0.2.5",
-    "SparkleUpdateInfoProvider": "0.1.0",
-    "StopProcessingIf": "0.1.0",
-    "Symlinker": "0.1.0",
-    "Unarchiver": "0.1.0",
-    "URLTextSearcher": "0.2.9",
-    "Versioner": "0.1.0",
-}
-
-# Processors for which %NAME%.app should not be present in the arguments.
-NO_NAME_VAR_IN_PROC_ARGS = (
-    "CodeSignatureVerifier",
-    "Versioner",
-    "PkgPayloadUnpacker",
-    "FlatPkgUnpacker",
-    "FileFinder",
-    "Copier",
-    "AppDmgVersioner",
-    "InstallFromDMG",
-)
+from pre_commit_hooks.util import validate_pkginfo_key_types, validate_required_keys
 
 
 def build_argument_parser():
@@ -88,12 +40,145 @@ def build_argument_parser():
     return parser
 
 
+def validate_processor_keys(process, filename):
+    """Ensure all items in Process array have a "Processor" specified."""
+
+    passed = True
+    missing_processor_keys = [x for x in process if "Processor" not in x]
+    if missing_processor_keys:
+        for missing_proc in missing_processor_keys:
+            print(
+                '{}: Item in processor array is missing "Processor" key: {}'.format(
+                    filename, missing_proc
+                )
+            )
+        passed = False
+
+    return passed
+
+
+def validate_endofcheckphase(process, filename):
+    """Ensure EndOfCheckPhase comes after a downloader."""
+
+    passed = True
+    downloader_idx = next(
+        (
+            idx
+            for (idx, d) in enumerate(process)
+            if d.get("Processor") in ("URLDownloader", "CURLDownloader")
+        ),
+        None,
+    )
+    endofcheck_idx = next(
+        (
+            idx
+            for (idx, d) in enumerate(process)
+            if d.get("Processor") == "EndOfCheckPhase"
+        ),
+        None,
+    )
+    if (
+        downloader_idx is not None
+        and endofcheck_idx is not None
+        and endofcheck_idx < downloader_idx
+    ):
+        print(
+            "{}: EndOfCheckPhase typically goes after a download processor, "
+            "not before.".format(filename)
+        )
+        passed = False
+
+    return passed
+
+
+def validate_minimumversion(process, min_vers, ignore_min_vers_before, filename):
+    """Ensure MinimumVersion is set appropriately for the processors used."""
+
+    # Processors for which a minimum version of AutoPkg is required.
+    proc_min_versions = {
+        "AppPkgCreator": "1.0.0",
+        "BrewCaskInfoProvider": "0.2.5",
+        "CodeSignatureVerifier": "0.3.1",
+        "CURLDownloader": "0.5.1",
+        "CURLTextSearcher": "0.5.1",
+        "DeprecationWarning": "1.1.0",
+        "EndOfCheckPhase": "0.1.0",
+        "FileFinder": "0.2.3",
+        "FileMover": "0.2.9",
+        "FlatPkgPacker": "0.2.4",
+        "FlatPkgUnpacker": "0.1.0",
+        "GitHubReleasesInfoProvider": "0.5.0",
+        "Installer": "0.4.0",
+        "InstallFromDMG": "0.4.0",
+        "MunkiCatalogBuilder": "0.1.0",
+        "MunkiImporter": "0.1.0",
+        "MunkiInstallsItemsCreator": "0.1.0",
+        "MunkiPkginfoMerger": "0.1.0",
+        "MunkiSetDefaultCatalog": "0.4.2",
+        "PackageRequired": "0.5.1",
+        "PathDeleter": "0.1.0",
+        "PkgCopier": "0.1.0",
+        "PkgExtractor": "0.1.0",
+        "PkgPayloadUnpacker": "0.1.0",
+        "PlistEditor": "0.1.0",
+        "PlistReader": "0.2.5",
+        "SparkleUpdateInfoProvider": "0.1.0",
+        "StopProcessingIf": "0.1.0",
+        "Symlinker": "0.1.0",
+        "Unarchiver": "0.1.0",
+        "URLTextSearcher": "0.2.9",
+        "Versioner": "0.1.0",
+    }
+
+    passed = True
+    for proc in [
+        x
+        for x in proc_min_versions
+        if LooseVersion(proc_min_versions[x]) >= LooseVersion(ignore_min_vers_before)
+    ]:
+        if proc in [x["Processor"] for x in process]:
+            if LooseVersion(min_vers) < LooseVersion(proc_min_versions[proc]):
+                print(
+                    "{}: {} processor requires minimum AutoPkg "
+                    "version {}".format(filename, proc, proc_min_versions[proc])
+                )
+                passed = False
+
+    return passed
+
+
+def validate_no_var_in_app_path(process, filename):
+    """Ensure %NAME% is not used in app paths that should be hard coded."""
+
+    # Processors for which %NAME%.app should not be present in the arguments.
+    no_name_var_in_proc_args = (
+        "CodeSignatureVerifier",
+        "Versioner",
+        "PkgPayloadUnpacker",
+        "FlatPkgUnpacker",
+        "FileFinder",
+        "Copier",
+        "AppDmgVersioner",
+        "InstallFromDMG",
+    )
+
+    passed = True
+    for process in process:
+        if process["Processor"] in no_name_var_in_proc_args:
+            for _, argvalue in process["Arguments"].items():
+                if isinstance(argvalue, str) and "%NAME%.app" in argvalue:
+                    print(
+                        "{}: Use actual app name instead of %NAME%.app in {} "
+                        "processor argument.".format(filename, process["Processor"])
+                    )
+                    passed = False
+
+    return passed
+
+
 def main(argv=None):
     """Main process."""
 
-    # Top level keys that all AutoPkg recipes should contain.
-    required_keys = ("Identifier", "Input")
-
     # Parse command line arguments.
     argparser = build_argument_parser()
     args = argparser.parse_args(argv)
@@ -102,20 +187,21 @@ def main(argv=None):
     for filename in args.filenames:
         try:
             recipe = plistlib.readPlist(filename)
-            for req_key in required_keys:
-                if req_key not in recipe:
-                    print("{}: missing required key {}".format(filename, req_key))
-                    retval = 1
-                    break  # No need to continue checking this file
 
         except (ExpatError, ValueError) as err:
             print("{}: plist parsing error: {}".format(filename, err))
             retval = 1
             break  # No need to continue checking this file
 
+        # Top level keys that all AutoPkg recipes should contain.
+        required_keys = ("Identifier",)
+        if not validate_required_keys(recipe, filename, required_keys):
+            retval = 1
+            break  # No need to continue checking this file
+
         if args.override_prefix and "Process" not in recipe:
             override_prefix = args.override_prefix
-            if not recipe.get("Identifier", "").startswith(override_prefix):
+            if not recipe["Identifier"].startswith(override_prefix):
                 print(
                     '{}: override identifier does not start with "{}"'.format(
                         filename, override_prefix
@@ -124,7 +210,7 @@ def main(argv=None):
                 retval = 1
         if args.recipe_prefix and "Process" in recipe:
             recipe_prefix = args.recipe_prefix
-            if not recipe.get("Identifier", "").startswith(recipe_prefix):
+            if not recipe["Identifier"].startswith(recipe_prefix):
                 print(
                     '{}: recipe identifier does not start with "{}"'.format(
                         filename, recipe_prefix
@@ -137,49 +223,36 @@ def main(argv=None):
             if not validate_pkginfo_key_types(input_key["pkginfo"], filename):
                 retval = 1
 
+            # TODO: Additional pkginfo checks here.
+
         # Warn about comments that would be lost during `plutil -convert xml1`
         with open(filename, "r") as openfile:
             recipe_text = openfile.read()
             if "<!--" in recipe_text and "-->" in recipe_text:
                 print(
-                    "{}: WARNING: Recommend converting from <!-- --> style comments to a "
-                    "Comment key where needed.".format(filename)
+                    "{}: WARNING: Recommend converting from <!-- --> style comments "
+                    "to a Comment key where needed.".format(filename)
                 )
 
         # Processor checks.
         if "Process" in recipe:
-            # Ensure MinimumVersion is set appropriately for the processors used.
+            process = recipe["Process"]
+
+            if not validate_processor_keys(process, filename):
+                retval = 1
+
+            if not validate_endofcheckphase(process, filename):
+                retval = 1
+
+            if not validate_no_var_in_app_path(process, filename):
+                retval = 1
+
             if "MinimumVersion" in recipe:
-                for proc in [
-                    x
-                    for x in PROC_MIN_VERSIONS
-                    if LooseVersion(PROC_MIN_VERSIONS[x])
-                    >= LooseVersion(args.ignore_min_vers_before)
-                ]:
-                    if proc in [x["Processor"] for x in recipe["Process"]]:
-                        if LooseVersion(recipe["MinimumVersion"]) < LooseVersion(
-                            PROC_MIN_VERSIONS[proc]
-                        ):
-                            print(
-                                "{}: {} processor requires minimum AutoPkg "
-                                "version {}".format(
-                                    filename, proc, PROC_MIN_VERSIONS[proc]
-                                )
-                            )
-                            retval = 1
-
-            # Ensure %NAME% is not used in app paths that should be hard coded.
-            for process in recipe["Process"]:
-                if process["Processor"] in NO_NAME_VAR_IN_PROC_ARGS:
-                    for _, argvalue in process["Arguments"].items():
-                        if isinstance(argvalue, str) and "%NAME%.app" in argvalue:
-                            print(
-                                "{}: Use actual app name instead of %NAME%.app in {} "
-                                "processor argument.".format(
-                                    filename, process["Processor"]
-                                )
-                            )
-                            retval = 1
+                min_vers = recipe["MinimumVersion"]
+                if not validate_minimumversion(
+                    process, min_vers, args.ignore_min_vers_before, filename
+                ):
+                    retval = 1
 
     return retval
 

diff --git a/pre_commit_hooks/check_munki_pkgsinfo.py b/pre_commit_hooks/check_munki_pkgsinfo.py
@@ -6,7 +6,7 @@
 import plistlib
 from xml.parsers.expat import ExpatError
 
-from pre_commit_hooks.util import validate_pkginfo_key_types
+from pre_commit_hooks.util import validate_pkginfo_key_types, validate_required_keys
 
 
 def build_argument_parser():
@@ -51,11 +51,9 @@ def main(argv=None):
 
         # Check for presence of required pkginfo keys.
         if args.required_keys:
-            for req_key in args.required_keys:
-                if not pkginfo.get(req_key):
-                    print('{}: missing required key "{}"'.format(filename, req_key))
-                    retval = 1
-                    break  # No need to continue checking this file
+            if not validate_required_keys(pkginfo, filename, args.required_keys):
+                retval = 1
+                break  # No need to continue checking this file
 
         # Ensure pkginfo keys have expected types.
         if not validate_pkginfo_key_types(pkginfo, filename):