Address review feedback

Signed-off-by: Maysun J Faisal <[email protected]>

Author: maysunfaisal

README.md

@@ -145,6 +145,14 @@ The function documentation can be accessed via [pkg.go.dev](https://pkg.go.dev/g
    // write to the devfile on disk
    err = devfile.WriteYamlDevfile()
    ```
+6. To parse the outerloop Kubernetes/OpenShift component's uri or inline content, call the read and parse functions
+   ```go
+   // Read the YAML content
+   values, err := ReadKubernetesYaml(src, fs)
+
+   // Get the Kubernetes resources
+   resources, err := ParseKubernetesYaml(values)
+   ```
 
 ## Projects using devfile/library
 

pkg/devfile/parser/reader.go

@@ -13,40 +13,46 @@ import (
 	routev1 "github.com/openshift/api/route/v1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	extensionsv1 "k8s.io/api/extensions/v1beta1"
 )
 
 // YamlSrc specifies the src of the yaml in either Path, URL or Data format
 type YamlSrc struct {
-	// Path is a relative or absolute yaml path.
+	// Path to the yaml file
 	Path string
-	// URL is the URL address of the specific yaml.
+	// URL of the yaml file
 	URL string
-	// Data is the yaml content in []byte format.
+	// Data is the yaml content in []byte format
 	Data []byte
 }
 
+// KubernetesResources struct contains the Deployments, Services,
+// Routes and Ingresses resources
+type KubernetesResources struct {
+	Deployments []appsv1.Deployment
+	Services    []corev1.Service
+	Routes      []routev1.Route
+	Ingresses   []extensionsv1.Ingress
+}
+
 // ReadKubernetesYaml reads a yaml Kubernetes file from either the Path, URL or Data provided.
-// It returns Deployments, Services, Routes resources as the primary Kubernetes resources.
-// Other Kubernetes resources are returned as []byte type. Consumers interested in other Kubernetes resources
-// are expected to Unmarshal it to the struct of the respective resource.
-func ReadKubernetesYaml(src YamlSrc, fs filesystem.Filesystem) ([]appsv1.Deployment, []corev1.Service, []routev1.Route, [][]byte, error) {
+// It returns all the parsed Kubernetes objects as an array of interface.
+// Consumers interested in the Kubernetes resources are expected to Unmarshal
+// it to the struct of the respective Kubernetes resource.
+func ReadKubernetesYaml(src YamlSrc, fs filesystem.Filesystem) ([]interface{}, error) {
 
 	var data []byte
 	var err error
 
 	if src.URL != "" {
 		data, err = util.DownloadFileInMemory(src.URL)
 		if err != nil {
-			return nil, nil, nil, nil, errors.Wrapf(err, "failed to download file %q", src.URL)
+			return nil, errors.Wrapf(err, "failed to download file %q", src.URL)
 		}
 	} else if src.Path != "" {
-		absPath, err := util.GetAbsPath(src.Path)
-		if err != nil {
-			return nil, nil, nil, nil, err
-		}
-		data, err = fs.ReadFile(absPath)
+		data, err = fs.ReadFile(src.Path)
 		if err != nil {
-			return nil, nil, nil, nil, errors.Wrapf(err, "failed to read yaml from path %q", src.Path)
+			return nil, errors.Wrapf(err, "failed to read yaml from path %q", src.Path)
 		}
 	} else if len(src.Data) > 0 {
 		data = src.Data
@@ -61,24 +67,34 @@ func ReadKubernetesYaml(src YamlSrc, fs filesystem.Filesystem) ([]appsv1.Deploym
 			if err == io.EOF {
 				break
 			}
-			return nil, nil, nil, nil, err
+			return nil, err
 		}
 		values = append(values, value)
 	}
 
+	return values, nil
+}
+
+// ParseKubernetesYaml Unmarshals the interface array of the Kubernetes resources
+// and returns it as a KubernetesResources struct. Only Deployment, Service, Route
+// and Ingress are processed. Consumers interested in other Kubernetes resources
+// are expected to parse the values interface array an Unmarshal it to their
+// desired Kuberenetes struct
+func ParseKubernetesYaml(values []interface{}) (KubernetesResources, error) {
 	var deployments []appsv1.Deployment
 	var services []corev1.Service
 	var routes []routev1.Route
-	var otherResources [][]byte
+	var ingresses []extensionsv1.Ingress
 
 	for _, value := range values {
 		var deployment appsv1.Deployment
 		var service corev1.Service
 		var route routev1.Route
+		var ingress extensionsv1.Ingress
 
 		byteData, err := k8yaml.Marshal(value)
 		if err != nil {
-			return nil, nil, nil, nil, err
+			return KubernetesResources{}, err
 		}
 
 		kubernetesMap := value.(map[string]interface{})
@@ -94,14 +110,20 @@ func ReadKubernetesYaml(src YamlSrc, fs filesystem.Filesystem) ([]appsv1.Deploym
 		case "Route":
 			err = k8yaml.Unmarshal(byteData, &route)
 			routes = append(routes, route)
-		default:
-			otherResources = append(otherResources, byteData)
+		case "Ingress":
+			err = k8yaml.Unmarshal(byteData, &ingress)
+			ingresses = append(ingresses, ingress)
 		}
 
 		if err != nil {
-			return nil, nil, nil, nil, err
+			return KubernetesResources{}, err
 		}
 	}
 
-	return deployments, services, routes, otherResources, nil
+	return KubernetesResources{
+		Deployments: deployments,
+		Services:    services,
+		Routes:      routes,
+		Ingresses:   ingresses,
+	}, nil
 }

pkg/devfile/parser/reader_test.go

@@ -4,16 +4,15 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"reflect"
 	"testing"
 
 	"github.com/devfile/library/pkg/testingutil/filesystem"
 	"github.com/devfile/library/pkg/util"
 	"github.com/stretchr/testify/assert"
-	corev1 "k8s.io/api/core/v1"
-	k8yaml "sigs.k8s.io/yaml"
 )
 
-func TestReadKubernetesYaml(t *testing.T) {
+func TestReadAndParseKubernetesYaml(t *testing.T) {
 	const serverIP = "127.0.0.1:9080"
 	var data []byte
 
@@ -62,6 +61,7 @@ func TestReadKubernetesYaml(t *testing.T) {
 		wantDeploymentNames []string
 		wantServiceNames    []string
 		wantRouteNames      []string
+		wantIngressNames    []string
 		wantOtherNames      []string
 	}{
 		{
@@ -70,32 +70,35 @@ func TestReadKubernetesYaml(t *testing.T) {
 				URL: "http://" + serverIP,
 			},
 			fs:                  filesystem.DefaultFs{},
-			wantDeploymentNames: []string{"deploy-sample"},
-			wantServiceNames:    []string{"service-sample"},
-			wantRouteNames:      []string{"route-sample"},
-			wantOtherNames:      []string{"pvc-sample"},
+			wantDeploymentNames: []string{"deploy-sample", "deploy-sample-2"},
+			wantServiceNames:    []string{"service-sample", "service-sample-2"},
+			wantRouteNames:      []string{"route-sample", "route-sample-2"},
+			wantIngressNames:    []string{"ingress-sample", "ingress-sample-2"},
+			wantOtherNames:      []string{"pvc-sample", "pvc-sample-2"},
 		},
 		{
 			name: "Read the YAML from the Path",
 			src: YamlSrc{
 				Path: "../../../tests/yamls/resources.yaml",
 			},
 			fs:                  filesystem.DefaultFs{},
-			wantDeploymentNames: []string{"deploy-sample"},
-			wantServiceNames:    []string{"service-sample"},
-			wantRouteNames:      []string{"route-sample"},
-			wantOtherNames:      []string{"pvc-sample"},
+			wantDeploymentNames: []string{"deploy-sample", "deploy-sample-2"},
+			wantServiceNames:    []string{"service-sample", "service-sample-2"},
+			wantRouteNames:      []string{"route-sample", "route-sample-2"},
+			wantIngressNames:    []string{"ingress-sample", "ingress-sample-2"},
+			wantOtherNames:      []string{"pvc-sample", "pvc-sample-2"},
 		},
 		{
 			name: "Read the YAML from the Data",
 			src: YamlSrc{
 				Data: data,
 			},
 			fs:                  filesystem.DefaultFs{},
-			wantDeploymentNames: []string{"deploy-sample"},
-			wantServiceNames:    []string{"service-sample"},
-			wantRouteNames:      []string{"route-sample"},
-			wantOtherNames:      []string{"pvc-sample"},
+			wantDeploymentNames: []string{"deploy-sample", "deploy-sample-2"},
+			wantServiceNames:    []string{"service-sample", "service-sample-2"},
+			wantRouteNames:      []string{"route-sample", "route-sample-2"},
+			wantIngressNames:    []string{"ingress-sample", "ingress-sample-2"},
+			wantOtherNames:      []string{"pvc-sample", "pvc-sample-2"},
 		},
 		{
 			name: "Bad URL",
@@ -125,27 +128,61 @@ func TestReadKubernetesYaml(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			deployments, services, routes, others, err := ReadKubernetesYaml(tt.src, tt.fs)
+			values, err := ReadKubernetesYaml(tt.src, tt.fs)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("unexpected error: %v", err)
 				return
 			}
-			for _, deploy := range deployments {
-				assert.Contains(t, tt.wantDeploymentNames, deploy.Name)
-			}
-			for _, svc := range services {
-				assert.Contains(t, tt.wantServiceNames, svc.Name)
-			}
-			for _, route := range routes {
-				assert.Contains(t, tt.wantRouteNames, route.Name)
+
+			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)
+				}
 			}
-			for _, other := range others {
-				pvc := corev1.PersistentVolumeClaim{}
-				err = k8yaml.Unmarshal(other, &pvc)
+
+			if len(values) > 0 {
+				resources, err := ParseKubernetesYaml(values)
 				if err != nil {
 					t.Error(err)
+					return
+				}
+
+				if reflect.DeepEqual(resources, KubernetesResources{}) {
+					t.Error("Kubernetes resources is empty, expected to contain some resources")
+				} else {
+					deployments := resources.Deployments
+					services := resources.Services
+					routes := resources.Routes
+					ingresses := resources.Ingresses
+
+					for _, deploy := range deployments {
+						assert.Contains(t, tt.wantDeploymentNames, deploy.Name)
+					}
+					for _, svc := range services {
+						assert.Contains(t, tt.wantServiceNames, svc.Name)
+					}
+					for _, route := range routes {
+						assert.Contains(t, tt.wantRouteNames, route.Name)
+					}
+					for _, ingress := range ingresses {
+						assert.Contains(t, tt.wantIngressNames, ingress.Name)
+					}
 				}
-				assert.Contains(t, tt.wantOtherNames, pvc.Name)
 			}
 		})
 	}