VTY: implicit node exit by de-indenting, not parent lookup

Note: This will break users' config files if they do not use consistent
indenting. (see below for a definition of "consistent".)

When reading VTY commands from a file, use indenting as means to implicitly
exit child nodes. Do not look for commands in the parent node implicitly.

The VTY so far implies 'exit' commands if a VTY line cannot be parsed on the
current node, but succeeds on the parent node. That is the mechanism by which
our VTY config files do not need 'exit' at the end of each child node.

We've hit problems with this in the following scenarios, which will show
improved user experience after this patch:

*) When both a parent and its child node have commands with identical names:

  cs7 instace 0
   point-code 1.2.3
   sccp-address osmo-msc
    point-code 0.0.1

If I put the parent's command below the child, it is still interpreted in the
context of the child node:

  cs7 instace 0
   sccp-address osmo-msc
    point-code 0.0.1
   point-code 1.2.3

Though the indenting lets me assume I am setting the cs7 instance's global PC
to 1.2.3, I'm actually overwriting osmo-msc's PC with 1.2.3 and discarding the
0.0.1.

*) When a software change moves a VTY command from a child to a parent. Say
'timezone' moved from 'bts' to 'network' level:

  network
   timezone 1 2

Say a user still has an old config file with 'timezone' on the child level:

  network
   bts 0
    timezone 1 2
    trx 0

The user would expect an error message that 'timezone' is invalid on the 'bts'
level. Instead, the VTY finds the parent node's 'timezone', steps out of 'bts'
to the 'network' level, and instead says that the 'trx' command does not exist.

Format:

Consistent means that two adjacent indenting lines have the exact
same indenting characters for the common length:

Weird mix if you ask me, but correct and consistent:

  ROOT
  <space>PARENT
  <space><tab><space>CHILD
  <space><tab><space><tab><tab>GRANDCHILD
  <space><tab><space><tab><tab>GRANDCHILD2
  <space>SIBLING

Inconsistent:

  ROOT
  <space>PARENT
  <tab><space>CHILD
  <space><space><tab>GRANDCHILD
  <space><tab><tab>GRANDCHILD2
  <tab>SIBLING

Also, when going back to a parent level, the exact same indenting must be used
as before in that node:

Incorrect:

  ROOT
  <tab>PARENT
  <tab><tab><tab>CHILD
  <tab><tab>SIBLING

As not really intended side effect, it is also permitted to indent the entire
file starting from the root level. We could guard against it but there's no
harm:

Correct and consistent:

  <tab>ROOT
  <tab><tab>PARENT
  <tab><tab><tab><tab>CHILD
  <tab><tab>SIBLING

Implementation:

Track parent nodes state: whenever a command enters a child node, push a parent
node onto an llist to remember the exact indentation characters used for that
level.

As soon as the first line on a child node is parsed, remember this new
indentation (which must have a longer strlen() than its parent level) to apply
to all remaining child siblings and grandchildren.

If the amount of spaces that indent a following VTY command are less than this
expected indentation, call vty_go_parent() until it matches up.

At any level, if the common length of indentation characters mismatch, abort
parsing in error.

Transitions to child node are spread across VTY implementations and are hard to
change. But transitions to the parent node are all handled by vty_go_parent().
By popping a parent from the list of parents in vty_go_parent(), we can also
detect that a command has changed the node without changing the parent, hence
it must have stepped into a child node, and we can push a parent frame.

The behavior on the interactive telnet VTY remains unchanged.

Change-Id: I24cbb3f6de111f2d31110c3c484c066f1153aac9

14 files changed, 95 insertions, 1 deletions

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 37378fbc..8935bf72 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -217,7 +217,18 @@ EXTRA_DIST = testsuite.at $(srcdir)/package.m4 $(TESTSUITE)		\
              logging/logging_test.ok logging/logging_test.err		\
              fr/fr_test.ok loggingrb/logging_test.ok			\
              loggingrb/logging_test.err	strrb/strrb_test.ok		\
