From 85e443cd085bf363f5da56aed6d95c14a4183a36 Mon Sep 17 00:00:00 2001 From: stdweird Date: Fri, 12 Jun 2020 23:29:13 +0200 Subject: [PATCH] ncm-metaconfig: switch to no-shell and support "ok to fail" commands --- .../main/pan/components/metaconfig/schema.pan | 12 ++++++-- ncm-metaconfig/src/main/perl/metaconfig.pm | 30 ++++++++++++++----- ncm-metaconfig/src/test/perl/commands.t | 26 +++++++++++----- .../src/test/resources/simple_commands.pan | 8 ++--- 4 files changed, 53 insertions(+), 23 deletions(-) diff --git a/ncm-metaconfig/src/main/pan/components/metaconfig/schema.pan b/ncm-metaconfig/src/main/pan/components/metaconfig/schema.pan index d82f0e0ff0..10e7e92802 100644 --- a/ncm-metaconfig/src/main/pan/components/metaconfig/schema.pan +++ b/ncm-metaconfig/src/main/pan/components/metaconfig/schema.pan @@ -130,16 +130,22 @@ type ${project.artifactId}_config = { 'contents' : ${project.artifactId}_extension @{Predefined conversions from EDG::WP4::CCM::TextRender} 'convert' ? ${project.artifactId}_textrender_convert - @{Actions (i.e. named registered commands) to run on pre, validation and/or post step. - These are independent of daemons and are executed at time of processing each service.} + @{Actions (i.e. names found in /software/components/metadata/commands) to run when processing the service. + Refer to the ${project.artifactId}_actions type definition for the available hooks + for when a command may be run.} 'actions' ? ${project.artifactId}_actions } = dict(); +@{Command must start with absolute path to executable. + If the executable is preceded with a '-', it means that the a command failure + treated as success w.r.t. reporting and continuation.} +type ${project.artifactId}_command = string with match(SELF, '^-?/'); + type ${project.artifactId}_component = { include structure_component 'services' : ${project.artifactId}_config{} with valid_absolute_file_paths(SELF) @{Command registry for allowed actions, keys should be used as action value} - 'commands' ? string{} + 'commands' ? ${project.artifactId}_command{} } with { foreach (esc_fn; srv; SELF['services']) { if (exists(srv['actions'])) { diff --git a/ncm-metaconfig/src/main/perl/metaconfig.pm b/ncm-metaconfig/src/main/perl/metaconfig.pm index 466195c37e..fbfe1df38c 100644 --- a/ncm-metaconfig/src/main/perl/metaconfig.pm +++ b/ncm-metaconfig/src/main/perl/metaconfig.pm @@ -153,6 +153,7 @@ use LC::Exception; use EDG::WP4::CCM::TextRender 18.6.1; use CAF::Service; use EDG::WP4::CCM::Path qw(unescape); +use Text::ParseWords qw(old_shellwords); use Readonly; # Has to correspond to what is allowed in the schema @@ -236,11 +237,23 @@ sub run_shell_command my $command = $commands->{$type}; if ($command) { - $self->debug(1, "Going to run $type command '$command'"); + my $error_on_fail = 1; + if ($command =~ m/^-/) { + $command =~ s/^-//; + $error_on_fail = 0; + } + my $cmd_ref = [old_shellwords($command)]; + if (!$cmd_ref) { + $self->error("Failed to split '$command'"); + return; + } + + $self->verbose("Going to run $type command '$command' as ['", + join("','", @$cmd_ref), "']", + $error_on_fail ? "" : " and no error reporting on fail"); my ($err, $out); my %opts = ( - shell => 1, log => $self, stdout => \$out, stderr => \$err, @@ -252,16 +265,17 @@ sub run_shell_command $opts{keeps_state} = 1; }; - CAF::Process->new([$command], %opts)->execute(); + CAF::Process->new($cmd_ref, %opts)->execute(); my $ec = $?; - my $report = $ec ? 'error' : 'verbose'; + my $report = $ec && $error_on_fail ? 'error' : 'verbose'; $self->$report("run $type command '$command' ", - ($ec ? 'failed' : 'ok'), - ,": stdout '$out'\n stderr '$err'", - ($input ? "\n stdin '$input'" : "")); - return $ec ? undef : 1; + ($ec ? 'failed' : 'ok'), + ($error_on_fail ? '' : ' (no error on fail set)'), + ,": stdout '$out'\n stderr '$err'", + ($input ? "\n stdin '$input'" : "")); + return $ec && $error_on_fail ? undef : 1; } else { $self->debug(5, "No $type command to run"); return 1; diff --git a/ncm-metaconfig/src/test/perl/commands.t b/ncm-metaconfig/src/test/perl/commands.t index bcd36455b1..fe370ac02c 100644 --- a/ncm-metaconfig/src/test/perl/commands.t +++ b/ncm-metaconfig/src/test/perl/commands.t @@ -1,7 +1,7 @@ use strict; use warnings; use Test::More; -use Test::Quattor qw(commands); +use Test::Quattor qw(commands commands_fail_pre); use NCM::Component::metaconfig; use Test::MockModule; @@ -23,6 +23,7 @@ sub clean my $cmp = NCM::Component::metaconfig->new('metaconfig'); my $cfg = get_config_for_profile('commands'); +my $cfg_fail = get_config_for_profile('commands_fail_pre'); clean(); @@ -34,15 +35,15 @@ ok($fh, "A file was actually created"); isa_ok($fh, "CAF::FileWriter"); # if default sysv init service changes, also modify the aii_command negative test -ok(command_history_ok(['cmd pre', 'cmd test', 'cmd changed', 'cmd post', 'service foo restart']), +ok(command_history_ok(['/cmd pre', '/cmd test', '/cmd changed', '/cmd post', 'service foo restart']), "commands run and serivce foo restarted"); clean(); # changed failed, post does not run, daemons does run -set_command_status('cmd changed', 1); +set_command_status('/cmd changed', 1); $cmp->Configure($cfg); -ok(command_history_ok(['cmd pre', 'cmd test', 'cmd changed', 'service foo restart'], ['cmd post']), +ok(command_history_ok(['/cmd pre', '/cmd test', '/cmd changed', 'service foo restart'], ['/cmd post']), "commands except post run and serivce foo restarted"); my $fcnt = get_file_contents("/foo/bar"); ok($orig ne "$fcnt", "orig content is not same as current $fcnt"); @@ -50,9 +51,9 @@ ok($orig ne "$fcnt", "orig content is not same as current $fcnt"); clean(); # test failed, changed, post and daemons do not run, content unmodified -set_command_status('cmd test', 1); +set_command_status('/cmd test', 1); $cmp->Configure($cfg); -ok(command_history_ok(['cmd pre', 'cmd test'], ['cmd changed', 'service foo restart', 'cmd post']), +ok(command_history_ok(['/cmd pre', '/cmd test'], ['/cmd changed', 'service foo restart', '/cmd post']), "commands run except changed, post and no serivce foo restarted"); $fcnt = get_file_contents("/foo/bar"); # unmodified @@ -60,13 +61,22 @@ is($orig, "$fcnt", "orig content is same as current $fcnt on test failed"); clean(); # pre failed, test, changed, post and daemons do not run, content unmodified -set_command_status('cmd pre', 1); +set_command_status('/cmd pre', 1); $cmp->Configure($cfg); -ok(command_history_ok(['cmd pre'], ['cmd test', 'cmd changed', 'service foo restart', 'cmd post']), +ok(command_history_ok(['/cmd pre'], ['/cmd test', '/cmd changed', 'service foo restart', '/cmd post']), "commands run except test, changed, post and no serivce foo restarted"); $fcnt = get_file_contents("/foo/bar"); # unmodified is($orig, "$fcnt", "orig content is same as current $fcnt on pre failed"); +clean(); +# Rerun with cmd_pre that can fail. Test still fails as usual +$cmp->Configure($cfg_fail); +ok(command_history_ok(['/cmd pre', '/cmd test'], ['/cmd changed', 'service foo restart', '/cmd post']), + "commands pre fails, but is ok so still runs test, and no changed, post and no serivce foo restarted"); +$fcnt = get_file_contents("/foo/bar"); +# unmodified +is($orig, "$fcnt", "orig content is same as current $fcnt on pre failed but ok to fail and test fails"); + done_testing(); diff --git a/ncm-metaconfig/src/test/resources/simple_commands.pan b/ncm-metaconfig/src/test/resources/simple_commands.pan index 5a20d95e48..d42b6db017 100644 --- a/ncm-metaconfig/src/test/resources/simple_commands.pan +++ b/ncm-metaconfig/src/test/resources/simple_commands.pan @@ -3,10 +3,10 @@ unique template simple_commands; include 'simple'; prefix "/software/components/metaconfig/commands"; -"cmd_pre" = "cmd pre"; -"cmd_test" = "cmd test"; -"cmd_changed" = "cmd changed"; -"cmd_post" = "cmd post"; +"cmd_pre" = "/cmd pre"; +"cmd_test" = "/cmd test"; +"cmd_changed" = "/cmd changed"; +"cmd_post" = "/cmd post"; prefix "/software/components/metaconfig/services/{/foo/bar}/actions"; "pre" = "cmd_pre";