How best to validate the keys of hashref arguments?

[cbeckley]

I have a subroutine in a module that uses Net::OpenSSH to run administration commands remotely. This sub gets its arguments through a hash ref.

sub ops_do_ssh_shell {
   my ($cmd) = @_;
#etc ...
}

sub ops_do_ssh_shell {
   my ($cmd) = @_;

   my $ssh;
   my $key_path = defined $cmd->{key} ? $cmd->{key} : $ENV{HOME} . '/.
+ssh/id_rsa';

   if ( ops_cmd_status($cmd) ) {
      $ssh = Net::OpenSSH->new($cmd->{host}, user => $cmd->{user}, key
+_path => $key_path);
      if ( $ssh->error ) {
         $cmd->{status} = 'failure';
         ($cmd->{ssh_retcode}, $cmd->{ssh_retmsg}) = (0 + $ssh->error,
+ '' . $ssh->error);
      }
   }

   if ( ops_cmd_status($cmd) ) {
      ($cmd->{output}, $cmd->{std_err}) = $ssh->capture2($cmd->{comman
+d});
      $cmd->{cmd_ret_code} = 0 + $ssh->error;
      if ( $ssh->error ) {
         $cmd->{status} = 'failure';
         $cmd->{cmd_ret_msg} = '' . $ssh->error;
      }
      chomp $cmd->{output};
      chomp $cmd->{std_err};
   }

   return $cmd;
}

And I invoke it like this:

my $cmd_status = ops_do_ssh_shell($cmd);

my @commands = (
   { name => 'command_name1',
     user => 'user1',
     host => 'server1.foo.com',
     command => 'rsync_command yadda yadda'
   },
   { name => 'command_name2',
     user => 'user2',
     host => 'server2.foo.com',
     command => 'rsync_command yadda yadda'
   }
);

for my $cmd (@commands) {
   if ($state->{status} eq 'success') {
      my $cmd_status = ops_do_ssh_shell($cmd);
      if ( (!defined $cmd_status->{cmd_ret_code}) or ($cmd_status->{cm
+d_ret_code} != 0) ) {
         $state->{status} = 'failure';
         $state->{stack_trace} = $cmd_status;
      }
   }
}

The keys in the $cmd hashref don't benefit from "use strict" and "use warnings". I've found Params::Check and Params::Validate, but is manual validation, even with help of these modules, the best way to achieve this "strictness"?

Or is my $cmd hash approaching the level of complexity that would benefit from implementing it as an object? Am I correct in thinking that would get the validation as a side effect?

Thank you for taking the time to look at this.

Thanks,
cbeckley

[haukex]

If I just want to check that no unknown args are passed, I'll sometimes write something like this:

use Carp;
my %FOO_KNOWN_ARGS = map {$_=>1} qw/ quz baz /;
sub foo {
     my %args = @_;
     $FOO_KNOWN_ARGS{$_} or croak "Invalid argument '$_'" for keys %ar
+gs;
     # ...
}
foo(quz=>1,baz=>1);
foo(quz=>1,baz=>1,bar=>1); # dies

Sometimes, to prevent typos, I'll use restricted hashes via Hash::Util:

use Hash::Util qw/lock_ref_keys/;
my $foo = {};
lock_ref_keys($foo, qw/ quz baz /);
$foo->{quz} = 1;
$foo->{bar} = 1; # dies

However, both of these solutions only cover validating the keys, not the values of the hashes.

For more advanced validation, I'd suggest Params::Validate.

Or, as you suggested, you can make a simple object, for this I'd recommend Moo over Moose, as the former is (with a few minor exceptions) a subset of the latter, and is more lightweight. You can implement constraints on the values manually using coderefs, or with something like Type::Tiny.

{   package Foo;
    use Moo;
    use Types::Standard qw/ Str Int /;
    use Type::Utils qw/ declare as where /;
    use namespace::clean;
    has foo => ( is=>'ro', required=>1,
        isa => sub { $_[0]=~/^bar\d+$/ or die "Invalid foo" } );
    has bar => ( is=>'ro', required=>1, isa => Int );
    my $Hostname = declare as Str, where { /^\w+(\.\w+)*$/ };
    has quz => ( is=>'ro', required=>1, isa => $Hostname );
    sub baz {
        my $self = shift;
        print "baz, my foo is ",$self->foo,"\n";
    }
}
my $foo = Foo->new(foo=>'bar123', bar=>4, quz=>'aa.bb.cc');
$foo->baz;
baz2($foo);
baz2({x=>1}); # dies

# possible alternative to Foo::baz
use Scalar::Util qw/blessed/;
sub baz2 {
    my $foo = shift;
    die "Not a Foo" unless blessed($foo) && $foo->isa('Foo');
    print "baz2, the foo is ",$foo->foo,"\n";
}

[cbeckley]

Wow, haukex, thank you. There's a lot of info in there. Restricted hashes from Hash::Util works beautifully and is exactly what I was looking for.

For the curious, I added a sub to my ssh module:

sub ops_new_cmd {
   my ($init_hash) = @_;
   my $new_cmd = {};
   lock_ref_keys($new_cmd, qw(name user host key command status ssh_re
+tcode ssh_retmsg output std_err cmd_ret_code cmd_ret_msg));
   for my $k (keys %$init_hash) { $new_cmd->{$k} = $init_hash->{$k}; }
   return $new_cmd
}

And then invoking it, from my initial example:

my @commands = (
   ops_new_cmd({ name => 'command_name1',
     user => 'user1',
     host => 'server1.foo.com',
     command => 'rsync_command yadda yadda'
   }),
   ops_new_cmd({ name => 'command_name2',
     user => 'user2',
     host => 'server2.foo.com',
     command => 'rsync_command yadda yadda'
   })
);

