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

Schema Ambiguity Error with openconfig-system re: NTP #334

Open
pheller opened this issue Jun 5, 2022 · 6 comments
Open

Schema Ambiguity Error with openconfig-system re: NTP #334

pheller opened this issue Jun 5, 2022 · 6 comments
Labels

Comments

@pheller
Copy link
Contributor

pheller commented Jun 5, 2022

vagrant@ubuntu1804.localdomain / # set system ntp servers server 1.2.3.4
CLI syntax error: "set system ntp servers server 1.2.3.4" is ambiguous

Using a clixon.xml with the following YANG configuration:

  <CLICON_YANG_DIR>/usr/local/share/clixon</CLICON_YANG_DIR>
  <CLICON_YANG_DIR>/usr/local/share/openconfig/public/release/models</CLICON_YANG_DIR>
  <CLICON_YANG_MODULE_MAIN>openconfig-system</CLICON_YANG_MODULE_MAIN>

The openconfig-system revision I'm using is 2021-07-20, and I do not see anything of interest in the YANG file itself.

Sure enough, when I print the parse trees, there is a duplicate:

vagrant@ubuntu1804.localdomain / # set system ntp servers server 1.2.3.4
<address>, act-lastkey;{
   config, act-container;{
      address, ac-leaf, act-leafconst;{
         <address>, ac-leaf, act-leafvar;
         <address>, ac-leaf, act-leafvar;
         <address>, ac-leaf, act-leafvar;
         <address>, ac-leaf, act-leafvar;
      }
      association-type, ac-leaf, act-leafconst;{
         (SERVER|PEER|POOL), ac-leaf, act-leafvar;
      }
      iburst, ac-leaf, act-leafconst;{
         <iburst>, ac-leaf, act-leafvar;
         <iburst>, ac-leaf, act-leafvar;
      }
      port, ac-leaf, act-leafconst;{
         <port>, ac-leaf, act-leafvar;
         <port>, ac-leaf, act-leafvar;
      }
      prefer, ac-leaf, act-leafconst;{
         <prefer>, ac-leaf, act-leafvar;
         <prefer>, ac-leaf, act-leafvar;
      }
      version, ac-leaf, act-leafconst;{
         <version>, ac-leaf, act-leafvar;
         <version>, ac-leaf, act-leafvar;
      }
   }
}
<address>, act-lastkey;{
   config, act-container;{
      address, ac-leaf, act-leafconst;{
         <address>, ac-leaf, act-leafvar;
         <address>, ac-leaf, act-leafvar;
         <address>, ac-leaf, act-leafvar;
         <address>, ac-leaf, act-leafvar;
      }
      association-type, ac-leaf, act-leafconst;{
         (SERVER|PEER|POOL), ac-leaf, act-leafvar;
      }
      iburst, ac-leaf, act-leafconst;{
         <iburst>, ac-leaf, act-leafvar;
         <iburst>, ac-leaf, act-leafvar;
      }
      port, ac-leaf, act-leafconst;{
         <port>, ac-leaf, act-leafvar;
         <port>, ac-leaf, act-leafvar;
      }
      prefer, ac-leaf, act-leafconst;{
         <prefer>, ac-leaf, act-leafvar;
         <prefer>, ac-leaf, act-leafvar;
      }
      version, ac-leaf, act-leafconst;{
         <version>, ac-leaf, act-leafvar;
         <version>, ac-leaf, act-leafvar;
      }
   }
}
  <cr>

However, when I debug clixon_cli on startup, the autocli does not seem to reveal any duplication.

@olofhagsand
Copy link
Member

This is a topic that comes back now and then, see eg #330, which changed the behavior in 5.7.
A temporary workaround is to make the following patch, which restores the preference setting to 5.6 status, but installing it in master would re-trigger #330:

