From 96a3ee98c6e13cd45315ccef6887b64f5ea27c20 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Tue, 27 Jul 2021 10:53:47 +0200 Subject: [PATCH] Removed default of `CLICON_RESTCONF_INSTALLDIR` * The default behaviour is changed to use the config $(sbindir) to locate `clixon_restconf` when starting restconf internally --- CHANGELOG.md | 12 ++++++++++- apps/backend/Makefile.in | 4 +++- apps/backend/backend_plugin_restconf.c | 15 +++++++++---- test/test_restconf_internal.sh | 3 --- test/test_restconf_internal_usecases.sh | 3 --- yang/clixon/clixon-config@2021-07-11.yang | 26 ++++++++++++++++------- 6 files changed, 43 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c89efc8..ed445805 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,11 +34,21 @@ Expected: September, 2021 ### C/CLI-API changes on existing features +### New features -Developers may need to change their code +* Restconf YANG PATCH according to RFC 8072 + * Experimental: enable by setting YANG_PATCH in include/clixon_custom.h + * Thanks to Alan Yaniger for providing this patch + +### API changes on existing protocol/config features + +Users may have to change how they access the system * Native Restconf is now default, not fcgi/nginx * That is, to configure with fcgi, you need to explicitly configure: `--with-restconf=fcgi` +* New clixon-config@2021-07-11.yang revision + * Removed default of `CLICON_RESTCONF_INSTALLDIR` + * The default behaviour is changed to use the config $(sbindir) to locate `clixon_restconf` when starting restconf internally ### Corrected Bugs diff --git a/apps/backend/Makefile.in b/apps/backend/Makefile.in index aecf95c7..4c55d23f 100644 --- a/apps/backend/Makefile.in +++ b/apps/backend/Makefile.in @@ -152,7 +152,9 @@ install-include: clixon_backend.h clixon_backend_handle.h clixon_backend_transac .SUFFIXES: .c .o .c.o: - $(CC) $(INCLUDES) $(CPPFLAGS) -D__PROGRAM__=\"$(APPL)\" $(CFLAGS) -c $< + # Note: CLIXON_CONFIG_SBINDIR is where clixon_restconf is believed to be installed, unless + # overruled by CLICON_RESTCONF_INSTALLDIR option + $(CC) $(INCLUDES) $(CPPFLAGS) -D__PROGRAM__=\"$(APPL)\" -DCLIXON_CONFIG_SBINDIR=\"$(sbindir)\" $(CFLAGS) -c $< # Just link test programs test.c : diff --git a/apps/backend/backend_plugin_restconf.c b/apps/backend/backend_plugin_restconf.c index 6b82417d..fe39c8c5 100644 --- a/apps/backend/backend_plugin_restconf.c +++ b/apps/backend/backend_plugin_restconf.c @@ -245,6 +245,7 @@ restconf_pseudo_process_control(clicon_handle h) int i; int nr; cbuf *cb = NULL; + char *dir = NULL; nr = 10; if ((argv = calloc(nr, sizeof(char *))) == NULL){ @@ -256,12 +257,18 @@ restconf_pseudo_process_control(clicon_handle h) clicon_err(OE_UNIX, errno, "cbuf_new"); goto done; } - /* CLICON_RESTCONF_INSTALLDIR is where we think clixon_restconf is installed - * Problem is where to define it? Now in config file, but maybe it should be in configure? - * Tried Makefile but didnt work on Docker since it was moved around. + /* Try to figure out where clixon_restconf is installed + * If config option CLICON_RESTCONF_INSTALLDIR is installed, use that. + * If not, use the Makefile * Use PATH? */ - cprintf(cb, "%s/clixon_restconf", clicon_option_str(h, "CLICON_RESTCONF_INSTALLDIR")); + if ((dir = clicon_option_str(h, "CLICON_RESTCONF_INSTALLDIR")) == NULL){ + if ((dir = CLIXON_CONFIG_SBINDIR) == NULL){ + clicon_err(OE_RESTCONF, EINVAL, "Both option CLICON_RESTCONF_INSTALLDIR and makefile constant CLIXON_CONFIG_SBINDIR are NULL which make sit not possible to know where clixon_restconf is installed(shouldnt happen)"); + goto done; + } + } + cprintf(cb, "%s/clixon_restconf", dir); argv[i++] = cbuf_get(cb); argv[i++] = "-f"; argv[i++] = clicon_option_str(h, "CLICON_CONFIGFILE"); diff --git a/test/test_restconf_internal.sh b/test/test_restconf_internal.sh index 41e449f5..4eb894f0 100755 --- a/test/test_restconf_internal.sh +++ b/test/test_restconf_internal.sh @@ -23,8 +23,6 @@ startupdb=$dir/startup_db RESTCONFDBG=$DBG RCPROTO=http # no ssl here -RESTCONFDIR=$(dirname $(which clixon_restconf)) - # log-destination in restconf xml: syslog or file : ${LOGDST:=syslog} # Set daemon command-line to -f @@ -54,7 +52,6 @@ cat < $cfg /usr/local/lib/$APPNAME/backend example_backend.so$ /usr/local/lib/$APPNAME/restconf - $RESTCONFDIR /usr/local/lib/$APPNAME/cli $APPNAME /usr/local/var/$APPNAME/$APPNAME.sock diff --git a/test/test_restconf_internal_usecases.sh b/test/test_restconf_internal_usecases.sh index 70a94c37..f57c7cf9 100755 --- a/test/test_restconf_internal_usecases.sh +++ b/test/test_restconf_internal_usecases.sh @@ -34,8 +34,6 @@ startupdb=$dir/startup_db RESTCONFDBG=$DBG RCPROTO=http # no ssl here -RESTCONFDIR=$(dirname $(which clixon_restconf)) - INVALIDADDR=251.1.1.1 # used by fourth usecase as invalid # log-destination in restconf xml: syslog or file @@ -68,7 +66,6 @@ cat < $cfg /usr/local/lib/$APPNAME/backend example_backend.so$ /usr/local/lib/$APPNAME/restconf - $RESTCONFDIR /usr/local/lib/$APPNAME/cli $APPNAME /usr/local/var/$APPNAME/$APPNAME.sock diff --git a/yang/clixon/clixon-config@2021-07-11.yang b/yang/clixon/clixon-config@2021-07-11.yang index dfc8d54b..f4d065c8 100644 --- a/yang/clixon/clixon-config@2021-07-11.yang +++ b/yang/clixon/clixon-config@2021-07-11.yang @@ -45,8 +45,13 @@ module clixon-config { revision 2021-07-11 { description - "Added option - CLICON_SYSTEM_CAPABILITIES"; + "Added option: + CLICON_SYSTEM_CAPABILITIES + Removed default value: + CLICON_RESTCONF_INSTALLDIR + Marked as obsolete: + CLICON_YANG_LIST_CHECK + (Will be) Released in Clixon 5.3"; } revision 2021-05-20 { description @@ -518,13 +523,18 @@ module clixon-config { } leaf CLICON_RESTCONF_INSTALLDIR { type string; - default "/usr/local/sbin"; description - "Path to dir of clixon-restconf daemon binary as used by backend if started internally - Discussion: Somewhat problematic to have it as run time option. It may think it - should be known at configure or install time, but for example the main docker - installation moves the binaries, and this may be true elsewehere too. - Maybe one could locate it via PATHs search"; + "If set, path to dir of clixon-restconf daemon binary as used by backend if + started internally (run-time). + If this path is not set, clixon_restconf will be looked for according to + configured installdir: $(sbindir) (install-time) + Since programs can be moved around at install/cross-compile time the installed + dir may be difficult to know at install time, which is the reason why + CLICON_RESTCONF_INSTALLDIR exists, in order to override the Makefile + installdir. + Note on the installdir, DESTDIR is not included since according to man pages: + by specifying DESTDIR should not change the operation of the software in + any way, so its value should not be included in any file contents. "; } leaf CLICON_RESTCONF_STARTUP_DONTUPDATE { type boolean;