requirements.txt not loaded outside of Docker context

[wesley-dean-gsa]

Describe the bug

When build/copy-and-convert-content.sh runs, there are two locations (lines 157 and 202) where python/convert_filetypes.py is invoked. The convert_filetypes.py script imports ruamel.yaml on line 10.

The Dockerfile adds and installs the requirements.txt that downloads the required modules (e.g., the aforementioned ruamel.yaml) so that the Docker image will build and run properly.

However, when run outside of the context of a pre-built Docker container, the required Python modules are not loaded in advance, such as when our (ASAP) builds process (example).

Who is the bug affecting?

ASAP

What is affected by this bug?

Our builds are failing. Because -e is not set in the shell script, the failing python3 command does not cause the script to fail which means errors in the script are not surfacing when our workflow runs. That is, it looks like our build is succeeding when, in fact, it is not.

When does this occur?

When copy-and-convert-content.sh is run without pip3 install -r requirement.txt beforehand (such as when not using a Docker image built with the supplied Dockerfile).

How do we replicate the issue?

1. Do NOT install (and/or verify that it is not installed) the ruamel.yaml module
2. Do NOT run pip3 install -r requirements.txt
3. Do run python3 copy-and-convert-content.sh

(this last step may require data to copy and convert in order for the error to occur)

Expected behavior (i.e. solution)

The copy-and-convert-content.sh script should do what it's supposed to do.

Other Comments

One may wish to update the documentation to include verbiage such as:

Before running build/ci-cd/copy-and-convert-content.sh, please be sure to run pip3 install -r build/ci-cd/python/requirements.txt to verify that the required modules are available.

Alternatively, one may add the aforementioned pip3 command to copy-and-convert-content.sh, but that would be redundant in many situations.

Lastly, please consider setting Bash's -e flag so that commands in the script that return non-zero response codes are surfaced when the script is run and using the env command for situations where Bash is not installed in /bin/:

#!/usr/bin/env bash

set -e

[wendellpiez]

@wesley-dean-gsa apologies for a little latency (due to annual leave on this side) and thanks for the analysis/debug.

[aj-stein-nist]

@wesley-dean-gsa, hi Wes! Long time no talk. Thanks for reporting this issue with all the necessary detail. Expect a PR in the near-term to review.

[aj-stein-nist]

OK @wesley-dean-gsa I did some digging between meetings and I realized an oversight in my initial code skim. Our CI/CD has updated significantly since the last git submodule update you are using in the branch behind that PR. We do not even call the copy-and-convert.sh script in a GitHub Actions step in the core OSCAL repo anymore, only oscal-content because, well, that is where current catalogs and example content needs it.

But, this repo is a submodule of that oscal-content repo and your codebase too, which is in turn forked from the official upstream fedramp-automation repo maintained by the FedRAMP PMO. So GitHub Action declarations will not be magically fixed by the necessary fix to oscal-content repo in a way that will just "fix the glitch for your team." I just talked with our tech lead Dave about next steps, so, on that note.

• Expect a PR to oscal-content that will show how to properly handle Python runtime and deps setup as a reference point for fixes to one, the other, or both fedramp-automation repositories (18F fork or GSA upstream for FedRAMP PMO): Ensure GitHub Actions Installs Python Dependencies Like Local Docker Environment oscal-content#103
• Expect a new issue, as part of our [EPIC] CI/CD Enhancements #1180, to review scripts error handling and proper set -e/set +e behavior in a more holistic way as Dave has been working on this with a given strategy and level of attention to where and where not we want fail fast errors thrown over time, we need to review this given feedback: Review Error Behavior in OSCAL CI/CD Scripts and GitHub Actions Automation #1222
• Expect a new issue, as part of our [EPIC] CI/CD Enhancements #1180, to modularize all GitHub Actions, which are very similar across a few of the core OSCAL project's repositories, into one single GitHub repository (new or exisiting), and modularize it accordingly and not have them copy-pasted repeated in metaschema, OSCAL, and oscal-content repos in repeat fashion. There will be "one source of truth" if possible. This will make sure with a long-term fix such issues like this can be fixed with updating the one submodule, and only that in your repo, and there is less risk of breakage and we can have it thoroughly tested: Modularize All GitHub Actions Setups #1223
• We will PR or collaborate with you on a PR to either 18F fork or the upstream GSA repo once we get guidance from the FedRAMP PMO in a Thursday sync meeting (this is following up on Update issue auto-assignment for new issues GSA/fedramp-automation#192 as I am not sure who inside of the FedRAMP PMO is or will be receiving PRs for several changes, some needed, others wanted).

Hope that clears things up. Expect more updates shortly. 👋

[aj-stein-nist]

@wesley-dean-gsa, re this:

We will PR or collaborate with you on a PR to either 18F fork or the upstream GSA repo once we get guidance from the FedRAMP PMO in a Thursday sync meeting (this is following up on GSA/fedramp-automation#192 as I am not sure who inside of the FedRAMP PMO is or will be receiving PRs for several changes, some needed, others wanted).

We just got off the call with the PMO we agreed to send changes to GSA/fedramp-automation. I will work with @volpet2014 and @david-waltermire-nist to bring those changes in, and you can pull them back into your master and relevant branches, OK?