Fix issue with variables not substituted in uri-referenced manifests (#5451)

pkg/deploy/deploy.go

@@ -59,7 +59,7 @@ func (o *deployHandler) ApplyImage(img v1alpha2.Component) error {
 func (o *deployHandler) ApplyKubernetes(kubernetes v1alpha2.Component) error {
 
 	// Validate if the GVRs represented by Kubernetes inlined components are supported by the underlying cluster
-	_, err := service.ValidateResourceExist(o.kubeClient, kubernetes, o.path)
+	_, err := service.ValidateResourceExist(o.kubeClient, o.devfileObj, kubernetes, o.path)
 	if err != nil {
 		return err
 	}
@@ -75,7 +75,7 @@ func (o *deployHandler) ApplyKubernetes(kubernetes v1alpha2.Component) error {
 	odolabels.SetProjectType(annotations, component.GetComponentTypeFromDevfileMetadata(o.devfileObj.Data.GetMetadata()))
 
 	// Get the Kubernetes component
-	u, err := libdevfile.GetK8sComponentAsUnstructured(kubernetes.Kubernetes, o.path, devfilefs.DefaultFs{})
+	u, err := libdevfile.GetK8sComponentAsUnstructured(o.devfileObj, kubernetes.Name, o.path, devfilefs.DefaultFs{})
 	if err != nil {
 		return err
 	}

pkg/devfile/adapters/kubernetes/component/adapter.go

@@ -196,13 +196,13 @@ func (a Adapter) Push(parameters common.PushParameters) (err error) {
 	}
 
 	// validate if the GVRs represented by Kubernetes inlined components are supported by the underlying cluster
-	err = service.ValidateResourcesExist(a.Client, k8sComponents, a.Context)
+	err = service.ValidateResourcesExist(a.Client, a.Devfile, k8sComponents, a.Context)
 	if err != nil {
 		return err
 	}
 
 	// create the Kubernetes objects from the manifest and delete the ones not in the devfile
-	err = service.PushKubernetesResources(a.Client, k8sComponents, labels, annotations, a.Context)
+	err = service.PushKubernetesResources(a.Client, a.Devfile, k8sComponents, labels, annotations, a.Context)
 	if err != nil {
 		return fmt.Errorf("failed to create service(s) associated with the component: %w", err)
 	}
@@ -246,14 +246,14 @@ func (a Adapter) Push(parameters common.PushParameters) (err error) {
 
 	// Update all services with owner references
 	err = a.Client.TryWithBlockOwnerDeletion(ownerReference, func(ownerRef metav1.OwnerReference) error {
-		return service.UpdateServicesWithOwnerReferences(a.Client, k8sComponents, ownerRef, a.Context)
+		return service.UpdateServicesWithOwnerReferences(a.Client, a.Devfile, k8sComponents, ownerRef, a.Context)
 	})
 	if err != nil {
 		return err
 	}
 
 	// create the Kubernetes objects from the manifest and delete the ones not in the devfile
-	needRestart, err := service.PushLinks(a.Client, k8sComponents, labels, a.deployment, a.Context)
+	needRestart, err := service.PushLinks(a.Client, a.Devfile, k8sComponents, labels, a.deployment, a.Context)
 	if err != nil {
 		return fmt.Errorf("failed to create service(s) associated with the component: %w", err)
 	}

pkg/libdevfile/command_apply.go

@@ -37,7 +37,7 @@ func (o *applyCommand) Execute(handler Handler) error {
 	}
 
 	if len(devfileComponents) != 1 {
-		return NewComponentsWithSameNameError()
+		return NewComponentsWithSameNameError(o.command.Apply.Component)
 	}
 
 	component, err := newComponent(o.devfileObj, devfileComponents[0])

pkg/libdevfile/component_kubernetes_utils.go

@@ -6,19 +6,16 @@ import (
 	"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
 	devfilefs "github.com/devfile/library/pkg/testingutil/filesystem"
 	"github.com/ghodss/yaml"
-	"github.com/redhat-developer/odo/pkg/util"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
 // GetK8sComponentAsUnstructured parses the Inlined/URI K8s of the devfile K8s component
-func GetK8sComponentAsUnstructured(component *v1alpha2.KubernetesComponent, context string, fs devfilefs.Filesystem) (unstructured.Unstructured, error) {
-	strCRD := component.Inlined
-	var err error
-	if component.Uri != "" {
-		strCRD, err = util.GetDataFromURI(component.Uri, context, fs)
-		if err != nil {
-			return unstructured.Unstructured{}, err
-		}
+func GetK8sComponentAsUnstructured(devfileObj parser.DevfileObj, componentName string,
+	context string, fs devfilefs.Filesystem) (unstructured.Unstructured, error) {
+
+	strCRD, err := GetK8sManifestWithVariablesSubstituted(devfileObj, componentName, context, fs)
+	if err != nil {
+		return unstructured.Unstructured{}, err
 	}
 
 	// convert the YAML definition into map[string]interface{} since it's needed to create dynamic resource
@@ -40,7 +37,7 @@ func ListKubernetesComponents(devfileObj parser.DevfileObj, path string) (list [
 	var u unstructured.Unstructured
 	for _, kComponent := range components {
 		if kComponent.Kubernetes != nil {
-			u, err = GetK8sComponentAsUnstructured(kComponent.Kubernetes, path, devfilefs.DefaultFs{})
+			u, err = GetK8sComponentAsUnstructured(devfileObj, kComponent.Name, path, devfilefs.DefaultFs{})
 			if err != nil {
 				return
 			}

pkg/libdevfile/errors.go

@@ -64,14 +64,18 @@ func (e ComponentNotExistError) Error() string {
 	return fmt.Sprintf("component %q does not exists", e.name)
 }
 
-type ComponentsWithSameNameError struct{}
+type ComponentsWithSameNameError struct {
+	name string
+}
 
-func NewComponentsWithSameNameError() ComponentsWithSameNameError {
-	return ComponentsWithSameNameError{}
+func NewComponentsWithSameNameError(name string) ComponentsWithSameNameError {
+	return ComponentsWithSameNameError{
+		name: name,
+	}
 }
 
 func (e ComponentsWithSameNameError) Error() string {
-	return "more than one component with the same name, should not happen"
+	return fmt.Sprintf("more than one component with the same name %q, should not happen", e.name)
 }
 
 // ComponentTypeNotFoundError is returned when no component with the specified type has been found in Devfile

pkg/libdevfile/libdevfile.go

@@ -2,11 +2,17 @@ package libdevfile
 
 import (
 	"fmt"
+	"regexp"
+	"strings"
 
 	"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
+	"github.com/devfile/api/v2/pkg/validation/variables"
 	"github.com/devfile/library/pkg/devfile/parser"
 	"github.com/devfile/library/pkg/devfile/parser/data"
 	"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
+	devfilefs "github.com/devfile/library/pkg/testingutil/filesystem"
+
+	"github.com/redhat-developer/odo/pkg/util"
 )
 
 type Handler interface {
@@ -193,3 +199,87 @@ func GetEndpointsFromDevfile(devfileObj parser.DevfileObj, ignoreExposures []v1a
 	}
 	return endpoints, nil
 }
+
+// GetK8sManifestWithVariablesSubstituted returns the full content of either a Kubernetes or an Openshift
+// Devfile component, either Inlined or referenced via a URI.
+// No matter how the component is defined, it returns the content with all variables substituted
+// using the global variables map defined in `devfileObj`.
+// An error is returned if the content references an invalid variable key not defined in the Devfile object.
+func GetK8sManifestWithVariablesSubstituted(devfileObj parser.DevfileObj, devfileCmpName string,
+	context string, fs devfilefs.Filesystem) (string, error) {
+
+	components, err := devfileObj.Data.GetComponents(common.DevfileOptions{FilterByName: devfileCmpName})
+	if err != nil {
+		return "", err
+	}
+
+	if len(components) == 0 {
+		return "", NewComponentNotExistError(devfileCmpName)
+	}
+
+	if len(components) != 1 {
+		return "", NewComponentsWithSameNameError(devfileCmpName)
+	}
+
+	devfileCmp := components[0]
+	componentType, err := common.GetComponentType(devfileCmp)
+	if err != nil {
+		return "", err
+	}
+
+	var content, uri string
+	switch componentType {
+	case v1alpha2.KubernetesComponentType:
+		content = devfileCmp.Kubernetes.Inlined
+		if devfileCmp.Kubernetes.Uri != "" {
+			uri = devfileCmp.Kubernetes.Uri
+		}
+
+	case v1alpha2.OpenshiftComponentType:
+		content = devfileCmp.Openshift.Inlined
+		if devfileCmp.Openshift.Uri != "" {
+			uri = devfileCmp.Openshift.Uri
+		}
+
+	default:
+		return "", fmt.Errorf("unexpected component type %s", componentType)
+	}
+
+	if uri != "" {
+		return loadResourceManifestFromUriAndResolveVariables(devfileObj, uri, context, fs)
+	}
+	return substituteVariables(devfileObj.Data.GetDevfileWorkspaceSpec().Variables, content)
+}
+
+func loadResourceManifestFromUriAndResolveVariables(devfileObj parser.DevfileObj, uri string,
+	context string, fs devfilefs.Filesystem) (string, error) {
+	content, err := util.GetDataFromURI(uri, context, fs)
+	if err != nil {
+		return content, err
+	}
+	return substituteVariables(devfileObj.Data.GetDevfileWorkspaceSpec().Variables, content)
+}
+
+// substituteVariables validates the string for a global variable in the given `devfileObj` and replaces it.
+// An error is returned if the string references an invalid variable key not defined in the Devfile object.
+//
+//Inspired from variables.validateAndReplaceDataWithVariable, which is unfortunately not exported
+func substituteVariables(devfileVars map[string]string, val string) (string, error) {
+	// example of the regex: {{variable}} / {{ variable }}
+	matches := regexp.MustCompile(`\{\{\s*(.*?)\s*\}\}`).FindAllStringSubmatch(val, -1)
+	var invalidKeys []string
+	for _, match := range matches {
+		varValue, ok := devfileVars[match[1]]
+		if !ok {
+			invalidKeys = append(invalidKeys, match[1])
+		} else {
+			val = strings.Replace(val, match[0], varValue, -1)
+		}
+	}
+
+	if len(invalidKeys) > 0 {
+		return val, &variables.InvalidKeysError{Keys: invalidKeys}
+	}
+
+	return val, nil
+}