Fix issue with build-images and deploy not working with Docker if…

… optional `buildContext` field is not set in Devfile (redhat-developer#5600) (redhat-developer#5657) * Add unit test highlighting the issue * Add integration tests to both `build-images` and `deploy` highlighting the issue * Use the Devfile directory as default build context path if not defined Per the Devfile spec [1], `buildContext` should be optional. [1] https://devfile.io/docs/devfile/2.2.0/user-guide/api-reference/ * Quote the build context path value
cdrage · Aug 31, 2022 · c90cdd0 · c90cdd0
1 parent 756b76f
commit c90cdd0
Show file tree

Hide file tree

Showing 5 changed files with 215 additions and 7 deletions.
diff --git a/pkg/devfile/image/docker_compatible.go b/pkg/devfile/image/docker_compatible.go
@@ -61,9 +61,12 @@ func getShellCommand(cmdName string, image *devfile.ImageComponent, devfilePath
 	imageName := image.ImageName
 	dockerfile := filepath.Join(devfilePath, image.Dockerfile.Uri)
 	buildpath := image.Dockerfile.BuildContext
+	if buildpath == "" {
+		buildpath = devfilePath
+	}
 	args := image.Dockerfile.Args
 
-	shell := fmt.Sprintf(`%s build -t "%s" -f "%s" %s`, cmdName, imageName, dockerfile, buildpath)
+	shell := fmt.Sprintf(`%s build -t "%s" -f "%s" "%s"`, cmdName, imageName, dockerfile, buildpath)
 	if len(args) > 0 {
 		shell = shell + " " + strings.Join(args, " ")
 	}

diff --git a/pkg/devfile/image/docker_compatible_test.go b/pkg/devfile/image/docker_compatible_test.go
@@ -1,13 +1,15 @@
 package image
 
 import (
+	"fmt"
 	"path/filepath"
 	"testing"
 
 	devfile "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 )
 
 func TestGetShellCommand(t *testing.T) {
+	devfilePath := filepath.Join("home", "user", "project1")
 	tests := []struct {
 		name        string
 		cmdName     string
@@ -33,8 +35,9 @@ func TestGetShellCommand(t *testing.T) {
 					},
 				},
 			},
-			devfilePath: filepath.Join("home", "user", "project1"),
-			want:        `cli build -t "registry.io/myimagename:tag" -f "` + filepath.Join("home", "user", "project1", "Dockerfile") + `" ${PROJECTS_ROOT}`,
+			devfilePath: devfilePath,
+			want: fmt.Sprintf(`cli build -t "registry.io/myimagename:tag" -f "%s" "${PROJECTS_ROOT}"`,
+				filepath.Join(devfilePath, "Dockerfile")),
 		},
 		{
 			name:    "test 2",
@@ -54,8 +57,9 @@ func TestGetShellCommand(t *testing.T) {
 					},
 				},
 			},
-			devfilePath: filepath.Join("home", "user", "project1"),
-			want:        `cli build -t "registry.io/myimagename:tag" -f "` + filepath.Join("home", "user", "project1", "Dockerfile") + `" ${PROJECTS_ROOT}`,
+			devfilePath: devfilePath,
+			want: fmt.Sprintf(`cli build -t "registry.io/myimagename:tag" -f "%s" "${PROJECTS_ROOT}"`,
+				filepath.Join(devfilePath, "Dockerfile")),
 		},
 		{
 			name:    "test with args",
@@ -76,8 +80,28 @@ func TestGetShellCommand(t *testing.T) {
 					},
 				},
 			},
-			devfilePath: filepath.Join("home", "user", "project1"),
-			want:        `cli build -t "registry.io/myimagename:tag" -f "` + filepath.Join("home", "user", "project1", "Dockerfile") + `" ${PROJECTS_ROOT} --flag value`,
+			devfilePath: devfilePath,
+			want: fmt.Sprintf(`cli build -t "registry.io/myimagename:tag" -f "%s" "${PROJECTS_ROOT}" --flag value`,
+				filepath.Join(devfilePath, "Dockerfile")),
+		},
+		{
+			name:    "test with no build context in Devfile",
+			cmdName: "cli",
+			image: &devfile.ImageComponent{
+				Image: devfile.Image{
+					ImageName: "registry.io/myimagename:tag",
+					ImageUnion: devfile.ImageUnion{
+						Dockerfile: &devfile.DockerfileImage{
+							DockerfileSrc: devfile.DockerfileSrc{
+								Uri: "Dockerfile.rhel",
+							},
+						},
+					},
+				},
+			},
+			devfilePath: devfilePath,
+			want: fmt.Sprintf(`cli build -t "registry.io/myimagename:tag" -f "%s" "%s"`,
+				filepath.Join(devfilePath, "Dockerfile.rhel"), devfilePath),
 		},
 	}
 

diff --git a/...s/source/devfiles/nodejs/issue-5600-devfile-with-image-component-and-no-buildContext.yaml b/...s/source/devfiles/nodejs/issue-5600-devfile-with-image-component-and-no-buildContext.yaml
@@ -0,0 +1,95 @@
+commands:
+- exec:
+    commandLine: npm install
+    component: runtime
+    group:
+      isDefault: true
+      kind: build
+    workingDir: $PROJECT_SOURCE
+  id: install
+- exec:
+    commandLine: npm start
+    component: runtime
+    group:
+      isDefault: true
+      kind: run
+    workingDir: $PROJECT_SOURCE
+  id: run
+- apply:
+    component: prod-image
+  id: build-image
+- apply:
+    component: outerloop-deploy
+  id: deploy-deployment
+- apply:
+    component: outerloop-service
+  id: deploy-service
+- composite:
+    commands:
+    - build-image
+    - deploy-deployment
+    - deploy-service
+    group:
+      isDefault: true
+      kind: deploy
+  id: deploy
+components:
+- container:
+    endpoints:
+    - name: http-3000
+      targetPort: 3000
+    image: registry.access.redhat.com/ubi8/nodejs-14:0.1.0
+    memoryLimit: 1024Mi
+    mountSources: true
+  name: runtime
+- image:
+    dockerfile:
+      uri: ./Dockerfile
+    imageName: localhost:5000/devfile-nodejs-deploy:0.1.0
+  name: prod-image
+- kubernetes:
+    inlined: |
+      kind: Deployment
+      apiVersion: apps/v1
+      metadata:
+        name: devfile-nodejs-deploy
+      spec:
+        replicas: 1
+        selector:
+          matchLabels:
+            app: devfile-nodejs-deploy
+        template:
+          metadata:
+            labels:
+              app: devfile-nodejs-deploy
+          spec:
+            containers:
+              - name: main
+                image: "localhost:5000/devfile-nodejs-deploy:0.1.0"
+                resources: {}
+  name: outerloop-deploy
+- kubernetes:
+    inlined: |
+      apiVersion: v1
+      kind: Service
+      metadata:
+        labels:
+          app: devfile-nodejs-deploy
+        name: devfile-nodejs-deploy
+      spec:
+        ports:
+        - name: http-3000
+          port: 3000
+          protocol: TCP
+          targetPort: 3000
+        selector:
+          app: devfile-nodejs-deploy
+        type: LoadBalancer
+  name: outerloop-service
+metadata:
+  language: javascript
+  name: my-nodejs-without-buildctx
+  projectType: nodejs
+schemaVersion: 2.2.0
+variables:
+  CONTAINER_IMAGE: localhost:5000/devfile-nodejs-deploy:0.1.0
diff --git a/tests/integration/devfile/cmd_devfile_build_images_test.go b/tests/integration/devfile/cmd_devfile_build_images_test.go
@@ -116,4 +116,46 @@ var _ = Describe("odo devfile build-images command tests", func() {
 			})
 		}
 	})
