Skip to content

Commit aa80c85

Browse files
soltyshsoltysh
committed
Bug 1420976 - Support passing reference-policy in import-image command
1 parent f617108 commit aa80c85

File tree

9 files changed

+162
-66
lines changed

9 files changed

+162
-66
lines changed

‎pkg/api/serialization_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,10 @@ func fuzzInternalObject(t *testing.T, forVersion unversioned.GroupVersion, item
272272
j.From.Kind = specs[c.Intn(len(specs))]
273273
}
274274
},
275+
func(j *image.TagReferencePolicy, c fuzz.Continue) {
276+
c.FuzzNoCustom(j)
277+
j.Type = image.SourceTagReferencePolicy
278+
},
275279
func(j *build.BuildConfigSpec, c fuzz.Continue) {
276280
c.FuzzNoCustom(j)
277281
j.RunPolicy = build.BuildRunPolicySerial

‎pkg/cmd/cli/cmd/importimage.go

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ func NewCmdImportImage(fullName string, f *clientcmd.Factory, out, errout io.Wri
5252
cmd.Flags().StringVar(&opts.From, "from", "", "A Docker image repository to import images from")
5353
cmd.Flags().BoolVar(&opts.Confirm, "confirm", false, "If true, allow the image stream import location to be set or changed")
5454
cmd.Flags().BoolVar(&opts.All, "all", false, "If true, import all tags from the provided source on creation or if --from is specified")
55+
cmd.Flags().StringVar(&opts.ReferencePolicy, "reference-policy", sourceReferencePolicy, "Allow to request pullthrough for external image when set to 'local'. Defaults to 'source'.")
5556
opts.Insecure = cmd.Flags().Bool("insecure", false, "If true, allow importing from registries that have invalid HTTPS certificates or are hosted via HTTP. This flag will take precedence over the insecure annotation.")
5657

5758
return cmd
@@ -66,10 +67,11 @@ type ImportImageOptions struct {
6667
Insecure *bool
6768

6869
// internal values
69-
Namespace string
70-
Name string
71-
Tag string
72-
Target string
70+
Namespace string
71+
Name string
72+
Tag string
73+
Target string
74+
ReferencePolicy string
7375

7476
CommandName string
7577

@@ -92,6 +94,9 @@ func (o *ImportImageOptions) Complete(f *clientcmd.Factory, cmd *cobra.Command,
9294
if !cmd.Flags().Lookup("insecure").Changed {
9395
o.Insecure = nil
9496
}
97+
if !cmd.Flags().Lookup("reference-policy").Changed {
98+
o.ReferencePolicy = ""
99+
}
95100

96101
namespace, _, err := f.DefaultNamespace()
97102
if err != nil {
@@ -423,13 +428,20 @@ func (o *ImportImageOptions) newImageStream() (*imageapi.ImageStream, *imageapi.
423428
if len(from) == 0 {
424429
from = o.Target
425430
}
426-
var stream *imageapi.ImageStream
427-
// create new ImageStream
431+
var (
432+
stream *imageapi.ImageStream
433+
isi *imageapi.ImageStreamImport
434+
)
435+
// create new ImageStream and accompanying ImageStreamImport
436+
// TODO: this should be removed along with the legacy path, we don't need to
437+
// create the IS in the new path, the import mechanism will do that for us,
438+
// this is only for the legacy path that we need to create the IS.
428439
if o.All {
429440
stream = &imageapi.ImageStream{
430441
ObjectMeta: kapi.ObjectMeta{Name: o.Name},
431442
Spec: imageapi.ImageStreamSpec{DockerImageRepository: from},
432443
}
444+
isi = o.newImageStreamImportAll(stream, from)
433445
} else {
434446
stream = &imageapi.ImageStream{
435447
ObjectMeta: kapi.ObjectMeta{Name: o.Name},
@@ -440,22 +452,31 @@ func (o *ImportImageOptions) newImageStream() (*imageapi.ImageStream, *imageapi.
440452
Kind: "DockerImage",
441453
Name: from,
442454
},
455+
ReferencePolicy: o.getReferencePolicy(),
443456
},
444457
},
445458
},
446459
}
447-
}
448-
// and accompanying ImageStreamImport
449-
var isi *imageapi.ImageStreamImport
450-
if o.All {
451-
isi = o.newImageStreamImportAll(stream, from)
452-
} else {
453460
isi = o.newImageStreamImportTags(stream, map[string]string{tag: from})
454461
}
455462

456463
return stream, isi
457464
}
458465

466+
func (o *ImportImageOptions) getReferencePolicy() imageapi.TagReferencePolicy {
467+
ref := imageapi.TagReferencePolicy{}
468+
if len(o.ReferencePolicy) == 0 {
469+
return ref
470+
}
471+
switch o.ReferencePolicy {
472+
case sourceReferencePolicy:
473+
ref.Type = imageapi.SourceTagReferencePolicy
474+
case localReferencePolicy:
475+
ref.Type = imageapi.LocalTagReferencePolicy
476+
}
477+
return ref
478+
}
479+
459480
func (o *ImportImageOptions) newImageStreamImport(stream *imageapi.ImageStream) (*imageapi.ImageStreamImport, bool) {
460481
isi := &imageapi.ImageStreamImport{
461482
ObjectMeta: kapi.ObjectMeta{
@@ -482,7 +503,8 @@ func (o *ImportImageOptions) newImageStreamImportAll(stream *imageapi.ImageStrea
482503
Kind: "DockerImage",
483504
Name: from,
484505
},
485-
ImportPolicy: imageapi.TagImportPolicy{Insecure: insecure},
506+
ImportPolicy: imageapi.TagImportPolicy{Insecure: insecure},
507+
ReferencePolicy: o.getReferencePolicy(),
486508
}
487509

488510
return isi
@@ -496,8 +518,9 @@ func (o *ImportImageOptions) newImageStreamImportTags(stream *imageapi.ImageStre
496518
Kind: "DockerImage",
497519
Name: from,
498520
},
499-
To: &kapi.LocalObjectReference{Name: tag},
500-
ImportPolicy: imageapi.TagImportPolicy{Insecure: insecure},
521+
To: &kapi.LocalObjectReference{Name: tag},
522+
ImportPolicy: imageapi.TagImportPolicy{Insecure: insecure},
523+
ReferencePolicy: o.getReferencePolicy(),
501524
})
502525
}
503526
return isi

‎pkg/cmd/cli/cmd/importimage_test.go

Lines changed: 69 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ func TestCreateImageImport(t *testing.T) {
2121
all bool
2222
confirm bool
2323
insecure *bool
24+
referencePolicy string
2425
err string
2526
expectedImages []imageapi.ImageImportSpec
2627
expectedRepository *imageapi.RepositoryImportSpec
@@ -338,6 +339,67 @@ func TestCreateImageImport(t *testing.T) {
338339
ImportPolicy: imageapi.TagImportPolicy{Insecure: false},
339340
}},
340341
},
342+
"import tag setting referencePolicy": {
343+
name: "testis:mytag",
344+
referencePolicy: localReferencePolicy,
345+
stream: &imageapi.ImageStream{
346+
ObjectMeta: kapi.ObjectMeta{Name: "testis", Namespace: "other"},
347+
Spec: imageapi.ImageStreamSpec{
348+
Tags: map[string]imageapi.TagReference{
349+
"mytag": {
350+
From: &kapi.ObjectReference{Kind: "DockerImage", Name: "repo.com/somens/someimage:mytag"},
351+
},
352+
},
353+
},
354+
},
355+
expectedImages: []imageapi.ImageImportSpec{{
356+
From: kapi.ObjectReference{Kind: "DockerImage", Name: "repo.com/somens/someimage:mytag"},
357+
To: &kapi.LocalObjectReference{Name: "mytag"},
358+
ReferencePolicy: imageapi.TagReferencePolicy{Type: imageapi.LocalTagReferencePolicy},
359+
}},
360+
},
361+
"import all from .spec.tags setting referencePolicy": {
362+
name: "testis",
363+
all: true,
364+
referencePolicy: localReferencePolicy,
365+
stream: &imageapi.ImageStream{
366+
ObjectMeta: kapi.ObjectMeta{Name: "testis", Namespace: "other"},
367+
Spec: imageapi.ImageStreamSpec{
368+
Tags: map[string]imageapi.TagReference{
369+
"mytag": {From: &kapi.ObjectReference{Kind: "DockerImage", Name: "repo.com/somens/someimage:mytag"}},
370+
"other": {From: &kapi.ObjectReference{Kind: "DockerImage", Name: "repo.com/somens/someimage:other"}},
371+
},
372+
},
373+
},
374+
expectedImages: []imageapi.ImageImportSpec{
375+
{
376+
From: kapi.ObjectReference{Kind: "DockerImage", Name: "repo.com/somens/someimage:mytag"},
377+
To: &kapi.LocalObjectReference{Name: "mytag"},
378+
ReferencePolicy: imageapi.TagReferencePolicy{Type: imageapi.LocalTagReferencePolicy},
379+
},
380+
{
381+
From: kapi.ObjectReference{Kind: "DockerImage", Name: "repo.com/somens/someimage:other"},
382+
To: &kapi.LocalObjectReference{Name: "other"},
383+
ReferencePolicy: imageapi.TagReferencePolicy{Type: imageapi.LocalTagReferencePolicy},
384+
},
385+
},
386+
},
387+
"import all from .spec.dockerImageRepository setting referencePolicy": {
388+
name: "testis",
389+
all: true,
390+
referencePolicy: localReferencePolicy,
391+
stream: &imageapi.ImageStream{
392+
ObjectMeta: kapi.ObjectMeta{Name: "testis", Namespace: "other"},
393+
Spec: imageapi.ImageStreamSpec{
394+
DockerImageRepository: "repo.com/somens/someimage",
395+
Tags: make(map[string]imageapi.TagReference),
396+
},
397+
},
398+
expectedRepository: &imageapi.RepositoryImportSpec{
399+
From: kapi.ObjectReference{Kind: "DockerImage", Name: "repo.com/somens/someimage"},
400+
ReferencePolicy: imageapi.TagReferencePolicy{Type: imageapi.LocalTagReferencePolicy},
401+
},
402+
},
341403
}
342404

