WIP: Extension validation for the YANG module versioning work#2446
WIP: Extension validation for the YANG module versioning work#2446xorrkaz wants to merge 8 commits intoCESNET:masterfrom
Conversation
Add the ietf-yang-semver and ietf-yang-revisions YANG modules along with their generated header files and test module copies. - ietf-yang-semver@2025-09-29: Defines version and recommended-min-version extensions - ietf-yang-revisions@2025-01-28: Defines non-backwards-compatible and recommended-min-date extensions
Add validation plugins for ietf-yang-semver and ietf-yang-revisions extensions: yang_semver.c: - Implements validation for ysv:version extension * Validates MAJOR.MINOR.PATCH format with optional modifiers * Ensures version uniqueness across module revisions * Checks for duplicate versions in same revision - Implements validation for ysv:recommended-min-version extension * Validates strict MAJOR.MINOR.PATCH format (no modifiers) * Ensures correct placement in import statements * Prevents duplicate declarations yang_revisions.c: - Implements validation for rev:non-backwards-compatible extension * Validates placement in revision statements * Prevents duplicate declarations - Implements validation for rev:recommended-min-date extension * Validates YYYY-MM-DD date format * Ensures correct placement in import statements * Prevents duplicate declarations
- Add external declarations for plugins_yang_semver and plugins_yang_revisions - Register both plugin sets during library initialization - Add source files to build system (CMakeLists.txt)
Split tests into separate modules for ietf-yang-semver and ietf-yang-revisions extensions: test_yang_semver.c: - Tests for ysv:version extension * Valid formats (basic, with modifiers, pre-release, metadata) * Invalid formats (wrong placement, malformed versions) * Version uniqueness across revisions - Tests for ysv:recommended-min-version extension * Valid MAJOR.MINOR.PATCH format * Invalid formats (with modifiers, pre-release) * Duplicate prevention test_yang_revisions.c: - Tests for rev:non-backwards-compatible extension * Valid usage in revision statements * Invalid placement and duplicates - Tests for rev:recommended-min-date extension * Valid date formats (YYYY-MM-DD) * Invalid date formats * Duplicate prevention Update CMakeLists.txt to register both test suites.
Add ietf-yang-semver and ietf-yang-revisions to the internal modules list so they are automatically available in all contexts. - Update context.c to include generated headers and module registrations - Update test_context.c to reflect new module count in test assertions
|
It seems you have understood the code perfectly and I see no major issues, just some minor ones. Not sure what additional changes you have in mind but I have way too much work at least until the IETF so I am not able to help. It also seems that the CI is broken but that is likely just some minor issue. |
This is an attempt to fix some out-of-bounds issues.
|
I think I licked the CI problem. Can you re-run the PR? And thanks for the quick look. |
|
Yes, it works now, only some formatting issue left. I can fix that, if you want. |
|
Should be all fixed. Give it another shot. I do want to give this some time to bake, though. I want more IETFers to look at it, and the YANG modules may rev prior to an RFC number being issued. |
| return LY_EVALID; | ||
| } | ||
|
|
||
| if ((year < 1000) || (year > 9999) || (month < 1) || (month > 12) || (day < 1) || (day > 31)) { |
There was a problem hiding this comment.
Should dates be verified to actually exist?
This rule would allow e.g. 2026-02-31.
RFC 7950 doesn't seem to give much guidance but points at
RFC 6991 which in turn points at RFC 3339 and ISO 8601.
There was a problem hiding this comment.
Good question. I considered this at the time with the code, but I don't know that other date validation is done, so I left it simpler. That said, a bad date has bitten YANG Catalog in the past. Let me think it over.
There was a problem hiding this comment.
Some internal revision date validation is performed, naturally. While it is not a public function, you can use it in an internal plugin.
4 participants
This is a WIP PR. An early review would be appreciated, but I want to see if others can bang on the code a bit, too.