MAISTRA-1982 Fix panic in conversion of logging level in v1 SMCP

pkg/apis/maistra/conversion/logging.go

@@ -54,25 +54,31 @@ func componentLogLevelsToString(logLevels v2.ComponentLogLevels) string {
 	return strings.Join(componentLogLevels, ",")
 }
 
-func componentLogLevelsFromString(levels string) v2.ComponentLogLevels {
+func componentLogLevelsFromString(levels string) (v2.ComponentLogLevels, error) {
 	componentLevels := strings.Split(levels, ",")
 	if len(componentLevels) == 0 {
-		return nil
+		return nil, nil
 	}
 	logLevels := v2.ComponentLogLevels{}
 	for _, componentLevel := range componentLevels {
 		pair := strings.SplitN(componentLevel, ":", 2)
+		if len(pair) < 2 || pair[0] == "" || pair[1] == "" {
+			return nil, fmt.Errorf("Invalid entry %q in spec.istio.globals.logging.level. The correct format is 'component:level[,component:level]'.", componentLevel)
+		}
 		logLevels[v2.EnvoyComponent(pair[0])] = v2.LogLevel(pair[1])
 	}
-	return logLevels
+	return logLevels, nil
 }
 
 func populateControlPlaneLoggingConfig(in *v1.HelmValues, out *v2.ControlPlaneSpec) error {
 	logging := &v2.LoggingConfig{}
 	setLogging := false
 
 	if componentLevels, ok, err := in.GetAndRemoveString("global.logging.level"); ok && len(componentLevels) > 0 {
-		logging.ComponentLevels = componentLogLevelsFromString(componentLevels)
+		logging.ComponentLevels, err = componentLogLevelsFromString(componentLevels)
+		if err != nil {
+			return err
+		}
 		setLogging = true
 	} else if err != nil {
 		return err
@@ -103,7 +109,10 @@ func populateProxyLoggingConfig(proxyValues *v1.HelmValues, logging *v2.ProxyLog
 		return false, err
 	}
 	if componentLevels, ok, err := proxyValues.GetAndRemoveString("componentLogLevel"); ok && len(componentLevels) > 0 {
-		logging.ComponentLevels = componentLogLevelsFromString(componentLevels)
+		logging.ComponentLevels, err = componentLogLevelsFromString(componentLevels)
+		if err != nil {
+			return false, err
+		}
 		setLogging = true
 	} else if err != nil {
 		return false, err

pkg/apis/maistra/conversion/logging_test.go

@@ -97,3 +97,85 @@ func TestLoggingConversionFromV2(t *testing.T) {
 		})
 	}
 }
+
+func TestComponentLogLevelsFromString(t *testing.T) {
+	testCases := []struct{
+		logLevelsString string
+		expectError bool
+		expectedLogLevels v2.ComponentLogLevels
+	} {
+		{
+			logLevelsString: "",
+			expectedLogLevels: nil,
+		},
+		{
+			logLevelsString: "admin:info",
+			expectedLogLevels: v2.ComponentLogLevels{
+				v2.EnvoyComponentAdmin: v2.LogLevelInfo,
+			},
+		},
+		{
+			logLevelsString: "admin:info,client:debug",
+			expectedLogLevels: v2.ComponentLogLevels{
+				v2.EnvoyComponentAdmin: v2.LogLevelInfo,
+				v2.EnvoyComponentClient: v2.LogLevelDebug,
+			},
+		},
+		{
+			logLevelsString: "unknown_component:info",
+			expectedLogLevels: v2.ComponentLogLevels{
+				"unknown_component": v2.LogLevelInfo,
+			},
+		},
+		{
+			logLevelsString: "admin:non_standard_level",
+			expectedLogLevels: v2.ComponentLogLevels{
+				v2.EnvoyComponentAdmin: "non_standard_level",
+			},
+		},
+		{
+			logLevelsString: "bad_format",
+			expectError: true,
+		},
+		{
+			logLevelsString: "no_level:",
+			expectError: true,
+		},
+		{
+			logLevelsString: ":no_component",
+			expectError: true,
+		},
+		{
+			logLevelsString: ":",
+			expectError: true,
+		},
+		{
+			logLevelsString: "consecutive_commas:info,,",
+			expectError: true,
+		},
+		{
+			logLevelsString: ",,,",
+			expectError: true,
+		},
+		{
+			logLevelsString: ":,:,:",
+			expectError: true,
+		},
+		{
+			logLevelsString: "bad_format,bad_format",
+			expectError: true,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run("string=" + tc.logLevelsString, func(t *testing.T) {
+			logLevels, err := componentLogLevelsFromString(tc.logLevelsString)
+			if tc.expectError {
+				if err == nil {
+					t.Fatal("Expected function call to fail, but it didn't")
+				}
+			} else {
+				assertEquals(t, tc.expectedLogLevels, logLevels)
+			}
+		})
+	}
+}