Allow using remote Dockerfiles (HTTP(S) only) for building images (#5976

)

* Add utility function in `docker_compatible.go` allowing to resolve a Dockerfile

For now, this only supports HTTP(S) and downloads the remote file to a temporary file.
The path to that temp. file is then returned to the caller.

In all other cases, the specified URI path is returned as is.
This means that non-HTTP(S) URIs will *not* get resolved, but will be returned as is.

Signed-off-by: Armel Soro <asoro@redhat.com>

* Allow using remote HTTP(S) Dockerfiles with both `deploy` and `build-images` commands

Signed-off-by: Armel Soro <asoro@redhat.com>

* Join resolved Dockerfile path with the Devfile one only if the former is relative

It actually does not make sense to join absolute Dockerfile paths,
as can be the case with temporary files created by downloading a remote Dockerfile

Signed-off-by: Armel Soro <asoro@redhat.com>

* Add integration test cases for `odo build-images`

Signed-off-by: Armel Soro <asoro@redhat.com>

* Add integration test cases for `odo deploy`

Signed-off-by: Armel Soro <asoro@redhat.com>

* Update doc for both `build-images` and `deploy` commands

Signed-off-by: Armel Soro <asoro@redhat.com>

* fixup! Add utility function in `docker_compatible.go` allowing to resolve a Dockerfile

* fixup! Allow using remote HTTP(S) Dockerfiles with both `deploy` and `build-images` commands

Signed-off-by: Armel Soro <asoro@redhat.com>

* fixup! Allow using remote HTTP(S) Dockerfiles with both `deploy` and `build-images` commands

* fixup! 878285b

docs/website/docs/command-reference/build-images.md

@@ -16,10 +16,11 @@ components:
   name: component-built-from-dockerfile
 ```
 
-The `uri` field indicates the relative path of the Dockerfile to use, relative to the directory containing the `devfile.yaml`. The devfile specification indicates that `uri` could also be an HTTP URL, but this case is not supported by odo yet.
+The `uri` field indicates the relative path of the Dockerfile to use, relative to the directory containing the `devfile.yaml`. 
+As indicated in the Devfile specification, `uri` could also be an HTTP or HTTPS URL.
 
 The `buildContext` indicates the directory used as build context. The default value is `${PROJECT_SOURCE}`.
 
 For each image component, odo executes either `podman` or `docker` (the first one found, in this order), to build the image with the specified Dockerfile, build context and arguments.
 
-If the `--push` flag is passed to the command, the images are be pushed to their registries after they are built.
+If the `--push` flag is passed to the command, the images will be pushed to their registries after they are built.

docs/website/docs/command-reference/deploy.md

@@ -65,6 +65,8 @@ components:
                   image: {{CONTAINER_IMAGE}}
 ```
 
+Note that the `uri` for the Dockerfile could also be an HTTP or HTTPS URL.
+
 ## Sustituting variables
 
 The Devfile can define variables to make the Devfile parameterizable. The Devfile can define values for these variables, and you 

pkg/deploy/deploy.go

@@ -17,6 +17,7 @@ import (
 	"github.com/redhat-developer/odo/pkg/libdevfile"
 	"github.com/redhat-developer/odo/pkg/log"
 	"github.com/redhat-developer/odo/pkg/service"
+	"github.com/redhat-developer/odo/pkg/testingutil/filesystem"
 )
 
 type DeployClient struct {
@@ -31,12 +32,13 @@ func NewDeployClient(kubeClient kclient.ClientInterface) *DeployClient {
 	}
 }
 
