-
Notifications
You must be signed in to change notification settings - Fork 184
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
Move web content from OSCAL, and other streamlining. #1824
Conversation
Hi Chris, as part of this PR I'd like to propose some additional removals/changes:
|
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.
This is looking good so far. I have added a few minor comments. In addition, I am not sure deleting xml/README.md
and json/README.md
is aggressive enough for a recursive delete if this makes sense, let me know.
@@ -28,29 +26,3 @@ jobs: | |||
commit_resources: true | |||
secrets: | |||
access_token: ${{ secrets.COMMIT_TOKEN }} | |||
validate-website-reference: |
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.
Question, unclear if it is blocking: we need to determine how we will generate schemas in these pipelines even if they are not committed inline anymore, and that means adapting metaschema-xslt, in this PR branch or another.
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.
I'm not sure if this has been addressed or not, or if it is still a potential issue -- @nikitawootten-nist ?
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.
If we are planning on removing auto-comitting actions as stated in the ADR, we should probably do the following:
- Streamline the schema and converter generation to be as simple to invoke as possible (another makefile?)
- Modify the existing action to build the schemas and converters without committing for PRs
- Also make the action upload schema artifacts on release
For existing releases we'll have to manually upload the correct artifacts (which is simple but a bit tedious, thankfully we only have a few releases so it shouldn't be too painful)
Should we align the ADR draft to support this?
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.
This work has been summarized in a follow-on issue: #1847
@aj-stein-nist and @Compton-NIST: All individual work will be done in branches in a personal fork of this repository. During my chat with AJ today around #1798, he indicated that working on a personal fork is no longer mandated. Should this file be updated? Similar guidance exists in the CONTRIBUTING.md file, the block starting at line 72. |
If given the option I intend to continue to work in a personal fork. However, I did not interpret the new guideline as discouraging this, only as not requiring it. However I note that work when not individual does not have to go in a personal fork, the alternative presumably being a feature branch in the main repo. Maybe we only need guidance on when to create a feature branch vs work in a fork. 🌻 |
2d8b7f9
to
bf9b66c
Compare
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.
I've noticed a significant regression and the work I had done in #1666 and #1754 is not present:
src/utils/resolver-2018
which I had previously deleted has reappeared.- The profile resolution stylesheets no longer have the helper script and readme updates
- The
src/utils/testing
scripts that I included in XSpec test suites script & CI #1754 is not present
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.
Thank you for the fantastic work done to split the OSCAL repo into multiple content-specific repos. Kudos! I approve the PR moving the website. I have some small comments, that, if not addressed now, we can address them later when doing a fine tunning.
| [OSCAL](https://github.com/usnistgov/OSCAL/) | The main OSCAL project that contains the source code for the OSCAL models. | | ||
| [OSCAL-Pages](https://github.com/usnistgov/OSCAL-Pages/) | The project that contains the public OSCAL website content. | | ||
| [OSCAL-Reference](https://github.com/usnistgov/OSCAL-Reference/) | The project that contains the model documentation and developer reference content. | | ||
| [OSCAL-Content](https://github.com/usnistgov/oscal-content/) | The project that contains examples of OSCAL model content. | |
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.
For consistency reasons, do we want to rename oscal-content
to become OSCAL-Content
? Or maybe OSCAL-content
and make all other projects follow the same convention? Consistency is what I am looking for, so we need to remember the convention and not specifics.
|
||
#### Content in OSCAL-Pages repository: | ||
|
||
A `published-pages` branch will also exist in OSCAL-Pages that contains the latest deployed content that is sent to the OSCAL repository. Additionally, there is a `nist-pages` branch in the OSCAL-Pages project that contains a blank page. This branch could be used to test a deployment of the website at `pages.nist.gov/OSCAL-Pages`, but it is important to be aware that this site could be indexed by search engines and cause confusion, as well as broken links. |
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.
What is the main
branch used for? Should a description be documented?
|
||
#### Streamlined Build Process | ||
|
||
The new reference repository operates off of one GitHub Actions Workflow that produces the documentation from `metaschema` and uses `Hugo` to publish the website. All of the scripted aspects of the model interpretation are executed using `make`, and the `Makefile` is located in the root of the repository. |
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.
I am assuming the metaschema
listed is the previous src/metaschema
which appears to remain in the OSCAL repository. Is this the proposed approach in this ADR?
1. We will separate one large repository into three distinct repositories. | ||
- Links to any content that is moved will break. | ||
|
||
2. We remove the schemas and converters from the repository. |
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.
The ADR does not document where the removed schemas and converters are relocated after removing them from the OSCAL repo.
- OSCAL | ||
- We should notify the public of the pending release, and the changes noted in this ADR. | ||
- For developers (and products) that link directly to content in the OSCAL repository, they may update their links to point to tagged releases, rather than using source files in the main branch. Future dependencies linking directly to source code in the OSCAL repository, particularly `main`, should be discouraged since this could impact the ability of the development team to improve source code as necessary to build OSCAL. | ||
- Future links to schemas and converters should use release assets (see [v1.0.6](https://github.com/usnistgov/OSCAL/releases/tag/v1.0.6) for an example of assets). We currently produce zip archives as a part of releases. We intend to also include the schemas and converters directly in the release assets without having to extract from the zip file. **This will be implemented in a future sprint.** |
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.
Guidance for submitting issues against different schemas is not captured in this ADR.
@@ -83,10 +83,6 @@ The OSCAL project uses a typical GitHub fork and pull request [workflow](https:/ | |||
This repository consists of the following directories and files pertaining to the OSCAL project: | |||
|
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.
Missing decisions
subdirectory from the list of subdirectories.
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.
The file indicates that the NIST team is maintaining FedRAMP baselines... I think we should update the information:
The NIST team is also maintaining OSCAL content that is updated to the latest OSCAL revision. The [OSCAL content repository](https://github.com/usnistgov/oscal-content/) provides OSCAL examples, in addition to:
The [NIST SP 800-53 revision 5 catalog](https://github.com/usnistgov/oscal-content/tree/main/nist.gov/SP800-53/rev5) and the security and privacy [NIST SP 800-53B baselines](https://github.com/usnistgov/oscal-content/tree/main/nist.gov/SP800-53/rev5).
The [NIST SP 800-53 revision 4 catalog](https://github.com/usnistgov/oscal-content/tree/main/nist.gov/SP800-53/rev4) and the [three NIST SP 800-53 revision 4 baselines](https://github.com/usnistgov/oscal-content/tree/main/nist.gov/SP800-53/rev4).
The [FedRAMP SP 800-53 revision 4 baselines](https://github.com/GSA/fedramp-automation/tree/master/dist/content/rev4/baselines).
The base branch was changed.
- Content was deleted and distributed between OSCAL-Pages and OSCAL-Reference. - Unnecessary scripts and code were pruned from the repo. - Workflows were reduced as necessary.
- Content was deleted and distributed between OSCAL-Pages and OSCAL-Reference. - Unnecessary scripts and code were pruned from the repo. - Workflows were reduced as necessary.
74e4436
to
2e3e889
Compare
I'm going to close this and reopen with the feature branch that was not squashed so that we have a more clean rebase to the updated develop branch. |
Committer Notes
In support of Issue #1802
Currently in rapid prototype mode with the team.
Remaining to determine:
Benefits:
All Submissions:
By submitting a pull request, you are agreeing to provide this contribution under the CC0 1.0 Universal public domain dedication.
(For reviewers: The wiki has guidance on code review and overall issue review for completeness.)
Changes to Core Features: