From 4baa1ed6a65f58afd667066885403e6189b56d2a Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Mon, 7 Jun 2021 07:46:48 -0400 Subject: [PATCH 01/30] Standardize the layout of the Lineage configuration file --- .github/lineage.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/lineage.yml b/.github/lineage.yml index 8dfc20b..49f9c4f 100644 --- a/.github/lineage.yml +++ b/.github/lineage.yml @@ -1,6 +1,5 @@ --- -version: "1" - lineage: skeleton: remote-url: https://github.com/cisagov/skeleton-generic.git +version: '1' From 249bbbb49292ef5f4ed85f0792b416e817604b10 Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Mon, 7 Jun 2021 07:53:22 -0400 Subject: [PATCH 02/30] Add to the cache keys for the GitHub Actions workflow This adds a hash of `setup.py` to the cache keys used in the GitHub Actions workflow for Python projects. --- .github/workflows/build.yml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 695e8cc..7ca5aeb 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -31,7 +31,8 @@ jobs: key: "${{ env.BASE_CACHE_KEY }}\ ${{ hashFiles('**/requirements-test.txt') }}-\ ${{ hashFiles('**/requirements.txt') }}-\ - ${{ hashFiles('**/.pre-commit-config.yaml') }}" + ${{ hashFiles('**/.pre-commit-config.yaml') }}-\ + ${{ hashFiles('setup.py') }}" restore-keys: | ${{ env.BASE_CACHE_KEY }} - name: Install dependencies @@ -61,7 +62,8 @@ jobs: path: ${{ env.PIP_CACHE_DIR }} key: "${{ env.BASE_CACHE_KEY }}\ ${{ hashFiles('**/requirements-test.txt') }}-\ - ${{ hashFiles('**/requirements.txt') }}" + ${{ hashFiles('**/requirements.txt') }}-\ + ${{ hashFiles('setup.py') }}" restore-keys: | ${{ env.BASE_CACHE_KEY }} - name: Install dependencies @@ -97,7 +99,8 @@ jobs: path: ${{ env.PIP_CACHE_DIR }} key: "${{ env.BASE_CACHE_KEY }}\ ${{ hashFiles('**/requirements-test.txt') }}-\ - ${{ hashFiles('**/requirements.txt') }}" + ${{ hashFiles('**/requirements.txt') }}-\ + ${{ hashFiles('setup.py') }}" restore-keys: | ${{ env.BASE_CACHE_KEY }} - name: Install dependencies @@ -127,7 +130,8 @@ jobs: with: path: ${{ env.PIP_CACHE_DIR }} key: "${{ env.BASE_CACHE_KEY }}\ - ${{ hashFiles('**/requirements.txt') }}" + ${{ hashFiles('**/requirements.txt') }}-\ + ${{ hashFiles('setup.py') }}" restore-keys: | ${{ env.BASE_CACHE_KEY }} - name: Install dependencies From 449eef422bf7cb8469a25996126fac26ae899fb4 Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Mon, 7 Jun 2021 16:53:03 -0400 Subject: [PATCH 03/30] Add comments explaining different hashFiles() argument The other cache keys for our GHA jobs are in the format '**/' so that any file with that name is used in the repository. However, for Python packages they may have a 'setup.py' as part of their internal codebase that does not impact environment requirements. As a result we only want to use the 'setup.py' that is in the root of the repository and is used to install the package. Co-authored-by: dav3r --- .github/workflows/build.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7ca5aeb..c054b90 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -28,6 +28,9 @@ jobs: path: | ${{ env.PIP_CACHE_DIR }} ${{ env.PRE_COMMIT_CACHE_DIR }} + # We do not use '**/setup.py' in the cache key so only the 'setup.py' + # file in the root of the repository is used. This is in case a Python + # package were to have a 'setup.py' as part of its internal codebase. key: "${{ env.BASE_CACHE_KEY }}\ ${{ hashFiles('**/requirements-test.txt') }}-\ ${{ hashFiles('**/requirements.txt') }}-\ @@ -60,6 +63,9 @@ jobs: py${{ steps.setup-python.outputs.python-version }}-" with: path: ${{ env.PIP_CACHE_DIR }} + # We do not use '**/setup.py' in the cache key so only the 'setup.py' + # file in the root of the repository is used. This is in case a Python + # package were to have a 'setup.py' as part of its internal codebase. key: "${{ env.BASE_CACHE_KEY }}\ ${{ hashFiles('**/requirements-test.txt') }}-\ ${{ hashFiles('**/requirements.txt') }}-\ @@ -97,6 +103,9 @@ jobs: py${{ steps.setup-python.outputs.python-version }}-" with: path: ${{ env.PIP_CACHE_DIR }} + # We do not use '**/setup.py' in the cache key so only the 'setup.py' + # file in the root of the repository is used. This is in case a Python + # package were to have a 'setup.py' as part of its internal codebase. key: "${{ env.BASE_CACHE_KEY }}\ ${{ hashFiles('**/requirements-test.txt') }}-\ ${{ hashFiles('**/requirements.txt') }}-\ @@ -129,6 +138,9 @@ jobs: py${{ steps.setup-python.outputs.python-version }}-" with: path: ${{ env.PIP_CACHE_DIR }} + # We do not use '**/setup.py' in the cache key so only the 'setup.py' + # file in the root of the repository is used. This is in case a Python + # package were to have a 'setup.py' as part of its internal codebase. key: "${{ env.BASE_CACHE_KEY }}\ ${{ hashFiles('**/requirements.txt') }}-\ ${{ hashFiles('setup.py') }}" From 1e8f8223910f41294c16bfebea332c80fd83573c Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Fri, 18 Jun 2021 03:33:26 -0400 Subject: [PATCH 04/30] Add style enforcement rules Add rules to enforce ATX-closed headers, dashes for unordered list elements, and `1.` for ordered list elements. --- .mdl_config.json | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.mdl_config.json b/.mdl_config.json index 7a6f3f8..8266cdb 100644 --- a/.mdl_config.json +++ b/.mdl_config.json @@ -1,4 +1,10 @@ { + "MD003": { + "style": "atx_closed" + }, + "MD004": { + "style": "dash" + }, "MD013": { "code_blocks": false, "tables": false @@ -6,5 +12,8 @@ "MD024": { "allow_different_nesting": true }, + "MD029": { + "style": "one" + }, "default": true } From afc6bd6f6d754f8c2bc5675411535b0c61e0ec78 Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Fri, 18 Jun 2021 03:37:08 -0400 Subject: [PATCH 05/30] Add rule for image headers Add