+
+	When("using a devfile.yaml containing an Image component with no build context", func() {
+
+		BeforeEach(func() {
+			helper.CopyExample(filepath.Join("source", "nodejs"), commonVar.Context)
+			helper.CopyExampleDevFile(
+				filepath.Join("source", "devfiles", "nodejs",
+					"issue-5600-devfile-with-image-component-and-no-buildContext.yaml"),
+				filepath.Join(commonVar.Context, "devfile.yaml"))
+			helper.CreateLocalEnv(commonVar.Context, "aname", commonVar.Project)
+		})
+
+		for _, scope := range []struct {
+			name    string
+			envvars []string
+		}{
+			{
+				name:    "Podman",
+				envvars: []string{"PODMAN_CMD=echo"},
+			},
+			{
+				name: "Docker",
+				envvars: []string{
+					"PODMAN_CMD=a-command-not-found-for-podman-should-make-odo-fallback-to-docker",
+					"DOCKER_CMD=echo",
+				},
+			},
+		} {
+			It(fmt.Sprintf("should build image via %s by defaulting build context to devfile path", scope.name), func() {
+				stdout := helper.Cmd("odo", "build-images").AddEnv(scope.envvars...).ShouldPass().Out()
+				lines, err := helper.ExtractLines(stdout)
+				Expect(err).ShouldNot(HaveOccurred())
+				nbLines := len(lines)
+				Expect(nbLines).To(BeNumerically(">", 2))
+				containerImage := "localhost:5000/devfile-nodejs-deploy:0.1.0" // from Devfile yaml file
+				dockerfilePath := filepath.Join(commonVar.Context, "Dockerfile")
+				buildCtx := commonVar.Context
+				Expect(lines[nbLines-2]).To(BeEquivalentTo(
+					fmt.Sprintf("build -t %s -f %s %s", containerImage, dockerfilePath, buildCtx)))
+			})
+		}
+	})
 })
diff --git a/tests/integration/devfile/cmd_devfile_deploy_test.go b/tests/integration/devfile/cmd_devfile_deploy_test.go
@@ -186,4 +186,48 @@ var _ = Describe("odo devfile deploy command tests", func() {
 			})
 		}
 	})
+
+	When("using a devfile.yaml containing an Image component with no build context", func() {
+
+		BeforeEach(func() {
+			helper.CopyExample(filepath.Join("source", "nodejs"), commonVar.Context)
+			helper.CopyExampleDevFile(
+				filepath.Join("source", "devfiles", "nodejs",
+					"issue-5600-devfile-with-image-component-and-no-buildContext.yaml"),
+				filepath.Join(commonVar.Context, "devfile.yaml"))
+		})
+
+		for _, scope := range []struct {
+			name    string
+			envvars []string
+		}{
+			{
+				name:    "Podman",
+				envvars: []string{"PODMAN_CMD=echo"},
+			},
+			{
+				name: "Docker",
+				envvars: []string{
+					"PODMAN_CMD=a-command-not-found-for-podman-should-make-odo-fallback-to-docker",
+					"DOCKER_CMD=echo",
+				},
+			},
+		} {
+			It(fmt.Sprintf("should build image via %s by defaulting build context to devfile path", scope.name), func() {
+				stdout := helper.Cmd("odo", "deploy").AddEnv(scope.envvars...).ShouldPass().Out()
+				lines, err := helper.ExtractLines(stdout)
+				Expect(err).ShouldNot(HaveOccurred())
+				Expect(lines).ShouldNot(BeEmpty())
+				containerImage := "localhost:5000/devfile-nodejs-deploy:0.1.0" // from Devfile yaml file
+				dockerfilePath := filepath.Join(commonVar.Context, "Dockerfile")
+				buildCtx := commonVar.Context
+				expected := fmt.Sprintf("build -t %s -f %s %s", containerImage, dockerfilePath, buildCtx)
+				i, found := helper.FindFirstElementIndexByPredicate(lines, func(s string) bool {
+					return s == expected
+				})
+				Expect(found).To(BeTrue(), "line not found: ["+expected+"]")
+				Expect(i).ToNot(BeZero(), "line not found at non-zero index: ["+expected+"]")
+			})
+		}
+	})
 })