diff --git a/api/vendor/github.com/openshift/assisted-service/models/host_validation_id.go b/api/vendor/github.com/openshift/assisted-service/models/host_validation_id.go index dcaeb894c10f..0cc422b24c31 100644 --- a/api/vendor/github.com/openshift/assisted-service/models/host_validation_id.go +++ b/api/vendor/github.com/openshift/assisted-service/models/host_validation_id.go @@ -155,6 +155,9 @@ const ( // HostValidationIDNoIPCollisionsInNetwork captures enum value "no-ip-collisions-in-network" HostValidationIDNoIPCollisionsInNetwork HostValidationID = "no-ip-collisions-in-network" + + // HostValidationIDNoIscsiNicBelongsToMachineCidr captures enum value "no-iscsi-nic-belongs-to-machine-cidr" + HostValidationIDNoIscsiNicBelongsToMachineCidr HostValidationID = "no-iscsi-nic-belongs-to-machine-cidr" ) // for schema @@ -162,7 +165,7 @@ var hostValidationIdEnum []interface{} func init() { var res []HostValidationID - if err := json.Unmarshal([]byte(`["connected","media-connected","has-inventory","has-min-cpu-cores","has-min-valid-disks","has-min-memory","machine-cidr-defined","has-cpu-cores-for-role","has-memory-for-role","hostname-unique","hostname-valid","belongs-to-machine-cidr","ignition-downloadable","belongs-to-majority-group","valid-platform-network-settings","ntp-synced","time-synced-between-host-and-service","container-images-available","lso-requirements-satisfied","ocs-requirements-satisfied","odf-requirements-satisfied","lvm-requirements-satisfied","mce-requirements-satisfied","mtv-requirements-satisfied","sufficient-installation-disk-speed","cnv-requirements-satisfied","sufficient-network-latency-requirement-for-role","sufficient-packet-loss-requirement-for-role","has-default-route","api-domain-name-resolved-correctly","api-int-domain-name-resolved-correctly","apps-domain-name-resolved-correctly","release-domain-name-resolved-correctly","compatible-with-cluster-platform","dns-wildcard-not-configured","disk-encryption-requirements-satisfied","non-overlapping-subnets","vsphere-disk-uuid-enabled","compatible-agent","no-skip-installation-disk","no-skip-missing-disk","no-ip-collisions-in-network"]`), &res); err != nil { + if err := json.Unmarshal([]byte(`["connected","media-connected","has-inventory","has-min-cpu-cores","has-min-valid-disks","has-min-memory","machine-cidr-defined","has-cpu-cores-for-role","has-memory-for-role","hostname-unique","hostname-valid","belongs-to-machine-cidr","ignition-downloadable","belongs-to-majority-group","valid-platform-network-settings","ntp-synced","time-synced-between-host-and-service","container-images-available","lso-requirements-satisfied","ocs-requirements-satisfied","odf-requirements-satisfied","lvm-requirements-satisfied","mce-requirements-satisfied","mtv-requirements-satisfied","sufficient-installation-disk-speed","cnv-requirements-satisfied","sufficient-network-latency-requirement-for-role","sufficient-packet-loss-requirement-for-role","has-default-route","api-domain-name-resolved-correctly","api-int-domain-name-resolved-correctly","apps-domain-name-resolved-correctly","release-domain-name-resolved-correctly","compatible-with-cluster-platform","dns-wildcard-not-configured","disk-encryption-requirements-satisfied","non-overlapping-subnets","vsphere-disk-uuid-enabled","compatible-agent","no-skip-installation-disk","no-skip-missing-disk","no-ip-collisions-in-network","no-iscsi-nic-belongs-to-machine-cidr"]`), &res); err != nil { panic(err) } for _, v := range res { diff --git a/client/vendor/github.com/openshift/assisted-service/models/host_validation_id.go b/client/vendor/github.com/openshift/assisted-service/models/host_validation_id.go index dcaeb894c10f..0cc422b24c31 100644 --- a/client/vendor/github.com/openshift/assisted-service/models/host_validation_id.go +++ b/client/vendor/github.com/openshift/assisted-service/models/host_validation_id.go @@ -155,6 +155,9 @@ const ( // HostValidationIDNoIPCollisionsInNetwork captures enum value "no-ip-collisions-in-network" HostValidationIDNoIPCollisionsInNetwork HostValidationID = "no-ip-collisions-in-network" + + // HostValidationIDNoIscsiNicBelongsToMachineCidr captures enum value "no-iscsi-nic-belongs-to-machine-cidr" + HostValidationIDNoIscsiNicBelongsToMachineCidr HostValidationID = "no-iscsi-nic-belongs-to-machine-cidr" ) // for schema @@ -162,7 +165,7 @@ var hostValidationIdEnum []interface{} func init() { var res []HostValidationID - if err := json.Unmarshal([]byte(`["connected","media-connected","has-inventory","has-min-cpu-cores","has-min-valid-disks","has-min-memory","machine-cidr-defined","has-cpu-cores-for-role","has-memory-for-role","hostname-unique","hostname-valid","belongs-to-machine-cidr","ignition-downloadable","belongs-to-majority-group","valid-platform-network-settings","ntp-synced","time-synced-between-host-and-service","container-images-available","lso-requirements-satisfied","ocs-requirements-satisfied","odf-requirements-satisfied","lvm-requirements-satisfied","mce-requirements-satisfied","mtv-requirements-satisfied","sufficient-installation-disk-speed","cnv-requirements-satisfied","sufficient-network-latency-requirement-for-role","sufficient-packet-loss-requirement-for-role","has-default-route","api-domain-name-resolved-correctly","api-int-domain-name-resolved-correctly","apps-domain-name-resolved-correctly","release-domain-name-resolved-correctly","compatible-with-cluster-platform","dns-wildcard-not-configured","disk-encryption-requirements-satisfied","non-overlapping-subnets","vsphere-disk-uuid-enabled","compatible-agent","no-skip-installation-disk","no-skip-missing-disk","no-ip-collisions-in-network"]`), &res); err != nil { + if err := json.Unmarshal([]byte(`["connected","media-connected","has-inventory","has-min-cpu-cores","has-min-valid-disks","has-min-memory","machine-cidr-defined","has-cpu-cores-for-role","has-memory-for-role","hostname-unique","hostname-valid","belongs-to-machine-cidr","ignition-downloadable","belongs-to-majority-group","valid-platform-network-settings","ntp-synced","time-synced-between-host-and-service","container-images-available","lso-requirements-satisfied","ocs-requirements-satisfied","odf-requirements-satisfied","lvm-requirements-satisfied","mce-requirements-satisfied","mtv-requirements-satisfied","sufficient-installation-disk-speed","cnv-requirements-satisfied","sufficient-network-latency-requirement-for-role","sufficient-packet-loss-requirement-for-role","has-default-route","api-domain-name-resolved-correctly","api-int-domain-name-resolved-correctly","apps-domain-name-resolved-correctly","release-domain-name-resolved-correctly","compatible-with-cluster-platform","dns-wildcard-not-configured","disk-encryption-requirements-satisfied","non-overlapping-subnets","vsphere-disk-uuid-enabled","compatible-agent","no-skip-installation-disk","no-skip-missing-disk","no-ip-collisions-in-network","no-iscsi-nic-belongs-to-machine-cidr"]`), &res); err != nil { panic(err) } for _, v := range res { diff --git a/internal/host/hostcommands/install_cmd.go b/internal/host/hostcommands/install_cmd.go index 1d048064cf59..385007e17f6e 100644 --- a/internal/host/hostcommands/install_cmd.go +++ b/internal/host/hostcommands/install_cmd.go @@ -382,20 +382,9 @@ func appendISCSIArgs(installerArgs []string, installationDisk *models.Disk, inve return nil, fmt.Errorf("Cannot parse iSCSI host IP %s: %w", installationDisk.Iscsi.HostIPAddress, err) } - nic, ok := lo.Find(inventory.Interfaces, func(nic *models.Interface) bool { - ips := nic.IPV4Addresses - if iSCSIHostIP.Is6() { - ips = nic.IPV6Addresses - } - _, ok := lo.Find(ips, func(ip string) bool { - prefix, err := netip.ParsePrefix(ip) - return err == nil && iSCSIHostIP.Compare(prefix.Addr()) == 0 - }) - return ok - }) - - if !ok { - return nil, fmt.Errorf("Cannot find the interface belonging to iSCSI host IP %s", iSCSIHostIP.String()) + nic, err := network.FindInterfaceByIP(iSCSIHostIP, inventory.Interfaces) + if err != nil { + return nil, fmt.Errorf("Cannot find the interface belonging to iSCSI host IP: %w", err) } dhcp := "dhcp" diff --git a/internal/host/hostcommands/install_cmd_test.go b/internal/host/hostcommands/install_cmd_test.go index b96d7b625e62..395849e81161 100644 --- a/internal/host/hostcommands/install_cmd_test.go +++ b/internal/host/hostcommands/install_cmd_test.go @@ -866,7 +866,7 @@ var _ = Describe("construct host install arguments", func() { inventory, _ := common.UnmarshalInventory(host.Inventory) _, err := constructHostInstallerArgs(cluster, host, inventory, infraEnv, log) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(Equal("Cannot find the interface belonging to iSCSI host IP 10.56.21.80")) + Expect(err.Error()).To(Equal("Cannot find the interface belonging to iSCSI host IP: Cannot find the network interface on which the IP 10.56.21.80 is set")) }) It("iSCSI installation disk - OCI - IPv4 address is left un-configured on machine networks interface", func() { cluster.MachineNetworks = []*models.MachineNetwork{{Cidr: "10.56.21.0/25"}} diff --git a/internal/host/refresh_status_preprocessor.go b/internal/host/refresh_status_preprocessor.go index 4269fe3f48e7..f350310f9e48 100644 --- a/internal/host/refresh_status_preprocessor.go +++ b/internal/host/refresh_status_preprocessor.go @@ -310,6 +310,10 @@ func newValidations(v *validator) []validation { id: NoIPCollisionsInNetwork, condition: v.noIPCollisionsInNetwork, }, + { + id: NoIscsiNicBelongsToMachineCidr, + condition: v.noIscsiNicBelongsToMachineCidr, + }, } } diff --git a/internal/host/statemachine.go b/internal/host/statemachine.go index 5e24dde169d1..be716adf9d7f 100644 --- a/internal/host/statemachine.go +++ b/internal/host/statemachine.go @@ -954,6 +954,7 @@ func NewHostStateMachine(sm stateswitch.StateMachine, th TransitionHandler) stat If(NoSkipInstallationDisk), If(NoSkipMissingDisk), If(NoIPCollisionsInNetwork), + If(NoIscsiNicBelongsToMachineCidr), /* * MGMT-15213: The release domain is not resolved correctly when there is a mirror or proxy. In this case * validation might fail, but the installation may succeed. diff --git a/internal/host/transition_test.go b/internal/host/transition_test.go index 02363f131508..b4e03623063f 100644 --- a/internal/host/transition_test.go +++ b/internal/host/transition_test.go @@ -6556,6 +6556,7 @@ var allValidationIDs = []validationID{ NoSkipMissingDisk, NoIPCollisionsInNetwork, IsReleaseDomainNameResolvedCorrectly, + NoIscsiNicBelongsToMachineCidr, } var allConditions = []conditionId{ @@ -6708,6 +6709,19 @@ var _ = Describe("State machine test - refresh transition", func() { Expect(stateMachine.Run(TransitionTypeRefresh, testState, &refreshHostArgs)).To(Succeed()) Expect(string(testState.State())).To(Equal(models.HostStatusKnown)) }) + + It("Moves from known to insufficient when validation fails", func() { + + refreshHostArgs.conditions[string(NoIscsiNicBelongsToMachineCidr)] = false + + Expect(stateMachine.Run(TransitionTypeRefresh, testState, &refreshHostArgs)).To(Succeed()) + Expect(string(testState.State())).To(Equal(models.HostStatusInsufficient)) + + refreshHostArgs.conditions[string(NoIscsiNicBelongsToMachineCidr)] = true + + Expect(stateMachine.Run(TransitionTypeRefresh, testState, &refreshHostArgs)).To(Succeed()) + Expect(string(testState.State())).To(Equal(models.HostStatusKnown)) + }) }) }) diff --git a/internal/host/validation_id.go b/internal/host/validation_id.go index dd142f70f377..078fc8da557f 100644 --- a/internal/host/validation_id.go +++ b/internal/host/validation_id.go @@ -52,6 +52,7 @@ const ( NoSkipInstallationDisk = validationID(models.HostValidationIDNoSkipInstallationDisk) NoSkipMissingDisk = validationID(models.HostValidationIDNoSkipMissingDisk) NoIPCollisionsInNetwork = validationID(models.HostValidationIDNoIPCollisionsInNetwork) + NoIscsiNicBelongsToMachineCidr = validationID(models.HostValidationIDNoIscsiNicBelongsToMachineCidr) ) func (v validationID) category() (string, error) { @@ -75,7 +76,8 @@ func (v validationID) category() (string, error) { IsDNSWildcardNotConfigured, NonOverlappingSubnets, IsReleaseDomainNameResolvedCorrectly, - NoIPCollisionsInNetwork: + NoIPCollisionsInNetwork, + NoIscsiNicBelongsToMachineCidr: return "network", nil case HasInventory, HasMinCPUCores, diff --git a/internal/host/validations_test.go b/internal/host/validations_test.go index cc0dac78326d..041dcf0a2b6e 100644 --- a/internal/host/validations_test.go +++ b/internal/host/validations_test.go @@ -2339,4 +2339,255 @@ var _ = Describe("Validations test", func() { Expect(ValidationError).To(Equal(status)) }) }) + Context("No iSCSI NIC belongs to machine networks", func() { + var ( + cluster common.Cluster + host models.Host + ) + BeforeEach(func() { + cluster = hostutil.GenerateTestCluster(clusterID) + hostId, infraEnvId := strfmt.UUID(uuid.New().String()), strfmt.UUID(uuid.New().String()) + + mockProviderRegistry.EXPECT().IsHostSupported(commontesting.EqPlatformType(models.PlatformTypeVsphere), gomock.Any()).Return(false, nil).AnyTimes() + + host = hostutil.GenerateTestHostByKind(hostId, infraEnvId, &clusterID, models.HostStatusKnown, models.HostKindHost, models.HostRoleMaster) + host.Inventory = hostutil.GenerateMasterInventory() + }) + + It("No installation disk", func() { + var inventory models.Inventory + err := json.Unmarshal([]byte(host.Inventory), &inventory) + Expect(err).ShouldNot(HaveOccurred()) + + host.InstallationDiskID = "not-foo" + inventory.Disks = []*models.Disk{ + {ID: "foo", DriveType: models.DriveTypeSSD}, + } + + inventoryByte, _ := json.Marshal(inventory) + host.Inventory = string(inventoryByte) + + Expect(db.Create(&cluster).Error).ToNot(HaveOccurred()) + Expect(db.Create(&host).Error).ShouldNot(HaveOccurred()) + mockAndRefreshStatus(&host) + + refreshedHost := hostutil.GetHostFromDB(*host.ID, host.InfraEnvID, db).Host + + _, _, ok := getValidationResult(refreshedHost.ValidationsInfo, NoIscsiNicBelongsToMachineCidr) + Expect(ok).To(BeFalse()) + }) + It("Not iSCSI drive", func() { + var inventory models.Inventory + err := json.Unmarshal([]byte(host.Inventory), &inventory) + Expect(err).ShouldNot(HaveOccurred()) + + host.InstallationDiskID = "install-disk" + inventory.Disks = []*models.Disk{ + {ID: "install-disk", DriveType: models.DriveTypeSSD}, + } + + inventoryByte, _ := json.Marshal(inventory) + host.Inventory = string(inventoryByte) + + Expect(db.Create(&cluster).Error).ToNot(HaveOccurred()) + Expect(db.Create(&host).Error).ShouldNot(HaveOccurred()) + mockAndRefreshStatus(&host) + + refreshedHost := hostutil.GetHostFromDB(*host.ID, host.InfraEnvID, db).Host + + _, _, ok := getValidationResult(refreshedHost.ValidationsInfo, NoIscsiNicBelongsToMachineCidr) + Expect(ok).To(BeFalse()) + }) + It("iSCSI drive - no network interface set with host IP", func() { + var inventory models.Inventory + err := json.Unmarshal([]byte(host.Inventory), &inventory) + Expect(err).ShouldNot(HaveOccurred()) + + host.InstallationDiskID = "install-disk" + inventory.Disks = []*models.Disk{ + { + ID: "install-disk", + DriveType: models.DriveTypeISCSI, + Iscsi: &models.Iscsi{ + HostIPAddress: "99.99.99.99", + }}, + } + + inventoryByte, _ := json.Marshal(inventory) + host.Inventory = string(inventoryByte) + + Expect(db.Create(&cluster).Error).ToNot(HaveOccurred()) + Expect(db.Create(&host).Error).ShouldNot(HaveOccurred()) + mockAndRefreshStatus(&host) + + refreshedHost := hostutil.GetHostFromDB(*host.ID, host.InfraEnvID, db).Host + + status, message, ok := getValidationResult(refreshedHost.ValidationsInfo, NoIscsiNicBelongsToMachineCidr) + Expect(ok).To(BeTrue()) + Expect(status).To(Equal(ValidationError)) + Expect(message).To(Equal("Cannot find network interface associated to iSCSI host IP address")) + }) + + DescribeTable( + "iSCSI drive - malformed iSCSI properties", + func(iSCSI *models.Iscsi, expectedMessage string) { + var inventory models.Inventory + err := json.Unmarshal([]byte(host.Inventory), &inventory) + Expect(err).ShouldNot(HaveOccurred()) + + host.InstallationDiskID = "install-disk" + inventory.Disks = []*models.Disk{ + { + ID: "install-disk", + DriveType: models.DriveTypeISCSI, + Iscsi: iSCSI, + }, + } + + inventoryByte, _ := json.Marshal(inventory) + host.Inventory = string(inventoryByte) + + Expect(db.Create(&cluster).Error).ToNot(HaveOccurred()) + Expect(db.Create(&host).Error).ShouldNot(HaveOccurred()) + mockAndRefreshStatus(&host) + + refreshedHost := hostutil.GetHostFromDB(*host.ID, host.InfraEnvID, db).Host + + status, message, ok := getValidationResult(refreshedHost.ValidationsInfo, NoIscsiNicBelongsToMachineCidr) + Expect(ok).To(BeTrue()) + Expect(status).To(Equal(ValidationError)) + Expect(message).To(Equal(expectedMessage)) + }, + Entry("iSCSI is nil", nil, "iSCSI installation disk is missing host IP address"), + Entry("Host IP address is empty", &models.Iscsi{}, "Cannot find network interface associated to iSCSI host IP address"), + Entry("Host IP address is invalid", &models.Iscsi{HostIPAddress: "invalid"}, "Cannot find network interface associated to iSCSI host IP address"), + ) + DescribeTable( + "iSCSI drive - machine networks", + func(iSCSIHostIPAdress string, machineNetworks []*models.MachineNetwork, interfaces []*models.Interface, expectedStatus ValidationStatus, expectedMessage string) { + var inventory models.Inventory + err := json.Unmarshal([]byte(host.Inventory), &inventory) + Expect(err).ShouldNot(HaveOccurred()) + + host.InstallationDiskID = "install-disk" + inventory.Disks = []*models.Disk{ + { + ID: "install-disk", + DriveType: models.DriveTypeISCSI, + Iscsi: &models.Iscsi{ + HostIPAddress: iSCSIHostIPAdress, + }, + }, + } + inventory.Interfaces = interfaces + + inventoryByte, _ := json.Marshal(inventory) + host.Inventory = string(inventoryByte) + cluster.MachineNetworks = machineNetworks + Expect(db.Create(&cluster).Error).ToNot(HaveOccurred()) + Expect(db.Create(&host).Error).ShouldNot(HaveOccurred()) + mockAndRefreshStatus(&host) + + refreshedHost := hostutil.GetHostFromDB(*host.ID, host.InfraEnvID, db).Host + + status, message, ok := getValidationResult(refreshedHost.ValidationsInfo, NoIscsiNicBelongsToMachineCidr) + Expect(ok).To(BeTrue()) + Expect(message).To(Equal(expectedMessage)) + Expect(status).To(Equal(expectedStatus)) + }, + Entry("Machine networks is nil", "192.168.1.1", nil, nil, ValidationPending, "Missing inventory or machine network CIDR"), + Entry("Machine networks is empty", "192.168.1.1", []*models.MachineNetwork{}, nil, ValidationPending, "Missing inventory or machine network CIDR"), + Entry("Machine networks IPv4 overlaps with host IPv4 address", "192.168.1.1", + []*models.MachineNetwork{ + { + Cidr: "192.168.1.0/24", + ClusterID: clusterID, + }, + }, + []*models.Interface{ + { + IPV4Addresses: []string{"192.168.1.1/24"}, + }, + }, + ValidationFailure, "Network interface connected to iSCSI disk cannot belong to machine network CIDRs"), + Entry("Machine networks IPv6 overlaps with host IPv6 address", "2001:0db8:85a3::8a2e:0370:7334", + []*models.MachineNetwork{ + { + Cidr: "2001:0db8:85a3::/48", + ClusterID: clusterID, + }, + }, + []*models.Interface{ + { + IPV6Addresses: []string{"2001:0db8:85a3::8a2e:0370:7334/48"}, + }, + }, + ValidationFailure, "Network interface connected to iSCSI disk cannot belong to machine network CIDRs"), + Entry("Machine networks IPv6 uses the same interface as host IPv4 address", "192.168.1.1", + []*models.MachineNetwork{ + { + Cidr: "2001:0db8:85a3::/48", + ClusterID: clusterID, + }, + }, + []*models.Interface{ + { + IPV4Addresses: []string{"192.168.1.1/24"}, + IPV6Addresses: []string{"2001:0db8:85a3::8a2e:0370:7334/48"}, + }, + }, + ValidationFailure, "Network interface connected to iSCSI disk cannot belong to machine network CIDRs"), + Entry("Machine networks IPv4 uses the same interface as host IPv6 address", "2001:0db8:85a3::8a2e:0370:7334", + []*models.MachineNetwork{ + { + Cidr: "192.168.1.1/24", + ClusterID: clusterID, + }, + }, + []*models.Interface{ + { + IPV4Addresses: []string{"192.168.1.1/24"}, + IPV6Addresses: []string{"2001:0db8:85a3::8a2e:0370:7334/48"}, + }, + }, + ValidationFailure, "Network interface connected to iSCSI disk cannot belong to machine network CIDRs"), + Entry("Machine networks IPv4 and host IPv4 address use different interfaces", "192.168.1.1", + []*models.MachineNetwork{ + { + Cidr: "192.168.2.1/24", + ClusterID: clusterID, + }, + }, + []*models.Interface{ + { + IPV4Addresses: []string{"192.168.1.1/24"}, + }, + { + IPV4Addresses: []string{"192.168.2.1/24"}, + }, + }, + ValidationSuccess, "Network interface connected to iSCSI disk does not belong to machine network CIDRs"), + Entry("Machine networks IPv4/IPv6 and host IPv4 address use different interfaces", "192.168.1.1", + []*models.MachineNetwork{ + { + Cidr: "192.168.2.1/24", + ClusterID: clusterID, + }, + { + Cidr: "2001:0db8:85a3::/48", + ClusterID: clusterID, + }, + }, + []*models.Interface{ + { + IPV4Addresses: []string{"192.168.1.1/24"}, + }, + { + IPV4Addresses: []string{"192.168.2.1/24"}, + IPV6Addresses: []string{"2001:0db8:85a3::8a2e:0370:7334/48"}, + }, + }, + ValidationSuccess, "Network interface connected to iSCSI disk does not belong to machine network CIDRs"), + ) + }) }) diff --git a/internal/host/validator.go b/internal/host/validator.go index 17ceaff14e09..ce06e35273c1 100644 --- a/internal/host/validator.go +++ b/internal/host/validator.go @@ -1829,3 +1829,38 @@ func (v *validator) inventoryHasIP(inventory *models.Inventory, ipAddress string } return false, nil } + +func (v *validator) noIscsiNicBelongsToMachineCidr(c *validationContext) (ValidationStatus, string) { + installationDisk, err := hostutil.GetHostInstallationDisk(c.host) + if err != nil || installationDisk == nil { + return ValidationSuccessSuppressOutput, "" + } + + if installationDisk.DriveType != models.DriveTypeISCSI { + // Validation is not relevant in this case + return ValidationSuccessSuppressOutput, "" + } + + if c.inventory == nil || !network.IsMachineCidrAvailable(c.cluster) { + return ValidationPending, "Missing inventory or machine network CIDR" + } + + if installationDisk.Iscsi == nil { + // If this is nil, the disk shouldn't have passed the eligilibily test in the first place + v.log.Warn("iSCSI installation disk is missing host IP address") + return ValidationError, "iSCSI installation disk is missing host IP address" + } + + nic, err := network.FindInterfaceByIPString(installationDisk.Iscsi.HostIPAddress, c.inventory.Interfaces) + if err != nil { + v.log.WithError(err).Warn("Cannot find network interface associated to iSCSI host IP address") + return ValidationError, "Cannot find network interface associated to iSCSI host IP address" + } + + found := network.IsInterfaceInPrimaryMachineNetCidr(v.log, c.cluster, nic) + if found { + return ValidationFailure, "Network interface connected to iSCSI disk cannot belong to machine network CIDRs" + } + + return ValidationSuccess, "Network interface connected to iSCSI disk does not belong to machine network CIDRs" +} diff --git a/internal/network/machine_network_cidr.go b/internal/network/machine_network_cidr.go index 0c4285ff264b..1393d0f1d2ec 100644 --- a/internal/network/machine_network_cidr.go +++ b/internal/network/machine_network_cidr.go @@ -13,6 +13,7 @@ import ( "github.com/openshift/assisted-service/internal/common" "github.com/openshift/assisted-service/models" "github.com/pkg/errors" + "github.com/samber/lo" "github.com/sirupsen/logrus" ) @@ -516,25 +517,63 @@ func GetDefaultRouteNetworkByFamily(h *models.Host, networks map[AddressFamily][ return ret, fmt.Errorf("can not find cidr by route: no inventory for host %s", h.ID.String()) } -func IsHostInPrimaryMachineNetCidr(log logrus.FieldLogger, cluster *common.Cluster, host *models.Host) bool { - // The host should belong to all the networks specified as Machine Networks. +// Parse the Machine Network CIDRs into IPNet +func parseMachineNetworks(machineNetworks []*models.MachineNetwork) ([]*net.IPNet, error) { + var parsedCidr []*net.IPNet + for _, machineNet := range machineNetworks { + _, machineIpnet, err := net.ParseCIDR(string(machineNet.Cidr)) + if err != nil { + return nil, err + } + parsedCidr = append(parsedCidr, machineIpnet) + } + return parsedCidr, nil +} + +// Check if a host belongs to all the networks specified as Machine Networks. +func IsHostInPrimaryMachineNetCidr(log logrus.FieldLogger, cluster *common.Cluster, host *models.Host) bool { // TODO(mko) This rule should be revised as soon as OCP supports multiple machineNetwork // entries using the same IP stack. + return forEachMachineNetwork(log, cluster, func(agg bool, machineIpnet *net.IPNet, index int) bool { + // initialize agg to true the first time this function is called + if index == 0 { + agg = true + } + + return agg && belongsToNetwork(log, host, machineIpnet) + }) +} +// Check if an interface is part of one of the networks specified as Machine Networks +func IsInterfaceInPrimaryMachineNetCidr(log logrus.FieldLogger, cluster *common.Cluster, nic *models.Interface) bool { + return forEachMachineNetwork(log, cluster, func(agg bool, machineIpnet *net.IPNet, index int) bool { + // initialize agg to false the first time this function is called + if index == 0 { + agg = false + } + + isIPv4 := IsIPV4CIDR(machineIpnet.String()) + found, _ := findMatchingIP(machineIpnet, nic, isIPv4) + return agg || found + }) +} + +type machineNetworkCheckFunc func(bool, *net.IPNet, int) bool + +// Function calling a given check fonction on each of the networks specified in cluster's Machine Networks. +func forEachMachineNetwork(log logrus.FieldLogger, cluster *common.Cluster, checkFunc machineNetworkCheckFunc) bool { if !IsMachineCidrAvailable(cluster) { return false } - ret := true - for _, machineNet := range cluster.MachineNetworks { - _, machineIpnet, err := net.ParseCIDR(string(machineNet.Cidr)) - if err != nil { - return false - } - ret = ret && belongsToNetwork(log, host, machineIpnet) + machineNetwork, err := parseMachineNetworks(cluster.MachineNetworks) + if err != nil { + log.WithError(err).Warn("Failed to parse machine networks") + return false } - return ret + + return lo.Reduce(machineNetwork, checkFunc, false) } type IPSet map[strfmt.IPv4]struct{} diff --git a/internal/network/machine_network_cidr_test.go b/internal/network/machine_network_cidr_test.go index 3c1c67972e55..e5c12d55a275 100644 --- a/internal/network/machine_network_cidr_test.go +++ b/internal/network/machine_network_cidr_test.go @@ -5,6 +5,7 @@ import ( "testing" . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" "github.com/openshift/assisted-service/internal/common" "github.com/openshift/assisted-service/models" @@ -333,6 +334,55 @@ var _ = Describe("inventory", func() { }) }) + + Context("IsHostInPrimaryMachineNetCidr", func() { + + var log logrus.FieldLogger + + BeforeEach(func() { + log = logrus.New() + }) + + DescribeTable( + "IsHostInPrimaryMachineNetCidr", + func(nics []*models.Interface, machineNetworks []*models.MachineNetwork, expectedResult bool) { + cluster := createCluster("", "", createInventory(nics...)) + cluster.MachineNetworks = machineNetworks + res := IsHostInPrimaryMachineNetCidr(log, cluster, cluster.Hosts[0]) + Expect(res).To(Equal(expectedResult)) + }, + Entry("MachineNetworks is empty", []*models.Interface{createInterface("1.2.3.4/24")}, []*models.MachineNetwork{}, false), + Entry("MachineNetworks is malformed", []*models.Interface{createInterface("1.2.3.4/24")}, []*models.MachineNetwork{{Cidr: "a.b.c.d"}}, false), + Entry("Interfaces is empty", []*models.Interface{}, []*models.MachineNetwork{{Cidr: "a.b.c.d"}}, false), + Entry("Interface IP is malformed", []*models.Interface{createInterface("a.b.c.d/24")}, []*models.MachineNetwork{{Cidr: "1.2.3.4/24"}}, false), + Entry("Host belongs to all machine network CIDRs", []*models.Interface{createInterface("1.2.3.4/24"), addIPv6Addresses(createInterface(), "2001:db8::1/48")}, []*models.MachineNetwork{{Cidr: "1.2.3.0/24"}, {Cidr: "2001:db8::/48"}}, true), + Entry("Host doesn't belong to all machine network CIDRs", []*models.Interface{createInterface("1.2.3.4/24")}, []*models.MachineNetwork{{Cidr: "1.2.3.0/24"}, {Cidr: "2001:db8::a1/120"}}, false), + ) + }) + Context("IsInterfaceInPrimaryMachineNetCidr", func() { + + var log logrus.FieldLogger + + BeforeEach(func() { + log = logrus.New() + }) + + DescribeTable( + "IsInterfaceInPrimaryMachineNetCidr", + func(nic *models.Interface, machineNetworks []*models.MachineNetwork, expectedResult bool) { + cluster := createCluster("", "", createInventory(nic)) + cluster.MachineNetworks = machineNetworks + res := IsInterfaceInPrimaryMachineNetCidr(log, cluster, nic) + Expect(res).To(Equal(expectedResult)) + }, + Entry("MachineNetworks is empty", createInterface("1.2.3.4/24"), []*models.MachineNetwork{}, false), + Entry("MachineNetworks is malformed", createInterface("1.2.3.4/24"), []*models.MachineNetwork{{Cidr: "a.b.c.d"}}, false), + Entry("Interface IP is malformed", createInterface("a.b.c.d/24"), []*models.MachineNetwork{{Cidr: "1.2.3.4/24"}}, false), + Entry("Interface belongs to a IPv4 machine network CIDR", addIPv6Addresses(createInterface("1.2.3.4/24"), "2001:db8::a1/48"), []*models.MachineNetwork{{Cidr: "1.2.3.4/24"}}, true), + Entry("Interface belongs to a IPv6 machine network CIDR", addIPv6Addresses(createInterface("1.2.3.4/24"), "2001:db8::a1/48"), []*models.MachineNetwork{{Cidr: "2001:db8::/48"}}, true), + Entry("Interface doesn't belong to any machine network CIDRs", createInterface("1.2.3.4/24", "2001:db8::a1/48"), []*models.MachineNetwork{{Cidr: "5.6.7.8/24"}, {Cidr: "2001:db9::/48"}}, false), + ) + }) }) func TestMachineNetworkCidr(t *testing.T) { diff --git a/internal/network/utils.go b/internal/network/utils.go index ab6209c6b864..a63a40eb1d4d 100644 --- a/internal/network/utils.go +++ b/internal/network/utils.go @@ -4,6 +4,7 @@ import ( "fmt" "net" "net/http" + "net/netip" "sort" "strings" @@ -448,3 +449,31 @@ func UpdateVipsTables(db *gorm.DB, cluster *common.Cluster, apiVipUpdated bool, return nil } + +// Find the interface on which the given IP (v4 or v6) is set. +func FindInterfaceByIP(ip netip.Addr, interfaces []*models.Interface) (*models.Interface, error) { + for _, intf := range interfaces { + addresses := intf.IPV4Addresses + addresses = append(addresses, intf.IPV6Addresses...) + for _, address := range addresses { + prefix, err := netip.ParsePrefix(address) + if err != nil { + return nil, err + } + if ip.Compare(prefix.Addr()) == 0 { + return intf, nil + } + } + } + + return nil, fmt.Errorf("Cannot find the network interface on which the IP %s is set", ip.String()) +} + +// Wrapper function for FindInterfaceByIP that it accepts the IP as a string. +func FindInterfaceByIPString(ipAddress string, interfaces []*models.Interface) (*models.Interface, error) { + ip, err := netip.ParseAddr(ipAddress) + if err != nil { + return nil, err + } + return FindInterfaceByIP(ip, interfaces) +} diff --git a/internal/network/utils_test.go b/internal/network/utils_test.go index 33fb4f568c2e..8578a8102f4e 100644 --- a/internal/network/utils_test.go +++ b/internal/network/utils_test.go @@ -2,6 +2,7 @@ package network import ( . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" "github.com/openshift/assisted-service/internal/common" "github.com/openshift/assisted-service/models" @@ -1012,3 +1013,40 @@ var _ = Describe("GetVips", func() { }) }) }) +var _ = Describe("FindInterfaceByIP", func() { + var allInterfaces []*models.Interface + var searchedInterface models.Interface + + BeforeEach(func() { + searchedInterface = models.Interface{ + IPV4Addresses: []string{"192.168.1.3/24"}, + IPV6Addresses: []string{"1001:db8:0:200::78/48", "1001:db8:0:200::79/48"}, + } + allInterfaces = []*models.Interface{ + { + IPV4Addresses: []string{"192.168.1.1/24"}, + }, + { + IPV6Addresses: []string{"1001:db8:0:200::80/48"}, + }, + &searchedInterface, + } + + }) + DescribeTable("FindInterfaceByIP", func(allInterfaces *[]*models.Interface, lookForIp string, expectedInterface *models.Interface, expectedError string) { + int, err := FindInterfaceByIPString(lookForIp, *allInterfaces) + if expectedError == "" { + Expect(err).To(BeNil()) + Expect(int).To(Equal(expectedInterface)) + } else { + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(Equal(expectedError)) + } + }, + Entry("Look for IPv4", &allInterfaces, "192.168.1.3", &searchedInterface, ""), + Entry("Look for IPv6", &allInterfaces, "1001:db8:0:200::79", &searchedInterface, ""), + Entry("Invalid searched IP", &allInterfaces, "invalid", nil, "ParseAddr(\"invalid\"): unable to parse IP"), + Entry("Empty list all interfaces", &[]*models.Interface{}, "192.168.1.3", nil, "Cannot find the network interface on which the IP 192.168.1.3 is set"), + Entry("Searched IP does not match any of the interfaces", &allInterfaces, "99.99.99.99", nil, "Cannot find the network interface on which the IP 99.99.99.99 is set"), + ) +}) diff --git a/models/host_validation_id.go b/models/host_validation_id.go index dcaeb894c10f..0cc422b24c31 100644 --- a/models/host_validation_id.go +++ b/models/host_validation_id.go @@ -155,6 +155,9 @@ const ( // HostValidationIDNoIPCollisionsInNetwork captures enum value "no-ip-collisions-in-network" HostValidationIDNoIPCollisionsInNetwork HostValidationID = "no-ip-collisions-in-network" + + // HostValidationIDNoIscsiNicBelongsToMachineCidr captures enum value "no-iscsi-nic-belongs-to-machine-cidr" + HostValidationIDNoIscsiNicBelongsToMachineCidr HostValidationID = "no-iscsi-nic-belongs-to-machine-cidr" ) // for schema @@ -162,7 +165,7 @@ var hostValidationIdEnum []interface{} func init() { var res []HostValidationID - if err := json.Unmarshal([]byte(`["connected","media-connected","has-inventory","has-min-cpu-cores","has-min-valid-disks","has-min-memory","machine-cidr-defined","has-cpu-cores-for-role","has-memory-for-role","hostname-unique","hostname-valid","belongs-to-machine-cidr","ignition-downloadable","belongs-to-majority-group","valid-platform-network-settings","ntp-synced","time-synced-between-host-and-service","container-images-available","lso-requirements-satisfied","ocs-requirements-satisfied","odf-requirements-satisfied","lvm-requirements-satisfied","mce-requirements-satisfied","mtv-requirements-satisfied","sufficient-installation-disk-speed","cnv-requirements-satisfied","sufficient-network-latency-requirement-for-role","sufficient-packet-loss-requirement-for-role","has-default-route","api-domain-name-resolved-correctly","api-int-domain-name-resolved-correctly","apps-domain-name-resolved-correctly","release-domain-name-resolved-correctly","compatible-with-cluster-platform","dns-wildcard-not-configured","disk-encryption-requirements-satisfied","non-overlapping-subnets","vsphere-disk-uuid-enabled","compatible-agent","no-skip-installation-disk","no-skip-missing-disk","no-ip-collisions-in-network"]`), &res); err != nil { + if err := json.Unmarshal([]byte(`["connected","media-connected","has-inventory","has-min-cpu-cores","has-min-valid-disks","has-min-memory","machine-cidr-defined","has-cpu-cores-for-role","has-memory-for-role","hostname-unique","hostname-valid","belongs-to-machine-cidr","ignition-downloadable","belongs-to-majority-group","valid-platform-network-settings","ntp-synced","time-synced-between-host-and-service","container-images-available","lso-requirements-satisfied","ocs-requirements-satisfied","odf-requirements-satisfied","lvm-requirements-satisfied","mce-requirements-satisfied","mtv-requirements-satisfied","sufficient-installation-disk-speed","cnv-requirements-satisfied","sufficient-network-latency-requirement-for-role","sufficient-packet-loss-requirement-for-role","has-default-route","api-domain-name-resolved-correctly","api-int-domain-name-resolved-correctly","apps-domain-name-resolved-correctly","release-domain-name-resolved-correctly","compatible-with-cluster-platform","dns-wildcard-not-configured","disk-encryption-requirements-satisfied","non-overlapping-subnets","vsphere-disk-uuid-enabled","compatible-agent","no-skip-installation-disk","no-skip-missing-disk","no-ip-collisions-in-network","no-iscsi-nic-belongs-to-machine-cidr"]`), &res); err != nil { panic(err) } for _, v := range res { diff --git a/restapi/embedded_spec.go b/restapi/embedded_spec.go index 85a8e50c6c53..f59b9e547209 100644 --- a/restapi/embedded_spec.go +++ b/restapi/embedded_spec.go @@ -8354,7 +8354,8 @@ func init() { "compatible-agent", "no-skip-installation-disk", "no-skip-missing-disk", - "no-ip-collisions-in-network" + "no-ip-collisions-in-network", + "no-iscsi-nic-belongs-to-machine-cidr" ] }, "host_network": { @@ -19149,7 +19150,8 @@ func init() { "compatible-agent", "no-skip-installation-disk", "no-skip-missing-disk", - "no-ip-collisions-in-network" + "no-ip-collisions-in-network", + "no-iscsi-nic-belongs-to-machine-cidr" ] }, "host_network": { diff --git a/swagger.yaml b/swagger.yaml index d13a2e94ea28..c03e55014c8d 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -6503,6 +6503,7 @@ definitions: - 'no-skip-installation-disk' - 'no-skip-missing-disk' - 'no-ip-collisions-in-network' + - 'no-iscsi-nic-belongs-to-machine-cidr' dhcp_allocation_request: type: object diff --git a/vendor/github.com/openshift/assisted-service/models/host_validation_id.go b/vendor/github.com/openshift/assisted-service/models/host_validation_id.go index dcaeb894c10f..0cc422b24c31 100644 --- a/vendor/github.com/openshift/assisted-service/models/host_validation_id.go +++ b/vendor/github.com/openshift/assisted-service/models/host_validation_id.go @@ -155,6 +155,9 @@ const ( // HostValidationIDNoIPCollisionsInNetwork captures enum value "no-ip-collisions-in-network" HostValidationIDNoIPCollisionsInNetwork HostValidationID = "no-ip-collisions-in-network" + + // HostValidationIDNoIscsiNicBelongsToMachineCidr captures enum value "no-iscsi-nic-belongs-to-machine-cidr" + HostValidationIDNoIscsiNicBelongsToMachineCidr HostValidationID = "no-iscsi-nic-belongs-to-machine-cidr" ) // for schema @@ -162,7 +165,7 @@ var hostValidationIdEnum []interface{} func init() { var res []HostValidationID - if err := json.Unmarshal([]byte(`["connected","media-connected","has-inventory","has-min-cpu-cores","has-min-valid-disks","has-min-memory","machine-cidr-defined","has-cpu-cores-for-role","has-memory-for-role","hostname-unique","hostname-valid","belongs-to-machine-cidr","ignition-downloadable","belongs-to-majority-group","valid-platform-network-settings","ntp-synced","time-synced-between-host-and-service","container-images-available","lso-requirements-satisfied","ocs-requirements-satisfied","odf-requirements-satisfied","lvm-requirements-satisfied","mce-requirements-satisfied","mtv-requirements-satisfied","sufficient-installation-disk-speed","cnv-requirements-satisfied","sufficient-network-latency-requirement-for-role","sufficient-packet-loss-requirement-for-role","has-default-route","api-domain-name-resolved-correctly","api-int-domain-name-resolved-correctly","apps-domain-name-resolved-correctly","release-domain-name-resolved-correctly","compatible-with-cluster-platform","dns-wildcard-not-configured","disk-encryption-requirements-satisfied","non-overlapping-subnets","vsphere-disk-uuid-enabled","compatible-agent","no-skip-installation-disk","no-skip-missing-disk","no-ip-collisions-in-network"]`), &res); err != nil { + if err := json.Unmarshal([]byte(`["connected","media-connected","has-inventory","has-min-cpu-cores","has-min-valid-disks","has-min-memory","machine-cidr-defined","has-cpu-cores-for-role","has-memory-for-role","hostname-unique","hostname-valid","belongs-to-machine-cidr","ignition-downloadable","belongs-to-majority-group","valid-platform-network-settings","ntp-synced","time-synced-between-host-and-service","container-images-available","lso-requirements-satisfied","ocs-requirements-satisfied","odf-requirements-satisfied","lvm-requirements-satisfied","mce-requirements-satisfied","mtv-requirements-satisfied","sufficient-installation-disk-speed","cnv-requirements-satisfied","sufficient-network-latency-requirement-for-role","sufficient-packet-loss-requirement-for-role","has-default-route","api-domain-name-resolved-correctly","api-int-domain-name-resolved-correctly","apps-domain-name-resolved-correctly","release-domain-name-resolved-correctly","compatible-with-cluster-platform","dns-wildcard-not-configured","disk-encryption-requirements-satisfied","non-overlapping-subnets","vsphere-disk-uuid-enabled","compatible-agent","no-skip-installation-disk","no-skip-missing-disk","no-ip-collisions-in-network","no-iscsi-nic-belongs-to-machine-cidr"]`), &res); err != nil { panic(err) } for _, v := range res {