Repository: openshift/assisted-service (language: python)

Issue:
MGMT-18694, MGMT-18575, OCPBUGS-34849: Don't require mapping for names matching physical interfaces
For static network interface names that match physical interfaces don't require mac-address mapping. This allows the names to be in the networkConfig section only to handle, for example, disabled interfaces.

## List all the issues related to this PR

- [ ] New Feature <!-- new functionality -->
- [ ] Enhancement <!-- refactor, code changes, improvement, that won't add new features -->
- [x] Bug fix
- [ ] Tests
- [ ] Documentation
- [ ] CI/CD <!-- Notice that changes for Dockerfiles/Jenkinsfiles aren't tested in CI due to a known bug. -->

## What environments does this code impact?

- [ ] Automation (CI, tools, etc)
- [ ] Cloud
- [ ] Operator Managed Deployments
- [x] None

## How was this code tested?

<!-- Please, select one or more if needed: -->

- [ ] assisted-test-infra environment
- [x] dev-scripts environment
- [ ] Reviewer's test appreciated
- [ ] Waiting for CI to do a full test run
- [ ] Manual (Elaborate on how it was tested)
- [x] No tests needed

## Checklist

- [ ] Title and description added to both, commit and PR.
- [ ] Relevant issues have been associated (see [CONTRIBUTING] guide)
- [ ] This change does not require a documentation update (docstring, `docs`, README, etc)
- [ ] Does this change include unit-tests (note that code changes require unit-tests)

## Reviewers Checklist

- Are the title and description (in both PR and commit) meaningful and clear?
- Is there a bug required (and linked) for this change?
- Should this PR be backported?

[Kubernetes community documentation]: https://github.com/kubernetes/community/blob/master/contributors/guide/pull-requests.md#commit-message-guidelines
[CONTRIBUTING]: https://github.com/openshift/assisted-service/blob/master/CONTRIBUTING.md



Discussion / hints:

