Skip to content

Commit

Permalink
add retry for 401 errors to image imported to try pull image without …
Browse files Browse the repository at this point in the history
…authentication. This is to eliminate case when we try pull public images with wrong/expired secret and it blocks all imports
  • Loading branch information
mjudeikis committed Feb 7, 2018
1 parent d5f8d8b commit a3c1f98
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 0 deletions.
2 changes: 2 additions & 0 deletions pkg/image/importer/importer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
13 changes: 13 additions & 0 deletions pkg/image/registry/imagestreamimport/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package imagestreamimport
import (
"fmt"
"net/http"
"strings"
"time"

"github.com/golang/glog"
Expand Down Expand Up @@ -199,6 +200,18 @@ 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.
for _, image := range isi.Status.Images {
if image.Status.Reason == metav1.StatusReasonUnauthorized && strings.Contains(image.Status.Message, "incorrect username or password") {
importCtx := registryclient.NewContext(r.transport, r.insecureTransport).WithCredentials(nil)
imports := r.importFn(importCtx)
if err := imports.Import(ctx.(gocontext.Context), isi, stream); err != nil {
return nil, kapierrors.NewInternalError(fmt.Errorf(image.Status.Message))
}
}
}

// 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
Expand Down
10 changes: 10 additions & 0 deletions test/cmd/images.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 --docker-email==dummy1@example.com'
os::cmd::expect_success 'oc create secret docker-registry dummy-secret2 --docker-server=docker.io --docker-username=dummy2 --docker-password=dummy2 --docker-email==dummy2@example.com'
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

0 comments on commit a3c1f98

Please sign in to comment.