343405
for name, test := range testCases {
@@ -348,12 +410,13 @@ func TestCreateImageImport(t *testing.T) {
348410
fake = testclient.NewSimpleFake(test.stream)
349411
}
350412
o := ImportImageOptions{
351-
Target: test.name,
352-
From: test.from,
353-
All: test.all,
354-
Insecure: test.insecure,
355-
Confirm: test.confirm,
356-
isClient: fake.ImageStreams("other"),
413+
Target: test.name,
414+
From: test.from,
415+
All: test.all,
416+
Insecure: test.insecure,
417+
ReferencePolicy: test.referencePolicy,
418+
Confirm: test.confirm,
419+
isClient: fake.ImageStreams("other"),
357420
}
358421
// we need to run Validate, because it sets appropriate Name and Tag
359422
if err := o.Validate(&cobra.Command{}); err != nil {

‎pkg/image/api/types.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,7 @@ type RepositoryImportSpec struct {
437437
From kapi.ObjectReference
438438

439439
ImportPolicy TagImportPolicy
440+
ReferencePolicy TagReferencePolicy
440441
IncludeManifest bool
441442
}
442443

@@ -457,6 +458,7 @@ type ImageImportSpec struct {
457458
To *kapi.LocalObjectReference
458459

459460
ImportPolicy TagImportPolicy
461+
ReferencePolicy TagReferencePolicy
460462
IncludeManifest bool
461463
}
462464

‎pkg/image/api/v1/types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,8 @@ type RepositoryImportSpec struct {
393393

394394
// ImportPolicy is the policy controlling how the image is imported
395395
ImportPolicy TagImportPolicy `json:"importPolicy,omitempty" protobuf:"bytes,2,opt,name=importPolicy"`
396+
// ReferencePolicy defines how other components should consume the image
397+
ReferencePolicy TagReferencePolicy `json:"referencePolicy,omitempty" protobuf:"bytes,4,opt,name=referencePolicy"`
396398
// IncludeManifest determines if the manifest for each image is returned in the response
397399
IncludeManifest bool `json:"includeManifest,omitempty" protobuf:"varint,3,opt,name=includeManifest"`
398400
}
@@ -417,6 +419,8 @@ type ImageImportSpec struct {
417419

418420
// ImportPolicy is the policy controlling how the image is imported
419421
ImportPolicy TagImportPolicy `json:"importPolicy,omitempty" protobuf:"bytes,3,opt,name=importPolicy"`
422+
// ReferencePolicy defines how other components should consume the image
423+
ReferencePolicy TagReferencePolicy `json:"referencePolicy,omitempty" protobuf:"bytes,5,opt,name=referencePolicy"`
420424
// IncludeManifest determines if the manifest for each image is returned in the response
421425
IncludeManifest bool `json:"includeManifest,omitempty" protobuf:"varint,4,opt,name=includeManifest"`
422426
}

‎pkg/image/controller/controller.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,10 @@ func (c *ImportController) Next(stream *api.ImageStream, notifier Notifier) erro
146146
continue
147147
}
148148
isi.Spec.Images = append(isi.Spec.Images, api.ImageImportSpec{
149-
From: kapi.ObjectReference{Kind: "DockerImage", Name: tagRef.From.Name},
150-
To: &kapi.LocalObjectReference{Name: tag},
151-
ImportPolicy: tagRef.ImportPolicy,
149+
From: kapi.ObjectReference{Kind: "DockerImage", Name: tagRef.From.Name},
150+
To: &kapi.LocalObjectReference{Name: tag},
151+
ImportPolicy: tagRef.ImportPolicy,
152+
ReferencePolicy: tagRef.ReferencePolicy,
152153
})
153154
}
154155
if repo := stream.Spec.DockerImageRepository; !partial && len(repo) > 0 {

‎pkg/image/controller/controller_test.go

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,26 @@ func TestControllerStart(t *testing.T) {
274274
},
275275
},
276276
},
277+
// test external repo
278+
{
279+
run: true,
280+
stream: &api.ImageStream{
281+
ObjectMeta: kapi.ObjectMeta{
282+
Name: "test",
283+
Namespace: "other",
284+
},
285+
Spec: api.ImageStreamSpec{
286+
Tags: map[string]api.TagReference{
287+
"1.1": {
288+
From: &kapi.ObjectReference{
289+
Kind: "DockerImage",
290+
Name: "some/repo:mytag",
291+
},
292+
},
293+
},
294+
},
295+
},
296+
},
277297
}
278298