Fix patch:
diff --git a/pkg/staticnetworkconfig/generator.go b/pkg/staticnetworkconfig/generator.go
index d8ac5e36906..1fefe886d8e 100644
--- a/pkg/staticnetworkconfig/generator.go
+++ b/pkg/staticnetworkconfig/generator.go
@@ -6,6 +6,7 @@ import (
 	"context"
 	"fmt"
 	"path/filepath"
+	"regexp"
 	"sort"
 	"strings"
 
@@ -167,7 +168,8 @@ func (s *StaticNetworkConfigGenerator) formatNMConnection(nmConnection string) (
 
 func (s *StaticNetworkConfigGenerator) validateInterfaceNamesExistence(macInterfaceMap models.MacInterfaceMap, networksConfig []StaticNetworkConfigData) error {
 	interfaceNames := lo.Map(macInterfaceMap, func(m *models.MacInterfaceMapItems0, _ int) string { return m.LogicalNicName })
-	var matched bool
+	// See 'man systemd.net-naming-scheme' for interface naming protocol
+	predicatableIfaceNamePattern := regexp.MustCompile("^en[PsvxXbucaipod]")
 	for _, nc := range networksConfig {
 		cfg, err := ini.LoadSources(ini.LoadOptions{IgnoreInlineComment: true}, []byte(nc.FileContents))
 		if err != nil {
@@ -177,7 +179,8 @@ func (s *StaticNetworkConfigGenerator) validateInterfaceNamesExistence(macInterf
 		connectionSection := cfg.Section("connection")
 		interfaceName, err := connectionSection.GetKey("interface-name")
 		if err != nil {
-			return errors.Wrapf(err, "failed to get interface-name for file %s", nc.FilePath)
+			// When the id field is provided the interface-name will not be present
+			continue
 		}
 		interfaceType, err := connectionSection.GetKey("type")
 		if err != nil {
@@ -186,23 +189,14 @@ func (s *StaticNetworkConfigGenerator) validateInterfaceNamesExistence(macInterf
 		switch interfaceType.String() {
 		case "802-3-ethernet", "ethernet":
 			if !lo.Contains(interfaceNames, interfaceName.String()) {
-				return errors.Errorf("mac-interface mapping for interface %s is missing", interfaceName.String())
-			}
-			matched = true
-		case "vlan":
-			vlanSection := cfg.Section("vlan")
-			parent, err := vlanSection.GetKey("parent")
-			if err != nil {
-				return errors.Wrapf(err, "failed to get parent of vlan %s", interfaceName.Name())
-			}
-			if lo.Contains(interfaceNames, parent.String()) {
-				matched = true
+				if !predicatableIfaceNamePattern.MatchString(interfaceName.String()) {
+					return errors.Errorf("mac-interface mapping for interface %s is missing and not a physical interface", interfaceName.String())
+				} else {
+					s.log.Infof("Interface %s has no mac-interface mapping but matches a physical interface", interfaceName.String())
+				}
 			}
 		}
 	}
-	if !matched {
-		return errors.Errorf("no mac address mapping can be associated with the available network interfaces %v", interfaceNames)
-	}
 	return nil
 }
 
@@ -220,6 +214,9 @@ func (s *StaticNetworkConfigGenerator) ValidateStaticConfigParams(staticNetworkC
 }
 
 func (s *StaticNetworkConfigGenerator) validateMacInterfaceName(hostIdx int, macInterfaceMap models.MacInterfaceMap) error {
+	if len(macInterfaceMap) == 0 {
+		return fmt.Errorf("at least one interface for host %d must be provided", hostIdx)
+	}
 	interfaceCheck := make(map[string]struct{}, len(macInterfaceMap))
 	macCheck := make(map[string]struct{}, len(macInterfaceMap))
 	for _, macInterface := range macInterfaceMap {


Test patch:
diff --git a/pkg/staticnetworkconfig/generator_test.go b/pkg/staticnetworkconfig/generator_test.go
index e8e1f1dc67a..0e3cd9b3bda 100644
--- a/pkg/staticnetworkconfig/generator_test.go
+++ b/pkg/staticnetworkconfig/generator_test.go
@@ -238,17 +238,6 @@ var _ = Describe("validate mac interface mapping", func() {
     vlan:
       base-iface: eth1
       id: 101`
-		It("vlan without underlying interface - no mapping", func() {
-			err := staticNetworkGenerator.ValidateStaticConfigParams([]*models.HostStaticNetworkConfig{
-				{
-					MacInterfaceMap: nil,
-					NetworkYaml:     withoutUnderlyingInterface,
-				},
-			})
-			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(ContainSubstring("no mac address mapping can be associated with the available network interfaces"))
-		})
-
 		It("vlan with underlying interface - no mapping", func() {
 			err := staticNetworkGenerator.ValidateStaticConfigParams([]*models.HostStaticNetworkConfig{
 				{
@@ -288,6 +277,85 @@ var _ = Describe("validate mac interface mapping", func() {
 			Expect(err).ToNot(HaveOccurred())
 		})
 	})
+
+	Context("no mapping for physical interfaces", func() {
+		withPhysicalInterface := `interfaces:
+  - name: eth0
+    type: ethernet
+    state: up
+    ipv4:
+      enabled: true
+      dhcp: false
+      address:
+        - ip: 192.0.2.1
+          prefix-length: 24
+  - name: eno12399np0
+    type: ethernet
+    state: up
+    ipv4:
+      enabled: false
+      dhcp: false`
+		withNoMappedInterfaces := `interfaces:
+  - name: eno12345
+    type: ethernet
+    state: up
+    ipv4:
+      enabled: false
+      dhcp: false
+  - name: eno12399np0
+    type: ethernet
+    state: up
+    ipv4:
+      enabled: false
+      dhcp: false`
+		withMacIdentifier := `interfaces:
+  - name: eth0
+    type: ethernet
+    state: up
+    identifier: mac-address
+    mac-address: f8:75:a4:a4:00:fe
+    ipv4:
+      enabled: false
+      dhcp: false`
+		It("no mapping needed for physical interface", func() {
+			err := staticNetworkGenerator.ValidateStaticConfigParams([]*models.HostStaticNetworkConfig{
+				{
+					MacInterfaceMap: []*models.MacInterfaceMapItems0{
+						{
+							LogicalNicName: "eth0",
+							MacAddress:     "f8:75:a4:a4:00:fe",
+						},
+					},
+					NetworkYaml: withPhysicalInterface,
+				},
+			})
+			Expect(err).ToNot(HaveOccurred())
+		})
+		It("at least one mapped interface is required", func() {
+			err := staticNetworkGenerator.ValidateStaticConfigParams([]*models.HostStaticNetworkConfig{
+				{
+					MacInterfaceMap: []*models.MacInterfaceMapItems0{},
+					NetworkYaml:     withNoMappedInterfaces,
+				},
+			})
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("at least one interface for host"))
+		})
+		It("mac-identifier field is allowed", func() {
+			err := staticNetworkGenerator.ValidateStaticConfigParams([]*models.HostStaticNetworkConfig{
+				{
+					MacInterfaceMap: []*models.MacInterfaceMapItems0{
+						{
+							LogicalNicName: "eth0",
+							MacAddress:     "f8:75:a4:a4:00:fe",
+						},
+					},
+					NetworkYaml: withMacIdentifier,
+				},
+			})
+			Expect(err).ToNot(HaveOccurred())
+		})
+	})
 })
 
 var _ = Describe("StaticNetworkConfig", func() {