Restore import path with uppercase characters in custom build ko example (#6286)

* Restore import path with uppercase characters

I wasn't able to reproduce the issue described in #5492 and #5505, so
this change restores the import path with the uppercase characters
and the `ko://` prefix in the custom build example.

I had to tweak some values to get the integration tests to run,
because I don't have access to the `k8s-skaffold` project. Let's see
if the build passes.

Additional minor changes:

- Bump the ko version in the custom build example.

- Add the full path to the ko binary in the custom build script, in case
  `$GOPATH/bin` is not on the `PATH`.

Once we move to Go 1.16 for our builds, we can use the `go install`
command to install ko in the custom build shell script.

* Look for ko binary in GOPATH/bin

It's difficult to know what's on the `PATH` in different environments.
This change to the custom builder example looks for the ko binary in the
`GOPATH/bin` directory.

* Remove tagPolicy from custom builder example

Hypothesis: `tagPolicy: sha256` doesn't behave correctly with images
sideloaded into kind snf k3d.

Also fix conditional in custom build example shell script to prevent
recompiling ko each time.

* Sanitize image names before deciding what to load

I was able to reproduce the issue locally and work out a fix. I'd be
happy for feedback on whether I've chosen the right place to fix it.

When determining which images to sideload (for kind and k3d), we compare
`localImages` and `deployerImages`. The former from `skaffold.yaml`, and
the latter from Kubernetes manifest files. The image names from
Kubernetes manifests are sanitized in
`pkg/skaffold/kubernetes/manifests/images.go#L51` (and `L113`) in the
call to docker.ParseReference.

The same doesn't happen to image names from `skaffold.yaml`. This change
sanitizes these image names just for determining whether to sideload the
images.

In other parts of the code we look up image pipelines from
`skaffold.yaml` using the image name, so I was hesitant to change how
`localImages` is used (with 'raw' image names).

The hypothesis from the previous commit is disproven, so I'm
adding back the `sha256` tag policy in the custom builder example. To
make the test case easier to identify from the build logs, I renamed the
pod in the custom builder example.

New hypothesis: Could this be related to the issues some users are
reporting with images not being sideloaded when using Helm? E.g., #5159

Author: halvards

integration/examples/custom/build.sh

@@ -1,16 +1,20 @@
 #!/usr/bin/env bash
 set -e
 
-if ! [ -x "$(command -v ko)" ]; then
+if ! [ -x "$(go env GOPATH)/bin/ko" ]; then
     pushd $(mktemp -d)
-    go mod init tmp; GOFLAGS= go get github.com/google/ko/cmd/ko@v0.6.0
+    curl -L https://github.com/google/ko/archive/v0.8.3.tar.gz | tar --strip-components 1 -zx
+    go build -o $(go env GOPATH)/bin/ko .
     popd
 fi
 
-output=$(ko publish --local --preserve-import-paths --tags= . | tee)
+output=$($(go env GOPATH)/bin/ko publish --local --preserve-import-paths --tags= . | tee)
 ref=$(echo "$output" | tail -n1)
 
 docker tag "$ref" "$IMAGE"
 if [[ "${PUSH_IMAGE}" == "true" ]]; then
+    echo "Pushing $IMAGE"
     docker push "$IMAGE"
+else
+    echo "Not pushing $IMAGE"
 fi

integration/examples/custom/k8s/pod.yaml

@@ -1,8 +1,8 @@
 apiVersion: v1
 kind: Pod
 metadata:
-  name: getting-started
+  name: getting-started-custom
 spec:
   containers:
-  - name: getting-started
-    image: github.com/googlecontainertools/skaffold/examples/custom
+  - name: getting-started-custom
+    image: ko://github.com/GoogleContainerTools/skaffold/examples/custom

integration/examples/custom/skaffold.yaml

@@ -2,7 +2,7 @@ apiVersion: skaffold/v2beta21
 kind: Config
 build:
   artifacts:
-  - image: github.com/googlecontainertools/skaffold/examples/custom
+  - image: ko://github.com/GoogleContainerTools/skaffold/examples/custom
     custom:
       buildCommand: ./build.sh
       dependencies:

integration/run_test.go

@@ -113,7 +113,7 @@ var tests = []struct {
 	{
 		description: "custom builder",
 		dir:         "examples/custom",
-		pods:        []string{"getting-started"},
+		pods:        []string{"getting-started-custom"},
 	},
 	{
 		description: "buildpacks Go",

pkg/skaffold/kubernetes/loader/load.go

@@ -28,6 +28,7 @@ import (
 	"k8s.io/client-go/tools/clientcmd/api"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl"
 	kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context"
@@ -60,7 +61,7 @@ func NewImageLoader(kubeContext string, cli *kubectl.CLI) *ImageLoader {
 func imagesToLoad(localImages, deployerImages, images []graph.Artifact) []graph.Artifact {
 	local := map[string]bool{}
 	for _, image := range localImages {
-		local[image.ImageName] = true
+		local[docker.SanitizeImageName(image.ImageName)] = true
 	}
 
 	tracked := map[string]bool{}
@@ -72,7 +73,7 @@ func imagesToLoad(localImages, deployerImages, images []graph.Artifact) []graph.
 
 	var res []graph.Artifact
 	for _, image := range images {
-		if tracked[image.ImageName] {
+		if tracked[docker.SanitizeImageName(image.ImageName)] {
 			res = append(res, image)
 		}
 	}

pkg/skaffold/kubernetes/loader/load_test.go

@@ -200,6 +200,13 @@ func TestImagesToLoad(t *testing.T) {
 			builtImages:    []graph.Artifact{{ImageName: "image1", Tag: "foo"}},
 			expectedImages: nil,
 		},
+		{
+			name:           "single image marked as local, with ko prefix and Go import path with uppercase characters",
+			localImages:    []graph.Artifact{{ImageName: "ko://example.com/Organization/Image1", Tag: "Foo"}},
+			deployerImages: []graph.Artifact{{ImageName: "example.com/organization/image1", Tag: "Foo"}},
+			builtImages:    []graph.Artifact{{ImageName: "ko://example.com/Organization/Image1", Tag: "Foo"}},
+			expectedImages: []graph.Artifact{{ImageName: "ko://example.com/Organization/Image1", Tag: "Foo"}},
+		},
 		{
 			name:           "two images marked as local",
 			localImages:    []graph.Artifact{{ImageName: "image1", Tag: "foo"}, {ImageName: "image2", Tag: "bar"}},