From e6a04fbd952f6ad9311b42a2ccf0358ba8aeba24 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Mon, 20 Oct 2025 16:24:32 +0200 Subject: [PATCH] cli: add 'validate', or '-n', dry run to copy command This commit adds config file validation to the copy command, discussed in #373. Allowing users to test their config files before restoring a backup. The feature could also be used for the automatic rollback when downgrading to an earlier version of the OS. Fixes #373 Signed-off-by: Joachim Wiberg --- doc/cli/introduction.md | 10 +++++++- src/bin/copy.c | 35 +++++++++++++++++++++------- src/klish-plugin-infix/src/infix.c | 24 ++++++++----------- src/klish-plugin-infix/xml/infix.xml | 1 + 4 files changed, 47 insertions(+), 23 deletions(-) diff --git a/doc/cli/introduction.md b/doc/cli/introduction.md index 5ddd2347..a104f8a4 100644 --- a/doc/cli/introduction.md +++ b/doc/cli/introduction.md @@ -191,4 +191,12 @@ change that, e.g., breaks networking, it is trivial to revert back by: admin@host-12-34-56:/> copy startup-config running-config ``` -Or restarting the device. +Or restart the device, for example if the change to the configuration +caused you to lose contact with the system (it happens to the best of +us). The system will start up from the last "save gave". + +> **Tip:** when restoring a backup of a configuration, or having manually +> edited a config file, you can validate it using system's YANG models, +> it is *not* applied if validation is successful: +> +> `copy /media/backup/old.cfg running-config validate` diff --git a/src/bin/copy.c b/src/bin/copy.c index 8dc76414..db675c05 100644 --- a/src/bin/copy.c +++ b/src/bin/copy.c @@ -34,6 +34,8 @@ struct infix_ds infix_config[] = { static const char *prognm = "copy"; static int timeout; +static int dry_run; +static int quiet; /* @@ -180,11 +182,12 @@ static const char *infix_ds(const char *text, struct infix_ds **ds) /* * Load configuration from a file and replace running datastore. * This triggers all sysrepo change callbacks, unlike sr_copy_config(). + * In dry-run mode, only validates the configuration without applying. */ static int replace_running(sr_conn_ctx_t *conn, sr_session_ctx_t *sess, const char *file, int timeout) { - uint32_t flags = LYD_PARSE_NO_STATE | LYD_PARSE_ONLY; + uint32_t flags = LYD_PARSE_NO_STATE | LYD_PARSE_ONLY | LYD_PARSE_STORE_ONLY | LYD_PARSE_STRICT; struct lyd_node *data = NULL; const struct ly_ctx *ctx; LY_ERR err; @@ -199,18 +202,26 @@ static int replace_running(sr_conn_ctx_t *conn, sr_session_ctx_t *sess, return 1; } - /* Replace running config (triggers callbacks, takes ownership on success) */ - rc = sr_replace_config(sess, NULL, data, timeout * 1000); - if (rc) { - emsg(sess, ERRMSG "unable to replace configuration, err %d: %s\n", - rc, sr_strerror(rc)); + if (dry_run) { + if (!quiet) { + printf("Configuration validated, %s: OK\n", file); + fflush(stdout); + } lyd_free_all(data); + rc = 0; + } else { + /* Replace running config, assumes ownership of 'data' */ + rc = sr_replace_config(sess, NULL, data, timeout * 1000); + if (rc) { + emsg(sess, ERRMSG "unable to replace configuration, err %d: %s\n", + rc, sr_strerror(rc)); + lyd_free_all(data); + } } return rc; } - static int copy(const char *src, const char *dst, const char *remote_user) { struct infix_ds *srcds = NULL, *dstds = NULL; @@ -450,6 +461,8 @@ static int usage(int rc) "\n" "Options:\n" " -h This help text\n" + " -n Dry-run, validate configuration without applying\n" + " -q Quiet mode, suppress informational messages\n" " -u USER Username for remote commands, like scp\n" " -t SEEC Timeout for the operation, or default %d sec\n" " -v Show version\n", prognm, timeout); @@ -464,10 +477,16 @@ int main(int argc, char *argv[]) timeout = fgetint("/etc/default/confd", "=", "CONFD_TIMEOUT"); - while ((c = getopt(argc, argv, "ht:u:v")) != EOF) { + while ((c = getopt(argc, argv, "hnqt:u:v")) != EOF) { switch(c) { case 'h': return usage(0); + case 'n': + dry_run = 1; + break; + case 'q': + quiet = 1; + break; case 't': timeout = atoi(optarg); break; diff --git a/src/klish-plugin-infix/src/infix.c b/src/klish-plugin-infix/src/infix.c index 41c17df5..5ee0d0ed 100644 --- a/src/klish-plugin-infix/src/infix.c +++ b/src/klish-plugin-infix/src/infix.c @@ -28,7 +28,7 @@ const uint8_t kplugin_infix_major = 1; const uint8_t kplugin_infix_minor = 0; -static void cd_home(kcontext_t *ctx) +static const char *cd_home(kcontext_t *ctx) { const char *user = "root"; ksession_t *session; @@ -43,6 +43,8 @@ static void cd_home(kcontext_t *ctx) pw = getpwnam(user); chdir(pw->pw_dir); + + return user; } static int systemf(const char *fmt, ...) @@ -201,8 +203,8 @@ int infix_copy(kcontext_t *ctx) { kpargv_t *pargv = kcontext_pargv(ctx); const char *src, *dst; - const char *username; char user[256] = ""; + char validate[8] = ""; kparg_t *parg; src = kparg_value(kpargv_find(pargv, "src")); @@ -214,26 +216,20 @@ int infix_copy(kcontext_t *ctx) if (parg) snprintf(user, sizeof(user), "-u %s", kparg_value(parg)); - cd_home(ctx); - username = ksession_user(kcontext_session(ctx)); + parg = kpargv_find(pargv, "validate"); + if (parg) + strlcpy(validate, "-n", sizeof(validate)); - return systemf("sudo -u %s copy %s %s %s", username, user, src, dst); + /* Ensure we run the copy command as the logged-in user, not root (klishd) */ + return systemf("doas -u %s copy %s %s %s %s", cd_home(ctx), validate, user, src, dst); } int infix_shell(kcontext_t *ctx) { - const char *user = "root"; - ksession_t *session; + const char *user = cd_home(ctx); pid_t pid; int rc; - session = kcontext_session(ctx); - if (session) { - user = ksession_user(session); - if (!user) - user = "root"; - } - pid = fork(); if (pid == -1) return -1; diff --git a/src/klish-plugin-infix/xml/infix.xml b/src/klish-plugin-infix/xml/infix.xml index c40e249d..6e2137e3 100644 --- a/src/klish-plugin-infix/xml/infix.xml +++ b/src/klish-plugin-infix/xml/infix.xml @@ -188,6 +188,7 @@ +