Unverified Commit 022d4934 authored by Francesc Guasch's avatar Francesc Guasch Committed by GitHub
Browse files

Refactor: display tls (#1545)

refactor: display TLS

* refactor(backend): fix duplicated process req
* feat: clean db leftovers
* detect properly duplicated port
* refactor: store id_vm + port
* refactor(test): check port conflict
* refactor(test): copy config for mock domains
* check enforce limits properly
* check better port conflicts
* fix(install): correct id_vm
* properly test TLS
* : store port and vm to check duplicates
* show displays TLS
* clean cache when desconnecting VMs
* test: allow link sizes attribute
* test(displays): check gone TLS display when migrating
* install: clean ports before create index
* test ports with rvd_back running
* make sure there is an n_order field
*test ports and unload nbd when testing spinoff
* install: clean duplicated indexes
* refactor(test): tet mojo in background
* refactor(frontend): hide display URL unless port
* vlc no TLS
* refactor(test): clean iptables after test
* refactor(ports): improved fix conflict
* refactor(ports): manage secondary displays
* refactor: remove display_file pre-generated
* cache connections only if writeable
* user may have been removed
* retry 10 times to solve conflict
parent 678fb012
......@@ -39,6 +39,6 @@ WriteMakefile(
},
test => {TESTS => 't/*.t t/*/*.t'},
clean => {FILES => ['t/.db', '/var/tmp/rvd_void'] }
clean => {FILES => ['t/.db', '/var/tmp/rvd_void','/var/tmp/node.lock','/var/tmp/fw.lock'] }
);
......@@ -189,7 +189,7 @@ sub _update_user_grants {
$sth->execute;
$sth->bind_columns(\$id);
while ($sth->fetch) {
my $user = Ravada::Auth::SQL->search_by_id($id);
my $user = Ravada::Auth::SQL->search_by_id($id) or confess "Unknown user id = $id";
next if $user->name() eq $USER_DAEMON_NAME;
my %grants = $user->grants();
......@@ -1082,6 +1082,10 @@ sub _add_indexes_generic($self) {
,domain_displays => [
"unique(id_domain,n_order)"
,"unique(id_domain,driver)"
,"unique(id_vm,port)"
]
,domain_ports => [
"unique(id_vm,public_port)"
]
,requests => [
"index(status,at_time)"
......@@ -1154,6 +1158,12 @@ sub _add_indexes_generic($self) {
$self->_clean_index_conflicts($table, $name);
print "+" if $FIRST_TIME_RUN;
if ($table eq 'domain_displays' && $name eq 'id_vm_port') {
my $sth_clean=$CONNECTOR->dbh->prepare(
"UPDATE domain_displays set port=NULL"
);
$sth_clean->execute;
}
my $sth = $CONNECTOR->dbh->prepare($sql);
$sth->execute();
}
......@@ -1474,9 +1484,7 @@ sub _sqlite_trigger($self, $dbh, $table,$field, $trigger) {
$dbh->do($sql);
}
sub _remove_field {
my $self = shift;
my ($table, $field ) = @_;
sub _remove_field($self, $table, $field) {
my $dbh = $CONNECTOR->dbh;
return if $CONNECTOR->dbh->{Driver}{Name} !~ /mysql/i;
......@@ -1486,7 +1494,7 @@ sub _remove_field {
$sth->finish;
return if !$row;
warn "INFO: removing $field to $table\n"
warn "INFO: removing $field from $table\n"
if !$FIRST_TIME_RUN && $0 !~ /\.t$/;
$dbh->do("alter table $table drop column $field");
......@@ -1566,12 +1574,14 @@ sub _sql_create_tables($self) {
domain_displays => {
id => 'integer NOT NULL PRIMARY KEY AUTO_INCREMENT'
,id_domain => 'integer NOT NULL references domains(id) on delete cascade'
,id_vm => 'int default null'
,port => 'char(5) DEFAULT NULL'
,ip => 'varchar(254)'
,listen_ip => 'varchar(254)'
,driver => 'char(40) not null'
,is_active => 'integer NOT NULL default 0'
,is_builtin => 'integer NOT NULL default 0'
,is_secondary => 'integer NOT NULL default 0'
,id_domain_port => 'integer DEFAULT NULL'
,n_order => 'integer NOT NULL'
,password => 'char(32)'
......@@ -1908,8 +1918,8 @@ sub _upgrade_tables {
$self->_upgrade_table('domains','autostart','int NOT NULL DEFAULT 0');
$self->_upgrade_table('domains','status','varchar(32) DEFAULT "shutdown"');
$self->_upgrade_table('domains','display','text');
#$self->_upgrade_table('domains','display_file','text DEFAULT NULL');
$self->_remove_field('domains','display_file');
$self->_upgrade_table('domains','info','TEXT DEFAULT NULL');
$self->_upgrade_table('domains','internal_id','varchar(64) DEFAULT NULL');
$self->_upgrade_table('domains','volatile_clones','int NOT NULL default 0');
......@@ -1950,10 +1960,14 @@ sub _upgrade_tables {
$self->_upgrade_table('volumes','name','char(200)');
$self->_upgrade_table('domain_displays', 'id_vm','int(1) DEFAULT NULL');
$self->_upgrade_table('domain_drivers_options','data', 'char(200) ');
$self->_upgrade_table('domain_ports', 'internal_ip','char(200)');
$self->_upgrade_table('domain_ports', 'restricted','int(1) DEFAULT 0');
$self->_upgrade_table('domain_ports', 'is_active','int(1) DEFAULT 0');
$self->_upgrade_table('domain_ports', 'is_secondary','int(1) DEFAULT 0');
$self->_upgrade_table('domain_ports', 'id_vm','int(1) DEFAULT NULL');
$self->_upgrade_table('messages','date_changed','timestamp DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP');
......@@ -2177,7 +2191,7 @@ sub _connect_vm {
my $vm = $self->vm->[$n];
if (!$connect) {
$vm->disconnect();
$vm->disconnect() if $vm;
} else {
$vm->connect();
}
......@@ -2816,6 +2830,7 @@ sub process_requests {
next if $request_type ne 'all' && $req->type ne $request_type;
next if $duplicated{$id_request}++;
next if $req->command !~ /shutdown/i
&& $self->_domain_working($id_domain, $id_request);
......@@ -2830,8 +2845,14 @@ sub process_requests {
for my $req (sort { $a->priority <=> $b->priority } @reqs) {
next if $req eq 'refresh_vms' && scalar@reqs > 2;
next if !$req->id;
next if $req->status() =~ /^(done|working)$/;
my $txt_retry = '';
$txt_retry = " retry=".$req->retry if $req->retry;
warn "[$request_type] $$ executing request id=".$req->id." ".$req->status()." retry=".($req->retry or '<UNDEF>')." "
warn ''.localtime." [$request_type] $$ executing request id=".$req->id." ".
"pid=".($req->pid or '')." ".$req->status()
."$txt_retry "
.$req->command
." ".Dumper($req->args) if $DEBUG || $debug;
......@@ -2842,13 +2863,18 @@ sub process_requests {
# $req->status("done") if $req->status() !~ /retry/;
next if !$DEBUG && !$debug;
warn "req ".$req->id." , command: ".$req->command." , status: ".$req->status()
." , error: '".($req->error or 'NONE')."'\n" if $DEBUG || $VERBOSE;
warn ''.localtime." req ".$req->id." , cmd: ".$req->command." ".$req->status()
." , err: '".($req->error or '')."'\n" if $DEBUG || $VERBOSE;
# sleep 1 if $DEBUG;
}
$self->_timeout_requests();
warn Dumper([map { $_->id." ".($_->pid or '')." ".$_->command." ".$_->status }
grep { $_->id } @reqs ])
if ($DEBUG || $debug ) && @reqs;
return scalar(@reqs);
}
sub _date_now($seconds = 0) {
......@@ -2880,7 +2906,7 @@ sub _timeout_requests($self) {
my $req = Ravada::Request->open($id);
my $timeout = $req->defined_arg('timeout') or next;
next if time - $start_time <= $timeout;
warn "request ".$req->pid." ".$req->command." timeout";
warn "request ".$req->pid." ".$req->command." timeout [".(time - $start_time)." <= $timeout";
push @requests,($req);
}
$sth->finish;
......@@ -2947,7 +2973,7 @@ sub process_all_requests {
my $self = shift;
my ($debug,$dont_fork) = @_;
$self->process_requests($debug, $dont_fork,'all');
return $self->process_requests($debug, $dont_fork,'all');
}
......@@ -2959,7 +2985,7 @@ Process all the priority requests, long and short
sub process_priority_requests($self, $debug=0, $dont_fork=0) {
$self->process_requests($debug, $dont_fork,'priority');
return $self->process_requests($debug, $dont_fork,'priority');
}
......@@ -3107,6 +3133,7 @@ sub _execute {
$self->_wait_pids;
return if !$self->_can_fork($request);
$self->disconnect_vm();
my $pid = fork();
die "I can't fork" if !defined $pid;
......@@ -3116,6 +3143,7 @@ sub _execute {
$self->_do_execute_command($sub, $request);
exit;
}
warn "forked $pid\n" if $DEBUG;
$self->_add_pid($pid, $request);
$request->pid($pid);
}
......@@ -3399,10 +3427,14 @@ sub _wait_pids {
last;
}
next if !$id_req;
my $request = Ravada::Request->open($id_req);
my $request;
eval { $request = Ravada::Request->open($id_req) };
warn $@ if $@ && $@ !~ /I can't find id/;
if ($request) {
$request->status('done') if $request->status =~ /working/i;
};
warn("$$ request id=$id_req ".$request->command." ".$request->status()
.", error='".($request->error or '')."'\n") if $DEBUG && $request;
}
}
......@@ -4016,7 +4048,7 @@ sub _cmd_rename_domain {
my $user = Ravada::Auth::SQL->search_by_id($uid);
my $domain = $self->search_domain_by_id($id_domain);
confess "Unkown domain ".Dumper($request) if !$domain;
confess "Unkown domain id=$id_domain ".Dumper($request) if !$domain;
$domain->rename(user => $user, name => $name);
......@@ -4155,7 +4187,7 @@ sub _cmd_domain_autostart($self, $request ) {
sub _cmd_refresh_vms($self, $request=undef) {
if ($request && (my $id_recent = $request->done_recently(30))) {
if ($request && !$request->defined_arg('_force') && (my $id_recent = $request->done_recently(30))) {
die "Command ".$request->command." run recently by $id_recent.\n";
}
......@@ -4170,6 +4202,7 @@ sub _cmd_refresh_vms($self, $request=undef) {
$self->_refresh_volatile_domains();
$self->_check_duplicated_prerouting();
$self->_check_duplicated_iptable();
$request->error('') if $request;
}
......@@ -4471,6 +4504,7 @@ sub _check_duplicated_prerouting($self, $request = undef ) {
my $iptables = $vm->iptables_list();
my %prerouting;
my %already_open;
my %already_clean;
for my $line (@{$iptables->{'nat'}}) {
my %args = @$line;
next if $args{A} ne 'PREROUTING' || !$args{dport};
......@@ -4479,7 +4513,7 @@ sub _check_duplicated_prerouting($self, $request = undef ) {
my $value = $args{$item} or next;
if ($prerouting{$value}) {
warn "clean duplicated prerouting "
.Dumper($prerouting{$value}, \%args) if $debug_ports;
.Dumper($prerouting{$value}, \%args)."\n" if $debug_ports;
$self->_reopen_ports($port) unless $already_open{$port}++;
$self->_delete_iptables_rule($vm,'nat', \%args);
......@@ -4492,6 +4526,36 @@ sub _check_duplicated_prerouting($self, $request = undef ) {
}
}
sub _check_duplicated_iptable($self, $request = undef ) {
my $debug_ports = $self->setting('/backend/debug_ports');
my $sth = $CONNECTOR->dbh->prepare(
"SELECT id FROM vms WHERE is_active=1 "
);
$sth->execute();
while ( my ($id) = $sth->fetchrow()) {
my $vm;
eval { $vm = Ravada::VM->open($id) };
warn $@ if $@;
if ($vm) {
my $iptables = $vm->iptables_list();
my %dupe;
my %already_open;
for my $line (@{$iptables->{'filter'}}) {
my %args = @$line;
next if $args{A} ne 'RAVADA';
my $rule = join(" ", map { $_." ".$args{$_} } sort keys %args);
if ($dupe{$rule}) {
warn "clean duplicated iptables rule ".Dumper($line);
$self->_delete_iptables_rule($vm,'filter', \%args);
}
$dupe{$rule}++;
}
}
}
}
sub _reopen_ports($self, $port) {
my $sth = $CONNECTOR->dbh->prepare("SELECT id_domain FROM domain_ports "
." WHERE public_port=?");
......@@ -4499,10 +4563,11 @@ sub _reopen_ports($self, $port) {
my ($id_domain) = $sth->fetchrow;
return if !$id_domain;
my $domain = Ravada::Domain->open($id_domain);
Ravada::Request->open_exposed_ports(
uid => Ravada::Utils::user_daemon->id
,id_domain => $id_domain
);
) if $domain->is_active;
}
sub _delete_iptables_rule($self, $vm, $table, $rule) {
......@@ -4512,10 +4577,12 @@ sub _delete_iptables_rule($self, $vm, $table, $rule) {
my $dport = delete $delete{dport};
my $m = delete $delete{m};
my $p = delete $delete{p};
my $j = delete $delete{j};
my @delete = ( t => $table, 'D' => $chain
, m => $m, p => $p, dport => $dport
, %delete
, 'to-destination' => $to_destination);
, m => $m, p => $p, dport => $dport);
push @delete,("j" => $j) if $j;
push @delete,( 'to-destination' => $to_destination) if $to_destination;
push @delete, %delete;
$vm->iptables(@delete);
}
......@@ -4675,7 +4742,6 @@ sub _cmd_set_base_vm {
sub _cmd_cleanup($self, $request) {
$self->_clean_volatile_machines( request => $request);
$self->_clean_temporary_users( );
$self->_clean_requests('cleanup', $request);
for my $cmd ( qw(cleanup enforce_limits refresh_vms
manage_pools refresh_machine screenshot
open_iptables ping_backend
......@@ -4956,12 +5022,13 @@ sub _enforce_limits_active($self, $request) {
}
for my $id_user(keys %domains) {
my $user = Ravada::Auth::SQL->search_by_id($id_user);
my %grants = $user->grants();
my %grants;
%grants = $user->grants() if $user;
my $start_limit = (defined($grants{'start_limit'}) && $grants{'start_limit'} > 0) ? $grants{'start_limit'} : $start_limit_default;
next if scalar @{$domains{$id_user}} <= $start_limit;
next if $user->is_admin;
next if $user->can_start_many;
next if $user && $user->is_admin;
next if $user && $user->can_start_many;
my @domains_user = sort { $a->start_time <=> $b->start_time
|| $a->id <=> $b->id }
......@@ -4982,7 +5049,7 @@ sub _enforce_limits_active($self, $request) {
);
return;
}
$user->send_message("Too many machines started. $active out of $start_limit. Stopping ".$domain->name);
$user->send_message("Too many machines started. $active out of $start_limit. Stopping ".$domain->name) if $user;
$active--;
if ($domain->can_hybernate && !$domain->is_volatile) {
$domain->hybernate($USER_DAEMON);
......@@ -5077,7 +5144,7 @@ sub _cmd_remove_expose($self, $request) {
}
sub _cmd_open_exposed_ports($self, $request) {
my $domain = Ravada::Domain->open($request->id_domain);
my $domain = Ravada::Domain->open($request->id_domain) or return;
$domain->open_exposed_ports();
Ravada::Request->refresh_machine_ports(
......
......@@ -704,6 +704,8 @@ sub _load_grants($self) {
$self->_load_aliases();
return if exists $self->{_grant};
_init_connector();
my $sth;
eval { $sth= $$CON->dbh->prepare(
"SELECT gt.name, gu.allowed, gt.enabled, gt.is_int"
......@@ -804,12 +806,16 @@ sub grant_admin_permissions($self,$user) {
my $sth = $$CON->dbh->prepare(
"SELECT name FROM grant_types "
." WHERE enabled=1"
." ORDER BY name"
);
$sth->execute();
my $grant_found=0;
while ( my ($name) = $sth->fetchrow) {
$self->grant($user,$name);
$grant_found++ if $name eq'grant';
}
$sth->finish;
confess if !$grant_found;
}
=head2 revoke_all_permissions
......
This diff is collapsed.
......@@ -123,6 +123,7 @@ sub list_disks {
my $self = shift;
my @disks = ();
return @disks if !$self->xml_description;
my $doc = XML::LibXML->load_xml(string => $self->xml_description);
for my $disk ($doc->findnodes('/domain/devices/disk')) {
......@@ -654,14 +655,37 @@ sub display_info($self, $user) {
my @graph = $xml->findnodes('/domain/devices/graphics')
or return;
my $n_order = 0;
my @display;
for my $graph ( @graph ) {
my ($type) = $graph->getAttribute('type');
my $display;
if ($type eq 'spice') {
push @display,(_display_info_spice($graph));
$display = _display_info_spice($graph);
} elsif ($type eq 'vnc' ) {
push @display,(_display_info_vnc($graph));
$display= _display_info_vnc($graph);
} else {
confess "I don't know how to check info for $type display";
}
$display->{port} = undef if $display->{port} && $display->{port}==-1;
$display->{is_secondary} = 0;
my $display_tls;
if (exists $display->{tls_port} && $display->{tls_port} && $self->_vm->tls_ca) {
my %display_tls = %$display;
$display_tls{port} = delete $display_tls{tls_port};
$display_tls{port} = undef if $display_tls{port} && $display_tls{port}==-1;
$display_tls{driver} .= "-tls";
$display_tls{n_order} = ++$n_order;
$display_tls{is_secondary} = 1;
lock_hash(%display_tls);
$display_tls = \%display_tls;
}
delete $display->{tls_port} if exists $display->{tls_port};
$display->{n_order} = ++$n_order;
lock_hash(%$display);
push @display,($display_tls) if $display_tls;
push @display,($display);
}
return $display[0] if !wantarray;
return @display;
......@@ -681,7 +705,7 @@ sub _display_info_vnc($graph) {
,ip => $address
,is_builtin => 1
);
$display{tls_port} = $tls_port if defined $tls_port;
$display{tls_port} = $tls_port if defined $tls_port && $tls_port;
$display{password} = $password;
$port = '' if !defined $port;
......@@ -693,8 +717,6 @@ sub _display_info_vnc($graph) {
$display{$item->getName()} = $value;
}
}
lock_hash(%display);
return \%display;
}
......@@ -726,7 +748,6 @@ sub _display_info_spice($graph) {
}
}
lock_hash(%display);
return \%display;
}
......@@ -1937,6 +1958,7 @@ sub _set_driver_generic {
sub _update_device_graphics($self, $driver, $data) {
my $doc = XML::LibXML->load_xml(string
=> $self->domain->get_xml_description());
$driver =~ s/-tls$//;
my $path = "/domain/devices/graphics\[\@type='$driver']";
my ($device ) = $doc->findnodes($path);
die "$path not found ".$self->name if !$device;
......@@ -2229,7 +2251,7 @@ sub _set_controller_display($self, $number, $data) {
}
sub remove_controller($self, $name, $index=0) {
sub remove_controller($self, $name, $index=0,$attribute_name=undef, $attribute_value=undef) {
my $sub = $REMOVE_CONTROLLER_SUB{$name};
die "I can't get controller $name for domain ".$self->name
......@@ -2237,7 +2259,14 @@ sub remove_controller($self, $name, $index=0) {
."\n".Dumper(\%REMOVE_CONTROLLER_SUB)
if !$sub;
my $ret = $sub->($self, $index);
my $ret;
#some hardware can be removed searching by attribute
if($name eq 'display') {
$ret = $sub->($self, 0, $attribute_name, $attribute_value);
} else {
$ret = $sub->($self, $index);
}
$self->xml_description_inactive();
return $ret;
}
......@@ -2264,8 +2293,8 @@ sub _remove_device($self, $index, $device, $attribute_name=undef, $attribute_val
." not removed, only ".($ind)." found in ".$self->name."\n";
}
sub _remove_controller_display($self, $index) {
$self->_remove_device($index,'graphics' );
sub _remove_controller_display($self, $index, $attribute_name=undef, $attribute_value=undef) {
$self->_remove_device($index,'graphics', $attribute_name,$attribute_value );
}
......@@ -2502,6 +2531,7 @@ sub _change_hardware_disk_bus($self, $index, $bus) {
sub _change_hardware_display($self, $index, $data) {
my $type = delete $data->{driver};
$type =~ s/-tls$//;
my $port = delete $data->{port};
confess if $port;
for my $item (keys %$data) {
......@@ -2596,7 +2626,6 @@ sub _change_hardware_network($self, $index, $data) {
sub reload_config($self, $doc) {
my $new_domain = $self->_vm->vm->define_domain($doc->toString);
$self->domain($new_domain);
$self->info(Ravada::Utils::user_daemon);
}
sub copy_config($self, $domain) {
......
......@@ -60,10 +60,11 @@ sub display_info {
};
$graph->{is_builtin} = 1;
$graph->{port} = undef if $graph->{port} && $graph->{port} eq 'auto';
push @display,($graph);
}
return $display[0] if wantarray;
return $display[0] if !wantarray;
return @display;
}
......@@ -101,6 +102,11 @@ sub _file_free_port() {
}
sub _reset_free_port(@) {
my $file_fp = _file_free_port();
unlink $file_fp or die $! if -e $file_fp;
}
sub _new_free_port($self, $used={} ) {
my $file_fp = _file_free_port();
......@@ -661,7 +667,6 @@ sub get_info {
my $self = shift;
my $info = $self->_value('info');
if (!$info->{memory}) {
warn Dumper($info);
$info = $self->_set_default_info();
}
lock_keys(%$info);
......@@ -686,6 +691,12 @@ sub _set_default_info($self, $listen_ip=undef) {
,mac => _new_mac()
,time => time
};
$info->{interfaces}->[0] = {
hwaddr => $info->{mac}
,address => $info->{ip}
};
$self->_store(info => $info);
$self->_set_display($listen_ip);
my $hardware = $self->_value('hardware');
......@@ -694,10 +705,6 @@ sub _set_default_info($self, $listen_ip=undef) {
next if $name eq 'disk' || $name eq 'display';
$self->set_controller($name, 1) unless exists $hardware->{$name}->[0];
}
$info->{interfaces}->[0] = {
hwaddress => $info->{mac}
,address => $info->{ip}
};
return $info;
}
......@@ -1012,7 +1019,7 @@ sub _check_port($self,@args) {
}
sub copy_config($self, $domain) {
my $config_new = $self->_load();
my $config_new = $domain->_load();
for my $field ( keys %$config_new ) {
my $value = $config_new->{$field};
$value = 0 if $field eq 'is_active';
......
......@@ -82,9 +82,10 @@ sub display($self, $user) {
}
sub display_info($self, $user) {
my $display = $self->_data('display');
return {} if !$display;
return decode_json($display);
my @displays = $self->_get_controller_display();
return {} if !wantarray && !scalar(@displays);
return $displays[0] if !wantarray;
return @displays;
}
......@@ -211,6 +212,7 @@ sub _get_controller_display($self) {
my %file_extension = (
'spice' => 'vv'
,'spice-tls' => 'tls.vv'
);
my $sth = $$CONNECTOR->dbh->prepare(
......@@ -246,22 +248,6 @@ sub _get_controller_display($self) {
return @displays;
}
sub _fix_ports_duplicated($self, $displays) {
for my $display (@$displays) {
next if $display->{is_builtin};
my $sth = $$CONNECTOR->dbh->prepare("SELECT * FROM domain_displays "
." WHERE port=?"
." AND id <> ?"
." AND is_active=1"
);
$sth->execute($display->{port}, $display->{id});
while ( my $duplicated = $sth->fetchrow_hashref()) {
next if $duplicated->{is_builtin} && $display->{is_builtin};