-	     vty/vty_test.ok comp128/comp128_test.ok			\
+	     vty/vty_test.ok \
+	     vty/fail_not_de-indented.cfg \
+	     vty/fail_tabs_and_spaces.cfg \
+	     vty/fail_too_much_indent.cfg \
+	     vty/ok.cfg \
+	     vty/ok_ignore_blank.cfg \
+	     vty/ok_ignore_comment.cfg \
+	     vty/ok_indented_root.cfg \
+	     vty/ok_more_spaces.cfg \
+	     vty/ok_tabs_and_spaces.cfg \
+	     vty/ok_tabs.cfg \
+	     comp128/comp128_test.ok					\
 	     utils/utils_test.ok stats/stats_test.ok			\
 	     bitvec/bitvec_test.ok msgb/msgb_test.ok bits/bitcomp_test.ok \
 	     sim/sim_test.ok tlv/tlv_test.ok abis/abis_test.ok		\
diff --git a/tests/testsuite.at b/tests/testsuite.at
index f148cf5a..1954e66b 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -174,6 +174,7 @@ AT_CLEANUP
 AT_SETUP([vty])
 AT_KEYWORDS([vty])
 cat $abs_srcdir/vty/vty_test.ok > expout
+cp $abs_srcdir/vty/*.cfg .
 AT_CHECK([$abs_top_builddir/tests/vty/vty_test], [0], [expout], [ignore])
 AT_CLEANUP
 
diff --git a/tests/vty/fail_not_de-indented.cfg b/tests/vty/fail_not_de-indented.cfg
new file mode 100644
index 00000000..5af833da
--- /dev/null
+++ b/tests/vty/fail_not_de-indented.cfg
@@ -0,0 +1,3 @@
+line vty
+ no login
+ log stderr
diff --git a/tests/vty/fail_tabs_and_spaces.cfg b/tests/vty/fail_tabs_and_spaces.cfg
new file mode 100644
index 00000000..fa6ce059
--- /dev/null
+++ b/tests/vty/fail_tabs_and_spaces.cfg
@@ -0,0 +1,4 @@
+line vty
+ 		no login
+	 	no login
+log stderr
diff --git a/tests/vty/fail_too_much_indent.cfg b/tests/vty/fail_too_much_indent.cfg
new file mode 100644
index 00000000..bacb3e1e
--- /dev/null
+++ b/tests/vty/fail_too_much_indent.cfg
@@ -0,0 +1,3 @@
+line vty
+  no login
+ log stderr
diff --git a/tests/vty/ok.cfg b/tests/vty/ok.cfg
new file mode 100644
index 00000000..d5ef23e4
--- /dev/null
+++ b/tests/vty/ok.cfg
@@ -0,0 +1,3 @@
+line vty
+ no login
+log stderr
diff --git a/tests/vty/ok_ignore_blank.cfg b/tests/vty/ok_ignore_blank.cfg
new file mode 100644
index 00000000..d16ff64e
--- /dev/null
+++ b/tests/vty/ok_ignore_blank.cfg
@@ -0,0 +1,7 @@
+line vty
+
+  no login
+
+  no login
+
+log stderr
diff --git a/tests/vty/ok_ignore_comment.cfg b/tests/vty/ok_ignore_comment.cfg
new file mode 100644
index 00000000..5813fc7c
--- /dev/null
+++ b/tests/vty/ok_ignore_comment.cfg
@@ -0,0 +1,7 @@
+line vty
+  ! comment
+  no login
+! comment
+  no login
+     ! comment
+log stderr
diff --git a/tests/vty/ok_indented_root.cfg b/tests/vty/ok_indented_root.cfg
new file mode 100644
index 00000000..313c6742
--- /dev/null
+++ b/tests/vty/ok_indented_root.cfg
@@ -0,0 +1,3 @@
+	line vty
+	 no login
+	log stderr
diff --git a/tests/vty/ok_more_spaces.cfg b/tests/vty/ok_more_spaces.cfg
new file mode 100644
index 00000000..b66a9c21
--- /dev/null
+++ b/tests/vty/ok_more_spaces.cfg
@@ -0,0 +1,4 @@
+line vty
+  no login
+  no login
+log stderr
diff --git a/tests/vty/ok_tabs.cfg b/tests/vty/ok_tabs.cfg
new file mode 100644
index 00000000..e94609b7
--- /dev/null
+++ b/tests/vty/ok_tabs.cfg
@@ -0,0 +1,4 @@
+line vty
+	no login
+	no login
+log stderr
diff --git a/tests/vty/ok_tabs_and_spaces.cfg b/tests/vty/ok_tabs_and_spaces.cfg
new file mode 100644
index 00000000..2049b732
--- /dev/null
+++ b/tests/vty/ok_tabs_and_spaces.cfg
@@ -0,0 +1,4 @@
+line vty
+	  no login
+	  no login
+log stderr
diff --git a/tests/vty/vty_test.c b/tests/vty/vty_test.c
index d84bf419..eba9995c 100644
--- a/tests/vty/vty_test.c
+++ b/tests/vty/vty_test.c
@@ -19,6 +19,7 @@
 
 #include <stdio.h>
 #include <string.h>
+#include <errno.h>
 
 #include <sys/types.h>
 #include <sys/socket.h>
@@ -288,6 +289,15 @@ static void test_stats_vty(void)
 	destroy_test_vty(&test, vty);
 }
 
+void test_exit_by_indent(const char *fname, int expect_rc)
+{
+	int rc;
+	printf("reading file %s, expecting rc=%d\n", fname, expect_rc);
+	rc = vty_read_config_file(fname, NULL);
+	printf("got rc=%d\n", rc);
+	OSMO_ASSERT(rc == expect_rc);
+}
+
 int main(int argc, char **argv)
 {
 	struct vty_app_info vty_info = {
@@ -322,6 +332,16 @@ int main(int argc, char **argv)
 	test_cmd_string_from_valstr();
 	test_node_tree_structure();
 	test_stats_vty();
+	test_exit_by_indent("ok.cfg", 0);
+	test_exit_by_indent("ok_more_spaces.cfg", 0);
+	test_exit_by_indent("ok_tabs.cfg", 0);
+	test_exit_by_indent("ok_tabs_and_spaces.cfg", 0);
+	test_exit_by_indent("ok_ignore_comment.cfg", 0);
+	test_exit_by_indent("ok_ignore_blank.cfg", 0);
+	test_exit_by_indent("fail_not_de-indented.cfg", -EINVAL);
+	test_exit_by_indent("fail_too_much_indent.cfg", -EINVAL);
+	test_exit_by_indent("fail_tabs_and_spaces.cfg", -EINVAL);
+	test_exit_by_indent("ok_indented_root.cfg", 0);
 
 	/* Leak check */
 	OSMO_ASSERT(talloc_total_blocks(stats_ctx) == 1);