You are correct, however, I am going to need to add some validation to the values at some point. I'm leaning toward using OO. Thank you for the examples there as well.

Thanks,
cbeckley

[cbeckley]

Ah, forgot to ask, in the previous node, the line of code:

for my $k (keys %$init_hash) { $new_cmd->{$k} = $init_hash->{$k}; }

Is there a more idiomatic way to say that?

Thanks,
cbeckley

[huck]

if you know what keys are allowed you can check for ones that are not allowed

my %allowed = map ( $_ => 1 } qw/ name user host command/; 

for my $cmd (@commands) {
  my $badkeys=''; 
  for my $key (keys %$cmd) { 
    unless ($allowed {$key) {$badkeys.=' '.$k} 
    } 
  if ($badkeys) { 
    print 'skipping due to badkeys found:'.$badkeys."\n"; 
    next; 
    } 
  if ($state->{status} eq 'success') {
...

[johngg]

Firstly, it looks like you have a typo in the "unless" line, $k should perhaps be $key.

Secondly, using a scalar and concatenating spaces and bad keys onto a string seems a little awkward. Using an array instead and taking advantage of how it is interpolated inside double quotes seems more natural. I'm not sure why you use a mixture of single and double quotes along with concatenation in your print statement, just interpolate inside double quotes.

use strict;
use warnings;

use feature qw{ say };

my %allowed = map { $_ => 1 } qw{ name user host command };

my @commands = (
   {
      name    => q{Fred Bloggs},
      user    => q{fbloggs},
      host    => q{red},
      command => q{ls},,
   },
   {
      name     => q{Charlie Farley},
      user     => q{cfarley},
      host     => q{green},
      zone     => q{A},
      building => q{Chaucer},
      command  => q{ps},
   },
   );

for my $command ( @commands )
{
    my @badKeys = grep { not exists $allowed{ $_ } } keys %$command;
    say
       @badKeys
       ? qq{Command invalid: bad keys: @badKeys}
       :  q{Command OK};
}

The output.

Command OK
Command invalid: bad keys: zone building

I hope this is of interest.

[cbeckley]

Thank you huck and johngg! I had tried looping through the keys to verify they were what I was expecting, but map and grep are dramatic improvements.

Thanks,
cbeckley

[LanX]

I'm not sure which kind of parameter checking you want, maybe counting a hash slice is just enough?

@allowed_keys = qw/name user host command/; 
my %allowed;
@allowed{@allowed_keys} = delete @$cmd{@allowed_keys}

any remaining entries you get from keys %$cmd can't be allowed, an if %allowed has less then 4 entries you might need to throw an error.

Alternatively you can preset %allowed with default values in the second line, which are overwritten if present.

Hope this helps.

[nysus]

I am writing a program that does the same thing with Net::OpenSSH. I've created a OO role written in Moose that wraps Net::OpenSSH and I send commands through the objects that use the role. I call it with: WebServerRemote->new({ssh => 'me@198.168.1.2'}); Then I execute commands like this: $self->grab('ls -l') where "grab" is a wrapper for the capture command. Here's a rough skeleton of it:

package MyOpenSSH 0.000001;

use Carp;
use Moose::Role;
use Modern::Perl;
use Net::OpenSSH;
use Params::Validate;

has 'ssh' => (is => 'rw', isa => 'Net::OpenSSH', required => 1, lazy =
+> 0, handles => qr/.*/, );

sub BUILD {
  # do object validation stuff here
}

sub exec {
  my $self = shift;
  my $opts = shift;

  if (ref $opts eq 'HASH') {
    my $cmd = shift;
    $self->ssh->system($opts, $cmd) || croak 'Command failed: ' . $sel
+f->ssh->error;
  } else {
    my $cmd = $opts;
    $self->ssh->system($cmd, @_) || croak 'Command failed: ' . $self->
+ssh->error;
  }
}

# wrapper for capture method
# this has been edited to simplify and fix a bug since last post.
sub grab {
  my $self = shift;

  return $self->ssh->capture(@_) if !$self->ssh->error;
  croak ('ssh command failed');
}

DISCLAIMER: this code is still a work in progress and was written mostly as an experiment as a way for me to get more proficient with Moose and I haven't worked out some details with it. But maybe this will inspire some ideas for you.

[cbeckley]

Given how common the use of Perl is for system administration and further, that you and I are both trying to do the same thing, I'm surprised this isn't

Something That Has Been Done Before

I've done several searches on various permutations of "remote" "automated" "administration" "framework" and haven't turned up any examples of what I expect already exists in somebody's lovingly tended CPAN module.

I turned up https://www.amazon.com/Automating-System-Administration-Perl-Efficient/dp/059600639X/ref=sr_1_1?s=books&ie=UTF8&qid=1489687502&sr=1-1&keywords=perl+system+administration, naturally ...

But nothing that address fleet administration in a configurable fashion.

Does something like this already exist?

Thanks,
cbeckley

[nysus]

I have been asking kind of the same question recently. I was told no such animal exists. Read through that discussion on how one might change privileged files securely, though. It's interesting. As pointed out in that thread, there are beasts out there like webmin (which I forgot about years ago) but it is full of security holes and it's ill-advised to use it.

For now, I'm just kind of writing modules for use on my own personal home network to help me automate set up of websites on local machines with an eye toward eventually being able to automate tasks on a live server as a long term goal. Thanks for the link to the book. Maybe I'll check it out.