-func (o *DeployClient) Deploy(devfileObj parser.DevfileObj, path string, appName string) error {
-	deployHandler := newDeployHandler(devfileObj, path, o.kubeClient, appName)
+func (o *DeployClient) Deploy(fs filesystem.Filesystem, devfileObj parser.DevfileObj, path string, appName string) error {
+	deployHandler := newDeployHandler(fs, devfileObj, path, o.kubeClient, appName)
 	return libdevfile.Deploy(devfileObj, deployHandler)
 }
 
 type deployHandler struct {
+	fs         filesystem.Filesystem
 	devfileObj parser.DevfileObj
 	path       string
 	kubeClient kclient.ClientInterface
@@ -45,8 +47,9 @@ type deployHandler struct {
 
 var _ libdevfile.Handler = (*deployHandler)(nil)
 
-func newDeployHandler(devfileObj parser.DevfileObj, path string, kubeClient kclient.ClientInterface, appName string) *deployHandler {
+func newDeployHandler(fs filesystem.Filesystem, devfileObj parser.DevfileObj, path string, kubeClient kclient.ClientInterface, appName string) *deployHandler {
 	return &deployHandler{
+		fs:         fs,
 		devfileObj: devfileObj,
 		path:       path,
 		kubeClient: kubeClient,
@@ -56,7 +59,7 @@ func newDeployHandler(devfileObj parser.DevfileObj, path string, kubeClient kcli
 
 // ApplyImage builds and pushes the OCI image to be used on Kubernetes
 func (o *deployHandler) ApplyImage(img v1alpha2.Component) error {
-	return image.BuildPushSpecificImage(o.path, img, true)
+	return image.BuildPushSpecificImage(o.fs, o.path, img, true)
 }
 
 // ApplyKubernetes applies inline Kubernetes YAML from the devfile.yaml file

pkg/deploy/interface.go

@@ -1,8 +1,14 @@
 package deploy
 
-import "github.com/devfile/library/pkg/devfile/parser"
+import (
+	"github.com/devfile/library/pkg/devfile/parser"
+
+	"github.com/redhat-developer/odo/pkg/testingutil/filesystem"
+)
 
 type Client interface {
-	// Deploy resources from a devfile located in path, for the specified appName
-	Deploy(devfileObj parser.DevfileObj, path string, appName string) error
+	// Deploy resources from a devfile located in path, for the specified appName.
+	// The filesystem specified is used to download and store the Dockerfiles needed to build the necessary container images,
+	// in case such Dockerfiles are referenced as remote URLs in the Devfile.
+	Deploy(fs filesystem.Filesystem, devfileObj parser.DevfileObj, path string, appName string) error
 }

pkg/devfile/image/docker_compatible.go

@@ -1,7 +1,6 @@
 package image
 
 import (
-	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -10,11 +9,15 @@ import (
 
 	devfile "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 	"github.com/fatih/color"
-	"github.com/redhat-developer/odo/pkg/log"
 	"k8s.io/klog"
+
+	dfutil "github.com/devfile/library/pkg/util"
+
+	"github.com/redhat-developer/odo/pkg/log"
+	"github.com/redhat-developer/odo/pkg/testingutil/filesystem"
 )
 
-// This backend uses a CLI compatible with the docker CLI (at least docker itself and podman)
+// DockerCompatibleBackend uses a CLI compatible with the docker CLI (at least docker itself and podman)
 type DockerCompatibleBackend struct {
 	name string
 }
@@ -26,17 +29,25 @@ func NewDockerCompatibleBackend(name string) *DockerCompatibleBackend {
 }
 
 // Build an image, as defined in devfile, using a Docker compatible CLI
-func (o *DockerCompatibleBackend) Build(image *devfile.ImageComponent, devfilePath string) error {
-
-	if strings.HasPrefix(image.Dockerfile.Uri, "http") {
-		return errors.New("HTTP URL for uri is not supported")
+func (o *DockerCompatibleBackend) Build(fs filesystem.Filesystem, image *devfile.ImageComponent, devfilePath string) error {
+
+	dockerfile, isTemp, err := resolveAndDownloadDockerfile(fs, image.Dockerfile.Uri)
+	if isTemp {
+		defer func(path string) {
+			if e := fs.Remove(path); e != nil {
+				klog.V(3).Infof("could not remove temporary Dockerfile at path %q: %v", path, err)
+			}
+		}(dockerfile)
+	}
+	if err != nil {
+		return err
 	}
 
 	// We use a "No Spin" since we are outputting to stdout / stderr
 	buildSpinner := log.SpinnerNoSpin("Building image locally")
 	defer buildSpinner.End(false)
 
-	err := os.Setenv("PROJECTS_ROOT", devfilePath)
+	err = os.Setenv("PROJECTS_ROOT", devfilePath)
 	if err != nil {
 		return err
 	}
@@ -46,7 +57,7 @@ func (o *DockerCompatibleBackend) Build(image *devfile.ImageComponent, devfilePa
 		return err
 	}
 
-	shellCmd := getShellCommand(o.name, image, devfilePath)
+	shellCmd := getShellCommand(o.name, image, devfilePath, dockerfile)
 	klog.V(4).Infof("Running command: %v", shellCmd)
 	for i, cmd := range shellCmd {
 		shellCmd[i] = os.ExpandEnv(cmd)
@@ -72,12 +83,47 @@ func (o *DockerCompatibleBackend) Build(image *devfile.ImageComponent, devfilePa
 	return nil
 }
 
+// resolveAndDownloadDockerfile resolves and downloads (if needed) the specified Dockerfile URI.
+// For now, it only supports resolving HTTP(S) URIs, in which case it downloads the remote file
+// to a temporary file. The path to that temporary file is then returned.
+//
+// In all other cases, the specified URI path is returned as is.
+// This means that non-HTTP(S) URIs will *not* get resolved, but will be returned as is.
+//
+// In addition to the path, a boolean and a potential error are returned. The boolean indicates whether
+// the returned path is a temporary one; in such case, it is the caller's responsibility to delete this file
+// once it is done working with it.
+func resolveAndDownloadDockerfile(fs filesystem.Filesystem, uri string) (string, bool, error) {
+	uriLower := strings.ToLower(uri)
+	if strings.HasPrefix(uriLower, "http://") || strings.HasPrefix(uriLower, "https://") {
+		s := log.Spinner("Downloading Dockerfile")
+		defer s.End(false)
+		tempFile, err := fs.TempFile("", "odo_*.dockerfile")
+		if err != nil {
+			return "", false, err
+		}
+		dockerfile := tempFile.Name()
+		err = dfutil.DownloadFile(dfutil.DownloadParams{
+			Request: dfutil.HTTPRequestParams{
+				URL: uri,
+			},
+			Filepath: dockerfile,
+		})
+		s.End(err == nil)
+		return dockerfile, true, err
+	}
+	return uri, false, nil
+}
+
 //getShellCommand creates the docker compatible build command from detected backend,
 //container image and devfile path
-func getShellCommand(cmdName string, image *devfile.ImageComponent, devfilePath string) []string {
+func getShellCommand(cmdName string, image *devfile.ImageComponent, devfilePath string, dockerfilePath string) []string {
 	var shellCmd []string
 	imageName := image.ImageName
-	dockerfile := filepath.Join(devfilePath, image.Dockerfile.Uri)
+	dockerfile := dockerfilePath
+	if !filepath.IsAbs(dockerfile) {
+		dockerfile = filepath.Join(devfilePath, dockerfilePath)
+	}
 	buildpath := image.Dockerfile.BuildContext
 	if buildpath == "" {
 		buildpath = devfilePath

pkg/devfile/image/docker_compatible_test.go

@@ -1,11 +1,17 @@
 package image
 
 import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"path/filepath"
 	"reflect"
+	"strings"
 	"testing"
 
 	devfile "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
+
+	"github.com/redhat-developer/odo/pkg/testingutil/filesystem"
 )
 
 func TestGetShellCommand(t *testing.T) {
@@ -107,14 +113,106 @@ func TestGetShellCommand(t *testing.T) {
 				"cli", "build", "-t", "registry.io/myimagename:tag", "-f", filepath.Join(devfilePath, "Dockerfile.rhel"), devfilePath,
 			},
 		},
+		{
+			name:    "using an absolute Dockerfile path",
+			cmdName: "cli",
+			image: &devfile.ImageComponent{
+				Image: devfile.Image{
+					ImageName: "registry.io/myimagename:tag",
+					ImageUnion: devfile.ImageUnion{
+						Dockerfile: &devfile.DockerfileImage{
+							DockerfileSrc: devfile.DockerfileSrc{
+								Uri: filepath.Join("/", "path", "to", "Dockerfile.rhel"),
+							},
+						},
+					},
+				},
+			},
+			devfilePath: devfilePath,
+			want: []string{
+				"cli", "build", "-t", "registry.io/myimagename:tag", "-f", filepath.Join("/", "path", "to", "Dockerfile.rhel"), devfilePath,
+			},
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := getShellCommand(tt.cmdName, tt.image, tt.devfilePath)
+			got := getShellCommand(tt.cmdName, tt.image, tt.devfilePath, tt.image.Dockerfile.Uri)
 			if !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("%s:\n  Expected %v,\n       got %v", tt.name, tt.want, got)
 			}
 		})
 	}
 }
+
+func Test_resolveAndDownloadDockerfile(t *testing.T) {
+	fakeFs := filesystem.NewFakeFs()
+
+	for _, tt := range []struct {
+		name       string
+		uriFunc    func() (*httptest.Server, string)
+		wantErr    bool
+		wantIsTemp bool
+		want       string
+	}{
+		{
+			name:    "local file",
+			uriFunc: func() (*httptest.Server, string) { return nil, "Dockerfile" },
+			want:    "Dockerfile",
+		},
+		{
+			name:    "remote file (non-HTTP)",
+			uriFunc: func() (*httptest.Server, string) { return nil, "ftp://example.com/Dockerfile" },
+			want:    "ftp://example.com/Dockerfile",
+		},
+		{
+			name: "remote file with error (HTTP)",
+			uriFunc: func() (*httptest.Server, string) {
+				s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					w.WriteHeader(http.StatusNotFound)
+				}))
+				return s, s.URL + "/404"
+			},
+			wantErr:    true,
+			wantIsTemp: true,
+		},
+		{
+			name: "remote file (HTTP)",
+			uriFunc: func() (*httptest.Server, string) {
+				s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					fmt.Fprintln(w, "FROM alpine:3.6")
+					fmt.Fprintln(w, "RUN echo Hello World")
+					fmt.Fprintln(w, "ENTRYPOINT [\"/bin/tail\"]")
+					fmt.Fprintln(w, "CMD [\"-f\", \"/dev/null\"]")
+				}))
+				return s, s.URL
+			},
+			wantIsTemp: true,
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			server, uri := tt.uriFunc()
+			if server != nil {
+				defer server.Close()
+			}
+			got, gotIsTemp, err := resolveAndDownloadDockerfile(fakeFs, uri)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("%s:\n  Expected error %v,\n       got %v", tt.name, tt.wantErr, err)
+			}
+			if gotIsTemp != tt.wantIsTemp {
+				t.Errorf("%s:\n  For 'isTemp', expected %v,\n       got %v", tt.name, tt.wantIsTemp, gotIsTemp)
+			}
+			if gotIsTemp {
+				defer func(fs filesystem.Filesystem, name string) {
+					_ = fs.Remove(name)
+				}(fakeFs, got)
+				// temp file is created, so we can't compare the path, but we can check the path is not blank
+				if strings.TrimSpace(got) == "" {
+					t.Errorf("%s:\n  Expected non-blank path,\n       got blank path: %s", tt.name, got)
+				}
+			} else if got != tt.want {
+				t.Errorf("%s:\n  Expected %v,\n       got %v", tt.name, tt.want, got)
+			}
+		})
+	}
+}