Feature hostname#2353
Open
frankiejol wants to merge 15 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses #2338 by resolving and surfacing a hostname (instead of an IP) in display connection information, so clients (notably RDP with SSL certificates) can use a certificate-valid name.
Changes:
- Add hostname resolution + caching for display endpoints and use it when generating RDP/SPICE connection files.
- Expand/adjust VM display tests to validate hostname propagation into stored display data and connection files.
- Minor reliability fixes in port/machine/cleanup logic and test helpers.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| t/vm/n20_name.t | Avoids /tmp display config filename collisions by making it unique per test run. |
| t/vm/94_ports_exposed.t | Updates expected default value for ports_exposed to match current behavior. |
| t/vm/23_display.t | Adds hostname-related display assertions and connection-file validation for multiple display drivers. |
| t/mojo/10_login.t | Improves failure clarity when a domain cannot be found (croak with message). |
| t/lib/Test/Ravada.pm | Adjusts cleanup/discovery behavior when removing old domains/requests. |
| lib/Ravada/VM.pm | Tweaks used-port listing logic (notably for display TLS ports). |
| lib/Ravada/Domain/KVM.pm | Adds a guard around machine-type parsing in migration XML normalization. |
| lib/Ravada/Domain.pm | Implements hostname resolution + caching and uses hostname in display info + RDP/SPICE file generation. |
| lib/Ravada.pm | Refactors exposed-port cleanup logic to update per-row and retry on conflicts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1478
to
+1484
| my ($in, $out, $err); | ||
| run3(['host',$ip], \$in, \$out, \$err); | ||
| my ($hostname) = $out =~ /pointer (.*)\./; | ||
| if (!$hostname) { | ||
| warn "I can't fetch hostname from $out" if !$hostname; | ||
| $hostname = $ip; | ||
| } |
Comment on lines
+176
to
+184
| my $name = gethostbyaddr($ip, AF_INET); | ||
| return $name if $name && $name !~ /\d+\.\d+\.\d+/; | ||
|
|
||
| my ($in, $out, $err); | ||
| run3(['host',$ip], \$in, \$out, \$err); | ||
| my ($hostname) = $out =~ /pointer (.*)\./; | ||
| ($hostname) = $out =~ /pointer (.*)\./; | ||
| die "I can't fetch hostname from $out" if !$hostname; | ||
|
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment on lines
812
to
816
| my @reqs; | ||
| for my $machine ( @$machines, @machines2) { | ||
| next unless $machine->{name} =~ /$base_name/; | ||
| remove_domain_and_clones_req($machine,$wait); | ||
| remove_domain_and_clones_req($machine,$wait,0); | ||
| } |
Comment on lines
+172
to
+183
| sub _get_hostname($ip) { | ||
|
|
||
| return $ip if $ip !~ /^\d+\.\d+\.\d+\.\d+$/; | ||
|
|
||
| my ($in, $out, $err); | ||
| run3(['host',$ip], \$in, \$out, \$err); | ||
| my ($hostname) = $out =~ /pointer (.*)\./; | ||
| ($hostname) = $out =~ /pointer (.*)\./; | ||
| die "I can't fetch hostname from $out" if !$hostname; | ||
|
|
||
| return $hostname; | ||
| } |
Comment on lines
+1479
to
+1489
| my ($in, $out, $err); | ||
| run3(['host',$ip], \$in, \$out, \$err); | ||
| my ($hostname) = $out =~ /pointer (.*)\./; | ||
| if (!$hostname) { | ||
| warn "I can't fetch hostname from $out"; | ||
| $hostname = $ip; | ||
| } | ||
| $CACHE_HOSTNAME{$ip} = $hostname; | ||
|
|
||
| return $hostname; | ||
| } |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
issue #2338
…ature/2338_hostname
Comment on lines
806
to
810
| sub remove_old_domains_req($wait=1, $run_request=0) { | ||
| my $base_name = base_domain_name(); | ||
| _discover(); | ||
| delete_request( 'enforce_limits', 'set_time', 'refresh_vm' ); | ||
| _discover() if $discover; | ||
| my $machines = rvd_front->list_machines(user_admin); |
Comment on lines
+1442
to
+1446
| if (!exists $display_new{hostname}) { | ||
| unlock_hash(%display_new); | ||
| $display_new{hostname} = $self->_cache_dns($display_new{ip}); | ||
| lock_hash(%display_new); | ||
| } |
Comment on lines
+1468
to
+1489
| return $ip if $ip !~ /^\d+\.\d+\.\d+\.\d+$/; | ||
|
|
||
| return $CACHE_HOSTNAME{$ip} if exists $CACHE_HOSTNAME{$ip}; | ||
|
|
||
| my $packed_ip = inet_aton($ip); | ||
| my $name = $packed_ip ? gethostbyaddr($packed_ip, AF_INET) : undef; | ||
| if ( $name && $name !~ /\d+\.\d+\.\d+/ ) { | ||
| $CACHE_HOSTNAME{$ip} = $name; | ||
| return $name; | ||
| } | ||
|
|
||
| my ($in, $out, $err); | ||
| run3(['host',$ip], \$in, \$out, \$err); | ||
| my ($hostname) = $out =~ /pointer (.*)\./; | ||
| if (!$hostname) { | ||
| warn "I can't fetch hostname from $out"; | ||
| $hostname = $ip; | ||
| } | ||
| $CACHE_HOSTNAME{$ip} = $hostname; | ||
|
|
||
| return $hostname; | ||
| } |
Comment on lines
+172
to
+183
| sub _get_hostname($ip) { | ||
|
|
||
| return $ip if $ip !~ /^\d+\.\d+\.\d+\.\d+$/; | ||
|
|
||
| my ($in, $out, $err); | ||
| run3(['host',$ip], \$in, \$out, \$err); | ||
| my ($hostname) = $out =~ /pointer (.*)\./; | ||
| ($hostname) = $out =~ /pointer (.*)\./; | ||
| die "I can't fetch hostname from $out" if !$hostname; | ||
|
|
||
| return $hostname; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolve the host name at the display connection fields
closes #2338