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

Date regex not updated to fixed (v1.04) version in json schema, or in main branch #1703

Closed
fsuits opened this issue Mar 7, 2023 · 16 comments
Assignees
Labels
Milestone

Comments

@fsuits
Copy link

fsuits commented Mar 7, 2023

Describe the bug

OSCAL v1.0.4 included a fix to the regex for dates but the json schema for catalog and possibly others does not parse all dates properly. The regex in the json schema does not appear to have ever changed from v1.0.0

The fix is described in the changelog for v1.0.4 which refers to the bug report #1260

The regex appears twice around line 900 in the catalog schema for published and last-modified
https://github.com/usnistgov/OSCAL/blob/v1.0.4/json/schema/oscal_catalog_schema.json

If you compare the regex near line 900 for published or last-modified it does not appear to have changed in v1.0.4 or main branch.

Who is the bug affecting

Developers who rely on the json schema and users of code based on that schema

What is affected by this bug

OSCAL Content

How do we replicate this issue

See the regex pattern near line 900 of https://github.com/usnistgov/OSCAL/blob/v1.0.4/json/schema/oscal_catalog_schema.json

^((2000|2400|2800|(19|2[0-9](0[48]|[2468][048]|[13579][26])))-02-29)|(((19|2[0-9])[0-9]{2})-02-(0[1-9]|1[0-9]|2[0-8]))|(((19|2[0-9])[0-9]{2})-(0[13578]|10|12)-(0[1-9]|[12][0-9]|3[01]))|(((19|2[0-9])[0-9]{2})-(0[469]|11)-(0[1-9]|[12][0-9]|30))T(2[0-3]|[01][0-9]):([0-5][0-9]):([0-5][0-9])(\\.[0-9]+)?(Z|[+-][0-9]{2}:[0-9]{2})$

See that the regex will only parse the date part of 2022-05-03T02:00:00Z

But it will parse the entire date/time for 2022-06-03T02:00:00Z

using a parser such as https://www.debuggex.com/

Expected behavior (i.e. solution)

The regex should be updated from v1.0.0 in the json schema so it parses all valid date/time strings properly.

Other comments

No response

@fsuits fsuits added the bug label Mar 7, 2023
@github-actions github-actions bot added this to Needs Triage in Issue Triage Mar 7, 2023
@Compton-US
Copy link
Contributor

For when this is reviewed, it looks like this shows up in multiple files in the repo, beyond the catalog.

find . -type f -name "*.*" -exec egrep -Hn -A0 -B0 "\"\^\(\(200" {} ";" | egrep "\)T\(" | cut -d ":" -f 1 | sort | uniq -c

Result:

   2 ./build/metaschema/test-suite/schema-generation/datatypes/datatypes-datetime-no-tz_json-schema.json
   2 ./build/metaschema/test-suite/schema-generation/datatypes/datatypes-datetime_json-schema.json
   4 ./build/metaschema/toolchains/xslt-M4/schema-gen/oscal-auto-datatypes.xsd
  10 ./json/schema/oscal_assessment-plan_schema.json
  14 ./json/schema/oscal_assessment-results_schema.json
   2 ./json/schema/oscal_catalog_schema.json
  14 ./json/schema/oscal_complete_schema.json
   2 ./json/schema/oscal_component_schema.json
  10 ./json/schema/oscal_poam_schema.json
   2 ./json/schema/oscal_profile_schema.json
   2 ./json/schema/oscal_ssp_schema.json

