Merge pull request #1522 from mtrmac/collapse-errors

Update for containers/image#1299

cmd/skopeo/inspect.go

@@ -18,6 +18,7 @@ import (
 	"github.com/containers/image/v5/transports"
 	"github.com/containers/image/v5/types"
 	"github.com/containers/skopeo/cmd/skopeo/inspect"
+	"github.com/docker/distribution/registry/api/errcode"
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
@@ -203,12 +204,26 @@ func (opts *inspectOptions) run(args []string, stdout io.Writer) (retErr error)
 		}
 		outputData.RepoTags, err = docker.GetRepositoryTags(ctx, sys, img.Reference())
 		if err != nil {
-			// some registries may decide to block the "list all tags" endpoint
-			// gracefully allow the inspect to continue in this case. Currently
-			// the IBM Bluemix container registry has this restriction.
-			// In addition, AWS ECR rejects it with 403 (Forbidden) if the "ecr:ListImages"
-			// action is not allowed.
-			if !strings.Contains(err.Error(), "401") && !strings.Contains(err.Error(), "403") {
+			// Some registries may decide to block the "list all tags" endpoint;
+			// gracefully allow the inspect to continue in this case:
+			fatalFailure := true
+			// - AWS ECR rejects it if the "ecr:ListImages" action is not allowed.
+			//   https://github.com/containers/skopeo/issues/726
+			var ec errcode.ErrorCoder
+			if ok := errors.As(err, &ec); ok && ec.ErrorCode() == errcode.ErrorCodeDenied {
+				fatalFailure = false
+			}
+			// - public.ecr.aws does not implement the endpoint at all, and fails with 404:
+			//   https://github.com/containers/skopeo/issues/1230
+			//   This is actually "code":"NOT_FOUND", and the parser doesn’t preserve that.
+			//   So, also check the error text.
+			if ok := errors.As(err, &ec); ok && ec.ErrorCode() == errcode.ErrorCodeUnknown {
+				var e errcode.Error
+				if ok := errors.As(err, &e); ok && e.Code == errcode.ErrorCodeUnknown && e.Message == "404 page not found" {
+					fatalFailure = false
+				}
+			}
+			if fatalFailure {
 				return fmt.Errorf("Error determining repository tags: %w", err)
 			}
 			logrus.Warnf("Registry disallows tag list retrieval; skipping")

cmd/skopeo/sync.go

@@ -216,15 +216,7 @@ func getImageTags(ctx context.Context, sysCtx *types.SystemContext, repoRef refe
 	}
 	tags, err := docker.GetRepositoryTags(ctx, sysCtx, dockerRef)
 	if err != nil {
-		var unauthorizedForCredentials docker.ErrUnauthorizedForCredentials
-		if errors.As(err, &unauthorizedForCredentials) {
-			// Some registries may decide to block the "list all tags" endpoint.
-			// Gracefully allow the sync to continue in this case.
-			logrus.Warnf("Registry disallows tag list retrieval: %s", err)
-			tags = nil
-		} else {
-			return nil, fmt.Errorf("Error determining repository tags for image %s: %w", name, err)
-		}
+		return nil, fmt.Errorf("Error determining repository tags for repo %s: %w", name, err)
 	}
 
 	return tags, nil

go.mod

@@ -4,9 +4,10 @@ go 1.17
 
 require (
 	github.com/containers/common v0.50.1
-	github.com/containers/image/v5 v5.23.0
+	github.com/containers/image/v5 v5.23.1-0.20221012204947-6ea53742be58
 	github.com/containers/ocicrypt v1.1.6
 	github.com/containers/storage v1.43.0
+	github.com/docker/distribution v2.8.1+incompatible
 	github.com/opencontainers/go-digest v1.0.0
 	github.com/opencontainers/image-spec v1.1.0-rc2
 	github.com/opencontainers/image-tools v1.0.0-rc3
@@ -31,7 +32,6 @@ require (
 	github.com/containers/libtrust v0.0.0-20200511145503-9c3a6c22cd9a // indirect
 	github.com/cyphar/filepath-securejoin v0.2.3 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
-	github.com/docker/distribution v2.8.1+incompatible // indirect
 	github.com/docker/docker v20.10.18+incompatible // indirect
 	github.com/docker/docker-credential-helpers v0.7.0 // indirect
 	github.com/docker/go-connections v0.4.0 // indirect

go.sum

@@ -386,8 +386,9 @@ github.com/containernetworking/plugins v1.0.1/go.mod h1:QHCfGpaTwYTbbH+nZXKVTxNB
 github.com/containernetworking/plugins v1.1.1/go.mod h1:Sr5TH/eBsGLXK/h71HeLfX19sZPp3ry5uHSkI4LPxV8=
 github.com/containers/common v0.50.1 h1:AYRAf1xyahNVRez49KIkREInNf36SQx1lyLY9M95wQI=
 github.com/containers/common v0.50.1/go.mod h1:XnWlXPyE9Ky+8v8MfYWJZFnejkprAkUeo0DTWmSiwcY=
-github.com/containers/image/v5 v5.23.0 h1:Uv/n8zsHVUBBJK2rfBUHbN4CutHHmsQeyi4f80lAOf8=
 github.com/containers/image/v5 v5.23.0/go.mod h1:EXFFGEsL99S6aqLqK2mQJ3yrNh6Q05UCHt4mhF9JNoM=
+github.com/containers/image/v5 v5.23.1-0.20221012204947-6ea53742be58 h1:VgX3CTXXkoSQFIr70Wsg59jioTwz5JUcV6q6BScWhh8=
+github.com/containers/image/v5 v5.23.1-0.20221012204947-6ea53742be58/go.mod h1:2JJxA5K1NFpA3FtrK+Csmdlj++5oveB7CsXhekEJsIU=
 github.com/containers/libtrust v0.0.0-20200511145503-9c3a6c22cd9a h1:spAGlqziZjCJL25C6F1zsQY05tfCKE9F5YwtEWWe6hU=
 github.com/containers/libtrust v0.0.0-20200511145503-9c3a6c22cd9a/go.mod h1:9rfv8iPl1ZP7aqh9YA68wnZv2NUDbXdcdPHVz0pFbPY=
 github.com/containers/ocicrypt v1.0.1/go.mod h1:MeJDzk1RJHv89LjsH0Sp5KTY3ZYkjXO/C+bKAeWFIrc=

integration/check_test.go

@@ -49,65 +49,63 @@ func (s *SkopeoSuite) TearDownSuite(c *check.C) {
 //func skopeoCmd()
 
 func (s *SkopeoSuite) TestVersion(c *check.C) {
-	wanted := fmt.Sprintf(".*%s version %s.*", skopeoBinary, version.Version)
-	assertSkopeoSucceeds(c, wanted, "--version")
+	assertSkopeoSucceeds(c, fmt.Sprintf(".*%s version %s.*", skopeoBinary, version.Version),
+		"--version")
 }
 
 func (s *SkopeoSuite) TestCanAuthToPrivateRegistryV2WithoutDockerCfg(c *check.C) {
-	wanted := ".*manifest unknown: manifest unknown.*"
-	assertSkopeoFails(c, wanted, "--tls-verify=false", "inspect", "--creds="+s.regV2WithAuth.username+":"+s.regV2WithAuth.password, fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url))
+	assertSkopeoFails(c, ".*manifest unknown.*",
+		"--tls-verify=false", "inspect", "--creds="+s.regV2WithAuth.username+":"+s.regV2WithAuth.password, fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url))
 }
 
 func (s *SkopeoSuite) TestNeedAuthToPrivateRegistryV2WithoutDockerCfg(c *check.C) {
-	wanted := ".*unauthorized: authentication required.*"
-	assertSkopeoFails(c, wanted, "--tls-verify=false", "inspect", fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url))
+	assertSkopeoFails(c, ".*authentication required.*",
+		"--tls-verify=false", "inspect", fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url))
 }
 
 func (s *SkopeoSuite) TestCertDirInsteadOfCertPath(c *check.C) {
-	wanted := ".*unknown flag: --cert-path.*"
-	assertSkopeoFails(c, wanted, "--tls-verify=false", "inspect", fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url), "--cert-path=/")
-	wanted = ".*unauthorized: authentication required.*"
-	assertSkopeoFails(c, wanted, "--tls-verify=false", "inspect", fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url), "--cert-dir=/etc/docker/certs.d/")
+	assertSkopeoFails(c, ".*unknown flag: --cert-path.*",
+		"--tls-verify=false", "inspect", fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url), "--cert-path=/")
+	assertSkopeoFails(c, ".*authentication required.*",
+		"--tls-verify=false", "inspect", fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url), "--cert-dir=/etc/docker/certs.d/")
 }
 
 // TODO(runcom): as soon as we can push to registries ensure you can inspect here
 // not just get image not found :)
 func (s *SkopeoSuite) TestNoNeedAuthToPrivateRegistryV2ImageNotFound(c *check.C) {
 	out, err := exec.Command(skopeoBinary, "--tls-verify=false", "inspect", fmt.Sprintf("docker://%s/busybox:latest", s.regV2.url)).CombinedOutput()
 	c.Assert(err, check.NotNil, check.Commentf(string(out)))
-	wanted := ".*manifest unknown.*"
-	c.Assert(string(out), check.Matches, "(?s)"+wanted) // (?s) : '.' will also match newlines
-	wanted = ".*unauthorized: authentication required.*"
-	c.Assert(string(out), check.Not(check.Matches), "(?s)"+wanted) // (?s) : '.' will also match newlines
+	c.Assert(string(out), check.Matches, "(?s).*manifest unknown.*")                                 // (?s) : '.' will also match newlines
+	c.Assert(string(out), check.Not(check.Matches), "(?s).*unauthorized: authentication required.*") // (?s) : '.' will also match newlines
 }
 
 func (s *SkopeoSuite) TestInspectFailsWhenReferenceIsInvalid(c *check.C) {
 	assertSkopeoFails(c, `.*Invalid image name.*`, "inspect", "unknown")
 }
 
 func (s *SkopeoSuite) TestLoginLogout(c *check.C) {
-	wanted := "^Login Succeeded!\n$"
-	assertSkopeoSucceeds(c, wanted, "login", "--tls-verify=false", "--username="+s.regV2WithAuth.username, "--password="+s.regV2WithAuth.password, s.regV2WithAuth.url)
+	assertSkopeoSucceeds(c, "^Login Succeeded!\n$",
+		"login", "--tls-verify=false", "--username="+s.regV2WithAuth.username, "--password="+s.regV2WithAuth.password, s.regV2WithAuth.url)
 	// test --get-login returns username
-	wanted = fmt.Sprintf("^%s\n$", s.regV2WithAuth.username)
-	assertSkopeoSucceeds(c, wanted, "login", "--tls-verify=false", "--get-login", s.regV2WithAuth.url)
+	assertSkopeoSucceeds(c, fmt.Sprintf("^%s\n$", s.regV2WithAuth.username),
+		"login", "--tls-verify=false", "--get-login", s.regV2WithAuth.url)
 	// test logout
-	wanted = fmt.Sprintf("^Removed login credentials for %s\n$", s.regV2WithAuth.url)
-	assertSkopeoSucceeds(c, wanted, "logout", s.regV2WithAuth.url)
+	assertSkopeoSucceeds(c, fmt.Sprintf("^Removed login credentials for %s\n$", s.regV2WithAuth.url),
+		"logout", s.regV2WithAuth.url)
 }
 
 func (s *SkopeoSuite) TestCopyWithLocalAuth(c *check.C) {
-	wanted := "^Login Succeeded!\n$"
-	assertSkopeoSucceeds(c, wanted, "login", "--tls-verify=false", "--username="+s.regV2WithAuth.username, "--password="+s.regV2WithAuth.password, s.regV2WithAuth.url)
+	assertSkopeoSucceeds(c, "^Login Succeeded!\n$",
+		"login", "--tls-verify=false", "--username="+s.regV2WithAuth.username, "--password="+s.regV2WithAuth.password, s.regV2WithAuth.url)
 	// copy to private registry using local authentication
 	imageName := fmt.Sprintf("docker://%s/busybox:mine", s.regV2WithAuth.url)
 	assertSkopeoSucceeds(c, "", "copy", "--dest-tls-verify=false", testFQIN+":latest", imageName)
 	// inspect from private registry
 	assertSkopeoSucceeds(c, "", "inspect", "--tls-verify=false", imageName)
 	// logout from the registry
-	wanted = fmt.Sprintf("^Removed login credentials for %s\n$", s.regV2WithAuth.url)
-	assertSkopeoSucceeds(c, wanted, "logout", s.regV2WithAuth.url)
+	assertSkopeoSucceeds(c, fmt.Sprintf("^Removed login credentials for %s\n$", s.regV2WithAuth.url),
+		"logout", s.regV2WithAuth.url)
 	// inspect from private registry should fail after logout
-	wanted = ".*unauthorized: authentication required.*"
-	assertSkopeoFails(c, wanted, "inspect", "--tls-verify=false", imageName)
+	assertSkopeoFails(c, ".*authentication required.*",
+		"inspect", "--tls-verify=false", imageName)
 }

