add retry for 401 errors to image imported to try pull image without authentication. This is to eliminate case when we try pull public images with wrong/expired secret and it blocks all imports

Author: mjudeikis

pkg/image/importer/importer.go

@@ -340,6 +340,8 @@ func formatRepositoryError(ref imageapi.DockerImageReference, err error) error {
 		err = kapierrors.NewUnauthorized(fmt.Sprintf("you may not have access to the Docker image %q", ref.Exact()))
 	case strings.HasSuffix(err.Error(), "no basic auth credentials"):
 		err = kapierrors.NewUnauthorized(fmt.Sprintf("you may not have access to the Docker image %q", ref.Exact()))
+	case strings.HasSuffix(err.Error(), "incorrect username or password"):
+		err = kapierrors.NewUnauthorized(fmt.Sprintf("incorrect username or password for image %q", ref.Exact()))
 	}
 	return err
 }

pkg/image/registry/imagestreamimport/rest.go

@@ -3,6 +3,7 @@ package imagestreamimport
 import (
 	"fmt"
 	"net/http"
+	"strings"
 	"time"
 
 	"github.com/golang/glog"
@@ -199,6 +200,34 @@ func (r *REST) Create(ctx apirequest.Context, obj runtime.Object, createValidati
 		return nil, kapierrors.NewInternalError(err)
 	}
 
+	// check imported images status. If we get authentication error (401), try import same image without authentication.
+	// Docker registry gives 401 on public images if you have wrong secret in your secret list.
+	// this block was introduced by PR #18012
+	var imageStatus []metav1.Status
+	importFailed := false
+	for _, image := range isi.Status.Images {
+		//cache all imports status
+		imageStatus = append(imageStatus, image.Status)
+		if image.Status.Reason == metav1.StatusReasonUnauthorized && strings.Contains(strings.ToLower(image.Status.Message), "username or password") {
+			importFailed = true
+		}
+	}
+	// try import IS without auth if it failed before
+	if importFailed {
+		importCtx := registryclient.NewContext(r.transport, r.insecureTransport).WithCredentials(nil)
+		imports := r.importFn(importCtx)
+		//TODO add check if we get error and run import outside the loop
+		if err := imports.Import(ctx.(gocontext.Context), isi, stream); err != nil {
+			return nil, kapierrors.NewInternalError(err)
+		}
+	}
+	//cycle through status and set old messages so not to confuse users
+	for key, image := range isi.Status.Images {
+		if image.Status.Reason == metav1.StatusReasonUnauthorized {
+			isi.Status.Images[key].Status = imageStatus[key]
+		}
+	}
+
 	// if we encountered an error loading credentials and any images could not be retrieved with an access
 	// related error, modify the message.
 	// TODO: set a status cause

test/cmd/images.sh

@@ -309,4 +309,14 @@ os::cmd::expect_success 'oc delete project merge-tags'
 echo "apply new imagestream tags: ok"
 os::test::junit::declare_suite_end
 
+# test importing images with wrong docker secrets
+os::test::junit::declare_suite_start "cmd/images${IMAGES_TESTS_POSTFIX:-}/import-public-images-with-fake-secret"
+os::cmd::expect_success 'oc new-project import-images'
+os::cmd::expect_success 'oc create secret docker-registry dummy-secret1 --docker-server=docker.io --docker-username=dummy1 --docker-password=dummy1 [email protected]'
+os::cmd::expect_success 'oc create secret docker-registry dummy-secret2 --docker-server=docker.io --docker-username=dummy2 --docker-password=dummy2 [email protected]'
+os::cmd::expect_success_and_text 'oc import-image example --from=openshift/hello-openshift --confirm' 'The import completed successfully'
+os::cmd::expect_success 'oc delete project import-images'
+echo "import public images with fake secret ok"
+os::test::junit::declare_suite_end
+
 os::test::junit::declare_suite_end