From 3e1a1da68b0b65f3f54d5ce6aae14de1e6ccab46 Mon Sep 17 00:00:00 2001
From: Milan Lenco <milan@zededa.com>
Date: Wed, 18 Sep 2024 12:08:26 +0200
Subject: [PATCH] Properly initialize AppNetworkStatus after invalid config was
 fixed

When the application network configuration fails validation,
the AppNetworkStatus will be marked with an error, while the AppNum and
MACGenerator fields will remain unset. If the user corrects the invalid
part of the configuration, the handleAppNetworkModify callback will be
triggered, and validation will succeed. However, before activating
the application network, the zedrouter must ensure that AppNum and
MACGenerator are initialized. Otherwise, the zedbox will panic with
a fatal error: "undefined MAC generator."

Issue addressed by this patch was introduced in version 11.5.0

Signed-off-by: Milan Lenco <milan@zededa.com>
(cherry picked from commit 3dc9624c758e2116d54dd6a4d719820a9a4cd2e5)
---
 pkg/pillar/cmd/zedrouter/appnetwork.go     | 26 ++++++++++++
 pkg/pillar/cmd/zedrouter/pubsubhandlers.go | 47 ++++++++++++----------
 2 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/pkg/pillar/cmd/zedrouter/appnetwork.go b/pkg/pillar/cmd/zedrouter/appnetwork.go
index 40a0609933b..8e8c8b059c8 100644
--- a/pkg/pillar/cmd/zedrouter/appnetwork.go
+++ b/pkg/pillar/cmd/zedrouter/appnetwork.go
@@ -404,3 +404,29 @@ func (z *zedrouter) doAppNetworkModifyAppIntfNum(appID uuid.UUID,
 	}
 	return nil
 }
+
+// For app already deployed (before node reboot), keep using the same MAC address
+// generator. Changing MAC addresses could break network config inside the app.
+func (z *zedrouter) selectMACGeneratorForApp(status *types.AppNetworkStatus) error {
+	appKey := types.UuidToNumKey{UUID: status.UUIDandVersion.UUID}
+	macGenerator, _, err := z.appMACGeneratorMap.Get(appKey)
+	if err != nil || macGenerator == types.MACGeneratorUnspecified {
+		// New app or an existing app but without MAC generator ID persisted.
+		if z.localLegacyMACAddr {
+			// Use older node-scoped MAC address generator.
+			macGenerator = types.MACGeneratorNodeScoped
+		} else {
+			// Use newer (and preferred) globally-scoped MAC address generator.
+			macGenerator = types.MACGeneratorGloballyScoped
+		}
+		// Remember which MAC generator is being used for this app.
+		err = z.appMACGeneratorMap.Assign(appKey, macGenerator, false)
+		if err != nil {
+			err = fmt.Errorf("failed to persist MAC generator ID for app %s/%s: %v",
+				status.UUIDandVersion.UUID, status.DisplayName, err)
+			return err
+		}
+	}
+	status.MACGenerator = macGenerator
+	return nil
+}
diff --git a/pkg/pillar/cmd/zedrouter/pubsubhandlers.go b/pkg/pillar/cmd/zedrouter/pubsubhandlers.go
index 4305111c596..80f2d386338 100644
--- a/pkg/pillar/cmd/zedrouter/pubsubhandlers.go
+++ b/pkg/pillar/cmd/zedrouter/pubsubhandlers.go
@@ -534,29 +534,12 @@ func (z *zedrouter) handleAppNetworkCreate(ctxArg interface{}, key string,
 	}
 	status.AppNum = appNum
 
-	// For app already deployed (before node reboot), keep using the same MAC address
-	// generator. Changing MAC addresses could break network config inside the app.
-	macGenerator, _, err := z.appMACGeneratorMap.Get(appNumKey)
-	if err != nil || macGenerator == types.MACGeneratorUnspecified {
-		// New app or an existing app but without MAC generator ID persisted.
-		if z.localLegacyMACAddr {
-			// Use older node-scoped MAC address generator.
-			macGenerator = types.MACGeneratorNodeScoped
-		} else {
-			// Use newer (and preferred) globally-scoped MAC address generator.
-			macGenerator = types.MACGeneratorGloballyScoped
-		}
-		// Remember which MAC generator is being used for this app.
-		err = z.appMACGeneratorMap.Assign(appNumKey, macGenerator, false)
-		if err != nil {
-			err = fmt.Errorf("failed to persist MAC generator ID for app %s/%s: %v",
-				config.UUIDandVersion.UUID, config.DisplayName, err)
-			z.log.Errorf("handleAppNetworkCreate(%v): %v", config.UUIDandVersion.UUID, err)
-			z.addAppNetworkError(&status, "handleAppNetworkCreate", err)
-			return
-		}
+	err = z.selectMACGeneratorForApp(&status)
+	if err != nil {
+		z.log.Errorf("handleAppNetworkCreate(%v): %v", config.UUIDandVersion.UUID, err)
+		z.addAppNetworkError(&status, "handleAppNetworkCreate", err)
+		return
 	}
-	status.MACGenerator = macGenerator
 	z.publishAppNetworkStatus(&status)
 
 	// Allocate application numbers on network instances.
@@ -617,6 +600,26 @@ func (z *zedrouter) handleAppNetworkModify(ctxArg interface{}, key string,
 		return
 	}
 
+	// Get or (less likely) allocate number to identify the application instance.
+	appNumKey := types.UuidToNumKey{UUID: newConfig.UUIDandVersion.UUID}
+	appNum, err := z.appNumAllocator.GetOrAllocate(appNumKey)
+	if err != nil {
+		err = fmt.Errorf("failed to allocate appNum for %s/%s: %v",
+			newConfig.UUIDandVersion.UUID, newConfig.DisplayName, err)
+		z.log.Errorf("handleAppNetworkModify(%v): %v", newConfig.UUIDandVersion.UUID, err)
+		z.addAppNetworkError(status, "handleAppNetworkModify", err)
+		return
+	}
+	status.AppNum = appNum
+
+	err = z.selectMACGeneratorForApp(status)
+	if err != nil {
+		z.log.Errorf("handleAppNetworkModify(%v): %v", newConfig.UUIDandVersion.UUID, err)
+		z.addAppNetworkError(status, "handleAppNetworkModify", err)
+		return
+	}
+	z.publishAppNetworkStatus(status)
+
 	// Update numbers allocated for application interfaces.
 	z.checkAppNetworkModifyAppIntfNums(newConfig, status)