diff --git a/tests/vty/vty_test.ok b/tests/vty/vty_test.ok
index 2b6d5a67..b2df1a11 100644
--- a/tests/vty/vty_test.ok
+++ b/tests/vty/vty_test.ok
@@ -108,4 +108,24 @@ Going to execute 'no stats reporter log'
 Returned: 0, Current node: 4 '%s(config)# '
 Going to execute 'no stats reporter statsd'
 Returned: 0, Current node: 4 '%s(config)# '
+reading file ok.cfg, expecting rc=0
+got rc=0
+reading file ok_more_spaces.cfg, expecting rc=0
+got rc=0
+reading file ok_tabs.cfg, expecting rc=0
+got rc=0
+reading file ok_tabs_and_spaces.cfg, expecting rc=0
+got rc=0
+reading file ok_ignore_comment.cfg, expecting rc=0
+got rc=0
+reading file ok_ignore_blank.cfg, expecting rc=0
+got rc=0
+reading file fail_not_de-indented.cfg, expecting rc=-22
+got rc=-22
+reading file fail_too_much_indent.cfg, expecting rc=-22
+got rc=-22
+reading file fail_tabs_and_spaces.cfg, expecting rc=-22
+got rc=-22
+reading file ok_indented_root.cfg, expecting rc=0
+got rc=0
 All tests passed