279299
for i, test := range testCases {
@@ -288,8 +308,13 @@ func TestControllerStart(t *testing.T) {
288308
t.Errorf("%d: unexpected error: %v", i, err)
289309
}
290310
if test.run {
291-
if len(fake.Actions()) == 0 {
311+
actions := fake.Actions()
312+
if len(actions) == 0 {
292313
t.Errorf("%d: expected remote calls: %#v", i, fake)
314+
continue
315+
}
316+
if !actions[0].Matches("create", "imagestreamimports") {
317+
t.Errorf("expected a create action: %#v", actions)
293318
}
294319
} else {
295320
if !kapi.Semantic.DeepEqual(test.stream, other) {
@@ -302,38 +327,6 @@ func TestControllerStart(t *testing.T) {
302327
}
303328
}
304329

305-
func TestControllerExternalRepo(t *testing.T) {
306-
fake := &client.Fake{}
307-
c := ImportController{streams: fake}
308-
309-
stream := api.ImageStream{
310-
ObjectMeta: kapi.ObjectMeta{
311-
Name: "test",
312-
Namespace: "other",
313-
},
314-
Spec: api.ImageStreamSpec{
315-
Tags: map[string]api.TagReference{
316-
"1.1": {
317-
From: &kapi.ObjectReference{
318-
Kind: "DockerImage",
319-
Name: "some/repo:mytag",
320-
},
321-
},
322-
},
323-
},
324-
}
325-
if err := c.Next(&stream, nil); err != nil {
326-
t.Errorf("unexpected error: %v", err)
327-
}
328-
actions := fake.Actions()
329-
if len(actions) != 1 {
330-
t.Fatalf("expected 1 actions, got %#v", actions)
331-
}
332-
if !actions[0].Matches("create", "imagestreamimports") {
333-
t.Errorf("expected a create action: %#v", actions)
334-
}
335-
}
336-
337330
func TestScheduledImport(t *testing.T) {
338331
fake := &client.Fake{}
339332
b := newScheduled(true, fake, 1, nil, nil)

0 commit comments

Comments
 (0)