Merge pull request #171 from yangcao77/rhtapbugs-113

fix the invalid kube yaml causing panic issue
devfile · Apr 18, 2023 · e75481b · e75481b
2 parents 0ce592a + d3d4fe8
commit e75481b
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 26 deletions.
diff --git a/pkg/devfile/parser/reader.go b/pkg/devfile/parser/reader.go
@@ -1,5 +1,5 @@
 //
-// Copyright 2022 Red Hat, Inc.
+// Copyright 2022-2023 Red Hat, Inc.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -121,7 +121,11 @@ func ParseKubernetesYaml(values []interface{}) (KubernetesResources, error) {
 			return KubernetesResources{}, err
 		}
 
-		kubernetesMap := value.(map[string]interface{})
+		var kubernetesMap map[string]interface{}
+		err = k8yaml.Unmarshal(byteData, &kubernetesMap)
+		if err != nil {
+			return KubernetesResources{}, err
+		}
 		kind := kubernetesMap["kind"]
 
 		switch kind {

diff --git a/pkg/devfile/parser/reader_test.go b/pkg/devfile/parser/reader_test.go
@@ -1,5 +1,5 @@
 //
-// Copyright 2022 Red Hat, Inc.
+// Copyright 2022-2023 Red Hat, Inc.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -72,7 +72,9 @@ func TestReadAndParseKubernetesYaml(t *testing.T) {
 		name                string
 		src                 YamlSrc
 		fs                  *afero.Afero
+		testParseYamlOnly   bool
 		wantErr             bool
+		wantParserErr       bool
 		wantDeploymentNames []string
 		wantServiceNames    []string
 		wantRouteNames      []string
@@ -111,6 +113,14 @@ func TestReadAndParseKubernetesYaml(t *testing.T) {
 			fs:      nil,
 			wantErr: true,
 		},
+		{
+			name: "Bad Path",
+			src: YamlSrc{
+				Path: "$%^&",
+			},
+			fs:      &fs,
+			wantErr: true,
+		},
 		{
 			name: "Read the YAML from the Data",
 			src: YamlSrc{
@@ -147,45 +157,55 @@ func TestReadAndParseKubernetesYaml(t *testing.T) {
 			fs:      nil,
 			wantErr: true,
 		},
+		{
+			name: "Invalid kube yaml Data",
+			src: YamlSrc{
+				Data: []byte("invalidyaml"),
+			},
+			fs:                nil,
+			testParseYamlOnly: true,
+			wantParserErr:     true,
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			values, err := ReadKubernetesYaml(tt.src, tt.fs)
 			if (err != nil) != tt.wantErr {
-				t.Errorf("unexpected error: %v", err)
+				t.Errorf("unexpected error: %v, wantErr: %v", err, tt.wantErr)
 				return
 			}
-
-			for _, value := range values {
-				kubernetesMap := value.(map[string]interface{})
-
-				kind := kubernetesMap["kind"]
-				metadataMap := kubernetesMap["metadata"].(map[string]interface{})
-				name := metadataMap["name"]
-
-				switch kind {
-				case "Deployment":
-					assert.Contains(t, tt.wantDeploymentNames, name)
-				case "Service":
-					assert.Contains(t, tt.wantServiceNames, name)
-				case "Route":
-					assert.Contains(t, tt.wantRouteNames, name)
-				case "Ingress":
-					assert.Contains(t, tt.wantIngressNames, name)
-				default:
-					assert.Contains(t, tt.wantOtherNames, name)
+			if !tt.testParseYamlOnly {
+				for _, value := range values {
+					kubernetesMap := value.(map[string]interface{})
+
+					kind := kubernetesMap["kind"]
+					metadataMap := kubernetesMap["metadata"].(map[string]interface{})
+					name := metadataMap["name"]
+
+					switch kind {
+					case "Deployment":
+						assert.Contains(t, tt.wantDeploymentNames, name)
+					case "Service":
+						assert.Contains(t, tt.wantServiceNames, name)
+					case "Route":
+						assert.Contains(t, tt.wantRouteNames, name)
+					case "Ingress":
+						assert.Contains(t, tt.wantIngressNames, name)
+					default:
+						assert.Contains(t, tt.wantOtherNames, name)
+					}
 				}
 			}
 
 			if len(values) > 0 {
 				resources, err := ParseKubernetesYaml(values)
-				if err != nil {
-					t.Error(err)
+				if (err != nil) != tt.wantParserErr {
+					t.Errorf("unexpected error: %v, wantErr: %v", err, tt.wantParserErr)
 					return
 				}
 
-				if reflect.DeepEqual(resources, KubernetesResources{}) {
+				if reflect.DeepEqual(resources, KubernetesResources{}) && !tt.wantParserErr {
 					t.Error("Kubernetes resources is empty, expected to contain some resources")
 				} else {
 					deployments := resources.Deployments