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

Added simple control call link checker Schematron cf Issue #534 #539

Merged

Conversation

wendellpiez
Copy link
Contributor

Committer Notes

Addressing the requirements of #534, this PR adds a small Schematron for simple link checking on profiles.

It is able to detect, when a profile imports from a catalog, whether controls are actually present in the catalog, when called (imported) by a profile.

It doesn't do fancier checking like checking against profile resolutions, or analysis. See the neighbor Schematron oscal-profile-sources.sch for this (while it depends on now-outmoded profile resolution).

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you squashed any non-relevant commits and commit messages? [instructions]
  • Do all automated CI/CD checks pass?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them? (see Issue Broken links in profiles #534)
  • Have you written new tests for your core changes, as applicable?
  • Have you included examples of how to use your new feature(s)?
  • Have you updated all OSCAL website and readme documentation affected by the changes you made? Changes to the OSCAL website can be made in the docs/content directory of your branch.

@@ -6,6 +6,7 @@

<sch:ns uri="http://csrc.nist.gov/ns/oscal/1.0" prefix="oscal"/>

<!-- (This XSLT is broken until profile resolution is (re)stabilized -->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wendellpiez Is this still broken? If so, we might want to hold off on this PR. Also, we might want to add this to the content validation CI/CD workflow for profiles.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Schematron has been broken for a while, it's really only the comment saying so that is new with the PR. At the same time, I think there's a fair chance that the necessary repair will be pretty straightforward. Agreed that the check could be run under CI. In principle any profile document ought to pass when it is correctly linked to its imported catalogs (or profiles once that is connected back up).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi again, actually I should qualify ... the suspect part of the Schematron is only its dependency on an XSLT for profile resolution. This does not make it completely useless but it is less useful that otherwise. Given a profile resolver (at least, a copy of the prototype it was built with, or a replacement) it might work fine. Probably the comment was added because in meeting the other requirement, I found that its link to the resolver wasn't operable (but I didn't have time to dig further).

Fine to push the PR back, inasmuch as the problem it addresses is not acute, and the data error that gave rise to it, has been corrected.

Copy link
Contributor

@david-waltermire david-waltermire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@david-waltermire david-waltermire merged commit bbd67aa into usnistgov:master Dec 19, 2019
@david-waltermire david-waltermire deleted the Issue534-profile-validation branch December 19, 2019 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants