From 88946b65ac8d82e7e886b825ff76d8d1bc51893a Mon Sep 17 00:00:00 2001 From: Stephanie Date: Tue, 18 Apr 2023 16:29:25 +0800 Subject: [PATCH 1/3] fix the invalid kube yaml causing panic issue Signed-off-by: Stephanie --- pkg/devfile/parser/reader.go | 9 ++++- pkg/devfile/parser/reader_test.go | 67 ++++++++++++++++++++----------- 2 files changed, 50 insertions(+), 26 deletions(-) diff --git a/pkg/devfile/parser/reader.go b/pkg/devfile/parser/reader.go index d55c2f27..be4a1a54 100644 --- 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,12 @@ func ParseKubernetesYaml(values []interface{}) (KubernetesResources, error) { return KubernetesResources{}, err } - kubernetesMap := value.(map[string]interface{}) + // 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 index 414ca4eb..f2749837 100644 --- 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,54 @@ func TestReadAndParseKubernetesYaml(t *testing.T) { fs: nil, wantErr: true, }, + { + name: "Invalid kube yaml Data", + src: YamlSrc{ + Data: []byte("invalidyaml"), + }, + fs: nil, + 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 From 5a0c2d54ff10c57b84f630fe130bd1b9de8b414a Mon Sep 17 00:00:00 2001 From: Stephanie Date: Tue, 18 Apr 2023 22:24:50 +0800 Subject: [PATCH 2/3] remove the commented code Signed-off-by: Stephanie --- pkg/devfile/parser/reader.go | 1 - pkg/devfile/parser/reader_test.go | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/devfile/parser/reader.go b/pkg/devfile/parser/reader.go index be4a1a54..a9c27d46 100644 --- a/pkg/devfile/parser/reader.go +++ b/pkg/devfile/parser/reader.go @@ -121,7 +121,6 @@ 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 { diff --git a/pkg/devfile/parser/reader_test.go b/pkg/devfile/parser/reader_test.go index f2749837..62bdbe89 100644 --- a/pkg/devfile/parser/reader_test.go +++ b/pkg/devfile/parser/reader_test.go @@ -162,8 +162,9 @@ func TestReadAndParseKubernetesYaml(t *testing.T) { src: YamlSrc{ Data: []byte("invalidyaml"), }, - fs: nil, - wantParserErr: true, + fs: nil, + testParseYamlOnly: true, + wantParserErr: true, }, } From d3d4fe8a783883a061308a4f3d74b75cbad7bd49 Mon Sep 17 00:00:00 2001 From: Stephanie Date: Tue, 18 Apr 2023 22:43:06 +0800 Subject: [PATCH 3/3] fix test failure Signed-off-by: Stephanie --- pkg/devfile/parser/reader_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/devfile/parser/reader_test.go b/pkg/devfile/parser/reader_test.go index 62bdbe89..508b0e58 100644 --- a/pkg/devfile/parser/reader_test.go +++ b/pkg/devfile/parser/reader_test.go @@ -175,7 +175,7 @@ func TestReadAndParseKubernetesYaml(t *testing.T) { t.Errorf("unexpected error: %v, wantErr: %v", err, tt.wantErr) return } - if tt.testParseYamlOnly { + if !tt.testParseYamlOnly { for _, value := range values { kubernetesMap := value.(map[string]interface{})