Could this be in metaschema?

   <xs:simpleType name="dateTime-with-timezone">
      <xs:restriction base="xs:string">
         <xs:whiteSpace value="preserve"/>
         <!-- type: string -->
         <!-- format: date-time -->
         <xs:pattern value="^((2000|2400|2800|(19|2[0-9](0[48]|[2468][048]|[13579][26])))-02-29)|(((19|2[0-9])[0-9]{2})-02-(0[1-9]|1[0-9]|2[0-8]))|(((19|2[0-9])[0-9]{2})-(0[13578]|10|12)-(0[1-9]|[12][0-9]|3[01]))|(((19|2[0-9])[0-9]{2})-(0[469]|11)-(0[1-9]|[12][0-9]|30))T(2[0-3]|[01][0-9]):([0-5][0-9]):([0-5][0-9])(\.[0-9]+)?(Z|[+-][0-9]{2}:[0-9]{2})$"/>
      </xs:restriction>
   </xs:simpleType>
        <map key="dateTime-with-timezone"><!-- DateTimeWithTimezoneDatatype -->
            <string key="type">string</string>
            <string key="format">date-time</string>
            <string key="pattern">^((2000|2400|2800|(19|2[0-9](0[48]|[2468][048]|[13579][26])))-02-29)|(((19|2[0-9])[0-9]{2})-02-(0[1-9]|1[0-9]|2[0-8]))|(((19|2[0-9])[0-9]{2})-(0[13578]|10|12)-(0[1-9]|[12][0-9]|3[01]))|(((19|2[0-9])[0-9]{2})-(0[469]|11)-(0[1-9]|[12][0-9]|30))T(2[0-3]|[01][0-9]):([0-5][0-9]):([0-5][0-9])(\.[0-9]+)?(Z|[+-][0-9]{2}:[0-9]{2})$</string>
        </map>

@david-waltermire
Copy link
Contributor

The Metaschema main branch has not been updated with this change yet, but develop has it.

OSCAL is pointing to the correct Metaschema submodule commit, which is in the develop commit history.

The actual problem is the XML and JSON schemas were not regenerated for the v1.0.4 release. I believe the CI/CD has not regenerated the XML and JSON schema files. A good way to fix this is to release a v1.0.5 with the correct schemas once the CI is confirmed to be working.

@aj-stein-nist
Copy link
Contributor

The Metaschema main branch has not been updated with this change yet, but develop has it.

OSCAL is pointing to the correct Metaschema submodule commit, which is in the develop commit history.

The actual problem is the XML and JSON schemas were not regenerated for the v1.0.4 release. I believe the CI/CD has not regenerated the XML and JSON schema files. A good way to fix this is to release a v1.0.5 with the correct schemas once the CI is confirmed to be working.

This is very good to know. Thanks for the catch, @david-waltermire-nist. I will quickly investigate the release and schema updates issue and factor in how we can work on this in the next sprint (Sprint 65) or a following one.

@aj-stein-nist
Copy link
Contributor

For Sprint 65 @aj-stein-nist is the driver for this issue.

@aj-stein-nist
Copy link
Contributor

@JustKuzya will find some time to pair with me and we begin work on this.

@aj-stein-nist
Copy link
Contributor

OK @fsuits thanks for confirming. @JustKuzya and I got around to confirming what @david-waltermire-nist is the case and we understand a few approaches to resolving this, each with different levels of effort. More to follow!

@aj-stein-nist
Copy link
Contributor

aj-stein-nist commented Mar 29, 2023

@JustKuzya and I are going to work with Dave on the Metaschema to try and hotfix the usnistgov/metaschema#229 for usnistgov/metaschema#224 to a main branch like what was used for v1.0.4 (instead of all changes inclusive from then current main of metaschema repo during v1.0.4 release of OSCAL to current develop commit) in usnistgov/metaschema#341. Replaying commits appears to not be sufficient, as not just file relocation occurred, but file renaming. If we cannot do it with this low-lift approach, we will move this into the next sprint and adjust accordingly.

@aj-stein-nist
Copy link
Contributor

After consultation with Dave last night, it seems we may move up to develop sub-module and test that first, or we will have to opt for messier git surgery with the cherry-picking than we thought. More to follow.

@aj-stein-nist
Copy link
Contributor

I am driving this one so I owe a status update: Dmitry and I worked on it but I held it up working on other issues before we move up the module. Working #1726 means plotting a way forward and acting on it is hard until that is out of the way (to re-run the reference docs and website model pieces will fail, and are very relevant to this work). More to follow!

@aj-stein-nist
Copy link
Contributor

I have been working on this again now that #1726 is out of the way, will need to review the proper integration of the Metaschema develop branch or usnistgov/metaschema#341. More to follow.

