Add initial GitHub integration

.bandit_baseline.json

@@ -0,0 +1,46 @@
+{
+  "errors": [],
+  "generated_at": "2020-12-03T19:42:28Z",
+  "metrics": {
+    "_totals": {
+      "CONFIDENCE.HIGH": 1.0,
+      "CONFIDENCE.LOW": 0.0,
+      "CONFIDENCE.MEDIUM": 0.0,
+      "CONFIDENCE.UNDEFINED": 0.0,
+      "SEVERITY.HIGH": 0.0,
+      "SEVERITY.LOW": 0.0,
+      "SEVERITY.MEDIUM": 1.0,
+      "SEVERITY.UNDEFINED": 0.0,
+      "loc": 303,
+      "nosec": 0
+    },
+    "aws/client.py": {
+      "CONFIDENCE.HIGH": 1.0,
+      "CONFIDENCE.LOW": 0.0,
+      "CONFIDENCE.MEDIUM": 0.0,
+      "CONFIDENCE.UNDEFINED": 0.0,
+      "SEVERITY.HIGH": 0.0,
+      "SEVERITY.LOW": 0.0,
+      "SEVERITY.MEDIUM": 1.0,
+      "SEVERITY.UNDEFINED": 0.0,
+      "loc": 303,
+      "nosec": 0
+    }
+  },
+  "results": [
+    {
+      "code": "149 \n150     filename = md5(str.encode(arguments)).hexdigest() + \".json\"\n151 \n",
+      "filename": "aws/client.py",
+      "issue_confidence": "HIGH",
+      "issue_severity": "MEDIUM",
+      "issue_text": "Use of insecure MD2, MD4, MD5, or SHA1 hash function.",
+      "line_number": 150,
+      "line_range": [
+        150
+      ],
+      "more_info": "https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html#b303-md5",
+      "test_id": "B303",
+      "test_name": "blacklist"
+    }
+  ]
+}

.gitignore

@@ -1,3 +1,12 @@
+# Frost specific files
+.all_python_files.tmp
+
+# IDE files
+.vscode/
+
+# lots of caches fit this pattern (pytest, mypy, ...)
+*cache/
+
 # Byte-compiled / optimized / DLL files
 __pycache__/
 *.py[cod]

.pre-commit-config.yaml

@@ -3,3 +3,61 @@ repos:
     rev: 19.10b0
     hooks:
     - id: black
+# See https://pre-commit.com for more information
+# See https://pre-commit.com/hooks.html for more hooks
+-   repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v2.4.0
+    hooks:
+    -   id: trailing-whitespace
+    -   id: end-of-file-fixer
+    -   id: check-ast
+    -   id: check-yaml
+    -   id: check-added-large-files
+    -   id: check-case-conflict
+    -   id: check-docstring-first
+    -   id: check-merge-conflict
+    -   id: check-toml
+
+## ## -   repo: https://github.com/asottile/pyupgrade
+## ##     rev: v2.7.2
+## ##     hooks:
+## ##     -   id: pyupgrade
+## ##         args: ["--py38-plus"]
+
+## ## - repo: https://github.com/jumanjihouse/pre-commit-hooks
+## ##   rev: master  # or specific git tag
+## ##   hooks:
+## ##     - id: forbid-binary
+## ##       ## TODO - disable for docs/_build/html
+## ##     - id: markdownlint # Configure in .mdlrc
+## ##       # TODO - disable MD012 (consecutive blank lines)
+## ##       #        disable MD013 (line length)
+## ##       #        fix MD031 (fenced code needs blank lines) README.md
+## ##     - id: shellcheck
+
+## ## - repo: https://github.com/IamTheFij/docker-pre-commit
+## ##   rev: v2.0.0
+## ##   hooks:
+## ##         - id: docker-compose-check
+## ##         - id: hadolint
+## ##           # TODO - disable DL3005 (apt upgrade)
+## ##           #        disable DL3008 (unpinned packages)
+## ##           #        disable DL3009 (left apt-get lists around)
+
+- repo: https://github.com/PyCQA/bandit
+  rev: 1.6.2
+  hooks:
+    - id: bandit
+      args:
+      - "--skip"
+      - "B101"          # use of assert
+      - "--baseline"
+      - ".bandit_baseline.json" # use of md5 for local filename
+      - "--quiet"
+
+- repo: https://github.com/Yelp/detect-secrets
+  rev: v0.14.2
+  hooks:
+    - id: detect-secrets
+      args: ['--baseline', '.secrets.baseline']
+      exclude: ".*/tests/.*|docs/_build/html/.buildinfo"

.secrets.baseline

@@ -0,0 +1,67 @@
+{
+  "custom_plugin_paths": [],
+  "exclude": {
+    "files": null,
+    "lines": null
+  },
+  "generated_at": "2020-12-03T20:08:48Z",
+  "plugins_used": [
+    {
+      "name": "AWSKeyDetector"
+    },
+    {
+      "name": "ArtifactoryDetector"
+    },
+    {
+      "base64_limit": 4.5,
+      "name": "Base64HighEntropyString"
+    },
+    {
+      "name": "BasicAuthDetector"
+    },
+    {
+      "name": "CloudantDetector"
+    },
+    {
+      "hex_limit": 3,
+      "name": "HexHighEntropyString"
+    },
+    {
+      "name": "IbmCloudIamDetector"
+    },
+    {
+      "name": "IbmCosHmacDetector"
+    },
+    {
+      "name": "JwtTokenDetector"
+    },
+    {
+      "keyword_exclude": null,
+      "name": "KeywordDetector"
+    },
+    {
+      "name": "MailchimpDetector"
+    },
+    {
+      "name": "PrivateKeyDetector"
+    },
+    {
+      "name": "SlackDetector"
+    },
+    {
+      "name": "SoftlayerDetector"
+    },
+    {
+      "name": "StripeDetector"
+    },
+    {
+      "name": "TwilioKeyDetector"
+    }
+  ],
+  "results": {},
+  "version": "0.14.2",
+  "word_list": {
+    "file": null,
+    "hash": null
+  }
+}

Makefile

@@ -1,3 +1,4 @@
+SHELL := bash -o pipefail
 
 TODAY := $(shell date '+%Y-%m-%d')
 
@@ -45,7 +46,7 @@ doc-build: check_venv
 	@# we regen the api docs every time -- they are not checked in.
 	rm -rf docs/source
 	sphinx-apidoc --no-toc -o docs/source .
-	@# TODO: Add new service modules below also in docs/Source.rst
+	@# Add new service modules below also in docs/Source.rst
 	for module in frost aws gcp gsuite; do \
 		sphinx-apidoc -f -o docs/source/$$module $$module ; \
 	done
@@ -55,17 +56,23 @@ doc-preview: check_venv
 	@#sphinx-autobuild "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)
 	sphinx-autobuild $(AUTOBUILD_OPTS) "docs/" "docs/_build/html/" $(SPHINXOPTS) $(O)
 
-doctest: check_venv
-	frost test -vv --doctest-modules --doctest-glob='*.py' -s --offline --debug-calls $(shell find . -type f -name '*.py' | grep -v venv | grep -v .pyenv | grep -v setup.py) \
-		--doctest-modules -s --offline --debug-calls
+# We need the list of all python file in several places, so only compute
+# it once.
+.all_python_files.tmp:
+	find . -type d \( -name venv -o -name .??\* \) -prune \
+		-o -not -name setup.py -name \*.py -print \
+		> $@
 
-coverage: check_venv
+doctest: check_venv .all_python_files.tmp
+	frost test -vv --doctest-modules -s --offline --debug-calls $$(cat .all_python_files.tmp)
+
+coverage: check_venv .all_python_files.tmp
 	frost test --cov-config .coveragerc --cov=. \
 		--aws-profiles example-account \
 		-o python_files=meta_test*.py \
 		-o cache_dir=./example_cache/ \
 		--offline \
-		$(shell find . -type f -name '*.py' | grep -v venv | grep -v .pyenv | grep -v setup.py)
+		$$(cat .all_python_files.tmp)
 	coverage report -m
 	coverage html
 
@@ -96,6 +103,7 @@ build-image:
 	docker build -t localhost/frost:latest .
 
 .PHONY: \
+	.all_python_files.tmp \
 	all \
 	awsci \
 	black \

aws/ec2/helpers.py

@@ -366,15 +366,15 @@ def ec2_instance_missing_tag_names(ec2_instance, required_tag_names):
     frozenset({'Name'})
     """
     tags = ec2_instance.get("Tags", [])
-    instance_tag_names = set(tag["Key"] for tag in tags if "Key" in tag)
+    instance_tag_names = {tag["Key"] for tag in tags if "Key" in tag}
     return required_tag_names - instance_tag_names
 
 
 def ebs_volume_attached_to_instance(ebs, volume_created_days_ago=90):
     """
     Check an ebs volume is attached to an instance. The "volume_created_days_ago"
     parameter allows checking for volumes that were created that many days ago.
-    
+
     >>> from datetime import datetime
     >>> from datetime import timezone
 

aws/rds/test_rds_db_instance_not_publicly_accessible_by_vpc_sg.py

@@ -28,9 +28,9 @@ def test_rds_db_instance_not_publicly_accessible_by_vpc_security_group(
     if not ec2_security_groups:
         assert not rds_db_instance["VpcSecurityGroups"]
     else:
-        assert set(sg["GroupId"] for sg in ec2_security_groups) == set(
+        assert {sg["GroupId"] for sg in ec2_security_groups} == {
             sg["VpcSecurityGroupId"] for sg in rds_db_instance["VpcSecurityGroups"]
-        )
+        }
 
         assert not any(
             does_vpc_security_group_grant_public_access(sg)

cache.py

@@ -1,6 +1,4 @@
-"""
-Patch for pytest cache to serialize datetime.datetime
-"""
+"""Patch for pytest cache to serialize datetime.datetime."""
 
 import datetime
 import functools
@@ -53,12 +51,12 @@ def datetime_encode_set(
     path = self._getvaluepath(key)
     try:
         path.parent.mkdir(exist_ok=True, parents=True)
-    except (IOError, OSError):
+    except OSError:
         self.warn("could not create cache path {path}", path=path)
         return
     try:
         f = path.open("w")
-    except (IOError, OSError):
+    except OSError:
         self.warn("cache could not write path {path}", path=path)
     else:
         with f:
@@ -82,7 +80,7 @@ def datetime_encode_get(
     try:
         with path.open("r") as f:
             return json.load(f, object_hook=json_iso_datetime_string_to_datetime)
-    except (ValueError, IOError, OSError):
+    except (ValueError, OSError):
         return default
 
 

conftest.py

@@ -1,5 +1,6 @@
 import argparse
 import datetime
+from typing import Any
 
 import pytest
 
@@ -9,14 +10,24 @@
 from aws.client import BotocoreClient
 from gcp.client import GCPClient
 from gsuite.client import GsuiteClient
+from github.client import GitHubClient
 
 import custom_config
 
 botocore_client = None
 gcp_client = None
 gsuite_client = None
+github_client = None
 custom_config_global = None
 
+# globals in conftest.py are hard to import from several levels down, so provide access function
+def get_client(client_name: str) -> Any:
+    # restrict to variables with defined suffix
+    suffix = "_client"
+    if client_name.endswith(suffix):
+        client_name = client_name[: -len(suffix)]
+    return globals()[f"{client_name}_client"]
+
 
 def pytest_addoption(parser):
     frost_parser = parser.getgroup("Frost", "Frost's custom arguments")
@@ -77,6 +88,7 @@ def pytest_configure(config):
     global botocore_client
     global gcp_client
     global gsuite_client
+    global github_client
     global custom_config_global
 
     # run with -p 'no:cacheprovider'
@@ -119,6 +131,11 @@ def pytest_configure(config):
         offline=config.getoption("--offline"),
     )
 
+    github_client = GitHubClient(
+        debug_calls=config.getoption("--debug-calls"),
+        offline=config.getoption("--offline"),
+    )
+
     custom_config_global = custom_config.CustomConfig(config.getoption("--config"))
     config.custom_config = custom_config_global
 
@@ -215,7 +232,7 @@ def update(self, values):
                 error_values.add(value)
         if error_values:
             # we want the non-duplicate values added
-            super().update(values - error_values)
+            super().update(set(values))
             raise DuplicateKeyError(
                 "Value(s) {!r} already present".format(error_values)
             )
@@ -276,17 +293,28 @@ def serialize_datetimes(obj):
 
 
 def extract_metadata(resource):
-    return {
-        metadata_key: resource[metadata_key]
+    # In some cases, `resource` will "have a dict" rather than "be a
+    # dict". (e.g. DataClasses instances). We do require that
+    # those non-dict classes have a "real" dict as the value of their
+    # __dict__ instance variable. (Runtime exception if not.)
+    if isinstance(resource, (dict,)):
+        # resource is (or inherits from) dict
+        target = resource
+    else:
+        # resource has an embedded dict that should be used
+        target = resource.__dict__
+    x = {
+        metadata_key: target[metadata_key]
         for metadata_key in METADATA_KEYS
-        if metadata_key in resource
+        if metadata_key in target
     }
+    return x
 
 
 def get_metadata_from_funcargs(funcargs):
     metadata = {}
     for k in funcargs:
-        if isinstance(funcargs[k], dict):
+        if isinstance(funcargs[k], dict) or hasattr(funcargs[k], "__dict__"):
             metadata = {**metadata, **extract_metadata(funcargs[k])}
     return metadata