Modularize YAML and JSON loading and add YAML::XS support #6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This makes it possible to specify the exact module to use, which the user might rely on, instead of relying on whatever order pkwalify decides to use, which might not give reliable results depending on what modules are currently installed.
sipwise-jenkins
pushed a commit
to sipwise/ngcpcfg
that referenced
this pull request
Apr 9, 2019
NGCP uses 'YAML::XS' as only the library to work with YAML files.
Meanwhile pkwalify has a long fallback list:
> @try_order = ('YAML::XS', 'YAML::Syck', 'YAML', 'JSON::XS', 'JSON');
It may hide some issues in validation schema (used by 'ngcpcfg check')
and bypass the testing on Gerrit reviews, but failed to 'ngcpcfg apply'
on the customer's system.
Example:
1) cfg_schema.git had a bug in validation schema committed long time ago
in mr6.1+ , commit b9d69e0b33d316c04d8b70536f1e166fec9a67ac.
The commit passed review and all internal testing:
> https://gerrit.mgm.sipwise.com/#/c/17825/ ->
> https://repoapi.mgm.sipwise.com/job/cfg-schema-ce-validation-docker/849/console.txt
> ...
> + ngcpcfg --validate check
> 2017-12-20 14:34:37: yml configs were validated successfully
The bug was simple, the space was missing in schema:
> "rtcp_logging_facility":{ type: str, required: yes }
The library 'YAML::XS' was not able to load the YAML file.
Meanwhile the library 'YAML' was able to load it and pkwalify happily reported us
"yml configs were validated successfully".
2) Later another developer has noticed the problem with YAML schema here
and fixed it in trunk, commit 4636eadb78e5e91dd6d93378a42a8df2ed84838a.
> - "rtcp_logging_facility":{ type: str, required: yes }
> + "rtcp_logging_facility": { type: str, required: yes }
Unfortunately the fix has been applied in trunk only.
3) Later the third developer committed new code which was valid
for 'YAML::XS' but invalid for 'YAML' library (which is fine, as NGCP uses 'YAML::XS').
Since in trunk pkwalify used 'YAML::XS' the changes passed all the testing:
> commit a79e54fb9fbdf0db61b4dfb9c52e572795e854af
> https://gerrit.mgm.sipwise.com/#/c/28486/ ->
> https://repoapi.mgm.sipwise.com/job/cfg-schema-ce-validation-docker/1987/console.txt
> ...
> 2019-04-05 17:20:08 06828d6f31f5: yml configs were validated successfully
4) Later the third developer backported the commit a79e54fb9fbdf0db
to the previous release mr6.5 (where commit 4636eadb78e was missing).
And nightly upgrade tests has failed on mr6.4->mr6.5 upgrade saying:
> 2019-04-09 04:06:27 sp1: Error: Invalid schema detected for /etc/ngcp-config/config.yml
> Cannot parse </usr/share/ngcp-cfg-schema/validate/config.yml>. Cumulated errors:
> YAML::XS::Load Error: The problem:
> did not find expected key
It happens because pkwalify was not able to load config.yml with any libraries it support.
We have to force pkwalify to use only the library NGCP is using 'YAML::XS'.
This is a commit about it.
Hopefully all the necessary changes have been done by Guillem 2 years ago,
and were uploaded upstream (and even available in default Debian buster):
> eserte/p5-Kwalify#6
Thanks to Guillem Jover for the library fix here and for the help with tracing this issue.
Change-Id: Idbd46b4048a03b5b01a0280a6dcd50406b4222dc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This merge adds a new -m option to pkwalify so that one can explicitly request which parser module to use. And also adds support for YAML::XS.
This should fix #3.