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

include-all type mismatch between Profile and Assessment Plan #1137

Closed
guyzyl opened this issue Feb 14, 2022 · 1 comment · Fixed by #1144
Closed

include-all type mismatch between Profile and Assessment Plan #1137

guyzyl opened this issue Feb 14, 2022 · 1 comment · Fixed by #1144
Assignees
Labels
Milestone

Comments

@guyzyl
Copy link
Contributor

guyzyl commented Feb 14, 2022

Describe the bug

Both the Profile and the Assessment Plan schemas have models that contain an include-all field.

In the Profile schema they are:

  • #assembly_oscal-profile_insert-controls
  • #assembly_oscal-profile_import

In Assessment Plan they are:

  • #assembly_oscal-assessment-common_reviewed-controls.control-selection
  • #assembly_oscal-assessment-common_reviewed-controls.control-objective-selections
  • #assembly_oscal-assessment-common_assessment-subject

The problem is that there's a mismatch in the include-all field's types between the Profile and Assessment Plan.
In the Profile schema they are of type #assembly_oscal-profile_include-all, compared to Assessment Plan where they defined as object.

The solution to this issue is to convert the Assessment Plan's include-all fields to be of type #assembly_oscal-profile_include-all.
Since they're currently accepting type object and #assembly_oscal-profile_include-all is an object, this solution is backwards compatible.

Expected behavior (i.e. solution)

Uniformal types between the different OSCAL layers.

Other Comments

I'd be more than happy to open a PR for this in case this bug gets "accepted".

@david-waltermire
Copy link
Contributor

I am looking at PR #1144 and this issue.

I believe we can reuse the include-all from the profile model as you suggest, since their syntax is the same. I'd like to cleanup a few small things with PR #1144. I'll provide feedback on the PR regarding these issues.

@david-waltermire david-waltermire linked a pull request Feb 25, 2022 that will close this issue
4 tasks
@david-waltermire david-waltermire self-assigned this Feb 25, 2022
@david-waltermire david-waltermire added this to the OSCAL 1.0.2 milestone Feb 25, 2022
david-waltermire pushed a commit to guyzyl/OSCAL that referenced this issue Feb 25, 2022
david-waltermire pushed a commit to guyzyl/OSCAL that referenced this issue Feb 26, 2022
david-waltermire added a commit that referenced this issue Feb 26, 2022
* Replace `define-assembly` for `include-all` with `assembly ref`
* Apply suggestions from code review
* Moved include-all to oscal_control-common_metaschema.xml to allow for proper importing.

Resolves #1137

Co-authored-by: David Waltermire <[email protected]>
stephenbanghart pushed a commit to stephenbanghart/OSCAL that referenced this issue Mar 14, 2022
…stgov#1144)

* Replace `define-assembly` for `include-all` with `assembly ref`
* Apply suggestions from code review
* Moved include-all to oscal_control-common_metaschema.xml to allow for proper importing.

Resolves usnistgov#1137

Co-authored-by: David Waltermire <[email protected]>
iMichaela pushed a commit to iMichaela/OSCAL that referenced this issue Apr 7, 2022
…stgov#1144)

* Replace `define-assembly` for `include-all` with `assembly ref`
* Apply suggestions from code review
* Moved include-all to oscal_control-common_metaschema.xml to allow for proper importing.

Resolves usnistgov#1137

Co-authored-by: David Waltermire <[email protected]>
Rene2mt pushed a commit to Rene2mt/OSCAL that referenced this issue May 17, 2022
…stgov#1144)

* Replace `define-assembly` for `include-all` with `assembly ref`
* Apply suggestions from code review
* Moved include-all to oscal_control-common_metaschema.xml to allow for proper importing.

Resolves usnistgov#1137

Co-authored-by: David Waltermire <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants