From 841b75be0cce03499587b8720da229d89c7e3953 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>
---
 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 4638c5e803..0420911b9c 100644
--- a/pkg/pillar/cmd/zedrouter/appnetwork.go
+++ b/pkg/pillar/cmd/zedrouter/appnetwork.go
@@ -410,3 +410,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 af058b099f..713bd78b91 100644
--- a/pkg/pillar/cmd/zedrouter/pubsubhandlers.go
+++ b/pkg/pillar/cmd/zedrouter/pubsubhandlers.go
@@ -502,29 +502,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.
@@ -585,6 +568,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)