and tags to the allowed list for MD033 (HTML elements) to support using an image as the first thing in a markdown file (header image). --- .mdl_config.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.mdl_config.json b/.mdl_config.json index 8266cdb..38bc045 100644 --- a/.mdl_config.json +++ b/.mdl_config.json @@ -15,5 +15,11 @@ "MD029": { "style": "one" }, + "MD033": { + "allowed_elements": [ + "h1", + "img" + ] + }, "default": true } From ce173f401d8eec4f2caf3cf8174a5b53f03222da Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Fri, 18 Jun 2021 03:51:38 -0400 Subject: [PATCH 06/30] Switch to a YAML markdownlint configuration file This converts the existing `.mdl_config.json` file to an equivalent `.mdl_config.yaml` file. The reference in the markdownlint pre-commit hook configuration is updated to match. Co-authored-by: Shane Frasier --- .mdl_config.json | 25 ------------------------- .mdl_config.yaml | 24 ++++++++++++++++++++++++ .pre-commit-config.yaml | 2 +- 3 files changed, 25 insertions(+), 26 deletions(-) delete mode 100644 .mdl_config.json create mode 100644 .mdl_config.yaml diff --git a/.mdl_config.json b/.mdl_config.json deleted file mode 100644 index 38bc045..0000000 --- a/.mdl_config.json +++ /dev/null @@ -1,25 +0,0 @@ -{ - "MD003": { - "style": "atx_closed" - }, - "MD004": { - "style": "dash" - }, - "MD013": { - "code_blocks": false, - "tables": false - }, - "MD024": { - "allow_different_nesting": true - }, - "MD029": { - "style": "one" - }, - "MD033": { - "allowed_elements": [ - "h1", - "img" - ] - }, - "default": true -} diff --git a/.mdl_config.yaml b/.mdl_config.yaml new file mode 100644 index 0000000..a04720a --- /dev/null +++ b/.mdl_config.yaml @@ -0,0 +1,24 @@ +--- + +default: true + +MD003: + style: "atx_closed" + +MD004: + style: "dash" + +MD013: + code_blocks: false + tables: false + +MD024: + allow_different_nesting: true + +MD029: + style: "one" + +MD033: + allowed_elements: + - h1 + - img diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cf0330d..6b87ab0 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -35,7 +35,7 @@ repos: hooks: - id: markdownlint args: - - --config=.mdl_config.json + - --config=.mdl_config.yaml - repo: https://github.com/pre-commit/mirrors-prettier rev: v2.3.0 hooks: From f2a423095efe2f1a96ce2621352ee7cb4dd0458a Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Fri, 18 Jun 2021 03:59:36 -0400 Subject: [PATCH 07/30] Add comments to markdownlint configuration Now that this is a YAML file we can add comments explaining the rule modifications we use. This will make it easier to edit or expand in the future. --- .mdl_config.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.mdl_config.yaml b/.mdl_config.yaml index a04720a..a2f08f3 100644 --- a/.mdl_config.yaml +++ b/.mdl_config.yaml @@ -1,24 +1,40 @@ --- +# Default state for all rules default: true +# MD003/heading-style/header-style - Heading style MD003: + # Enforce the ATX-closed style of header style: "atx_closed" +# MD004/ul-style - Unordered list style MD004: + # Enforce dashes for unordered lists style: "dash" +# MD013/line-length - Line length MD013: + # Do not enforce for code blocks code_blocks: false + # Do not enforce for tables tables: false +# MD024/no-duplicate-heading/no-duplicate-header - Multiple headings with the +# same content MD024: + # Allow headers with the same content as long as they are not in the same + # parent heading allow_different_nesting: true +# MD029/ol-prefix - Ordered list item prefix MD029: + # Enforce the `1.` style for ordered lists style: "one" +# MD033/no-inline-html - Inline HTML MD033: + # The h1 and img elements are allowed to permit header images allowed_elements: - h1 - img From d4781ee177698490b740c5dc0e3ae90282c70618 Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Thu, 24 Jun 2021 01:38:44 -0400 Subject: [PATCH 08/30] Add the validate_manifest hook from pre-commit This hook will validate any pre-commit hook manifest files in the repository. --- .pre-commit-config.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cf0330d..82a5e46 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -47,6 +47,12 @@ repos: args: - --strict + # pre-commit hooks + - repo: https://github.com/pre-commit/pre-commit + rev: v2.13.0 + hooks: + - id: validate_manifest + # Shell script hooks - repo: https://github.com/lovesegfault/beautysh rev: v6.1.0 From 7ae8bea9125fbf458c0fb824c7d64a6f33c6b2c2 Mon Sep 17 00:00:00 2001 From: Jeremy Frasier Date: Fri, 25 Jun 2021 09:09:16 -0400 Subject: [PATCH 09/30] Add assertion to verify that the root logger's logging level is set correctly See the discussion here for more context: https://github.com/cisagov/pe-reports/pull/6#discussion_r657329612 --- tests/test_example.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_example.py b/tests/test_example.py index 3a22848..fe08eb7 100644 --- a/tests/test_example.py +++ b/tests/test_example.py @@ -83,6 +83,9 @@ def test_log_levels(level): assert ( logging.root.hasHandlers() is True ), "root logger should now have a handler" + assert ( + logging.getLevelName(logging.root.getEffectiveLevel()) == level.upper() + ), f"root logger level should be set to {level.upper()}" assert return_code == 0, "main() should return success (0)" From 106af21c04ae34d0402b9cfc59f386e2756776bd Mon Sep 17 00:00:00 2001 From: Jeremy Frasier Date: Fri, 9 Jul 2021 13:34:16 -0400 Subject: [PATCH 10/30] Install terraform and packer for the linting job We should be doing this because the Packer and Terraform pre-commit hooks leverage the corresponding executables; therefore, it makes sense to go ahead and install the particular versions of those executables that we support. Also add support for optionally debugging via tmate. See also cisagov/skeleton-generic#74. --- .github/workflows/build.yml | 63 ++++++++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5c65f71..8fa1b2f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -8,13 +8,16 @@ on: types: [apb] env: + CURL_CACHE_DIR: ~/.cache/curl PIP_CACHE_DIR: ~/.cache/pip PRE_COMMIT_CACHE_DIR: ~/.cache/pre-commit + RUN_TMATE: ${{ secrets.RUN_TMATE }} jobs: lint: runs-on: ubuntu-latest steps: + - uses: cisagov/setup-env-github-action@develop - uses: actions/checkout@v2 - id: setup-python uses: actions/setup-python@v2 @@ -23,17 +26,72 @@ jobs: - uses: actions/cache@v2 env: BASE_CACHE_KEY: "${{ github.job }}-${{ runner.os }}-\ - py${{ steps.setup-python.outputs.python-version }}-" + py${{ steps.setup-python.outputs.python-version }}-\ + go${{ env.GO_VERSION }}-\ + packer${{ env.PACKER_VERSION }}-\ + tf${{ env.TERRAFORM_VERSION }}-" with: + # Note that the .terraform directory IS NOT included in the + # cache because if we were caching, then we would need to use + # the `-upgrade=true` option. This option blindly pulls down the + # latest modules and providers instead of checking to see if an + # update is required. That behavior defeats the benefits of caching. + # so there is no point in doing it for the .terraform directory. path: | ${{ env.PIP_CACHE_DIR }} ${{ env.PRE_COMMIT_CACHE_DIR }} + ${{ env.CURL_CACHE_DIR }} + ${{ steps.go-cache.outputs.dir }} key: "${{ env.BASE_CACHE_KEY }}\ ${{ hashFiles('**/requirements-test.txt') }}-\ ${{ hashFiles('**/requirements.txt') }}-\ ${{ hashFiles('**/.pre-commit-config.yaml') }}" restore-keys: | ${{ env.BASE_CACHE_KEY }} + - uses: actions/setup-go@v2 + with: + go-version: '1.16' + - name: Store installed Go version + run: | + echo "GO_VERSION="\ + "$(go version | sed 's/^go version go\([0-9.]\+\) .*/\1/')" \ + >> $GITHUB_ENV + - name: Lookup go cache directory + id: go-cache + run: | + echo "::set-output name=dir::$(go env GOCACHE)" + - name: Install Packer + run: | + mkdir -p ${{ env.CURL_CACHE_DIR }} + PACKER_ZIP="packer_${PACKER_VERSION}_linux_amd64.zip" + curl --output ${{ env.CURL_CACHE_DIR }}/"${PACKER_ZIP}" \ + --time-cond ${{ env.CURL_CACHE_DIR }}/"${PACKER_ZIP}" \ + --location \ + "https://releases.hashicorp.com/packer/${PACKER_VERSION}/${PACKER_ZIP}" + sudo unzip -o -d /usr/local/bin \ + ${{ env.CURL_CACHE_DIR }}/"${PACKER_ZIP}" + - name: Install Terraform + run: | + mkdir -p ${{ env.CURL_CACHE_DIR }} + TERRAFORM_ZIP="terraform_${TERRAFORM_VERSION}_linux_amd64.zip" + curl --output ${{ env.CURL_CACHE_DIR }}/"${TERRAFORM_ZIP}" \ + --time-cond ${{ env.CURL_CACHE_DIR }}/"${TERRAFORM_ZIP}" \ + --location \ + "https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/${TERRAFORM_ZIP}" + sudo unzip -d /opt/terraform \ + ${{ env.CURL_CACHE_DIR }}/"${TERRAFORM_ZIP}" + sudo ln -s /opt/terraform/terraform /usr/bin/terraform + sudo mv /usr/local/bin/terraform /usr/local/bin/terraform-default + sudo ln -s /opt/terraform/terraform /usr/local/bin/terraform + - name: Install Terraform-docs + run: GO111MODULE=on go get github.com/terraform-docs/terraform-docs + - name: Find and initialize Terraform directories + run: | + for path in $(find . -not \( -type d -name ".terraform" -prune \) \ + -type f -iname "*.tf" -exec dirname "{}" \; | sort -u); do \ + echo "Initializing '$path'..."; \ + terraform init -input=false -backend=false "$path"; \ + done - name: Install dependencies run: | python -m pip install --upgrade pip @@ -42,3 +100,6 @@ jobs: run: pre-commit install-hooks - name: Run pre-commit on all files run: pre-commit run --all-files + - name: Setup tmate debug session + uses: mxschmitt/action-tmate@v3 + if: env.RUN_TMATE From c4810439814c3ebed6dd05f3b690b460670cd878 Mon Sep 17 00:00:00 2001 From: Shane Frasier Date: Fri, 9 Jul 2021 22:43:09 -0400 Subject: [PATCH 11/30] Break out the curl cache creation into its own step Co-authored-by: Nick M. <50747025+mcdonnnj@users.noreply.github.com> --- .github/workflows/build.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8fa1b2f..bc632c3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -60,9 +60,10 @@ jobs: id: go-cache run: | echo "::set-output name=dir::$(go env GOCACHE)" + - name: Setup curl cache + run: mkdir -p ${{ env.CURL_CACHE_DIR }} - name: Install Packer run: | - mkdir -p ${{ env.CURL_CACHE_DIR }} PACKER_ZIP="packer_${PACKER_VERSION}_linux_amd64.zip" curl --output ${{ env.CURL_CACHE_DIR }}/"${PACKER_ZIP}" \ --time-cond ${{ env.CURL_CACHE_DIR }}/"${PACKER_ZIP}" \ @@ -72,7 +73,6 @@ jobs: ${{ env.CURL_CACHE_DIR }}/"${PACKER_ZIP}" - name: Install Terraform run: | - mkdir -p ${{ env.CURL_CACHE_DIR }} TERRAFORM_ZIP="terraform_${TERRAFORM_VERSION}_linux_amd64.zip" curl --output ${{ env.CURL_CACHE_DIR }}/"${TERRAFORM_ZIP}" \ --time-cond ${{ env.CURL_CACHE_DIR }}/"${TERRAFORM_ZIP}" \ From 70414cff28c661c3b76425edf5021f213f505413 Mon Sep 17 00:00:00 2001 From: Jeremy Frasier Date: Fri, 9 Jul 2021 22:46:47 -0400 Subject: [PATCH 12/30] Remove unnecessary line in tasks There is no reason to create /usr/bin/terraform. This is a vestige of an earlier age. Co-authored-by: Nick M. <50747025+mcdonnnj@users.noreply.github.com> --- .github/workflows/build.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bc632c3..3946d90 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -80,7 +80,6 @@ jobs: "https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/${TERRAFORM_ZIP}" sudo unzip -d /opt/terraform \ ${{ env.CURL_CACHE_DIR }}/"${TERRAFORM_ZIP}" - sudo ln -s /opt/terraform/terraform /usr/bin/terraform sudo mv /usr/local/bin/terraform /usr/local/bin/terraform-default sudo ln -s /opt/terraform/terraform /usr/local/bin/terraform - name: Install Terraform-docs From b629f7f623490217fbd43d76fd77b4638cd4a4ec Mon Sep 17 00:00:00 2001 From: Jeremy Frasier Date: Fri, 9 Jul 2021 22:48:21 -0400 Subject: [PATCH 13/30] Modify the Packer installation to model that of Terraform The Terraform installation does not destroy the existing system Terraform installation, and neither should the Packer installation. Co-authored-by: Nick M. <50747025+mcdonnnj@users.noreply.github.com> --- .github/workflows/build.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3946d90..871bee7 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -69,8 +69,10 @@ jobs: --time-cond ${{ env.CURL_CACHE_DIR }}/"${PACKER_ZIP}" \ --location \ "https://releases.hashicorp.com/packer/${PACKER_VERSION}/${PACKER_ZIP}" - sudo unzip -o -d /usr/local/bin \ + sudo unzip -d /opt/packer \ ${{ env.CURL_CACHE_DIR }}/"${PACKER_ZIP}" + sudo mv /usr/local/bin/packer /usr/local/bin/packer-default + sudo ln -s /opt/packer/packer /usr/local/bin/packer - name: Install Terraform run: | TERRAFORM_ZIP="terraform_${TERRAFORM_VERSION}_linux_amd64.zip" From 181d1b2fafa211fb7cae5b6023e1b5271b59bbda Mon Sep 17 00:00:00 2001 From: Jeremy Frasier Date: Sat, 10 Jul 2021 22:36:45 -0400 Subject: [PATCH 14/30] Install a specific version of terraform-docs Note that this change is dependent on the merging of cisagov/setup-env-github-action#31. Co-authored-by: Nick M. <50747025+mcdonnnj@users.noreply.github.com> --- .github/workflows/build.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 871bee7..c8a1426 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -85,7 +85,9 @@ jobs: sudo mv /usr/local/bin/terraform /usr/local/bin/terraform-default sudo ln -s /opt/terraform/terraform /usr/local/bin/terraform - name: Install Terraform-docs - run: GO111MODULE=on go get github.com/terraform-docs/terraform-docs + run: | + GO111MODULE=on go get \ + github.com/terraform-docs/terraform-docs@${TERRAFORM_DOCS_VERSION} - name: Find and initialize Terraform directories run: | for path in $(find . -not \( -type d -name ".terraform" -prune \) \ From bb6e566e3a8e1069ca2c6a1f441f67fc4c176685 Mon Sep 17 00:00:00 2001 From: Jeremy Frasier Date: Sun, 11 Jul 2021 21:59:58 -0400 Subject: [PATCH 15/30] Move go installation so that it takes place before the cache task Some variables defined in the go installation are used in the cache task, so the go installation must happen first. Co-authored-by: Nick M. <50747025+mcdonnnj@users.noreply.github.com> --- .github/workflows/build.yml | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c8a1426..04159c2 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -23,6 +23,20 @@ jobs: uses: actions/setup-python@v2 with: python-version: 3.9 + # GO_VERSION and GOCACHE are used by the cache task, so the go + # installation must happen before that. + - uses: actions/setup-go@v2 + with: + go-version: '1.16' + - name: Store installed Go version + run: | + echo "GO_VERSION="\ + "$(go version | sed 's/^go version go\([0-9.]\+\) .*/\1/')" \ + >> $GITHUB_ENV + - name: Lookup go cache directory + id: go-cache + run: | + echo "::set-output name=dir::$(go env GOCACHE)" - uses: actions/cache@v2 env: BASE_CACHE_KEY: "${{ github.job }}-${{ runner.os }}-\ @@ -48,18 +62,6 @@ jobs: ${{ hashFiles('**/.pre-commit-config.yaml') }}" restore-keys: | ${{ env.BASE_CACHE_KEY }} - - uses: actions/setup-go@v2 - with: - go-version: '1.16' - - name: Store installed Go version - run: | - echo "GO_VERSION="\ - "$(go version | sed 's/^go version go\([0-9.]\+\) .*/\1/')" \ - >> $GITHUB_ENV - - name: Lookup go cache directory - id: go-cache - run: | - echo "::set-output name=dir::$(go env GOCACHE)" - name: Setup curl cache run: mkdir -p ${{ env.CURL_CACHE_DIR }} - name: Install Packer From 337d1efb8f72c11cae6b83f3f5e63e8187599470 Mon Sep 17 00:00:00 2001 From: Shane Frasier Date: Mon, 12 Jul 2021 09:06:41 -0400 Subject: [PATCH 16/30] Capitalize Go for consistency Co-authored-by: Nick M. <50747025+mcdonnnj@users.noreply.github.com> --- .github/workflows/build.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 04159c2..d84b7da 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -23,7 +23,7 @@ jobs: uses: actions/setup-python@v2 with: python-version: 3.9 - # GO_VERSION and GOCACHE are used by the cache task, so the go + # GO_VERSION and GOCACHE are used by the cache task, so the Go # installation must happen before that. - uses: actions/setup-go@v2 with: @@ -33,7 +33,7 @@ jobs: echo "GO_VERSION="\ "$(go version | sed 's/^go version go\([0-9.]\+\) .*/\1/')" \ >> $GITHUB_ENV - - name: Lookup go cache directory + - name: Lookup Go cache directory id: go-cache run: | echo "::set-output name=dir::$(go env GOCACHE)" From 8ee2116f428f1738540f0cdf261f1e4f1c15d092 Mon Sep 17 00:00:00 2001 From: Shane Frasier Date: Tue, 13 Jul 2021 17:02:13 -0400 Subject: [PATCH 17/30] Prefer the newer "go install" syntax As of [Go 1.16](https://tip.golang.org/doc/go1.16#go-command) the `GO111MODULE` environment variable defaults to `on` and `go get` has been deprecated for module installation. Co-authored-by: Nick M. <50747025+mcdonnnj@users.noreply.github.com> --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d84b7da..73f345a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -88,7 +88,7 @@ jobs: sudo ln -s /opt/terraform/terraform /usr/local/bin/terraform - name: Install Terraform-docs run: | - GO111MODULE=on go get \ + go install \ github.com/terraform-docs/terraform-docs@${TERRAFORM_DOCS_VERSION} - name: Find and initialize Terraform directories run: | From e2a729d0b11ab74207a3bb77367d8e9d8c577889 Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Tue, 13 Jul 2021 17:42:36 -0400 Subject: [PATCH 18/30] Install the shfmt tool for GHA The `shfmt` tool does not ship on the GitHub Actions runners so we must install it manually. --- .github/workflows/build.yml | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5c65f71..9dd5f7a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -15,25 +15,44 @@ jobs: lint: runs-on: ubuntu-latest steps: + - uses: cisagov/setup-env-github-action@develop - uses: actions/checkout@v2 - id: setup-python uses: actions/setup-python@v2 with: python-version: 3.9 + # GO_VERSION and GOCACHE are used by the cache task, so the Go + # installation must happen before that. + - uses: actions/setup-go@v2 + with: + go-version: '1.16' + - name: Store installed Go version + run: | + echo "GO_VERSION="\ + "$(go version | sed 's/^go version go\([0-9.]\+\) .*/\1/')" \ + >> $GITHUB_ENV + - name: Lookup Go cache directory + id: go-cache + run: | + echo "::set-output name=dir::$(go env GOCACHE)" - uses: actions/cache@v2 env: BASE_CACHE_KEY: "${{ github.job }}-${{ runner.os }}-\ - py${{ steps.setup-python.outputs.python-version }}-" + py${{ steps.setup-python.outputs.python-version }}-\ + go${{ env.GO_VERSION }}-" with: path: | ${{ env.PIP_CACHE_DIR }} ${{ env.PRE_COMMIT_CACHE_DIR }} + ${{ steps.go-cache.outputs.dir }} key: "${{ env.BASE_CACHE_KEY }}\ ${{ hashFiles('**/requirements-test.txt') }}-\ ${{ hashFiles('**/requirements.txt') }}-\ ${{ hashFiles('**/.pre-commit-config.yaml') }}" restore-keys: | ${{ env.BASE_CACHE_KEY }} + - name: Install shfmt + run: go install mvdan.cc/sh/v3/cmd/shfmt@${SHFMT_VERSION} - name: Install dependencies run: | python -m pip install --upgrade pip From 406b6880bd25a8592ad235102d4e832e05ab38e3 Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Tue, 13 Jul 2021 17:53:55 -0400 Subject: [PATCH 19/30] Replace the beautysh hook with pre-commit-shfmt We have had a difficult time with how beautysh parses some shellscripts. I went in pursuit of an alternative and I believe shfmt to be a good alternative. Co-authored-by: Shane Frasier --- .pre-commit-config.yaml | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cf0330d..c915aa9 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -48,13 +48,20 @@ repos: - --strict # Shell script hooks - - repo: https://github.com/lovesegfault/beautysh - rev: v6.1.0 + - repo: https://github.com/cisagov/pre-commit-shfmt + rev: v0.0.2 hooks: - - id: beautysh + - id: shfmt args: - - --indent-size + # Indent by two spaces + - -i - '2' + # Binary operators may start a line + - -bn + # Switch cases are indented + - -ci + # Redirect operators are followed by a space + - -sr - repo: https://github.com/detailyang/pre-commit-shell rev: 1.0.5 hooks: From 2b48e75b23cb80af9e97098da2dd6b9fb5eea2e4 Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Tue, 13 Jul 2021 18:08:51 -0400 Subject: [PATCH 20/30] Apply changes from the shfmt pre-commit hook --- setup-env | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/setup-env b/setup-env index 1579e04..5d7f673 100755 --- a/setup-env +++ b/setup-env @@ -4,7 +4,8 @@ set -o nounset set -o errexit set -o pipefail -USAGE=$(cat << 'END_OF_LINE' +USAGE=$( + cat << 'END_OF_LINE' Configure a developement environment for this repository. It does the following: @@ -35,17 +36,17 @@ FORCE=0 PARAMS="" # Parse command line arguments -while (( "$#" )); do +while (("$#")); do case "$1" in - -f|--force) + -f | --force) FORCE=1 shift ;; - -h|--help) + -h | --help) echo "${USAGE}" exit 0 ;; - -i|--install-hooks) + -i | --install-hooks) INSTALL_HOOKS=1 shift ;; @@ -160,7 +161,8 @@ pre-commit install ${INSTALL_HOOKS:+"--install-hooks"} # This could fail if the remotes are already setup, but that is ok. set +o errexit -eval "$(python3 << 'END_OF_LINE' +eval "$( + python3 << 'END_OF_LINE' from pathlib import Path import yaml import sys From 1df17570c764a6765e7b699bcbbc09ca42fcabc9 Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Wed, 14 Jul 2021 18:23:59 -0400 Subject: [PATCH 21/30] Remove unnecessary boilerplate This package has a `__main__.py` file which removes the need to include this boilerplate. --- src/example/example.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/example/example.py b/src/example/example.py index 73faa33..a0dcb68 100644 --- a/src/example/example.py +++ b/src/example/example.py @@ -102,7 +102,3 @@ def main() -> int: # Stop logging and clean up logging.shutdown() return 0 - - -if __name__ == "__main__": - sys.exit(main()) From ad5009c1956f476cf9968e1d28bb7da64cfb289b Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Wed, 14 Jul 2021 18:29:45 -0400 Subject: [PATCH 22/30] Explicitly call sys.exit for errors Instead of returning a value the main loop should explicitly call `sys.exit()` if an error is encountered that would result in a non-zero exit code. --- src/example/example.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/example/example.py b/src/example/example.py index a0dcb68..28b304e 100644 --- a/src/example/example.py +++ b/src/example/example.py @@ -45,7 +45,7 @@ def example_div(dividend: float, divisor: float) -> float: return dividend / divisor -def main() -> int: +def main() -> None: """Set up logging and call the example function.""" args: Dict[str, str] = docopt.docopt(__doc__, version=__version__) # Validate and convert arguments as needed @@ -73,7 +73,7 @@ def main() -> int: except SchemaError as err: # Exit because one or more of the arguments were invalid print(err, file=sys.stderr) - return 1 + sys.exit(1) # Assign validated arguments to variables dividend: int = validated_args[""] @@ -101,4 +101,3 @@ def main() -> int: # Stop logging and clean up logging.shutdown() - return 0 From 8a8785d16f5cb43b8e55141e7b350e7b0d79d844 Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Wed, 14 Jul 2021 18:31:49 -0400 Subject: [PATCH 23/30] Update function type hints The type hints for the example_div function indicate it takes two float values, but the command line interface indicates (and checks) that it takes integer values. --- src/example/example.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/example/example.py b/src/example/example.py index 28b304e..9267cc5 100644 --- a/src/example/example.py +++ b/src/example/example.py @@ -35,7 +35,7 @@ DEFAULT_ECHO_MESSAGE: str = "Hello World from the example default!" -def example_div(dividend: float, divisor: float) -> float: +def example_div(dividend: int, divisor: int) -> float: """Print some logging messages.""" logging.debug("This is a debug message") logging.info("This is an info message") From 29ade6fa6ea6187d4059a7c6a8ef630458337c1e Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Wed, 14 Jul 2021 18:36:43 -0400 Subject: [PATCH 24/30] Do not use f-strings in logging messages C-style '%' formatting should be used to any calls to the `logging` library. --- src/example/example.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/example/example.py b/src/example/example.py index 9267cc5..d3eda19 100644 --- a/src/example/example.py +++ b/src/example/example.py @@ -85,11 +85,11 @@ def main() -> None: format="%(asctime)-15s %(levelname)s %(message)s", level=log_level.upper() ) - logging.info(f"{dividend} / {divisor} == {example_div(dividend, divisor)}") + logging.info("%d / %d == %f", dividend, divisor, example_div(dividend, divisor)) # Access some data from an environment variable message: str = os.getenv("ECHO_MESSAGE", DEFAULT_ECHO_MESSAGE) - logging.info(f'ECHO_MESSAGE="{message}"') + logging.info('ECHO_MESSAGE="%s"', message) # Access some data from our package data (see the setup.py) secret_message: str = ( @@ -97,7 +97,7 @@ def main() -> None: .decode("utf-8") .strip() ) - logging.info(f'Secret="{secret_message}"') + logging.info('Secret="%s"', secret_message) # Stop logging and clean up logging.shutdown() From 015d47e7e9d4d4570bb42396e5a94495edb5b19d Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Wed, 14 Jul 2021 18:45:04 -0400 Subject: [PATCH 25/30] Update testing for exit code changes The testing needs to be updated now that `example.main()` only returns a `None` and error exits are done by directly calling `sys.exit()`. --- tests/test_example.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/tests/test_example.py b/tests/test_example.py index 3a22848..5ea4c5c 100644 --- a/tests/test_example.py +++ b/tests/test_example.py @@ -79,18 +79,26 @@ def test_log_levels(level): assert ( logging.root.hasHandlers() is False ), "root logger should not have handlers yet" - return_code = example.example.main() + return_code = None + try: + example.example.main() + except SystemExit as sys_exit: + return_code = sys_exit.code + assert return_code is None, "main() should return success" assert ( logging.root.hasHandlers() is True ), "root logger should now have a handler" - assert return_code == 0, "main() should return success (0)" def test_bad_log_level(): """Validate bad log-level argument returns error.""" with patch.object(sys, "argv", ["bogus", "--log-level=emergency", "1", "1"]): - return_code = example.example.main() - assert return_code == 1, "main() should return failure" + return_code = None + try: + example.example.main() + except SystemExit as sys_exit: + return_code = sys_exit.code + assert return_code == 1, "main() should exit with error" @pytest.mark.parametrize("dividend, divisor, quotient", div_params) @@ -124,5 +132,9 @@ def test_zero_division(): def test_zero_divisor_argument(): """Verify that a divisor of zero is handled as expected.""" with patch.object(sys, "argv", ["bogus", "1", "0"]): - return_code = example.example.main() + return_code = None + try: + example.example.main() + except SystemExit as sys_exit: + return_code = sys_exit.code assert return_code == 1, "main() should exit with error" From 1708b5c45ac12f9acfb447e532c4fc5746a80d23 Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Tue, 13 Jul 2021 11:27:26 -0400 Subject: [PATCH 26/30] Update pre-commit hooks This is performed by running `pre-commit autoupdate`, but with the `ansible-lint` hook held back manually. --- .pre-commit-config.yaml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cf0330d..cf319bf 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -37,7 +37,7 @@ repos: args: - --config=.mdl_config.json - repo: https://github.com/pre-commit/mirrors-prettier - rev: v2.3.0 + rev: v2.3.2 hooks: - id: prettier - repo: https://github.com/adrienverge/yamllint @@ -68,7 +68,7 @@ repos: args: - --config=.bandit.yml - repo: https://github.com/psf/black - rev: 21.5b2 + rev: 21.7b0 hooks: - id: black - repo: https://gitlab.com/pycqa/flake8 @@ -78,15 +78,15 @@ repos: additional_dependencies: - flake8-docstrings - repo: https://github.com/PyCQA/isort - rev: 5.8.0 + rev: 5.9.2 hooks: - id: isort - repo: https://github.com/pre-commit/mirrors-mypy - rev: v0.812 + rev: v0.910 hooks: - id: mypy - repo: https://github.com/asottile/pyupgrade - rev: v2.19.1 + rev: v2.21.2 hooks: - id: pyupgrade @@ -123,7 +123,7 @@ repos: # Docker hooks - repo: https://github.com/IamTheFij/docker-pre-commit - rev: v2.0.0 + rev: v2.0.1 hooks: - id: docker-compose-check From 63cf76fd694d13e431f820835a8fb0a4a993a3f9 Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Tue, 20 Jul 2021 12:55:26 -0400 Subject: [PATCH 27/30] Changes from the pre-commit-shfmt hook --- bump_version.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bump_version.sh b/bump_version.sh index c0bf25c..e1324b8 100755 --- a/bump_version.sh +++ b/bump_version.sh @@ -12,12 +12,11 @@ HELP_INFORMATION="bump_version.sh (show|major|minor|patch|prerelease|build|final old_version=$(sed -n "s/^__version__ = \"\(.*\)\"$/\1/p" $VERSION_FILE) -if [ $# -ne 1 ] -then +if [ $# -ne 1 ]; then echo "$HELP_INFORMATION" else case $1 in - major|minor|patch|prerelease|build) + major | minor | patch | prerelease | build) new_version=$(python -c "import semver; print(semver.bump_$1('$old_version'))") echo Changing version from "$old_version" to "$new_version" # A temp file is used to provide compatability with macOS development From 77c19cb1c5a43e0f100f04caaaa913e8baaab9de Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Tue, 20 Jul 2021 13:04:28 -0400 Subject: [PATCH 28/30] Add stubs required by mypy pre-commit hook With the release of 0.900 mypy moved to a modular stub model. This means that only stdlib stubs are "baked in" and everything else must be installed separately. Since the mypy hook runs in a venv the stubs it recommends need to be installed as additional dependencies. --- .pre-commit-config.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 74f3836..db9f507 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -108,6 +108,8 @@ repos: rev: v0.910 hooks: - id: mypy + additional_dependencies: + - types-setuptools - repo: https://github.com/asottile/pyupgrade rev: v2.21.2 hooks: From 3452b9de398c48d73f06d2fecc666d07341a9038 Mon Sep 17 00:00:00 2001 From: Jeremy Frasier Date: Tue, 20 Jul 2021 15:14:19 -0400 Subject: [PATCH 29/30] Install mypy type stubs as part of setup-env We already do this as part of the mypy pre-commit hook, but this way mypy is also ready to run manually in the developer's local Python virtual env. --- setup-env | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/setup-env b/setup-env index 5d7f673..6dd2079 100755 --- a/setup-env +++ b/setup-env @@ -12,7 +12,7 @@ It does the following: - Verifies pyenv and pyenv-virtualenv are installed. - Creates a Python virtual environment. - Configures the activation of the virtual enviroment for the repo directory. - - Installs the requirements needed for development. + - Installs the requirements needed for development (including mypy type stubs). - Installs git pre-commit hooks. - Configures git upstream remote "lineage" repositories. @@ -154,6 +154,9 @@ for req_file in "requirements-dev.txt" "requirements-test.txt" "requirements.txt fi done +# Install all necessary mypy type stubs +mypy --install-types src/ + # Install git pre-commit hooks now or later. pre-commit install ${INSTALL_HOOKS:+"--install-hooks"} From 50f7e4bc871ab499508f5a0a24a740a3515839e6 Mon Sep 17 00:00:00 2001 From: Nicholas McDonnell <50747025+mcdonnnj@users.noreply.github.com> Date: Mon, 9 Aug 2021 14:10:34 -0400 Subject: [PATCH 30/30] Run shfmt tool on the repository This was done with `pre-commit run --all-files` to ensure our desired configuration was used. --- build.sh | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/build.sh b/build.sh index 50feda1..ae75eb0 100755 --- a/build.sh +++ b/build.sh @@ -8,13 +8,10 @@ set -o pipefail # requirements and then exit. function check_dependencies { required_tools="pip python zip" - for tool in $required_tools - do - if [ -z "$(command -v "$tool")" ] - then + for tool in $required_tools; do + if [ -z "$(command -v "$tool")" ]; then echo "This script requires the following tools to run:" - for item in $required_tools - do + for item in $required_tools; do echo "- $item" done exit 1 @@ -88,13 +85,11 @@ cp lambda_handler.py "$BUILD_DIR" # Zip it all up. ### OUTPUT_DIR="/output" -if [ ! -d "$OUTPUT_DIR" ] -then +if [ ! -d "$OUTPUT_DIR" ]; then mkdir "$OUTPUT_DIR" fi -if [ -e "$OUTPUT_DIR/$ZIP_FILE" ] -then +if [ -e "$OUTPUT_DIR/$ZIP_FILE" ]; then rm "$OUTPUT_DIR/$ZIP_FILE" fi