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

Json schema for include-control in assessment plan is inconsistent with website documentation #769

Closed
butler54 opened this issue Oct 14, 2020 · 2 comments · Fixed by #758
Assignees
Labels
bug closable model-refactor Used to mark issues related to model refactoring for the Metaschema v4 transition. Scope: Modeling Issues targeted at development of OSCAL formats Scope: Website Issues targeted at the OSCAL project website.

Comments

@butler54
Copy link

Describe the bug

Current website documentation for assessment plan - specifically the json schema map here: https://pages.nist.gov/OSCAL/documentation/schema/assessment-layer/assessment-plan/json-model-map/

states that the STRVALUE within the include-control object is optional. Underlying json schema (e.g. https://github.com/usnistgov/OSCAL/blob/master/json/schema/oscal_assessment-plan_schema.json) has the field as required:

     "include-control" : 
      { "title" : "Include Control",
        "description" : "Identifies an individual control to include.",
        "$id" : "#/definitions/include-control",
        "type" : "object",
        "properties" : 
        { "control-id" : 
          { "title" : "Control Identifier Reference",
            "description" : "A reference to a control identifier.",
            "type" : "string" },
          "STRVALUE" : 
          { "type" : "string" } },
        "required" : 
        [ "STRVALUE",
          "control-id" ],
        "additionalProperties" : false },

Who is the bug affecting?

Users of the documentation & assessment plan

What is affected by this bug?

Users reading the document will face unexpected errors when validating.

When does this occur?

When validating assessment plans where user has chosen not to set at STRVALUE for a include-control based on viewing the json schema map

How do we replicate the issue?

Create an assessment plan with one include-control. Do not set STRVALUE, and run arv over the json file with the current schema.

Expected behavior (i.e. solution)

It appears on first inspection that the STRVALUE should not be required. I'm not sure whether this is a metaschema generation issue. Appears to be linked to #388

Other Comments

External linked issue: oscal-compass/compliance-trestle#150

@wendellpiez
Copy link
Contributor

@david-waltermire-nist could you please add this to the board? It can be linked with #750

Adding to unit tests to see to it that correct defaulting is provided for the JSON value key (including on empty fields), is probably also appropriate.

@david-waltermire david-waltermire added the model-refactor Used to mark issues related to model refactoring for the Metaschema v4 transition. label Oct 30, 2020
@iMichaela iMichaela added the Scope: Website Issues targeted at the OSCAL project website. label Oct 30, 2020
@wendellpiez
Copy link
Contributor

One more remark. There are actually two issues here, one hiding behind the other:

  1. Representation of required fields and discrepancy between docs and schema(s)
  2. Which fields should be required? In particular, when / where are JSON "field value" properties required?

The second question might be framed as "what is the correct representation in JSON for XML <call control-id="ac-1"/> or <prop name="status"/>?

Note that the rule might be different for fields marked as string, or "empty".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug closable model-refactor Used to mark issues related to model refactoring for the Metaschema v4 transition. Scope: Modeling Issues targeted at development of OSCAL formats Scope: Website Issues targeted at the OSCAL project website.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants