kie-issues#1549: kn-workflow-plugin check for the presence of an imag…

…e in the local Docker image does not cover all cases (apache#2685)

packages/kn-plugin-workflow/go.mod

@@ -12,6 +12,7 @@ require (
 	github.com/apache/incubator-kie-tools/packages/sonataflow-operator/api v0.0.0
 	github.com/apache/incubator-kie-tools/packages/sonataflow-operator/workflowproj v0.0.0
 	github.com/beevik/etree v1.2.0
+	github.com/docker/distribution v2.8.2+incompatible
 	github.com/docker/docker v24.0.9+incompatible
 	github.com/docker/go-connections v0.4.0
 	github.com/jstemmer/go-junit-report/v2 v2.0.0
@@ -31,7 +32,6 @@ require (
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
 	github.com/dgraph-io/ristretto v0.1.1 // indirect
-	github.com/docker/distribution v2.8.2+incompatible // indirect
 	github.com/docker/go-units v0.5.0 // indirect
 	github.com/dprotaso/go-yit v0.0.0-20220510233725-9ba8df137936 // indirect
 	github.com/dustin/go-humanize v1.0.1 // indirect
@@ -88,6 +88,7 @@ require (
 	github.com/spf13/cast v1.5.1 // indirect
 	github.com/spf13/jwalterweatherman v1.1.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
+	github.com/stretchr/objx v0.5.0 // indirect
 	github.com/subosito/gotenv v1.6.0 // indirect
 	github.com/vmware-labs/yaml-jsonpath v0.3.2 // indirect
 	golang.org/x/crypto v0.21.0 // indirect

packages/kn-plugin-workflow/go.sum

@@ -347,6 +347,7 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
 github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
+github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
 github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=

packages/kn-plugin-workflow/pkg/common/containers.go

@@ -20,22 +20,25 @@
 package common
 
 import (
+	"bufio"
+	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
+	"github.com/docker/distribution/reference"
 	"io"
 	"os"
 	"os/exec"
 	"os/signal"
 	"runtime"
 	"strings"
 	"syscall"
+	"time"
 
 	"github.com/apache/incubator-kie-tools/packages/kn-plugin-workflow/pkg/metadata"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
-	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/client"
 	"github.com/docker/docker/pkg/stdcopy"
 	"github.com/docker/go-connections/nat"
@@ -51,6 +54,10 @@ type DockerLogMessage struct {
 	ID     string `json:"id,omitempty"`
 }
 
+type DockerClient interface {
+	ImageList(ctx context.Context, options types.ImageListOptions) ([]types.ImageSummary, error)
+}
+
 func getDockerClient() (*client.Client, error) {
 	cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation())
 	if err != nil {
@@ -198,29 +205,71 @@ func GracefullyStopTheContainerWhenInterrupted(containerTool string) {
 	}()
 }
 
-func pullDockerImage(cli *client.Client, ctx context.Context) (io.ReadCloser, error) {
+func pullDockerImage(cli *client.Client, ctx context.Context) error {
 	// Check if the image exists locally.
-	// For that we should check only the image name and tag, removing the registry,
-	// as `docker image ls --filter reference=<image_full_url>` will return empty if the image_full_url is not the first tag
-	// of an image.
-	imageNameWithoutRegistry := strings.Split(metadata.DevModeImage, "/")
-	imageFilters := filters.NewArgs()
-	imageFilters.Add("reference", fmt.Sprintf("*/%s", imageNameWithoutRegistry[len(imageNameWithoutRegistry)-1]))
-	images, err := cli.ImageList(ctx, types.ImageListOptions{Filters: imageFilters})
+	exists, err := CheckImageExists(cli, ctx, metadata.DevModeImage)
 	if err != nil {
-		return nil, fmt.Errorf("error listing images: %s", err)
+		return fmt.Errorf("error listing images: %s", err)
 	}
 
 	// If the image is not found locally, pull it from the remote registry
-	if len(images) == 0 {
-		reader, err := cli.ImagePull(ctx, metadata.DevModeImage, types.ImagePullOptions{})
-		if err != nil {
-			return nil, fmt.Errorf("\nError pulling image: %s. Error is: %s", metadata.DevModeImage, err)
+	if !exists {
+		fmt.Printf("\n⏳ Retrieving (%s), this could take some time...\n", metadata.DevModeImage)
+
+		ctx, cancel := context.WithTimeout(ctx, 1*time.Minute)
+		defer cancel()
+
+		reader, writer := io.Pipe()
+		defer writer.Close()
+
+		var stderr bytes.Buffer
+
+		go func() {
+			scanner := bufio.NewScanner(reader)
+			for scanner.Scan() {
+				fmt.Print(".")
+			}
+		}()
+
+		// we use local docker client to pull the image
+		cmd := exec.CommandContext(ctx, "docker", "pull", metadata.DevModeImage)
+		cmd.Stdout = writer
+		cmd.Stderr = &stderr
+
+		if err := cmd.Start(); err != nil {
+			return fmt.Errorf("\nError pulling image: %s. Error is: %s", metadata.DevModeImage, err)
+		}
+
+		if err := cmd.Wait(); err != nil {
+			return fmt.Errorf("\nError pulling image: %s. Error is: %s", metadata.DevModeImage, stderr.String())
 		}
-		return reader, nil
+		fmt.Println("\n🎉 Successfully pulled the image")
+	}
+
+	return nil
+}
+
+func CheckImageExists(cli DockerClient, ctx context.Context, imageName string) (bool, error) {
+	named, err := reference.ParseNormalizedNamed(imageName)
+
+	if tagged, ok := named.(reference.Tagged); ok {
+		imageName = fmt.Sprintf("%s:%s", reference.Path(named), tagged.Tag())
+	} else {
+		imageName = fmt.Sprintf("%s:%s", reference.Path(named), "latest")
+	}
+	images, err := cli.ImageList(ctx, types.ImageListOptions{All: true})
+	if err != nil {
+		return false, fmt.Errorf("error listing images: %s", err)
 	}
 
-	return nil, nil
+	for _, image := range images {
+		for _, tag := range image.RepoTags {
+			if strings.HasSuffix(tag, imageName) {
+				return true, nil
+			}
+		}
+	}
+	return false, nil
 }
 
 func processDockerImagePullLogs(reader io.ReadCloser) error {
@@ -286,24 +335,18 @@ func startDockerContainer(cli *client.Client, ctx context.Context, resp containe
 }
 
 func runDockerContainer(portMapping string, path string) error {
-	ctx := context.Background()
+	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
+	defer cancel()
+
 	cli, err := getDockerClient()
 	if err != nil {
 		return err
 	}
-
-	reader, err := pullDockerImage(cli, ctx)
+	err = pullDockerImage(cli, ctx)
 	if err != nil {
 		return err
 	}
 
-	if reader != nil {
-		fmt.Printf("\n⏳ Retrieving (%s), this could take some time...\n", metadata.DevModeImage)
-		if err := processDockerImagePullLogs(reader); err != nil {
-			return err
-		}
-	}
-
 	resp, err := createDockerContainer(cli, ctx, portMapping, path)
 	if err != nil {
 		return err

packages/kn-plugin-workflow/pkg/common/containers_test.go

@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package common
+
+import (
+	"context"
+	"github.com/docker/docker/api/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+
+	"testing"
+)
+
+type MockDockerClient struct {
+	mock.Mock
+}
+
+func (m *MockDockerClient) ImageList(ctx context.Context, options types.ImageListOptions) ([]types.ImageSummary, error) {
+	args := m.Called(ctx, options)
+	return args.Get(0).([]types.ImageSummary), args.Error(1)
+}
+
+func TestCheckImageExists(t *testing.T) {
+
+	tests := []struct {
+		lookup   string
+		images   []string
+		expected bool
+	}{
+		{"docker.io/example/app-image:latest", []string{"docker.io/example/app-image:latest"}, true},
+		{"docker.io/demo/service-image:1.0", []string{"demo/service-image:1.0"}, true},
+
+		{"docker.io/testuser/sample-app", []string{"docker.io/testuser/sample-app:latest"}, true},
+		{"docker.io/testuser/sample-app", []string{"testuser/sample-app:latest"}, true},
+
+		{"testuser/sample-app:dev", []string{"docker.io/testuser/sample-app:dev"}, true},
+		{"testuser/sample-app:dev", []string{"testuser/sample-app:dev"}, true},
+
+		{"docker.io/example/app-image:latest", []string{"app-image:latest"}, false},
+		{"docker.io/testuser/sample-app", []string{"sample-app:latest"}, false},
+		{"testuser/sample-app:dev", []string{"sample-app:dev"}, false},
+	}
+
+	for _, test := range tests {
+		ctx := context.Background()
+		mockClient := new(MockDockerClient)
+
+		mockClient.On("ImageList", ctx, mock.Anything).Return([]types.ImageSummary{
+			{
+				RepoTags: test.images,
+			},
+		}, nil)
+
+		exists, err := CheckImageExists(mockClient, ctx, test.lookup)
+		assert.NoError(t, err, "Error should be nil")
+		assert.True(t, exists == test.expected, "Expected %t, got %t", test.expected, exists)
+	}
+}