integration/copy_test.go

@@ -1036,7 +1036,8 @@ func (s *CopySuite) TestCopyVerifyingMirroredSignatures(c *check.C) {
 	assertSkopeoSucceeds(c, "", "--policy", policy, "--registries.d", registriesDir, "--registries-conf", "fixtures/registries.conf", "copy", "--src-tls-verify=false", regPrefix+"remap:remapped", dirDest)
 	// To be extra clear about the semantics, verify that the signedPrefix (primary) location never exists
 	// and only the remapped prefix (mirror) is accessed.
-	assertSkopeoFails(c, ".*initializing source docker://localhost:5006/myns/mirroring-primary:remapped:.*manifest unknown: manifest unknown.*", "--policy", policy, "--registries.d", registriesDir, "--registries-conf", "fixtures/registries.conf", "copy", "--src-tls-verify=false", regPrefix+"primary:remapped", dirDest)
+	assertSkopeoFails(c, ".*initializing source docker://localhost:5006/myns/mirroring-primary:remapped:.*manifest unknown.*",
+		"--policy", policy, "--registries.d", registriesDir, "--registries-conf", "fixtures/registries.conf", "copy", "--src-tls-verify=false", regPrefix+"primary:remapped", dirDest)
 }
 
 func (s *SkopeoSuite) TestCopySrcWithAuth(c *check.C) {

integration/sync_test.go

@@ -534,7 +534,7 @@ func (s *SyncSuite) TestFailsNoSourceImages(c *check.C) {
 	assertSkopeoFails(c, ".*No images to sync found in .*",
 		"sync", "--scoped", "--dest-tls-verify=false", "--src", "dir", "--dest", "docker", tmpDir, v2DockerRegistryURL)
 
-	assertSkopeoFails(c, ".*No images to sync found in .*",
+	assertSkopeoFails(c, ".*Error determining repository tags for repo docker.io/library/hopefully_no_images_will_ever_be_called_like_this: fetching tags list: requested access to the resource is denied.*",
 		"sync", "--scoped", "--dest-tls-verify=false", "--src", "docker", "--dest", "docker", "hopefully_no_images_will_ever_be_called_like_this", v2DockerRegistryURL)
 }
 
@@ -544,11 +544,11 @@ func (s *SyncSuite) TestFailsWithDockerSourceNoRegistry(c *check.C) {
 	tmpDir := c.MkDir()
 
 	//untagged
-	assertSkopeoFails(c, ".*invalid status code from registry 404.*",
+	assertSkopeoFails(c, ".*StatusCode: 404.*",
 		"sync", "--scoped", "--src", "docker", "--dest", "dir", regURL, tmpDir)
 
 	//tagged
-	assertSkopeoFails(c, ".*invalid status code from registry 404.*",
+	assertSkopeoFails(c, ".*StatusCode: 404.*",
 		"sync", "--scoped", "--src", "docker", "--dest", "dir", regURL+":thetag", tmpDir)
 }
 
@@ -557,11 +557,11 @@ func (s *SyncSuite) TestFailsWithDockerSourceUnauthorized(c *check.C) {
 	tmpDir := c.MkDir()
 
 	//untagged
-	assertSkopeoFails(c, ".*Registry disallows tag list retrieval.*",
+	assertSkopeoFails(c, ".*requested access to the resource is denied.*",
 		"sync", "--scoped", "--src", "docker", "--dest", "dir", repo, tmpDir)
 
 	//tagged
-	assertSkopeoFails(c, ".*unauthorized: authentication required.*",
+	assertSkopeoFails(c, ".*requested access to the resource is denied.*",
 		"sync", "--scoped", "--src", "docker", "--dest", "dir", repo+":thetag", tmpDir)
 }
 
@@ -570,7 +570,7 @@ func (s *SyncSuite) TestFailsWithDockerSourceNotExisting(c *check.C) {
 	tmpDir := c.MkDir()
 
 	//untagged
-	assertSkopeoFails(c, ".*invalid status code from registry 404.*",
+	assertSkopeoFails(c, ".*repository name not known to registry.*",
 		"sync", "--scoped", "--src-tls-verify=false", "--src", "docker", "--dest", "dir", repo, tmpDir)
 
 	//tagged

systemtest/040-local-registry-auth.bats

@@ -24,22 +24,22 @@ function setup() {
 @test "auth: credentials on command line" {
     # No creds
     run_skopeo 1 inspect --tls-verify=false docker://localhost:5000/nonesuch
-    expect_output --substring "unauthorized: authentication required"
+    expect_output --substring "authentication required"
 
     # Wrong user
     run_skopeo 1 inspect --tls-verify=false --creds=baduser:badpassword \
                docker://localhost:5000/nonesuch
-    expect_output --substring "unauthorized: authentication required"
+    expect_output --substring "authentication required"
 
     # Wrong password
     run_skopeo 1 inspect --tls-verify=false --creds=$testuser:badpassword \
                docker://localhost:5000/nonesuch
-    expect_output --substring "unauthorized: authentication required"
+    expect_output --substring "authentication required"
 
     # Correct creds, but no such image
     run_skopeo 1 inspect --tls-verify=false --creds=$testuser:$testpassword \
                docker://localhost:5000/nonesuch
-    expect_output --substring "manifest unknown: manifest unknown"
+    expect_output --substring "manifest unknown"
 
     # These should pass
     run_skopeo copy --dest-tls-verify=false --dcreds=$testuser:$testpassword \
@@ -64,7 +64,7 @@ function setup() {
     podman logout localhost:5000
 
     run_skopeo 1 inspect --tls-verify=false docker://localhost:5000/busybox:mine
-    expect_output --substring "unauthorized: authentication required"
+    expect_output --substring "authentication required"
 }
 
 @test "auth: copy with --src-creds and --dest-creds" {
@@ -94,7 +94,7 @@ function setup() {
 
     # inspect without authfile: should fail
     run_skopeo 1 inspect --tls-verify=false docker://localhost:5000/busybox:mine
-    expect_output --substring "unauthorized: authentication required"
+    expect_output --substring "authentication required"
 
     # inspect with authfile: should work
     run_skopeo inspect --tls-verify=false --authfile $TESTDIR/test.auth docker://localhost:5000/busybox:mine