Skip to content

Commit 8a05370

Browse files
author
OpenShift Bot
authored
Merge pull request #13632 from soltysh/bug1435588
Merged by openshift-bot
2 parents fdd7cc1 + b3b252a commit 8a05370

File tree

3 files changed

+188
-131
lines changed

3 files changed

+188
-131
lines changed

‎pkg/cmd/cli/cmd/tag.go

+34-6
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,13 @@ func NewCmdTag(fullName string, f *clientcmd.Factory, out io.Writer) *cobra.Comm
9292
Run: func(cmd *cobra.Command, args []string) {
9393
kcmdutil.CheckErr(opts.Complete(f, cmd, args, out))
9494
kcmdutil.CheckErr(opts.Validate())
95-
kcmdutil.CheckErr(opts.RunTag())
95+
kcmdutil.CheckErr(opts.Run())
9696
},
9797
}
9898

9999
cmd.Flags().StringVar(&opts.sourceKind, "source", opts.sourceKind, "Optional hint for the source type; valid values are 'imagestreamtag', 'istag', 'imagestreamimage', 'isimage', and 'docker'.")
100100
cmd.Flags().BoolVarP(&opts.deleteTag, "delete", "d", opts.deleteTag, "Delete the provided spec tags.")
101-
cmd.Flags().BoolVar(&opts.aliasTag, "alias", false, "Should the destination tag be updated whenever the source tag changes. Defaults to false.")
101+
cmd.Flags().BoolVar(&opts.aliasTag, "alias", false, "Should the destination tag be updated whenever the source tag changes. Applies only to a single image stream. Defaults to false.")
102102
cmd.Flags().BoolVar(&opts.referenceTag, "reference", false, "Should the destination tag continue to pull from the source namespace. Defaults to false.")
103103
cmd.Flags().BoolVar(&opts.scheduleTag, "scheduled", false, "Set a Docker image to be periodically imported from a remote repository. Defaults to false.")
104104
cmd.Flags().BoolVar(&opts.insecureTag, "insecure", false, "Set to true if importing the specified Docker image requires HTTP or has a self-signed certificate. Defaults to false.")
@@ -271,6 +271,25 @@ func (o *TagOptions) Complete(f *clientcmd.Factory, cmd *cobra.Command, args []s
271271
return nil
272272
}
273273

