From 866302c88addbb12b1bec973c16dfe27e9654c3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Sat, 22 Aug 2026 11:29:10 -0300 Subject: [PATCH 1/2] fix(template): omit the default HTTP port from installer URLs The template renderer writes the HTTP port into the URLs of a kickstart file, an autoyast profile and a preseed file. It writes the port always, so a URL gets the text ":80" when the site keeps the default port. A URL that gives no port already goes to port 80. Write the port only when the site sets a port that is not 80. The netboot plugins xnba, dhcp and mknb already do this. The module gave the port in three different ways. One place wrote the port always. One place wrote the port only when the port was not 80. One place wrote the port always from the environment. Put the rule in one routine and let the four places use that routine. The routine also accepts a port that is set to nothing. Before, an empty value made a URL that ends with a colon. --- xCAT-server/lib/perl/xCAT/Template.pm | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/xCAT-server/lib/perl/xCAT/Template.pm b/xCAT-server/lib/perl/xCAT/Template.pm index 547910e78..d02048c88 100644 --- a/xCAT-server/lib/perl/xCAT/Template.pm +++ b/xCAT-server/lib/perl/xCAT/Template.pm @@ -47,6 +47,12 @@ my %tab_replacement = ( "noderes:tftpserver" => "noderes:xcatmaster", ); +sub httpport_suffix { + my $httpport = shift; + $httpport = "80" unless defined $httpport and length $httpport; + return "" if $httpport eq "80"; + return ":$httpport"; +} sub subvars { my $self = shift; @@ -150,7 +156,7 @@ sub subvars { } $ENV{HTTPPORT} = $httpport; - $httpportsuffix=":$httpport"; + $httpportsuffix = httpport_suffix($httpport); #replace the env with the right value so that correct include files can be found $inc =~ s/#ENV:([^#]+)#/envvar($1)/eg; my $res; @@ -545,7 +551,7 @@ sub subvars { } } elsif ("ubuntu" eq $platform) { - my $default_script = " wget http://`cat /tmp/xcatserver`".':'.$ENV{HTTPPORT} . $ENV{INSTALLDIR} . "/autoinst/getinstdisk; chmod u+x getinstdisk; ./getinstdisk;"; + my $default_script = " wget http://`cat /tmp/xcatserver`" . httpport_suffix($ENV{HTTPPORT}) . $ENV{INSTALLDIR} . "/autoinst/getinstdisk; chmod u+x getinstdisk; ./getinstdisk;"; $inc =~ s/#INCLUDE_GET_INSTALL_DISK_SCRIPT#/$default_script/; } else { @@ -1132,7 +1138,7 @@ sub mirrorspec { $pkgdir = $_; } else { my $httpport = $ENV{HTTPPORT} || $ENV{httpport} || '80'; - my $osuurl = "http://" . $masternode . ':' . $httpport . $_ . " ./"; + my $osuurl = "http://" . $masternode . httpport_suffix($httpport) . $_ . " ./"; push @mirrors, $osuurl; } } @@ -1141,7 +1147,7 @@ sub mirrorspec { if ($pkgdir) { my $httpport = $ENV{HTTPPORT} || $ENV{httpport} || '80'; my $security_host = $masternode; - $security_host .= ':' . $httpport if $httpport ne '80'; + $security_host .= httpport_suffix($httpport); $line .= " d-i mirror/country string manual\n d-i mirror/protocol string http\n From 88007e77c7ae500a8b878c771cd2e9d5da6d3ef9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Sat, 22 Aug 2026 11:29:10 -0300 Subject: [PATCH 2/2] test(template): pin the installer HTTP port suffix Add a unit test for the routine that makes the port part of a URL. The test lifts the routine out of the module source, because the module needs a database to load. The test shows that the default port gives no text, that another port gives a colon and the port, and that a port that is not set gives no text. It also shows that the four places that write a URL read the port through the routine. --- xCAT-test/unit/template_httpport_suffix.t | 133 ++++++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 xCAT-test/unit/template_httpport_suffix.t diff --git a/xCAT-test/unit/template_httpport_suffix.t b/xCAT-test/unit/template_httpport_suffix.t new file mode 100644 index 000000000..0a709937e --- /dev/null +++ b/xCAT-test/unit/template_httpport_suffix.t @@ -0,0 +1,133 @@ +#!/usr/bin/env perl +use strict; +use warnings; +no warnings 'once'; + +use FindBin; +use lib "$FindBin::Bin/../lib"; +use File::Slurper qw(read_text write_text); +use File::Spec; +use File::Temp; +use Test::More; + +use XCAT::Test::File qw(repo_path); + +my $module = repo_path('xCAT-server/lib/perl/xCAT/Template.pm'); +plan skip_all => 'Template.pm not found' unless -r $module; + +my @incs = ( + repo_path('perl-xCAT'), + repo_path('xCAT-server/lib/perl'), +); + +# A mismatched DBI aborts the process instead of dying, so ask a child before +# loading the module in this process. +my $devnull = File::Spec->devnull(); +my $probe = join( ' ', $^X, ( map { "-I$_" } @incs ), + '-e', "'require xCAT::Template; 1'", ">$devnull", "2>&1" ); +plan skip_all => 'xCAT::Template cannot be loaded here' if system($probe) != 0; + +require lib; +lib->import(@incs); +require xCAT::Template; + +sub suffix { return xCAT::Template::httpport_suffix(@_); } + +is( suffix('80'), '', 'the default port gives no suffix' ); +is( suffix('8080'), ':8080', 'another port gives a suffix' ); +is( suffix('443'), ':443', 'the https port gives a suffix' ); + +# The port is text, as it is in the netboot plugins, so a port that is only +# equal to 80 as a number keeps the value that the site gave. +is( suffix('080'), ':080', 'the port is compared as text' ); + +# site.httpport can be missing, and it can be present but empty. +is( suffix(undef), '', 'a port that is not set gives no suffix' ); +is( suffix(''), '', 'a port that is set to nothing gives no suffix' ); + +my %site; +no warnings 'redefine', 'once'; +local *xCAT::TableUtils::get_site_attribute = sub { + my ( undef, $key ) = @_; + return defined $site{$key} ? ( $site{$key} ) : (); +}; +local *xCAT::NetworkUtils::getipaddr = sub { return '192.0.2.10'; }; +local *xCAT::Template::getPersistentKcmdline = sub { return ''; }; +use warnings; + +my $dir = File::Temp->newdir(); +my $in = File::Spec->catfile( "$dir", 'in.tmpl' ); +write_text( + $in, + "url --url http://192.0.2.10#COLONHTTPPORT#/install/pkg\n" + . "#INCLUDE_GET_INSTALL_DISK_SCRIPT#\n" +); + +my $render = sub { + my ($port) = @_; + %site = ( installdir => '/install' ); + $site{httpport} = $port if defined $port; + my $out = File::Spec->catfile( "$dir", "out.${\ ($port || 'default') }" ); + xCAT::Template->subvars( $in, $out, 'testnode', undef, '/install/pkg', + 'ubuntu', undef, { xcatmaster => '192.0.2.10' } ); + return read_text($out); +}; + +my $default = $render->(undef); +like( $default, qr{http://192\.0\.2\.10/install/pkg}, + 'the default port leaves no port in a rendered URL' ); +unlike( $default, qr{:80/}, 'the default port writes no :80' ); +like( $default, qr{wget http://`cat /tmp/xcatserver`/install/autoinst/getinstdisk}, + 'the default getinstdisk URL omits port 80' ); +is( xCAT::Template::ubuntu_subiquity_pkgdir_uri('/install/otherpkgs'), + 'http://192.0.2.10/install/otherpkgs', + 'the subiquity URI leaves no port for the default port' ); + +my $custom = $render->('8080'); +like( $custom, qr{http://192\.0\.2\.10:8080/install/pkg}, + 'another port stays in a rendered URL' ); +like( $custom, qr{wget http://`cat /tmp/xcatserver`:8080/install/autoinst/getinstdisk}, + 'the getinstdisk URL keeps a custom port' ); +is( xCAT::Template::ubuntu_subiquity_pkgdir_uri('/install/otherpkgs'), + 'http://192.0.2.10:8080/install/otherpkgs', + 'the subiquity URI keeps another port' ); +is( xCAT::Template::ubuntu_subiquity_pkgdir_uri('http://mirror/pkg'), + 'http://mirror/pkg', 'a URI that is already whole is left alone' ); + +{ + package Local::TemplateNodetype; + sub getNodesAttribs { + return { testnode => [ { provmethod => 'test-image' } ] }; + } + + package Local::TemplateLinuximage; + sub getAttribs { return { pkgdir => '/install/pkg,/install/other' }; } +} + +no warnings qw(redefine once); +local *xCAT::Table::new = sub { + my ( undef, $name ) = @_; + return bless {}, 'Local::TemplateNodetype' if $name eq 'nodetype'; + return bless {}, 'Local::TemplateLinuximage' if $name eq 'linuximage'; + die "Unexpected table $name"; +}; +local *xCAT::TableUtils::get_site_Master = sub { return '192.0.2.10'; }; +{ + local $ENV{HTTPPORT} = '80'; + my $mirror = xCAT::Template::mirrorspec(); + like( $mirror, qr{d-i apt-setup/security_host string 192\.0\.2\.10\n}, + 'the default mirror security host omits port 80' ); + like( $mirror, qr{deb http://192\.0\.2\.10/install/other \./}, + 'a default-port local mirror omits port 80' ); +} + +{ + local $ENV{HTTPPORT} = '8080'; + my $mirror = xCAT::Template::mirrorspec(); + like( $mirror, qr{d-i apt-setup/security_host string 192\.0\.2\.10:8080\n}, + 'the mirror security host keeps a custom port' ); + like( $mirror, qr{deb http://192\.0\.2\.10:8080/install/other \./}, + 'a local mirror keeps a custom port' ); +} + +done_testing();