Skip to content

Commit 63176d5

Browse files
s3 mirrors should not error parsing repository name
The semi-recent change to force canonical parsing incorrectly broke this code path. While we are in here, clean up the client invocations, correctly support ~/.aws/credentials, and add better debugging info. Also, the WithDescriptor() option is required for creation so ensure it is always set.
1 parent ed28c6c commit 63176d5

2 files changed

Lines changed: 21 additions & 5 deletions

File tree

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ var (
4444
blobs (instead of uploading). The source bucket also supports the suffix "/[store]", which
4545
will transform blob identifiers into the form the Docker registry uses on disk, allowing
4646
you to mirror directly from an existing S3-backed Docker registry. Credentials for S3
47-
may be stored in your docker credential file and looked up by host.
47+
may be stored in your docker credential file and looked up by host, or loaded via the normal
48+
AWS client locations for ENV or file.
4849
4950
Images in manifest list format will be copied as-is unless you use --filter-by-os to restrict
5051
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
542543
return nil
543544
}
544545

546+
if c.destinationType == DestinationS3 {
547+
options = append(options, WithDescriptor(blob))
548+
}
549+
545550
w, err := c.to.Create(ctx, options...)
546551
// no-op
547552
if err == ErrAlreadyExists {

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,19 @@ func (d *s3Driver) newObject(server *url.URL, region string, insecure bool, secu
7373
creds = credentials.NewChainCredentials([]credentials.Provider{
7474
&s3CredentialStore{store: d.Creds, url: securityDomain},
7575
&credentials.EnvProvider{},
76+
&credentials.SharedCredentialsProvider{},
7677
})
7778

78-
awsConfig.WithS3ForcePathStyle(true)
79-
awsConfig.WithEndpoint(server.String())
8079
awsConfig.WithCredentials(creds)
8180
awsConfig.WithRegion(region)
8281
awsConfig.WithDisableSSL(insecure)
83-
if klog.V(6) {
82+
83+
switch {
84+
case bool(klog.V(10)):
85+
awsConfig.WithLogLevel(aws.LogDebugWithHTTPBody | aws.LogDebugWithRequestErrors | aws.LogDebugWithSigning)
86+
case bool(klog.V(8)):
87+
awsConfig.WithLogLevel(aws.LogDebugWithRequestErrors)
88+
case bool(klog.V(6)):
8489
awsConfig.WithLogLevel(aws.LogDebug)
8590
}
8691

@@ -110,10 +115,16 @@ func (d *s3Driver) Repository(ctx context.Context, server *url.URL, repoName str
110115
if err != nil {
111116
return nil, err
112117
}
113-
named, err := reference.ParseNamed(parts[2])
118+
119+
ref, err := reference.Parse(parts[2])
114120
if err != nil {
115121
return nil, err
116122
}
123+
named, ok := ref.(reference.Named)
124+
if !ok {
125+
return nil, fmt.Errorf("%s is not a valid repository name", parts[2])
126+
}
127+
117128
repo := &s3Repository{
118129
ctx: ctx,
119130
s3: s3obj,

0 commit comments

Comments
 (0)