From b8a3d98d575c75338d66c1318e40e294af1bb50c Mon Sep 17 00:00:00 2001 From: CrystalChun Date: Thu, 22 Feb 2024 17:06:33 -0800 Subject: [PATCH] [release-4.14] MGMT-16843: Ensure valid hostname during install Cherry pick of PR https://github.com/openshift/assisted-installer/pull/788 https://issues.redhat.com/browse/MGMT-16843 Some processes during install time depend on the hostname and an invalid hostname (e.g. 'localhost', having capital letters, etc.) that could cause the install to fail. Extra validation is added when checking the hostname to ensure it is valid. A random hostname will be assigned if the current hostname is invalid. --- Dockerfile.assisted-installer-build | 2 +- src/installer/installer.go | 14 +- src/installer/installer_test.go | 48 ++++- .../pkg/validations/validations.go | 175 ++++++++++++++++++ vendor/modules.txt | 1 + 5 files changed, 226 insertions(+), 14 deletions(-) create mode 100644 vendor/github.com/openshift/assisted-service/pkg/validations/validations.go diff --git a/Dockerfile.assisted-installer-build b/Dockerfile.assisted-installer-build index 0eb4d9796..3aabceb5f 100644 --- a/Dockerfile.assisted-installer-build +++ b/Dockerfile.assisted-installer-build @@ -2,7 +2,7 @@ FROM registry.ci.openshift.org/openshift/release:golang-1.19 ENV GO111MODULE=on ENV GOFLAGS="" -COPY --from=quay.io/app-sre/golangci-lint:v1.53.1 /usr/bin/golangci-lint /usr/bin/golangci-lint +RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b /usr/bin v1.53.1 RUN yum install -y docker && \ yum clean all diff --git a/src/installer/installer.go b/src/installer/installer.go index 0c679e5a9..3a49c3d77 100644 --- a/src/installer/installer.go +++ b/src/installer/installer.go @@ -14,6 +14,7 @@ import ( "github.com/google/uuid" "github.com/openshift/assisted-installer/src/main/drymock" "github.com/openshift/assisted-service/pkg/secretdump" + "github.com/openshift/assisted-service/pkg/validations" "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/thoas/go-funk" @@ -304,7 +305,7 @@ func (i *installer) startBootstrap() error { remove the work done by 30-local-dns-prepender. This will cause DNS issue in bootkube and it will fail to complete successfully */ - err = i.checkLocalhostName() + err = i.checkHostname() if err != nil { i.log.Error(err) return err @@ -856,24 +857,25 @@ func (i *installer) createSingleNodeMasterIgnition() (string, error) { return singleNodeMasterIgnitionPath, nil } -func (i *installer) checkLocalhostName() error { +func (i *installer) checkHostname() error { if i.DryRunEnabled { return nil } - i.log.Infof("Start checking localhostname") + i.log.Infof("Start checking hostname") hostname, err := i.ops.GetHostname() if err != nil { i.log.Errorf("Failed to get hostname from kernel, err %s157", err) return err } - if hostname != "localhost" { - i.log.Infof("hostname is not localhost, no need to do anything") + + if err := validations.ValidateHostname(hostname); err == nil && hostname != "localhost" { + i.log.Infof("hostname [%s] is not localhost or invalid, no need to do anything", hostname) return nil } data := fmt.Sprintf("random-hostname-%s", uuid.New().String()) - i.log.Infof("write data into /etc/hostname") + i.log.Infof("Hostname [%s] is invalid, generated random hostname [%s] and writing data into /etc/hostname") return i.ops.CreateRandomHostname(data) } diff --git a/src/installer/installer_test.go b/src/installer/installer_test.go index 14a86b11c..c9c4ea794 100644 --- a/src/installer/installer_test.go +++ b/src/installer/installer_test.go @@ -27,6 +27,7 @@ import ( "github.com/openshift/assisted-installer/src/k8s_client" "github.com/openshift/assisted-installer/src/ops" "github.com/openshift/assisted-service/models" + "github.com/openshift/assisted-service/pkg/validations" ) func TestValidator(t *testing.T) { @@ -239,7 +240,8 @@ var _ = Describe("installer HostRoleMaster role", func() { } checkLocalHostname := func(hostname string, err error) { mockops.EXPECT().GetHostname().Return(hostname, err).Times(1) - if hostname == "localhost" { + validateErr := validations.ValidateHostname(hostname) + if hostname == "localhost" || validateErr != nil { mockops.EXPECT().CreateRandomHostname(gomock.Any()).Return(nil).Times(1) } } @@ -319,7 +321,38 @@ var _ = Describe("installer HostRoleMaster role", func() { {string(models.HostStageRebooting)}, }) bootstrapSetup() - checkLocalHostname("not localhost", nil) + checkLocalHostname("notlocalhost", nil) + restartNetworkManager(nil) + prepareControllerSuccess() + startServicesSuccess() + WaitMasterNodesSucccess() + waitForBootkubeSuccess() + bootkubeStatusSuccess() + waitForETCDBootstrapSuccess() + bootstrapETCDStatusSuccess() + resolvConfSuccess() + waitForControllerSuccessfully(conf.ClusterID) + //HostRoleMaster flow: + downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign") + writeToDiskSuccess(gomock.Any()) + reportLogProgressSuccess() + setBootOrderSuccess(gomock.Any()) + uploadLogsSuccess(true) + ironicAgentDoesntExist() + rebootSuccess() + ret := installerObj.InstallNode() + Expect(ret).Should(BeNil()) + }) + It("bootstrap role happy flow with invalid hostname", func() { + updateProgressSuccess([][]string{{string(models.HostStageStartingInstallation), conf.Role}, + {string(models.HostStageWaitingForControlPlane), waitingForBootstrapToPrepare}, + {string(models.HostStageWaitingForControlPlane), waitingForMastersStatusInfo}, + {string(models.HostStageInstalling), string(models.HostRoleMaster)}, + {string(models.HostStageWritingImageToDisk)}, + {string(models.HostStageRebooting)}, + }) + bootstrapSetup() + checkLocalHostname("InvalidHostname", nil) restartNetworkManager(nil) prepareControllerSuccess() startServicesSuccess() @@ -406,7 +439,7 @@ var _ = Describe("installer HostRoleMaster role", func() { }) extractIgnitionToFS("extract failure", fmt.Errorf("extract failed")) bootstrapSetup() - checkLocalHostname("not localhost", nil) + checkLocalHostname("notlocalhost", nil) restartNetworkManager(nil) prepareControllerSuccess() startServicesSuccess() @@ -456,7 +489,7 @@ var _ = Describe("installer HostRoleMaster role", func() { {string(models.HostStageWaitingForControlPlane), waitingForBootstrapToPrepare}, }) bootstrapSetup() - checkLocalHostname("not localhost", nil) + checkLocalHostname("notlocalhost", nil) err := fmt.Errorf("Failed to restart NetworkManager") restartNetworkManager(err) //HostRoleMaster flow: @@ -512,7 +545,7 @@ var _ = Describe("installer HostRoleMaster role", func() { }) extractIgnitionToFS("extract failure", fmt.Errorf("extract failed")) bootstrapSetup() - checkLocalHostname("not localhost", nil) + checkLocalHostname("notlocalhost", nil) restartNetworkManager(nil) prepareControllerSuccess() startServicesSuccess() @@ -931,7 +964,8 @@ var _ = Describe("installer HostRoleMaster role", func() { } checkLocalHostname := func(hostname string, err error) { mockops.EXPECT().GetHostname().Return(hostname, err).Times(1) - if hostname == "localhost" { + validateErr := validations.ValidateHostname(hostname) + if hostname == "localhost" || validateErr != nil { mockops.EXPECT().CreateRandomHostname(gomock.Any()).Return(nil).Times(1) } } @@ -1000,7 +1034,7 @@ var _ = Describe("installer HostRoleMaster role", func() { // single node bootstrap flow singleNodeBootstrapSetup() err := fmt.Errorf("Failed to restart NetworkManager") - checkLocalHostname("not localhost", err) + checkLocalHostname("notlocalhost", err) ret := installerObj.InstallNode() Expect(ret).Should(Equal(err)) }) diff --git a/vendor/github.com/openshift/assisted-service/pkg/validations/validations.go b/vendor/github.com/openshift/assisted-service/pkg/validations/validations.go new file mode 100644 index 000000000..f0cc2da00 --- /dev/null +++ b/vendor/github.com/openshift/assisted-service/pkg/validations/validations.go @@ -0,0 +1,175 @@ +package validations + +import ( + "fmt" + "net" + "net/http" + "net/url" + "regexp" + "strings" + + "github.com/asaskevich/govalidator" + "github.com/pkg/errors" + "github.com/thoas/go-funk" +) + +const ( + baseDomainRegex = "^([a-z0-9]+(-[a-z0-9]+)*)+$" + dnsNameRegex = "^([a-z0-9]+(-[a-z0-9]+)*[.])+[a-z]{2,}$" + hostnameRegex = `^[a-z0-9][a-z0-9\-\.]{0,61}[a-z0-9]$` + installerArgsValuesRegex = `^[A-Za-z0-9@!#$%*()_+-=//.,";':{}\[\]]+$` +) + +var allowedFlags = []string{"--append-karg", "--delete-karg", "-n", "--copy-network", "--network-dir", "--save-partlabel", "--save-partindex", "--image-url", "--image-file"} + +func ValidateInstallerArgs(args []string) error { + argsRe := regexp.MustCompile("^-+.*") + valuesRe := regexp.MustCompile(installerArgsValuesRegex) + + for _, arg := range args { + if argsRe.MatchString(arg) { + if !funk.ContainsString(allowedFlags, arg) { + return fmt.Errorf("found unexpected flag %s for installer - allowed flags are %v", arg, allowedFlags) + } + continue + } + + if !valuesRe.MatchString(arg) { + return fmt.Errorf("found unexpected chars in value %s for installer", arg) + } + } + + return nil +} + +func ValidateDomainNameFormat(dnsDomainName string) (int32, error) { + matched, err := regexp.MatchString(baseDomainRegex, dnsDomainName) + if err != nil { + return http.StatusInternalServerError, errors.Wrapf(err, "Single DNS base domain validation for %s", dnsDomainName) + } + if matched { + return 0, nil + } + matched, err = regexp.MatchString(dnsNameRegex, dnsDomainName) + if err != nil { + return http.StatusInternalServerError, errors.Wrapf(err, "DNS name validation for %s", dnsDomainName) + } + if !matched { + return http.StatusBadRequest, errors.Errorf("DNS format mismatch: %s domain name is not valid", dnsDomainName) + } + return 0, nil +} + +func ValidateHostname(name string) error { + matched, err := regexp.MatchString(hostnameRegex, name) + if err != nil { + return errors.Wrapf(err, "Hostname validation for %s", name) + } + if !matched { + return errors.Errorf(`Hostname format mismatch: %s name is not valid. + Hostname must have a maximum length of 64 characters, + start and end with a lowercase alphanumerical character, + and can only contain lowercase alphanumerical characters, dashes, and periods.`, name) + } + return nil +} + +func AllStrings(vs []string, f func(string) bool) bool { + for _, v := range vs { + if !f(v) { + return false + } + } + return true +} + +func ValidateAdditionalNTPSource(commaSeparatedNTPSources string) bool { + return AllStrings(strings.Split(commaSeparatedNTPSources, ","), ValidateNTPSource) +} + +func ValidateNTPSource(ntpSource string) bool { + if addr := net.ParseIP(ntpSource); addr != nil { + return true + } + + if err := ValidateHostname(ntpSource); err == nil { + return true + } + + return false +} + +// ValidateHTTPFormat validates the HTTP and HTTPS format +func ValidateHTTPFormat(theurl string) error { + u, err := url.Parse(theurl) + if err != nil { + return fmt.Errorf("URL '%s' format is not valid: %w", theurl, err) + } + if !(u.Scheme == "http" || u.Scheme == "https") { + return errors.Errorf("The URL scheme must be http(s) and specified in the URL: '%s'", theurl) + } + return nil +} + +// ValidateHTTPProxyFormat validates the HTTP Proxy and HTTPS Proxy format +func ValidateHTTPProxyFormat(proxyURL string) error { + if !govalidator.IsURL(proxyURL) { + return errors.Errorf("Proxy URL format is not valid: '%s'", proxyURL) + } + u, err := url.Parse(proxyURL) + if err != nil { + return errors.Errorf("Proxy URL format is not valid: '%s'", proxyURL) + } + if u.Scheme == "https" { + return errors.Errorf("The URL scheme must be http; https is currently not supported: '%s'", proxyURL) + } + if u.Scheme != "http" { + return errors.Errorf("The URL scheme must be http and specified in the URL: '%s'", proxyURL) + } + return nil +} + +// ValidateNoProxyFormat validates the no-proxy format which should be a comma-separated list +// of destination domain names, domains, IP addresses or other network CIDRs. A domain can be +// prefaced with '.' to include all subdomains of that domain. +func ValidateNoProxyFormat(noProxy string) error { + if noProxy == "*" { + return nil + } + domains := strings.Split(noProxy, ",") + for _, s := range domains { + s = strings.TrimPrefix(s, ".") + if govalidator.IsIP(s) { + continue + } + + if govalidator.IsCIDR(s) { + continue + } + + if govalidator.IsDNSName(s) { + continue + } + return errors.Errorf("NO Proxy format is not valid: '%s'. "+ + "NO Proxy is a comma-separated list of destination domain names, domains, IP addresses or other network CIDRs. "+ + "A domain can be prefaced with '.' to include all subdomains of that domain. Use '*' to bypass proxy for all destinations with OpenShift 4.8 or later.", noProxy) + } + return nil +} + +func ValidateTags(tags string) error { + if tags == "" { + return nil + } + if !AllStrings(strings.Split(tags, ","), IsValidTag) { + errMsg := "Invalid format for Tags: %s. Tags should be a comma-separated list (e.g. tag1,tag2,tag3). " + + "Each tag can consist of the following characters: Alphanumeric (aA-zZ, 0-9), underscore (_) and white-spaces." + return errors.Errorf(errMsg, tags) + } + return nil +} + +func IsValidTag(tag string) bool { + tagRegex := `^\w+( \w+)*$` // word characters and whitespace + return regexp.MustCompile(tagRegex).MatchString(tag) +} diff --git a/vendor/modules.txt b/vendor/modules.txt index d1a58445a..af57ae16e 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -428,6 +428,7 @@ github.com/openshift/assisted-service/pkg/ocm github.com/openshift/assisted-service/pkg/requestid github.com/openshift/assisted-service/pkg/secretdump github.com/openshift/assisted-service/pkg/transaction +github.com/openshift/assisted-service/pkg/validations github.com/openshift/assisted-service/restapi github.com/openshift/assisted-service/restapi/operations github.com/openshift/assisted-service/restapi/operations/events