Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1435588 - Forbid creating aliases across different Image Streams. #13632

Merged
merged 1 commit into from
Apr 6, 2017
Merged

Bug 1435588 - Forbid creating aliases across different Image Streams. #13632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 34 additions & 6 deletions pkg/cmd/cli/cmd/tag.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,13 @@ func NewCmdTag(fullName string, f *clientcmd.Factory, out io.Writer) *cobra.Comm
Run: func(cmd *cobra.Command, args []string) {
kcmdutil.CheckErr(opts.Complete(f, cmd, args, out))
kcmdutil.CheckErr(opts.Validate())
kcmdutil.CheckErr(opts.RunTag())
kcmdutil.CheckErr(opts.Run())
},
}

cmd.Flags().StringVar(&opts.sourceKind, "source", opts.sourceKind, "Optional hint for the source type; valid values are 'imagestreamtag', 'istag', 'imagestreamimage', 'isimage', and 'docker'.")
cmd.Flags().BoolVarP(&opts.deleteTag, "delete", "d", opts.deleteTag, "Delete the provided spec tags.")
cmd.Flags().BoolVar(&opts.aliasTag, "alias", false, "Should the destination tag be updated whenever the source tag changes. Defaults to false.")
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.")
cmd.Flags().BoolVar(&opts.referenceTag, "reference", false, "Should the destination tag continue to pull from the source namespace. Defaults to false.")
cmd.Flags().BoolVar(&opts.scheduleTag, "scheduled", false, "Set a Docker image to be periodically imported from a remote repository. Defaults to false.")
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.")
Expand Down Expand Up @@ -271,6 +271,25 @@ func (o *TagOptions) Complete(f *clientcmd.Factory, cmd *cobra.Command, args []s
return nil
}

// isCrossImageStream verifies if destination is the same image stream as source. Returns true
// if any of the destination image stream is different and error from parsing
// image stream tag.
func isCrossImageStream(namespace string, srcRef imageapi.DockerImageReference, destNamespace []string, destNameAndTag []string) (bool, error) {
for i, ns := range destNamespace {
if namespace != ns {
return true, nil
}
name, _, ok := imageapi.SplitImageStreamTag(destNameAndTag[i])
if !ok {
return false, fmt.Errorf("%q must be of the form <stream_name>:<tag>", destNameAndTag[i])
}
if srcRef.Name != name {
return true, nil
}
}
return false, nil
}

