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

Docker image name incorrectly identified as registry in FROM #59

Closed
daimor opened this issue Oct 11, 2019 · 5 comments
Closed

Docker image name incorrectly identified as registry in FROM #59

daimor opened this issue Oct 11, 2019 · 5 comments
Assignees
Labels

Comments

@daimor
Copy link

daimor commented Oct 11, 2019

Docker hub has images from verified publishers, and such image names starts with store/
For example
store/intersystems/iris-community link
store/intersystems/irishealth link
store/splunk/universalforwarder link
on so on

At the moment parser recognizes store as registry but in this case it is just a part of image name

To be solved for microsoft/vscode-docker#1337

@rcjsuen
Copy link
Owner

rcjsuen commented Oct 11, 2019

@daimor This opens up an interesting can of worms as you could have any arbitrary word in front of the image's name and have that be a Docker registry given your local DNS settings or whatever. I wonder how Docker implements this on their end. Could they be hardcoding the word store in their handling of Dockerfiles?

@daimor
Copy link
Author

daimor commented Oct 11, 2019

Well, I found this code, which used by docker-cli

// splitDockerDomain splits a repository name to domain and remotename string.
// If no valid domain is found, the default domain is used. Repository name
// needs to be already validated before.
func splitDockerDomain(name string) (domain, remainder string) {
	i := strings.IndexRune(name, '/')
	if i == -1 || (!strings.ContainsAny(name[:i], ".:") && name[:i] != "localhost") {
		domain, remainder = defaultDomain, name
	} else {
		domain, remainder = name[:i], name[i+1:]
	}
	if domain == legacyDefaultDomain {
		domain = defaultDomain
	}
	if domain == defaultDomain && !strings.ContainsRune(remainder, '/') {
		remainder = officialRepoName + "/" + remainder
	}
	return
}

So, it actually means, that localhost is hardcoded, and any domain name should contain dots . or colon :
if no localhost and does not contains ., : it is just image name.

@rcjsuen
Copy link
Owner

rcjsuen commented Oct 11, 2019

@daimor Well then. You learn something new every day as I always like to say...

Thanks for digging this up!

@rcjsuen rcjsuen self-assigned this Oct 12, 2019
@rcjsuen rcjsuen added the bug label Oct 12, 2019
@rcjsuen
Copy link
Owner

rcjsuen commented Oct 12, 2019

import { DockerfileParser, From } from './main';
const dockerfile = DockerfileParser.parse("FROM store/base/image");
const from = (dockerfile.getInstructions()[0]) as From;
console.log(from.getImageName());
base/image

We should get store/base/image instead.

@rcjsuen rcjsuen changed the title Docker images from verified publishers should be correctly recognized Docker image name incorrectly identified as registry in FROM Oct 12, 2019
rcjsuen added a commit that referenced this issue Oct 12, 2019
Docker images that did not include a hostname were incorrectly being
flagged as having one.

Signed-off-by: Remy Suen <remy.suen@gmail.com>
@rcjsuen
Copy link
Owner

rcjsuen commented Oct 12, 2019

@daimor Thanks for the bug report. Have a great weekend!

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

No branches or pull requests

2 participants