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.

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

Conversation

soltysh
Copy link
Contributor

@soltysh soltysh commented Apr 5, 2017

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1435588.

As mentioned in #13595 (comment) this PR forbids creating aliases across different Image Streams.

@smarterclayton @mfojtik ptal

@soltysh
Copy link
Contributor Author

soltysh commented Apr 5, 2017

[test]

},
}

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.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should explain that is only for tags like :latest or :stable, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do a doc pass after this merges and will explain in details our intentions there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't capitalize image stream

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@@ -271,6 +271,25 @@ func (o *TagOptions) Complete(f *clientcmd.Factory, cmd *cobra.Command, args []s
return nil
}

// Verifies if destination is the same image stream as source. Returns true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isCrossImageStream verifies

@mfojtik
Copy link
Contributor

mfojtik commented Apr 5, 2017

LGTM

lets wait for the test pass

@soltysh
Copy link
Contributor Author

soltysh commented Apr 5, 2017

Previous failure was #13067.

@soltysh
Copy link
Contributor Author

soltysh commented Apr 5, 2017

Comments addressed, I don't see any objections so merging as is.

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to b3b252a

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to b3b252a

@smarterclayton
Copy link
Contributor

smarterclayton commented Apr 5, 2017 via email

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/583/) (Base Commit: 801b564)

@openshift-bot
Copy link
Contributor

openshift-bot commented Apr 5, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/261/) (Base Commit: fdd7cc1) (Image: devenv-rhel7_6119)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants