8000 fix issue #12281 by dalanlan · Pull Request #12527 · moby/moby · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix issue #12281 #12527

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

fix issue #12281 #12527

wants to merge 1 commit into from

Conversation

dalanlan
Copy link
Contributor

Fix issue #12281
Reject the client pull request for special case scratch.
Signed-off-by: Simei He hesimei@zju.edu.cn

@dalanlan
fix issue 12281. Reject the cli pull request for scratch.
39ed0ac 
Signed-off-by: Simei He <hesimei@zju.edu.cn>
@dalanlan
Copy link
Contributor Author

@thaJeztah @dmcgowan

@dmcgowan dmcgowan self-assigned this Apr 20, 2015
@dmcgowan
Copy link
Member

Setting this to design review to discuss whether we want to do this. The code looks good though.

@mmdriley
Copy link
Contributor

Really seems like this should use validateRepositoryName rather than duplicate that logic.

https://github.com/docker/docker/blob/fe53c277857ba77caf9189b1d6d095835c1b792c/registry/config.go#L250

@dalanlan
Copy link
Contributor Author

Thx @mmdriley
I thought about that before, however considering the function you mentioned is called in multiple cases build,commit,import,etc, i wouldnt regard it as an appropriate solution to go that deep.

@mmdriley
Copy link
Contributor

I suggest using validateRepositoryName precisely because it's so commonly used. That's no accident -- it was a lot of work to centralize repository name parsing and validation.

jlhawn
jlhawn reviewed Apr 22, 2015
View reviewed changes
graph/pull.go
@@ -40,6 +59,10 @@ func (s *TagStore) Pull(image string, tag string, imagePullConfig *ImagePullConf
return err
}

if err := validateRepoNotScratch(repoInfo.RemoteName); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

please use validateRepoName(repoInfo.LocalName) instead. Defining a separate validateRepoNotScratch() function is not necessary.

@jlhawn
Copy link
Contributor
jlhawn commented Apr 22, 2015

needs rebase

@jlhawn jlhawn mentioned this pull request Apr 22, 2015
Merged
@jlhawn
Copy link
Contributor
jlhawn commented Apr 22, 2015

@dalanlan I'm not sure if you are available to update this PR, so I've quickly submitted another here: #12655

@jessfraz
Copy link
Contributor

closing as there is a new pr #12655

@jessfraz jessfraz closed this Apr 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@dmcgowan dmcgowan

Labels
status/1-design-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dalanlan @dmcgowan @mmdriley @jlhawn @jessfraz @GordonTheTurtle
0