@aj-stein-nist
Copy link
Contributor

A.J. will make an effort to share a BlueJeans link to let others pair when he works on this today and other following days in the sprint.

aj-stein-nist added a commit to aj-stein-nist/OSCAL-forked that referenced this issue Apr 3, 2023
aj-stein-nist added a commit to usnistgov/oscal-content that referenced this issue Apr 4, 2023
aj-stein-nist added a commit to usnistgov/oscal-content that referenced this issue Apr 4, 2023
aj-stein-nist added a commit to usnistgov/oscal-content that referenced this issue Apr 4, 2023
@aj-stein-nist
Copy link
Contributor

Working through this and will likely draft a pre-release for review and public comment soon as things look good. But in the meantime, I consulted with Dave and other team members (namely myself): there was not checklist for QCing a release branch before tag and how to work that out, so I am starting that.

https://github.com/usnistgov/OSCAL/wiki/OSCAL-Release-Checklist

@aj-stein-nist aj-stein-nist moved this from Needs Triage to Volunteer Needed in Issue Triage Apr 11, 2023
@aj-stein-nist aj-stein-nist moved this from Volunteer Needed to Assigned to Developer in Issue Triage Apr 11, 2023
aj-stein-nist added a commit to usnistgov/oscal-content that referenced this issue Apr 12, 2023
aj-stein-nist added a commit that referenced this issue Apr 14, 2023
* Update metaschema for current datatype regexes for #1703.

* Back-port Metaschema XSD relocation.

As part of the OSCAL build process, we validate the structure of the
metaschema definitions of the OSCAL models themselves against the schema
of Metaschema definitions XSD from the metaschema repo. Recently, a path
update was fixed in the OSCAL develop branch but not in the release-1.0
branch or main because these changes occurred after 1.0.4 publication.
The files were relocated and reorganized. This adjustment must be made
to account for that.

Pull release/main back into develop should be minor and a rebase may not
be necessary. If so, we will adjust accordingly. In develop, see This
commit:

84d2d46

* Workaround maven.restlet.org service cert expiry. (#1602)

This is pulling over workaround as reported in usnistgov/oscal-content#139 with the WIP workaround submodule.

usnistgov/oscal-content@26f0fe1

* Update metaschema submodule again.
@aj-stein-nist
Copy link
Contributor

I have some final testing to work on that I had not wrapped up on Monday. Regrettably, moving this up to Sprint 67.

@aj-stein-nist
Copy link
Contributor

I am in the process of determining if it will be a problem to overwrite the develop branch model reference docs documentation before and after a v1.0.5 release @AleJo2995 pointed out it will be a problem before (I picked up on this Friday/Monday during testing; thanks for reminding me again, kind Internet person). But I am still not sure I have to correct the release-1.0 tag overwrite consistently with the workflows as-is or because I prepped a hotfix release incorrectly. More to follo

@aj-stein-nist
Copy link
Contributor

I guess there is nothing stopping me from just 1) manually triggering the generate-model-documentation.sh off of develop and have it commit the updated model ref docs back to main, then 2) run the website rebuild from main -> nist-pages.

  1. https://github.com/usnistgov/OSCAL/actions/runs/4737438341
  2. https://github.com/usnistgov/OSCAL/actions/runs/4737496625

@AleJo2995 I had to have a think and realize this is the a quick way to correct the issue and work around the last commit from the release branch. So simple, why didn't I think of those steps in order before!? I will be able to update this comment with a result in a minute.

@aj-stein-nist
Copy link
Contributor

Pre-release complete. Follow-on communication about the pre-release window will need to happen and it will later turn into a full release (barring any bug reports on this hotfix), but that is outside the scope of the fix and this issue can now be officially closed.

Issue Triage automation moved this from Assigned to Developer to Done Apr 19, 2023
@oscalbuilder oscalbuilder removed this from Done in Issue Triage Apr 19, 2023
@aj-stein-nist aj-stein-nist added this to the v1.0.6 milestone Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

When branches are created from issues, their pull requests are automatically linked.

5 participants