[FAB-6399] Benign panic in config update computation

This CR adds a check for the length of a config item's path and
triggers a panic if it is zero. The decision to still trigger
a panic stems from the fact that if the item's path is zero, it
means something unexpected occurred in the validation logic and
we should not proceed. The user who originally reported this bug
likely hit a config issue that is no longer possible after the
fix for FAB-5606.

Change-Id: I4052174c04e0810714823c3ca8a533ab2f261804
Signed-off-by: Will Lahti <wtlahti@us.ibm.com>

Author: wlahti

common/configtx/update.go

@@ -178,7 +178,10 @@ func (cm *configManager) policyForItem(item comparable) (policies.Policy, bool)
 	// If the mod_policy path is relative, get the right manager for the context
 	// if the mod_policy path is absolute (starts with /) evaluate at the root
 	if len(modPolicy) > 0 && modPolicy[0] != policies.PathSeparator[0] {
-		// path is always at least of length 1
+		// path should always be at least length 1, panic if not
+		if len(item.path) == 0 {
+			logger.Panicf("Empty path for item %s", item.key)
+		}
 		var ok bool
 		manager, ok = manager.Manager(item.path[1:])
 		if !ok {

common/configtx/update_test.go

@@ -152,41 +152,62 @@ func TestPolicyForItem(t *testing.T) {
 		},
 	}
 
-	policy, ok := cm.policyForItem(comparable{
-		path: []string{"root"},
-		ConfigValue: &cb.ConfigValue{
-			ModPolicy: "rootPolicy",
-		},
+	t.Run("Root manager", func(t *testing.T) {
+		policy, ok := cm.policyForItem(comparable{
+			path: []string{"root"},
+			ConfigValue: &cb.ConfigValue{
+				ModPolicy: "rootPolicy",
+			},
+		})
+		assert.True(t, ok)
+		assert.Equal(t, policy, rootPolicy, "Should have found relative policy off the root manager")
 	})
-	assert.True(t, ok)
-	assert.Equal(t, policy, rootPolicy, "Should have found relative policy off the root manager")
 
-	policy, ok = cm.policyForItem(comparable{
-		path: []string{"root", "wrong"},
-		ConfigValue: &cb.ConfigValue{
-			ModPolicy: "rootPolicy",
-		},
+	t.Run("Nonexistent manager", func(t *testing.T) {
+		_, ok := cm.policyForItem(comparable{
+			path: []string{"root", "wrong"},
+			ConfigValue: &cb.ConfigValue{
+				ModPolicy: "rootPolicy",
+			},
+		})
+		assert.False(t, ok, "Should not have found rootPolicy off a nonexistent manager")
 	})
-	assert.False(t, ok, "Should not have found rootPolicy off a non-existent manager")
 
-	policy, ok = cm.policyForItem(comparable{
-		path: []string{"root", "foo"},
-		ConfigValue: &cb.ConfigValue{
-			ModPolicy: "foo",
-		},
+	t.Run("Foo manager", func(t *testing.T) {
+		policy, ok := cm.policyForItem(comparable{
+			path: []string{"root", "foo"},
+			ConfigValue: &cb.ConfigValue{
+				ModPolicy: "foo",
+			},
+		})
+		assert.True(t, ok)
+		assert.Equal(t, policy, fooPolicy, "Should have found relative foo policy off the foo manager")
 	})
-	assert.True(t, ok)
-	assert.Equal(t, policy, fooPolicy, "Should not have found relative foo policy the foo manager")
 
-	policy, ok = cm.policyForItem(comparable{
-		key:  "foo",
-		path: []string{"root"},
-		ConfigGroup: &cb.ConfigGroup{
-			ModPolicy: "foo",
-		},
+	t.Run("Foo group", func(t *testing.T) {
+		policy, ok := cm.policyForItem(comparable{
+			key:  "foo",
+			path: []string{"root"},
+			ConfigGroup: &cb.ConfigGroup{
+				ModPolicy: "foo",
+			},
+		})
+		assert.True(t, ok)
+		assert.Equal(t, policy, fooPolicy, "Should have found relative foo policy for foo group")
+	})
+
+	t.Run("Empty item path", func(t *testing.T) {
+		policyForItemEmptyPath := func() {
+			cm.policyForItem(comparable{
+				key:  "foo",
+				path: []string{},
+				ConfigGroup: &cb.ConfigGroup{
+					ModPolicy: "foo",
+				},
+			})
+		}
+		assert.Panics(t, policyForItemEmptyPath)
 	})
-	assert.True(t, ok)
-	assert.Equal(t, policy, fooPolicy, "Should have found relative foo policy for foo group")
 }
 
 func TestValidateModPolicy(t *testing.T) {