-
Notifications
You must be signed in to change notification settings - Fork 133
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
Update content to use OSCAL v1.1.1 models #204
Conversation
@aj-stein-nist Is the title of the PR wrong? It lists OSCAL 1.0.0 |
Woops, fixed. :-) |
Add this back as part of sprint work to build new Makefile(s) and recreate profile resolution tooling for local and GHA CI/CD for usnistgov/oscal-content#204.
This was removed in site migration work and did not appear to be relocated, as determined during the course of usnistgov/oscal-content#204. We can rely on the metaschema-xslt project, but given OSCAL has the profile resolver tool committed in its repo tree, it stands to reason to rely on its own independent Maven POM file, even if now redundant, as they may diverge in the future.
* Correct relative path to previous Maven POM dep file Add this back as part of sprint work to build new Makefile(s) and recreate profile resolution tooling for local and GHA CI/CD for usnistgov/oscal-content#204. * Add back pom.xml for Maven deps This was removed in site migration work and did not appear to be relocated, as determined during the course of usnistgov/oscal-content#204. We can rely on the metaschema-xslt project, but given OSCAL has the profile resolver tool committed in its repo tree, it stands to reason to rely on its own independent Maven POM file, even if now redundant, as they may diverge in the future.
This script is being added to facilitate running XSLT stylesheets, such as the XML/JSON JSON/XML conversion utilities that will be used in our oscal-content and related pipelines. This script will be committed to coincide with testing and review of the Makefile in usnistgov/oscal-content#204.
This script is being added to facilitate running XSLT stylesheets, such as the XML/JSON JSON/XML conversion utilities that will be used in our oscal-content and related pipelines. This script will be committed to coincide with testing and review of the Makefile in usnistgov/oscal-content#204.
This is important and still needs to be finished. I will continue work on this in the first half of Sprint 74 and intend to finish it as it is an important prerequisite for many upcoming changes given the tentative roadmap in usnistgov/OSCAL#1867. |
65c7bc2
to
56a8e5e
Compare
* Correct relative path to previous Maven POM dep file Add this back as part of sprint work to build new Makefile(s) and recreate profile resolution tooling for local and GHA CI/CD for usnistgov/oscal-content#204. * Add back pom.xml for Maven deps This was removed in site migration work and did not appear to be relocated, as determined during the course of usnistgov/oscal-content#204. We can rely on the metaschema-xslt project, but given OSCAL has the profile resolver tool committed in its repo tree, it stands to reason to rely on its own independent Maven POM file, even if now redundant, as they may diverge in the future.
This script is being added to facilitate running XSLT stylesheets, such as the XML/JSON JSON/XML conversion utilities that will be used in our oscal-content and related pipelines. This script will be committed to coincide with testing and review of the Makefile in usnistgov/oscal-content#204.
* Correct relative path to previous Maven POM dep file Add this back as part of sprint work to build new Makefile(s) and recreate profile resolution tooling for local and GHA CI/CD for usnistgov/oscal-content#204. * Add back pom.xml for Maven deps This was removed in site migration work and did not appear to be relocated, as determined during the course of usnistgov/oscal-content#204. We can rely on the metaschema-xslt project, but given OSCAL has the profile resolver tool committed in its repo tree, it stands to reason to rely on its own independent Maven POM file, even if now redundant, as they may diverge in the future.
This script is being added to facilitate running XSLT stylesheets, such as the XML/JSON JSON/XML conversion utilities that will be used in our oscal-content and related pipelines. This script will be committed to coincide with testing and review of the Makefile in usnistgov/oscal-content#204.
3b14a16
to
74c7398
Compare
As picked up in review with Nikita, we are converting the source XML files to JSON and YAML, but not all. We need to adjust the make vars that collect the source files for conversion to minified JSON to Include the resolved catalogs from profiles from the previous step. Reviewing the auto-committed test content confirm these files were being missed in conversion to JSON and YAML and we obviously need those.
Per discussion with Nikita during PR review and pairing, we should try to adjust the jq and yq dep install and management approach to be a little more ergonomic. As for xmllint, it was requested we align that to work like the dep mgmt in the usnistgov/OSCAL repo, since that is a compiled dep that is not offered statically and is best handled as a concern by the developer in their operating system, or by a dedicated install step in GHA workflows. Additionally, for these reasons, the clean wrapper target will not include deleting it as a default, but can be added later by a developer by explicitly requesting it.
Per PR review and consultation from Nikita, add in additional wrapper targets for artifacts and checks for better organization and to make a cleaner hierarchy for the make all target.
As discussed in the branch under review in #204, we now have absolute paths in the resolved catalogs based on the full paths to the profile, which is less than idea for published catalogs. Comment with explanation: #204 (comment) Example: https://github.com/aj-stein-nist/oscal-content/blob/dc5500ed9371b485fc21cb095d2fb9db6e2a1fd3/nist.gov/SP800-53/rev5/xml/NIST_SP-800-53_rev5_LOW-baseline-resolved-profile_catalog.xml#L11 To work around this for the time being, we are using sed in the Makefile to clean up once profile resolution is done and before the XML source is converted to the target JSON and YAML.
After debugging with @nikitawootten-nist, we determined during pairing that the SECONDEXPANSION calls, that are necessary to avoid some more hacky kludges with macros and functions. As it stands, when running with concurrent jobs (`-jN` with N>=2) it fails. We determined the cause of the failures for `convert-min-json-content` and `convert-yaml-content` targets are because those target prerequisites are flawed. This change will be greedy and presume all respective XML files for JSON targets and all JSON files for YAML targets. This will limit the upper bound of concurrent jobs for these targets, but makes it correct and safe.
99669b4
to
bbdcac7
Compare
Doing some more testing but the concurrency fix as I committed is still not working perfectly. https://github.com/aj-stein-nist/oscal-content/actions/runs/6592140253/job/17912204423 Catching up on Makefile docs and tutorials while debugging locally and GHA to figure out if I did it in the wrong target location or this is not sufficient as discussed during pairing. 😭 |
65292a1
to
5a07896
Compare
It seems with c881404 we are very close (thanks to @nikitawootten-nist for #219) but I still see a regression in the |
* Rearranged targets, fixed build race conditions * Added a check for MacOS/Linux semantics
f95c749
to
e96e2b5
Compare
I removed the testing commit to change the HOME_REPO, but this is ready for final review. Sample from my fork simiulating the merge to /cc @usnistgov/itl-oscal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All works, all approved. Small nitpick below but it can be safely ignored
Edit: I deleted the nit as it may cause confusion down the line if oscal content includes resolved profiles with multiple inputs.
As discussed in the branch under review in #204, we now have absolute paths in the resolved catalogs based on the full paths to the profile, which is less than idea for published catalogs. Comment with explanation: #204 (comment) Example: https://github.com/aj-stein-nist/oscal-content/blob/dc5500ed9371b485fc21cb095d2fb9db6e2a1fd3/nist.gov/SP800-53/rev5/xml/NIST_SP-800-53_rev5_LOW-baseline-resolved-profile_catalog.xml#L11 To work around this for the time being, we are using sed in the Makefile to clean up once profile resolution is done and before the XML source is converted to the target JSON and YAML.
As discussed in the branch under review in usnistgov#204, we now have absolute paths in the resolved catalogs based on the full paths to the profile, which is less than idea for published catalogs. Comment with explanation: usnistgov#204 (comment) Example: https://github.com/aj-stein-nist/oscal-content/blob/dc5500ed9371b485fc21cb095d2fb9db6e2a1fd3/nist.gov/SP800-53/rev5/xml/NIST_SP-800-53_rev5_LOW-baseline-resolved-profile_catalog.xml#L11 To work around this for the time being, we are using sed in the Makefile to clean up once profile resolution is done and before the XML source is converted to the target JSON and YAML.
As discussed in the branch under review in usnistgov#204, we now have absolute paths in the resolved catalogs based on the full paths to the profile, which is less than idea for published catalogs. Comment with explanation: usnistgov#204 (comment) Example: https://github.com/aj-stein-nist/oscal-content/blob/dc5500ed9371b485fc21cb095d2fb9db6e2a1fd3/nist.gov/SP800-53/rev5/xml/NIST_SP-800-53_rev5_LOW-baseline-resolved-profile_catalog.xml#L11 To work around this for the time being, we are using sed in the Makefile to clean up once profile resolution is done and before the XML source is converted to the target JSON and YAML.
Committer Notes
This updates oscal-content
make
-based orchestration and touches up catalogs for #116. Guidance on these changes, future development, and future governance are further detailed in usnistgov/OSCAL#1947.Example content with the updated tooling work and simulating this branch being merged can be found on my work (the last commit changing the
HOME_REPO
variable to work on this branch has been dropped, but was used to complete this final testing).aj-stein-nist/oscal-content@c8a5374#diff-ecf37eddbbe732cfc8cb391c407ae4ad8f00500d1590cc65bb08569c80697ff2
All Submissions:
Have you squashed any non-relevant commits and commit messages? [instructions] (NOTE: Will do after review to aid review process and properly document many changes, big and small, potentially will not squash.)In this case, I believe it to be important that we keep all the commits for team understanding of significant changes in the repo. I will not squash these.Changes to Core Features:
Have you added an explanation of what your changes do and why you'd like us to include them?Have you written new tests for your core changes, as applicable?Have you included examples of how to use your new feature(s)?