diff --git a/apps/cli/cli_main.c b/apps/cli/cli_main.c
index f199799e..10bb625f 100644
--- a/apps/cli/cli_main.c
+++ b/apps/cli/cli_main.c
@@ -787,7 +787,7 @@ main(int    argc,
      * It used to be 1 in Clixon. But see eg https://github.com/clicon/clixon/issues/330
      * There may be cases where there will be "ambiguous".
      */
-    cligen_preference_mode_set(cli_cligen(h), 0);
+    cligen_preference_mode_set(cli_cligen(h), 1);
 
     /* Call start function in all plugins before we go interactive 
      */

Therefore, more work is needed to properly resolve it. Until then, please use the work-around.

@pheller
Copy link
Contributor Author

pheller commented Jun 24, 2022

Ok, I added this patch, but there is more to it...

vagrant@ubuntu2004 ~ % clixon_cli -f /usr/local/etc/example.xml
vagrant@ubuntu2004.localdomain /> show config
 cli                   json                  netconf               text                  xml
vagrant@ubuntu2004.localdomain /> show configuration cli
set system ntp config enabled false
set system ntp config enable-ntp-auth false
set system ssh-server config enable true
set system ssh-server config protocol-version V2
set system telnet-server config enable false
vagrant@ubuntu2004.localdomain /> set system ntp servers server 1.2.3.4 ?
  <cr>
vagrant@ubuntu2004.localdomain /> set system ntp servers server 1.2.3.4
vagrant@ubuntu2004.localdomain /> set system ntp servers server 1.2.3.4 ?
  <cr>
vagrant@ubuntu2004.localdomain /> show configuration cli
set system ntp config enabled false
set system ntp config enable-ntp-auth false
set system ntp servers server 1.2.3.4
set system ntp servers server 1.2.3.4 config port 123
set system ntp servers server 1.2.3.4 config version 4
set system ntp servers server 1.2.3.4 config association-type SERVER
set system ntp servers server 1.2.3.4 config iburst false
set system ntp servers server 1.2.3.4 config prefer false
set system ssh-server config enable true
set system ssh-server config protocol-version V2
set system telnet-server config enable false
vagrant@ubuntu2004.localdomain /> validate
Jun 24 18:38:44: Validate failed. Edit and try again or discard changes: application bad-element Leafref validation failed: No leaf 1.2.3.4 matching path ../config/address in openconfig-system.yang:740 <bad-element>1.2.3.4</bad-element>
CLI command error

So, while the patch does allow entering the command, the autogenerated CLI seems to omit the entire YANG hierarchy within the server container. Thus, I cannot create the requisite target of the leafref and therefore the error above.

@olofhagsand
Copy link
Member

olofhagsand commented Jul 11, 2022

I see the issue has something to do with the autocli code generated from a type in openconfig-inet-types.yang:

  typedef host {
    type union {
      type ip-address;
      type domain-name;
    }

where both ip-address and domain-name are patterns that somehow interfere.
And the issues happens when a string matches patterns in bith types, such as "1.2.3.4".

@olofhagsand
Copy link
Member

This resulted in a rather large CLIgen update, see
clicon/cligen@3042f44
which is described in Section 3.14 of the CLIgen tutorial.
In short, the problem was due to a "variable preference ambiguity" of two overlapping regexp:s in the openconfig spec.
The CLIgenupdate makes the ambiguity tie-break more powerful (and documented) which is used in Clixon CLI by using a a generic "non-terminal" tie-breaking mechanism.
Need to update CLIgen for this to apply.

@pheller
Copy link
Contributor Author

pheller commented Dec 30, 2022

I've tested this some more, and it's unclear to me how the YANG-based automatic CLI could make use of this in a fine-grained manner. I do see that cli_main.c sets preference mode to 2.

With mode set to 2, and absent any preference keywords in the generated cligen configuration, I will still get ambiguity errors given the OpenConfig NTP schema and the union of ip-address and domain-name, as you pointed out above.

vagrant@ubuntu1804.localdomain / # set system ntp servers server 1.2.3.4 config address 1.2.3.4
CLI syntax error: "set system ntp servers server 1.2.3.4 config address 1.2.3.4" is ambiguous
vagrant@ubuntu1804.localdomain / # edit system ntp servers server 1.2.3.4
CLI syntax error: "edit system ntp servers server 1.2.3.4" is ambiguous

Looking further into the RFC 7950, I find the following in 9.12:

When interpreting an XML encoding, a value is validated consecutively
against each member type, in the order they are specified in the
"type" statement, until a match is found. The type that matched will
be the type of the value for the node that was validated, and the
encoding is interpreted according to the rules for that type.

So, it seems that preference mode 1 might be more aligned with the RFC intention. I've adjusted cli_main.c to use mode 1, and operation is closer to what I'd expect, but still not quite right.

It seems, with this particular schema ambiguity, the parser does not show completions when the first alternative is taken, though it is an interior node in the schema:

vagrant@ubuntu1804.localdomain / # set system ntp servers server 1.2.3.4 ?
  <cr>

But, I can change the working point and see the completions:

vagrant@ubuntu1804.localdomain / # edit system ntp servers server 1.2.3.4
vagrant@ubuntu1804.localdomain server=1.2.3.4/ # set
  config                Configuration data for an NTP server.
vagrant@ubuntu1804.localdomain server=1.2.3.4/ # set config add
 <address>
vagrant@ubuntu1804.localdomain server=1.2.3.4/ # set config address 1.2.3.4
vagrant@ubuntu1804.localdomain server=1.2.3.4/ #

When providing a value that does not match the first alternative, but rather the second and final, things work as expected:

vagrant@ubuntu1804.localdomain / # set system ntp servers server foo ?
  <cr>
  config                Configuration data for an NTP server.
vagrant@ubuntu1804.localdomain / # set system ntp servers server foo config address foo
vagrant@ubuntu1804.localdomain / #

@olofhagsand olofhagsand reopened this Dec 31, 2022
@troglobit
Copy link
Contributor

Can confirm, I'm using the ietf-system yang model, which is very similar. Still see the "is ambiguous" output with latest master. With the cligen_preference_mode_set(..., 1); workaround it works better.

troglobit added a commit to kernelkit/clixon that referenced this issue Mar 14, 2023
We have the exact same problem with ntp servers in ietf-system as in
clicon#334

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants