Merge pull request #22663 from smarterclayton/fix_mirror

Fix two issues with s3 mirroring and extend to release images

Author: openshift-merge-robot

pkg/oc/cli/admin/release/mirror.go

@@ -34,7 +34,8 @@ import (
 // NewMirrorOptions creates the options for mirroring a release.
 func NewMirrorOptions(streams genericclioptions.IOStreams) *MirrorOptions {
 	return &MirrorOptions{
-		IOStreams: streams,
+		IOStreams:       streams,
+		ParallelOptions: imagemanifest.ParallelOptions{MaxPerRegistry: 6},
 	}
 }
 
@@ -153,7 +154,7 @@ func (o *MirrorOptions) Run() error {
 
 	var recreateRequired bool
 	var hasPrefix bool
-	var targetFn func(name string) imagereference.DockerImageReference
+	var targetFn func(name string) mirror.MirrorReference
 	var dst string
 	if len(o.ToImageStream) > 0 {
 		dst = imagereference.DockerImageReference{
@@ -167,13 +168,13 @@ func (o *MirrorOptions) Run() error {
 
 	if strings.Contains(dst, "${component}") {
 		format := strings.Replace(dst, "${component}", replaceComponentMarker, -1)
-		dstRef, err := imagereference.Parse(format)
+		dstRef, err := mirror.ParseMirrorReference(format)
 		if err != nil {
 			return fmt.Errorf("--to must be a valid image reference: %v", err)
 		}
-		targetFn = func(name string) imagereference.DockerImageReference {
+		targetFn = func(name string) mirror.MirrorReference {
 			value := strings.Replace(dst, "${component}", name, -1)
-			ref, err := imagereference.Parse(value)
+			ref, err := mirror.ParseMirrorReference(value)
 			if err != nil {
 				klog.Fatalf("requested component %q could not be injected into %s: %v", name, dst, err)
 			}
@@ -183,22 +184,25 @@ func (o *MirrorOptions) Run() error {
 		recreateRequired = replaceCount > 1 || (replaceCount == 1 && !strings.Contains(dstRef.Tag, replaceComponentMarker))
 
 	} else {
-		ref, err := imagereference.Parse(dst)
+		ref, err := mirror.ParseMirrorReference(dst)
 		if err != nil {
 			return fmt.Errorf("--to must be a valid image repository: %v", err)
 		}
 		if len(ref.ID) > 0 || len(ref.Tag) > 0 {
 			return fmt.Errorf("--to must be to an image repository and may not contain a tag or digest")
 		}
-		targetFn = func(name string) imagereference.DockerImageReference {
+		targetFn = func(name string) mirror.MirrorReference {
 			copied := ref
 			copied.Tag = name
 			return copied
 		}
 		hasPrefix = true
 	}
 
-	o.TargetFn = targetFn
+	o.TargetFn = func(name string) imagereference.DockerImageReference {
+		ref := targetFn(name)
+		return ref.DockerImageReference
+	}
 
 	if recreateRequired {
 		return fmt.Errorf("when mirroring to multiple repositories, use the new release command with --from-release and --mirror")
@@ -255,10 +259,11 @@ func (o *MirrorOptions) Run() error {
 				Name:        o.ToRelease,
 			})
 		} else if !o.SkipRelease {
+			dstRef := targetFn("release")
 			mappings = append(mappings, mirror.Mapping{
-				Type:        mirror.DestinationRegistry,
 				Source:      srcRef,
-				Destination: targetFn("release"),
+				Type:        dstRef.Type(),
+				Destination: dstRef.Combined(),
 				Name:        "release",
 			})
 		}
@@ -278,10 +283,11 @@ func (o *MirrorOptions) Run() error {
 			return fmt.Errorf("image-references should only contain pointers to images by digest: %s", tag.From.Name)
 		}
 
+		dstMirrorRef := targetFn(tag.Name)
 		mappings = append(mappings, mirror.Mapping{
-			Type:        mirror.DestinationRegistry,
 			Source:      from,
-			Destination: targetFn(tag.Name),
+			Type:        dstMirrorRef.Type(),
+			Destination: dstMirrorRef.Combined(),
 			Name:        tag.Name,
 		})
 		klog.V(2).Infof("Mapping %#v", mappings[len(mappings)-1])

pkg/oc/cli/image/mirror/mappings.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path"
 	"strings"
 	"sync"
 
@@ -36,6 +37,65 @@ func parseSource(ref string) (reference.DockerImageReference, error) {
 	return src, nil
 }
 
+type AWSReference struct {
+	Bucket string
+	Region string
+}
+
+type MirrorReference struct {
+	reference.DockerImageReference
+	AWS *AWSReference
+}
+
+func (r MirrorReference) Type() DestinationType {
+	if r.AWS != nil {
+		return DestinationS3
+	}
+	return DestinationRegistry
+}
+
+func (r MirrorReference) Combined() reference.DockerImageReference {
+	if r.AWS == nil {
+		return r.DockerImageReference
+	}
+	copied := r.DockerImageReference
+	copied.Registry = "s3.amazonaws.com"
+	copied.Namespace = path.Join(r.AWS.Region, r.AWS.Bucket, r.Namespace)
+	return copied
+}
+
+func ParseMirrorReference(ref string) (MirrorReference, error) {
+	var dst MirrorReference
+	switch {
+	case strings.HasPrefix(ref, "s3://"):
+		ref = strings.TrimPrefix(ref, "s3://")
+		dst.AWS = &AWSReference{}
+	}
+	image, err := reference.Parse(ref)
+	if err != nil {
+		return dst, fmt.Errorf("%q is not a valid image reference: %v", ref, err)
+	}
+	if len(dst.ID) != 0 {
+		return dst, fmt.Errorf("you must specify a tag for DST or leave it blank to only push by digest")
+	}
+	dst.DockerImageReference = image
+	if dst.AWS != nil {
+		parts := strings.SplitN(image.RepositoryName(), "/", 3)
+		if len(parts) < 3 {
+			return dst, fmt.Errorf("s3 target URLs must have at least 3 path segments: REGION/BUCKET/REPO[/...]")
+		}
+		dst.AWS.Region = parts[0]
+		dst.AWS.Bucket = parts[1]
+		dst.Registry = fmt.Sprintf("%s.s3.amazonaws.com", parts[0])
+		dst.Name = path.Base(parts[2])
+		dst.Namespace = path.Dir(parts[2])
+		if dst.Namespace == "." {
+			dst.Namespace = ""
+		}
+	}
+	return dst, nil
+}
+
 func parseDestination(ref string) (reference.DockerImageReference, DestinationType, error) {
 	dstType := DestinationRegistry
 	switch {

pkg/oc/cli/image/mirror/mirror.go

@@ -44,7 +44,8 @@ var (
 		blobs (instead of uploading). The source bucket also supports the suffix "/[store]", which
 		will transform blob identifiers into the form the Docker registry uses on disk, allowing
 		you to mirror directly from an existing S3-backed Docker registry. Credentials for S3
-		may be stored in your docker credential file and looked up by host.
+		may be stored in your docker credential file and looked up by host, or loaded via the normal
+		AWS client locations for ENV or file.
 
 		Images in manifest list format will be copied as-is unless you use --filter-by-os to restrict
 		the allowed images to copy in a manifest list. This flag has no effect on regular images.
@@ -542,6 +543,10 @@ func copyBlob(ctx context.Context, plan *workPlan, c *repositoryBlobCopy, blob d
 		return nil
 	}
 
+	if c.destinationType == DestinationS3 {
+		options = append(options, WithDescriptor(blob))
+	}
+
 	w, err := c.to.Create(ctx, options...)
 	// no-op
 	if err == ErrAlreadyExists {

pkg/oc/cli/image/mirror/s3.go

@@ -73,14 +73,19 @@ func (d *s3Driver) newObject(server *url.URL, region string, insecure bool, secu
 	creds = credentials.NewChainCredentials([]credentials.Provider{
 		&s3CredentialStore{store: d.Creds, url: securityDomain},
 		&credentials.EnvProvider{},
+		&credentials.SharedCredentialsProvider{},
 	})
 
-	awsConfig.WithS3ForcePathStyle(true)
-	awsConfig.WithEndpoint(server.String())
 	awsConfig.WithCredentials(creds)
 	awsConfig.WithRegion(region)
 	awsConfig.WithDisableSSL(insecure)
-	if klog.V(6) {
+
+	switch {
+	case bool(klog.V(10)):
+		awsConfig.WithLogLevel(aws.LogDebugWithHTTPBody | aws.LogDebugWithRequestErrors | aws.LogDebugWithSigning)
+	case bool(klog.V(8)):
+		awsConfig.WithLogLevel(aws.LogDebugWithRequestErrors)
+	case bool(klog.V(6)):
 		awsConfig.WithLogLevel(aws.LogDebug)
 	}
 
@@ -110,10 +115,16 @@ func (d *s3Driver) Repository(ctx context.Context, server *url.URL, repoName str
 	if err != nil {
 		return nil, err
 	}
-	named, err := reference.ParseNamed(parts[2])
+
+	ref, err := reference.Parse(parts[2])
 	if err != nil {
 		return nil, err
 	}
+	named, ok := ref.(reference.Named)
+	if !ok {
+		return nil, fmt.Errorf("%s is not a valid repository name", parts[2])
+	}
+
 	repo := &s3Repository{
 		ctx:      ctx,
 		s3:       s3obj,