LYD_OPT_NOEXTDEPS no longer exists, alternatives?

[bradh352]

The transition manual from v1 to v2 says:

Many validation flags were removed because they became obsolete (LYD_OPT_DESTRUCT, LYD_OPT_WHENAUTODEL, LYD_OPT_NOEXTDEPS, LYD_OPT_DATA_NO_YANGLIB, LYD_OPT_VAL_DIFF) or we consider them redundant (LYD_OPT_OBSOLETE, LYD_OPT_NOSIBLINGS, LYD_OPT_DATA_ADD_YANGLIB).

In sonic-mgmt-common, it is using LYD_OPT_NOEXTDEPS for partial tree validations. This validation still wants to check for things like valid data for constraints/patterns/length, min/max, mandatory, and so forth ... but does not want to check outside of the current module for leafrefs/must/when. The code does not have the full data tree available at this point when calling lyd_validate_all(), hence the use of this flag.

Is there an alternative approach that can be used? Or would you accept patches to reimplement a flag like LYD_VALIDATE_NOEXTDEPS to restore the prior behavior?

[michalvasko]

The flag LYD_PARSE_ONLY should be used in these cases. Using LYD_OPT_NOEXTDEPS in libyang v1 gave you no useful information (you do not know whether the data are valid or not) so it was removed and we do not wish it reimplemented.

[bradh352]

The flag LYD_PARSE_ONLY should be used in these cases. Using LYD_OPT_NOEXTDEPS in libyang v1 gave you no useful information (you do not know whether the data are valid or not) so it was removed and we do not wish it reimplemented.

PARSE flags aren't even in scope for the SONiC codepaths here aslyd_parse_data_*() isn't being called, instead lyd_new_inner(), lyd_new_list2(), lyd_new_term() are being directly called to build the data tree. I'm not the original author of this SONiC code and its definitely doing some things that I would view as questionable decisions in its use of libyang to begin with and is actually doing its own leafref/must/when validations using some form of lazy loading validation of prior config. This is used by gNMI and RESTCONF in SONiC where small partial updates are made to the running configuration. This solution was done I think because of the cost of the serialization between the SONiC config format and YANG JSON .... its a bit obtuse due to limitations of data formats REDIS can express.

That said, I'm sort of stuck in something that would probably be a multi-month effort to rework this code to be more sane, but I'm really just trying to port it from libyang1 to libyang3 which I thought would just be a day or two. I'm already a week in. This is actually the last project in SONiC using libyang, everything else was fairly easy to port once you got those other changes merged in we needed...

In case you're curious, this is the project being ported ... its written in Go, which adds to the complexity calling out to a C library ... sonic-net/sonic-mgmt-common#157

[michalvasko]

I have realized the functionality may have been implemented for a completely different purpose, try using LYD_VALIDATE_NOT_FINAL.

In case you're curious, this is the project being ported

Based on your description and all the previous issues, not really :)

[bradh352]

I have realized the functionality may have been implemented for a completely different purpose, try using LYD_VALIDATE_NOT_FINAL.

Tried that a while ago, the issue is it doesn't validate min/max. There may be other things it doesn't validate that were previously expected to be validated, that was just the first set of issues I hit.

In case you're curious, this is the project being ported

Based on your description and all the previous issues, not really :)

Good choice :)

[michalvasko]

Tried that a while ago, the issue is it doesn't validate min/max. There may be other things it doesn't validate that were previously expected to be validated, that was just the first set of issues I hit.

Actually, it did not work correctly then. Because if there was a list node with when referencing data in another module, for example, it would not be possible to correctly validate min/max restrictions of such a list without that foreign data. This new flag does exactly what is documented. In other words, only "static" or "local" checks that are not affected by any other nodes except the one being validated. Anything else may be validated incorrectly.

[bradh352]

Tried that a while ago, the issue is it doesn't validate min/max. There may be other things it doesn't validate that were previously expected to be validated, that was just the first set of issues I hit.

Actually, it did not work correctly then. Because if there was a list node with when referencing data in another module, for example, it would not be possible to correctly validate min/max restrictions of such a list without that foreign data. This new flag does exactly what is documented. In other words, only "static" or "local" checks that are not affected by any other nodes except the one being validated. Anything else may be validated incorrectly.

It really does sound like the whole partial / incremental validation concept is just flawed. I think I'm just going to have to see what the performance hit really is by serializing the entire config, merging in said incremental changes, then running the full validation.