274+
// isCrossImageStream verifies if destination is the same image stream as source. Returns true
275+
// if any of the destination image stream is different and error from parsing
276+
// image stream tag.
277+
func isCrossImageStream(namespace string, srcRef imageapi.DockerImageReference, destNamespace []string, destNameAndTag []string) (bool, error) {
278+
for i, ns := range destNamespace {
279+
if namespace != ns {
280+
return true, nil
281+
}
282+
name, _, ok := imageapi.SplitImageStreamTag(destNameAndTag[i])
283+
if !ok {
284+
return false, fmt.Errorf("%q must be of the form <stream_name>:<tag>", destNameAndTag[i])
285+
}
286+
if srcRef.Name != name {
287+
return true, nil
288+
}
289+
}
290+
return false, nil
291+
}
292+
274293
// Validate validates all the required options for the tag command.
275294
func (o TagOptions) Validate() error {
276295
// Validate client and writer
@@ -319,15 +338,24 @@ func (o TagOptions) Validate() error {
319338
if o.sourceKind != "DockerImage" && (o.scheduleTag || o.insecureTag) {
320339
return errors.New("only Docker images can have importing flags set")
321340
}
322-
if o.aliasTag && (o.scheduleTag || o.insecureTag) {
323-
return errors.New("cannot set a Docker image tag as an alias and also set import flags")
341+
if o.aliasTag {
342+
if o.scheduleTag || o.insecureTag {
343+
return errors.New("cannot set a Docker image tag as an alias and also set import flags")
344+
}
345+
cross, err := isCrossImageStream(o.namespace, o.ref, o.destNamespace, o.destNameAndTag)
346+
if err != nil {
347+
return err
348+
}
349+
if cross {
350+
return errors.New("cannot set alias across different Image Streams")
351+
}
324352
}
325353

326354
return nil
327355
}
328356

329-
// RunTag contains all the necessary functionality for the OpenShift cli tag command.
330-
func (o TagOptions) RunTag() error {
357+
// Run contains all the necessary functionality for the OpenShift cli tag command.
358+
func (o TagOptions) Run() error {
331359
var tagReferencePolicy imageapi.TagReferencePolicyType
332360
switch o.referencePolicy {
333361
case sourceReferencePolicy:

‎pkg/cmd/cli/cmd/tag_test.go

+125-90
Original file line numberDiff line numberDiff line change
@@ -58,110 +58,145 @@ func testData() []*imageapi.ImageStream {
5858
},
5959
},
6060
},
61+
{
62+
ObjectMeta: api.ObjectMeta{Name: "django", Namespace: "yourproject", ResourceVersion: "11", CreationTimestamp: unversioned.Now()},
63+
Spec: imageapi.ImageStreamSpec{
64+
DockerImageRepository: "",
65+
Tags: map[string]imageapi.TagReference{
66+
"tip": {
67+
From: &api.ObjectReference{
68+
Name: "python",
69+
Namespace: "openshift",
70+
Kind: "ImageStreamTag",
71+
},
72+
},
73+
},
74+
},
75+
},
6176
}
6277
}
6378

64-
func TestRunTag_AddAccrossNamespaces(t *testing.T) {
79+
func TestTag(t *testing.T) {
6580
streams := testData()
66-
client := testclient.NewSimpleFake(streams[2], streams[0])
67-
client.PrependReactor("create", "imagestreamtags", func(action core.Action) (handled bool, ret runtime.Object, err error) {
68-
return true, nil, kapierrors.NewMethodNotSupported(imageapi.Resource("imagestreamtags"), "create")
69-
})
70-
client.PrependReactor("update", "imagestreamtags", func(action core.Action) (handled bool, ret runtime.Object, err error) {
71-
return true, nil, kapierrors.NewMethodNotSupported(imageapi.Resource("imagestreamtags"), "update")
72-
})
7381

74-
test := struct {
82+
testCases := map[string]struct {
83+
data []runtime.Object
7584
opts *TagOptions
7685
expectedActions []testAction
77-
expectedErr error
86+
validateErr string
87+
runErr string
7888
}{
79-
opts: &TagOptions{
80-
out: os.Stdout,
81-
osClient: client,
82-
ref: imageapi.DockerImageReference{
83-
Namespace: "openshift",
84-
Name: "ruby",
85-
Tag: "latest",
89+
"tag across namespaces": {
90+
data: []runtime.Object{streams[2], streams[0]},
91+
opts: &TagOptions{
92+
ref: imageapi.DockerImageReference{
93+
Namespace: "openshift",
94+
Name: "ruby",
95+
Tag: "latest",
96+
},
97+
referencePolicy: sourceReferencePolicy,
98+
namespace: "myproject2",
99+
sourceKind: "ImageStreamTag",
100+
destNamespace: []string{"yourproject"},
101+
destNameAndTag: []string{"rails:tip"},
102+
},
103+
expectedActions: []testAction{
104+
{verb: "update", resource: "imagestreamtags"},
105+
{verb: "create", resource: "imagestreamtags"},
106+
{verb: "get", resource: "imagestreams"},
107+
{verb: "update", resource: "imagestreams"},
86108
},
87-
namespace: "myproject2",
88-
sourceKind: "ImageStreamTag",
89-
destNamespace: []string{"yourproject"},
90-
destNameAndTag: []string{"rails:tip"},
91109
},
92-
expectedActions: []testAction{
93-
{verb: "update", resource: "imagestreamtags"},
94-
{verb: "create", resource: "imagestreamtags"},
95-
{verb: "get", resource: "imagestreams"},
96-
{verb: "update", resource: "imagestreams"},
110+
"alias tag across namespaces": {
111+
data: []runtime.Object{streams[2], streams[0]},
112+
opts: &TagOptions{
113+
ref: imageapi.DockerImageReference{
114+
Namespace: "openshift",
115+
Name: "ruby",
116+
Tag: "latest",
117+
},
118+
aliasTag: true,
119+
referencePolicy: sourceReferencePolicy,
120+
namespace: "myproject2",
121+
sourceKind: "ImageStreamTag",
122+
destNamespace: []string{"yourproject"},
123+
destNameAndTag: []string{"rails:tip"},
124+
},
125+
validateErr: "cannot set alias across different Image Streams",
97126
},
98-
expectedErr: nil,
99-
}
100-
101-
if err := test.opts.RunTag(); err != test.expectedErr {
102-
t.Fatalf("error mismatch: expected %v, got %v", test.expectedErr, err)
103-
}
104-
105-
got := client.Actions()
106-
if len(test.expectedActions) != len(got) {
107-
t.Fatalf("action length mismatch: expectedc %d, got %d", len(test.expectedActions), len(got))
108-
}
109-
110-
for i, action := range test.expectedActions {
111-
if !got[i].Matches(action.verb, action.resource) {
112-
t.Errorf("action mismatch: expected %s %s, got %s %s", action.verb, action.resource, got[i].GetVerb(), got[i].GetResource())
113-
}
114-
}
115-
}
116-
117-
func TestRunTag_AddOld(t *testing.T) {
118-
streams := testData()
119-
client := testclient.NewSimpleFake(streams[0])
120-
client.PrependReactor("create", "imagestreamtags", func(action core.Action) (handled bool, ret runtime.Object, err error) {
121-
return true, nil, kapierrors.NewMethodNotSupported(imageapi.Resource("imagestreamtags"), "create")
122-
})
123-
client.PrependReactor("update", "imagestreamtags", func(action core.Action) (handled bool, ret runtime.Object, err error) {
124-
return true, nil, kapierrors.NewMethodNotSupported(imageapi.Resource("imagestreamtags"), "update")
125-
})
126-
127-
test := struct {
128-
opts *TagOptions
129-
expectedActions []testAction
130-
expectedErr error
131-
}{
132-
opts: &TagOptions{
133-
out: os.Stdout,
134-
osClient: client,
135-
ref: imageapi.DockerImageReference{
136-
Namespace: "openshift",
137-
Name: "ruby",
138-
Tag: "2.0",
127+
"alias tag across image streams": {
128+
data: []runtime.Object{streams[3], streams[0]},
129+
opts: &TagOptions{
130+
ref: imageapi.DockerImageReference{
131+
Namespace: "yourproject",
132+
Name: "rails",
133+
Tag: "latest",
134+
},
135+
aliasTag: true,
136+
referencePolicy: sourceReferencePolicy,
137+
namespace: "myproject2",
138+
sourceKind: "ImageStreamTag",
139+
destNamespace: []string{"yourproject"},
140+
destNameAndTag: []string{"python:alias"},
139141
},
140-
sourceKind: "ImageStreamTag",
141-
destNamespace: []string{"yourproject"},
142-
destNameAndTag: []string{"rails:tip"},
142+
validateErr: "cannot set alias across different Image Streams",
143143
},
144-
expectedActions: []testAction{
145-
{verb: "update", resource: "imagestreamtags"},
146-
{verb: "create", resource: "imagestreamtags"},
147-
{verb: "get", resource: "imagestreams"},
148-
{verb: "update", resource: "imagestreams"},
144+
"add old": {
145+
data: []runtime.Object{streams[0]},
146+
opts: &TagOptions{
147+
ref: imageapi.DockerImageReference{
148+
Namespace: "openshift",
149+
Name: "ruby",
150+
Tag: "2.0",
151+
},
152+
referencePolicy: sourceReferencePolicy,
153+
sourceKind: "ImageStreamTag",
154+
destNamespace: []string{"yourproject"},
155+
destNameAndTag: []string{"rails:tip"},
156+
},
157+
expectedActions: []testAction{
158+
{verb: "update", resource: "imagestreamtags"},
159+
{verb: "create", resource: "imagestreamtags"},
160+
{verb: "get", resource: "imagestreams"},
161+
{verb: "update", resource: "imagestreams"},
162+
},
149163
},
150-
expectedErr: nil,
151164
}
152165

153-
if err := test.opts.RunTag(); err != test.expectedErr {
154-
t.Fatalf("error mismatch: expected %v, got %v", test.expectedErr, err)
155-
}
166+
for name, test := range testCases {
167+
client := testclient.NewSimpleFake(test.data...)
168+
client.PrependReactor("create", "imagestreamtags", func(action core.Action) (handled bool, ret runtime.Object, err error) {
169+
return true, nil, kapierrors.NewMethodNotSupported(imageapi.Resource("imagestreamtags"), "create")
170+
})
171+
client.PrependReactor("update", "imagestreamtags", func(action core.Action) (handled bool, ret runtime.Object, err error) {
172+
return true, nil, kapierrors.NewMethodNotSupported(imageapi.Resource("imagestreamtags"), "update")
173+
})
174+
175+
test.opts.out = os.Stdout
176+
test.opts.osClient = client
177+
178+
err := test.opts.Validate()
179+
if (err == nil && len(test.validateErr) != 0) || (err != nil && err.Error() != test.validateErr) {
180+
t.Errorf("%s: validation error mismatch: expected %v, got %v", name, test.validateErr, err)
181+
}
182+
if err != nil {
183+
continue
184+
}
156185

157-
got := client.Actions()
158-
if len(test.expectedActions) != len(got) {
159-
t.Fatalf("action length mismatch: expectedc %d, got %d", len(test.expectedActions), len(got))
160-
}
186+
err = test.opts.Run()
187+
if (err == nil && len(test.runErr) != 0) || (err != nil && err.Error() != test.runErr) {
188+
t.Errorf("%s: run error mismatch: expected %v, got %v", name, test.runErr, err)
189+
}
161190

162-
for i, action := range test.expectedActions {
163-
if !got[i].Matches(action.verb, action.resource) {
164-
t.Errorf("action mismatch: expected %s %s, got %s %s", action.verb, action.resource, got[i].GetVerb(), got[i].GetResource())
191+
got := client.Actions()
192+
if len(test.expectedActions) != len(got) {
193+
t.Fatalf("%s: action length mismatch: expected %d, got %d", name, len(test.expectedActions), len(got))
194+
}
195+
for i, action := range test.expectedActions {
196+
if !got[i].Matches(action.verb, action.resource) {
197+
t.Errorf("%s: [%o] action mismatch: expected %s %s, got %s %s",
198+
name, i, action.verb, action.resource, got[i].GetVerb(), got[i].GetResource())
199+
}
165200
}
166201
}
167202
}
@@ -199,7 +234,7 @@ func TestRunTag_DeleteOld(t *testing.T) {
199234
expectedErr: nil,
200235
}
201236

202-
if err := test.opts.RunTag(); err != test.expectedErr {
237+
if err := test.opts.Run(); err != test.expectedErr {
203238
t.Fatalf("error mismatch: expected %v, got %v", test.expectedErr, err)
204239
}
205240

@@ -245,7 +280,7 @@ func TestRunTag_AddNew(t *testing.T) {
245280
expectedErr: nil,
246281
}
247282

248-
if err := test.opts.RunTag(); err != test.expectedErr {
283+
if err := test.opts.Run(); err != test.expectedErr {
249284
t.Fatalf("error mismatch: expected %v, got %v", test.expectedErr, err)
250285
}
251286

@@ -294,7 +329,7 @@ func TestRunTag_AddRestricted(t *testing.T) {
294329
expectedErr: nil,
295330
}
296331

297-
if err := test.opts.RunTag(); err != test.expectedErr {
332+
if err := test.opts.Run(); err != test.expectedErr {
298333
t.Fatalf("error mismatch: expected %v, got %v", test.expectedErr, err)
299334
}
300335

@@ -334,7 +369,7 @@ func TestRunTag_DeleteNew(t *testing.T) {
334369
expectedErr: nil,
335370
}
336371

337-
if err := test.opts.RunTag(); err != test.expectedErr {
372+
if err := test.opts.Run(); err != test.expectedErr {
338373
t.Fatalf("error mismatch: expected %v, got %v", test.expectedErr, err)
339374
}
340375

0 commit comments

Comments
 (0)