// Validate validates all the required options for the tag command.
func (o TagOptions) Validate() error {
// Validate client and writer
Expand Down Expand Up @@ -319,15 +338,24 @@ func (o TagOptions) Validate() error {
if o.sourceKind != "DockerImage" && (o.scheduleTag || o.insecureTag) {
return errors.New("only Docker images can have importing flags set")
}
if o.aliasTag && (o.scheduleTag || o.insecureTag) {
return errors.New("cannot set a Docker image tag as an alias and also set import flags")
if o.aliasTag {
if o.scheduleTag || o.insecureTag {
return errors.New("cannot set a Docker image tag as an alias and also set import flags")
}
cross, err := isCrossImageStream(o.namespace, o.ref, o.destNamespace, o.destNameAndTag)
if err != nil {
return err
}
if cross {
return errors.New("cannot set alias across different Image Streams")
}
}

return nil
}

// RunTag contains all the necessary functionality for the OpenShift cli tag command.
func (o TagOptions) RunTag() error {
// Run contains all the necessary functionality for the OpenShift cli tag command.
func (o TagOptions) Run() error {
var tagReferencePolicy imageapi.TagReferencePolicyType
switch o.referencePolicy {
case sourceReferencePolicy:
Expand Down
215 changes: 125 additions & 90 deletions pkg/cmd/cli/cmd/tag_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,110 +58,145 @@ func testData() []*imageapi.ImageStream {
},
},
},
{
ObjectMeta: api.ObjectMeta{Name: "django", Namespace: "yourproject", ResourceVersion: "11", CreationTimestamp: unversioned.Now()},
Spec: imageapi.ImageStreamSpec{
DockerImageRepository: "",
Tags: map[string]imageapi.TagReference{
"tip": {
From: &api.ObjectReference{
Name: "python",
Namespace: "openshift",
Kind: "ImageStreamTag",
},
},
},
},
},
}
}

func TestRunTag_AddAccrossNamespaces(t *testing.T) {
func TestTag(t *testing.T) {
streams := testData()
client := testclient.NewSimpleFake(streams[2], streams[0])
client.PrependReactor("create", "imagestreamtags", func(action core.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, kapierrors.NewMethodNotSupported(imageapi.Resource("imagestreamtags"), "create")
})
client.PrependReactor("update", "imagestreamtags", func(action core.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, kapierrors.NewMethodNotSupported(imageapi.Resource("imagestreamtags"), "update")
})

test := struct {
testCases := map[string]struct {
data []runtime.Object
opts *TagOptions
expectedActions []testAction
expectedErr error
validateErr string
runErr string
}{
opts: &TagOptions{
out: os.Stdout,
osClient: client,
ref: imageapi.DockerImageReference{
Namespace: "openshift",
Name: "ruby",
Tag: "latest",
"tag across namespaces": {
data: []runtime.Object{streams[2], streams[0]},
opts: &TagOptions{
ref: imageapi.DockerImageReference{
Namespace: "openshift",
Name: "ruby",
Tag: "latest",
},
referencePolicy: sourceReferencePolicy,
namespace: "myproject2",
sourceKind: "ImageStreamTag",
destNamespace: []string{"yourproject"},
destNameAndTag: []string{"rails:tip"},
},
expectedActions: []testAction{
{verb: "update", resource: "imagestreamtags"},
{verb: "create", resource: "imagestreamtags"},
{verb: "get", resource: "imagestreams"},
{verb: "update", resource: "imagestreams"},
},
namespace: "myproject2",
sourceKind: "ImageStreamTag",
destNamespace: []string{"yourproject"},
destNameAndTag: []string{"rails:tip"},
},
expectedActions: []testAction{
{verb: "update", resource: "imagestreamtags"},
{verb: "create", resource: "imagestreamtags"},
{verb: "get", resource: "imagestreams"},
{verb: "update", resource: "imagestreams"},
"alias tag across namespaces": {
data: []runtime.Object{streams[2], streams[0]},
opts: &TagOptions{
ref: imageapi.DockerImageReference{
Namespace: "openshift",
Name: "ruby",
Tag: "latest",
},
aliasTag: true,
referencePolicy: sourceReferencePolicy,
namespace: "myproject2",
sourceKind: "ImageStreamTag",
destNamespace: []string{"yourproject"},
destNameAndTag: []string{"rails:tip"},
},
validateErr: "cannot set alias across different Image Streams",
},
expectedErr: nil,
}

if err := test.opts.RunTag(); err != test.expectedErr {
t.Fatalf("error mismatch: expected %v, got %v", test.expectedErr, err)
}

got := client.Actions()
if len(test.expectedActions) != len(got) {
t.Fatalf("action length mismatch: expectedc %d, got %d", len(test.expectedActions), len(got))
}

for i, action := range test.expectedActions {
if !got[i].Matches(action.verb, action.resource) {
t.Errorf("action mismatch: expected %s %s, got %s %s", action.verb, action.resource, got[i].GetVerb(), got[i].GetResource())
}
}
}

func TestRunTag_AddOld(t *testing.T) {
streams := testData()
client := testclient.NewSimpleFake(streams[0])
client.PrependReactor("create", "imagestreamtags", func(action core.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, kapierrors.NewMethodNotSupported(imageapi.Resource("imagestreamtags"), "create")
})
client.PrependReactor("update", "imagestreamtags", func(action core.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, kapierrors.NewMethodNotSupported(imageapi.Resource("imagestreamtags"), "update")
})

test := struct {
opts *TagOptions
expectedActions []testAction
expectedErr error
}{
opts: &TagOptions{
out: os.Stdout,
osClient: client,
ref: imageapi.DockerImageReference{
Namespace: "openshift",
Name: "ruby",
Tag: "2.0",
"alias tag across image streams": {
data: []runtime.Object{streams[3], streams[0]},
opts: &TagOptions{
ref: imageapi.DockerImageReference{
Namespace: "yourproject",
Name: "rails",
Tag: "latest",
},
aliasTag: true,
referencePolicy: sourceReferencePolicy,
namespace: "myproject2",
sourceKind: "ImageStreamTag",
destNamespace: []string{"yourproject"},
destNameAndTag: []string{"python:alias"},
},
sourceKind: "ImageStreamTag",
destNamespace: []string{"yourproject"},
destNameAndTag: []string{"rails:tip"},
validateErr: "cannot set alias across different Image Streams",
},
expectedActions: []testAction{
{verb: "update", resource: "imagestreamtags"},
{verb: "create", resource: "imagestreamtags"},
{verb: "get", resource: "imagestreams"},
{verb: "update", resource: "imagestreams"},
"add old": {
data: []runtime.Object{streams[0]},
opts: &TagOptions{
ref: imageapi.DockerImageReference{
Namespace: "openshift",
Name: "ruby",
Tag: "2.0",
},
referencePolicy: sourceReferencePolicy,
sourceKind: "ImageStreamTag",
destNamespace: []string{"yourproject"},
destNameAndTag: []string{"rails:tip"},
},
expectedActions: []testAction{
{verb: "update", resource: "imagestreamtags"},
{verb: "create", resource: "imagestreamtags"},
{verb: "get", resource: "imagestreams"},
{verb: "update", resource: "imagestreams"},
},
},
expectedErr: nil,
}

if err := test.opts.RunTag(); err != test.expectedErr {
t.Fatalf("error mismatch: expected %v, got %v", test.expectedErr, err)
}
for name, test := range testCases {
client := testclient.NewSimpleFake(test.data...)
client.PrependReactor("create", "imagestreamtags", func(action core.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, kapierrors.NewMethodNotSupported(imageapi.Resource("imagestreamtags"), "create")
})
client.PrependReactor("update", "imagestreamtags", func(action core.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, kapierrors.NewMethodNotSupported(imageapi.Resource("imagestreamtags"), "update")
})

test.opts.out = os.Stdout
test.opts.osClient = client

err := test.opts.Validate()
if (err == nil && len(test.validateErr) != 0) || (err != nil && err.Error() != test.validateErr) {
t.Errorf("%s: validation error mismatch: expected %v, got %v", name, test.validateErr, err)
}
if err != nil {
continue
}

got := client.Actions()
if len(test.expectedActions) != len(got) {
t.Fatalf("action length mismatch: expectedc %d, got %d", len(test.expectedActions), len(got))
}
err = test.opts.Run()
if (err == nil && len(test.runErr) != 0) || (err != nil && err.Error() != test.runErr) {
t.Errorf("%s: run error mismatch: expected %v, got %v", name, test.runErr, err)
}

for i, action := range test.expectedActions {
if !got[i].Matches(action.verb, action.resource) {
t.Errorf("action mismatch: expected %s %s, got %s %s", action.verb, action.resource, got[i].GetVerb(), got[i].GetResource())
got := client.Actions()
if len(test.expectedActions) != len(got) {
t.Fatalf("%s: action length mismatch: expected %d, got %d", name, len(test.expectedActions), len(got))
}
for i, action := range test.expectedActions {
if !got[i].Matches(action.verb, action.resource) {
t.Errorf("%s: [%o] action mismatch: expected %s %s, got %s %s",
name, i, action.verb, action.resource, got[i].GetVerb(), got[i].GetResource())
}
}
}
}
Expand Down Expand Up @@ -199,7 +234,7 @@ func TestRunTag_DeleteOld(t *testing.T) {
expectedErr: nil,
}

if err := test.opts.RunTag(); err != test.expectedErr {
if err := test.opts.Run(); err != test.expectedErr {
t.Fatalf("error mismatch: expected %v, got %v", test.expectedErr, err)
}

Expand Down Expand Up @@ -245,7 +280,7 @@ func TestRunTag_AddNew(t *testing.T) {
expectedErr: nil,
}

if err := test.opts.RunTag(); err != test.expectedErr {
if err := test.opts.Run(); err != test.expectedErr {
t.Fatalf("error mismatch: expected %v, got %v", test.expectedErr, err)
}

Expand Down Expand Up @@ -294,7 +329,7 @@ func TestRunTag_AddRestricted(t *testing.T) {
expectedErr: nil,
}

if err := test.opts.RunTag(); err != test.expectedErr {
if err := test.opts.Run(); err != test.expectedErr {
t.Fatalf("error mismatch: expected %v, got %v", test.expectedErr, err)
}

Expand Down Expand Up @@ -334,7 +369,7 @@ func TestRunTag_DeleteNew(t *testing.T) {
expectedErr: nil,
}

if err := test.opts.RunTag(); err != test.expectedErr {
if err := test.opts.Run(); err != test.expectedErr {
t.Fatalf("error mismatch: expected %v, got %v", test.expectedErr, err)
}

Expand Down
Loading