Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rev 5 Baselines Reference XML Version of the Catalog #109

Open
hahsan-ti opened this issue Jun 6, 2022 · 6 comments · Fixed by usnistgov/OSCAL#1301
Open

Rev 5 Baselines Reference XML Version of the Catalog #109

hahsan-ti opened this issue Jun 6, 2022 · 6 comments · Fixed by usnistgov/OSCAL#1301
Labels
bug The issue is a bug report.

Comments

@hahsan-ti
Copy link

The Rev 5 baseline JSON files reference NIST_SP-800-53_rev5_catalog.xml under imports. Is there a reason why the XML file is referenced and not the JSON file?

@hahsan-ti hahsan-ti added the question The issue contains a question that needs to be answered. label Jun 6, 2022
@hahsan-ti
Copy link
Author

The Rev 4 baselines do the same; however, the file is referenced in back-matter but with a JSON media type.

@aj-stein-nist
Copy link
Contributor

aj-stein-nist commented Jun 7, 2022

Hmmm, that is odd. convert_filetypes.py should be converting this by way copy-and-convert-content.sh script that is executed by the Copy and Conversion phase of this GHA workflow for content processing. I am reviewing the scripts and Python code locally to see how this is getting missed.

@aj-stein-nist
Copy link
Contributor

aj-stein-nist commented Jun 8, 2022

The lack of tests around this small but purposeful little Python script has come to bite me in the rear. Maybe I need to write some around this to avoid this moving forward.

I recall during feedback that during code review in ad hoc reviews or during formal review in usnistgov/OSCAL#1070 I had slipped on parameterized file extension (like "from XML" and "to JSON" as arguments getting passed into the functions correctly. Somehow, and I am not sure how this slipped by me/us, I missed process_xml function (when "from XML" and "to whatever" with whatever being you're choice on an XML document are passed in as parameters. That was not done here, even though design and code docs say as much.) :-(

Good (JSON): https://github.com/usnistgov/OSCAL/blob/dc2220124f4e2a7246697508a837d68e95a77908/build/ci-cd/python/convert_filetypes.py#L315

Bad (XML, source of this bug report): https://github.com/usnistgov/OSCAL/blob/dc2220124f4e2a7246697508a837d68e95a77908/build/ci-cd/python/convert_filetypes.py

Also bad (YAML): https://github.com/usnistgov/OSCAL/blob/dc2220124f4e2a7246697508a837d68e95a77908/build/ci-cd/python/convert_filetypes.py#L406

I am opening up a PR, but I think it might be time to test this thing more effectively. Will follow up with Dave tomorrow.

@aj-stein-nist aj-stein-nist added bug The issue is a bug report. and removed question The issue contains a question that needs to be answered. labels Jun 8, 2022
aj-stein-nist added a commit to aj-stein-nist/OSCAL-forked that referenced this issue Jun 8, 2022
aj-stein-nist added a commit to aj-stein-nist/OSCAL-forked that referenced this issue Jun 8, 2022
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Jun 10, 2022
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Jun 10, 2022
aj-stein-nist added a commit to aj-stein-nist/oscal-content-forked that referenced this issue Jun 13, 2022
@aj-stein-nist aj-stein-nist reopened this Jun 13, 2022
@aj-stein-nist
Copy link
Contributor

@david-waltermire-nist, how should we handle pushing up the submodule checkout to ensure this bug (and other enhancements) are merged into the current release of the catalog. Let me know me and I will work with you to sort it out.

@aj-stein-nist
Copy link
Contributor

I will include this in sprint: it can be resolved by updating the OSCAL submodule in the respective branches (develop or main or both upon further review, but likely develop only and progress through release).

@aj-stein-nist
Copy link
Contributor

This was revolved partially but other work referenced in #116 is still blocking this from completion and resolution in develop and main as indicated in previous replies. Current schedule means we should be able to put this to rest in Sprint 65. Moving this issue tentatively until then.

@aj-stein-nist aj-stein-nist removed their assignment Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug report.
Projects
Status: Blocked
3 participants