ParseDockerImageReference use docker/distribution reference parser #11245

legionus · 2016-10-06T13:21:35Z

We can't remove ParseDockerImageReference and just use docker/distribution/reference
to parse DockerImageReference because we have to support few special cases related
to docker.io. Keeping it in mind, we can reimplement ParseDockerImageReference
to use new parser.

In addition we got new limits on the length of the DockerImageReference.
So, after refactoring our restrictions are pretty the same as in docker/distribution.

Fix #11211

@ncdc @miminar please review.

legionus · 2016-10-06T13:26:13Z

[test]

legionus · 2016-10-06T14:15:20Z

This parser have a few issues:

it tries to apply DockerDefaultNamespace, but not DockerDefaultRegistry. We already have DockerClientDefaults function for that reason. In my opinion, the parser must either apply all default values or not to use them at all. But problem is that if we change ParseDockerImageReference, then we will break backward compatibility.
we need to move parser with our special black magic to separate module to able use it in S2I. They also parse DockerImageReference, but doing it the other (not fully compatible) way.

miminar · 2016-10-06T13:56:23Z

pkg/image/api/helper.go

+		ref.Registry, ref.Name = reference.SplitHostname(named)
+	} else if named, ok := dockerRef.(reference.Tagged); ok {
+		ref.Tag = named.Tag()
+	} else if named, ok := dockerRef.(reference.Digested); ok {


I believe Tagged and Digested can be considered invalid.

OK, but in this case type switch no longer needed.

Ignore my previous comment. They are valid references.

miminar · 2016-10-06T14:08:24Z

pkg/image/api/helper.go

+
+	repoParts := strings.Split(ref.Name, "/")
+
+	if !isRegistryName(ref.Registry) && len(repoParts) == 1 {


A note for isRegistryName(): use strings.ContainsAny. Also document that it needs to be called with just one path component (no slash allowed).

You forget about localhost. So I think this function useful here.

It is. This is a suggestion to modify the isRegistryName() function itself and annotate it with a godoc.

miminar · 2016-10-06T14:09:34Z

pkg/image/api/helper.go

+		ref.Registry = ""
+	}
+
+	if ref.Namespace == "" && len(repoParts) >= 2 {


nit: len(ref.Namespace) == 0

Make sure repoParts[0] doesn't look like a registry?

miminar · 2016-10-06T14:12:55Z

pkg/image/api/helper.go

+		ref.Namespace, ref.Name = repoParts[0], strings.Join(repoParts[1:], "/")
+	}
+
+	if ref.Namespace == "" && IsRegistryDockerHub(ref.Registry) {


nit: len(ref.Namespace) == 0

miminar · 2016-10-06T14:23:48Z

pkg/image/api/helper_test.go

+			Tag:       "tag",
+		},
+		{
+			// docker.io/namespace/name == 255 chars without implicit namespace


s/implicit/explicit/ ?

miminar · 2016-10-06T14:32:19Z

pkg/image/api/validation/validation.go

@@ -196,7 +196,9 @@ func ValidateImageStreamTagReference(tagRef api.TagReference, fldPath *field.Pat
 		}
 		switch tagRef.From.Kind {
 		case "DockerImage":
-			if ref, err := api.ParseDockerImageReference(tagRef.From.Name); err == nil && tagRef.ImportPolicy.Scheduled && len(ref.ID) > 0 {
+			if ref, err := api.ParseDockerImageReference(tagRef.From.Name); err != nil {


... && len(tagRef.From.Name) > 0, which is already covered above.

miminar · 2016-10-06T14:47:15Z

I'd rather do it right than stick with the old behavior. Another example is parsing of bar/foo/baz resulting in Registry == "bar". That's wrong. Especially now that we support multiple slashes in repository name.

miminar · 2016-10-06T14:52:36Z

pkg/image/api/helper_test.go

@@ -297,6 +298,10 @@ func TestParseDockerImageReference(t *testing.T) {
 			Err:  true,
 		},
 		{
+			From: "abc@badid",


Could you add From: "@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", From: "sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25" and From: ":tag"?

legionus · 2016-10-06T15:04:46Z

I'd rather do it right than stick with the old behavior. Another example is parsing of bar/foo/baz resulting in Registry == "bar". That's wrong. Especially now that we support multiple slashes in repository name.

@miminar This behavior right according reference.Parse(). Why it's not right ?

miminar · 2016-10-06T15:17:17Z

This behavior right according reference.Parse(). Why it's not right ?

package main
import ("fmt"; "github.com/docker/docker/reference")
func main() {
    ref, err := reference.ParseNamed("bar/foo/baz")
    if err != nil {
        fmt.Println("failed to parse reference: %v", err)
    }
    fmt.Printf("hostname=%q, remotename=%q\n", ref.Hostname(), ref.RemoteName())
}

$ go run above-script.go
hostname="docker.io", remotename="bar/foo/baz"

Which makes more sense to me.

Although if you replace "github.com/docker/docker/reference" with "github.com/docker/distribution/reference", and use reference.SplitHostname(), you'll end up with bar as a hostname and foo/baz as a name.

legionus · 2016-10-07T09:20:23Z

Although if you replace "github.com/docker/docker/reference" with "github.com/docker/distribution/reference", and use reference.SplitHostname(), you'll end up with bar as a hostname and foo/baz as a name.

@miminar Using github.com/docker/docker/reference will cost us a bunch of dependencies:

github.com/docker/docker/daemon/graphdriver
github.com/docker/docker/image
github.com/docker/docker/image/v1
github.com/docker/docker/layer
github.com/docker/docker/pkg/archive
github.com/docker/docker/pkg/chrootarchive
github.com/docker/docker/pkg/fileutils
github.com/docker/docker/pkg/idtools
github.com/docker/docker/pkg/ioutils
github.com/docker/docker/pkg/plugins
github.com/docker/docker/pkg/plugins/transport
github.com/docker/docker/pkg/pools
github.com/docker/docker/pkg/promise
github.com/docker/docker/pkg/random
github.com/docker/docker/pkg/reexec
github.com/docker/docker/pkg/stringid
github.com/docker/docker/pkg/version
github.com/docker/docker/reference
github.com/vbatts/tar-split/archive/tar
github.com/vbatts/tar-split/tar/asm
github.com/vbatts/tar-split/tar/storage

I'm not sure we want it.

mfojtik · 2016-10-07T09:26:36Z

@miminar i don't like vendoring 15 more deps just to get this helper, not saying that we can't even use the helper as it is as we need to get the namespace still..

miminar · 2016-10-07T09:44:25Z

@legionus, @mfojtik, yeah, that's too many. Let's stick with docker/distribution/reference

legionus · 2016-10-07T09:45:59Z

I took the function to separate the host name and repository from the docker.

--- FAIL: TestParseDockerImageReference (0.00s)
    helper_test.go:341: foo: registry: expected "", got "docker.io"
    helper_test.go:344: foo: namespace: expected "", got "library"
    helper_test.go:341: foo:tag: registry: expected "", got "docker.io"
    helper_test.go:344: foo:tag: namespace: expected "", got "library"
    helper_test.go:341: foo@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25: registry: expected "", got "docker.io"
    helper_test.go:344: foo@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25: namespace: expected "", got "library"
    helper_test.go:341: bar/foo: registry: expected "", got "docker.io"
    helper_test.go:341: bar/foo:tag: registry: expected "", got "docker.io"
    helper_test.go:341: bar/foo@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25: registry: expected "", got "docker.io"
    helper_test.go:341: bar/foo/baz: registry: expected "bar", got "docker.io"
    helper_test.go:344: bar/foo/baz: namespace: expected "foo", got "bar"
    helper_test.go:347: bar/foo/baz: name: expected "baz", got "foo/baz"
    helper_test.go:341: bar/library/baz: registry: expected "bar", got "docker.io"
    helper_test.go:344: bar/library/baz: namespace: expected "library", got "bar"
    helper_test.go:347: bar/library/baz: name: expected "baz", got "library/baz"
    helper_test.go:341: bar/foo/baz:tag: registry: expected "bar", got "docker.io"
    helper_test.go:344: bar/foo/baz:tag: namespace: expected "foo", got "bar"
    helper_test.go:347: bar/foo/baz:tag: name: expected "baz", got "foo/baz"
    helper_test.go:341: bar/foo/baz@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25: registry: expected "bar", got "docker.io"
    helper_test.go:344: bar/foo/baz@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25: namespace: expected "foo", got "bar"
    helper_test.go:347: bar/foo/baz@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25: name: expected "baz", got "foo/baz"
    helper_test.go:341: index.docker.io/bar: registry: expected "index.docker.io", got "docker.io"
    helper_test.go:341: index.docker.io/library/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb:tag: registry: expected "index.docker.io", got "docker.io"
    helper_test.go:341: index.docker.io/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb:tag: registry: expected "index.docker.io", got "docker.io"
    helper_test.go:335: index.docker.io/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb:tag: unexpected non-error
    helper_test.go:341: bar/foo/baz/biz: registry: expected "bar", got "docker.io"
    helper_test.go:344: bar/foo/baz/biz: namespace: expected "foo", got "bar"
    helper_test.go:347: bar/foo/baz/biz: name: expected "baz/biz", got "foo/baz/biz"
FAIL

legionus · 2016-10-07T09:50:50Z

@miminar I think this is unacceptable as well. we will break everyone.

miminar · 2016-10-07T10:16:09Z

I think this is unacceptable as well. we will break everyone.

By everyone you mean us? 😄

This PR attempts to get the parsing right. Different output for the same input is expected if the previous parsing was wrong. We just need to make sure that everything works as expected.

I wouldn't default to docker.io for empty registry. Docker image reference without a registry is still a valid reference. We have DockerClientDefaults() for defaulting to docker.io.

legionus · 2016-10-07T10:27:30Z

This PR attempts to get the parsing right. Different output for the same input is expected if the previous parsing was wrong. We just need to make sure that everything works as expected.

OK :)

I wouldn't default to docker.io for empty registry. Docker image reference without a registry is still a valid reference. We have DockerClientDefaults() for defaulting to docker.io.

If we do not want to use docker.io for empty registry then we shouldn't use library as default namespace and leave it empty for same reason (we have DockerClientDefaults()).

miminar · 2016-10-07T11:38:34Z

pkg/image/api/helper.go

+	repoParts := strings.Split(ref.Name, "/")
+
+	if len(repoParts) >= 2 {
+		if isRegistryName(repoParts[0]) {


Is this even possible to satisfy? It is with localhost in name (e.g. registry.com/app/localhost/image). But it looks to me like a valid reference.

miminar

LGTM. Just a note: the pkg/image/api/utils modules is intended to be imported in openshift/source-to-image to use the same parser and avoid duplicating the same hacks in different way.

@ncdc would you take a look as well?

miminar · 2016-10-07T13:00:15Z

pkg/image/api/helper.go

-			break
-		}
-		return ref, fmt.Errorf("the docker pull spec %q must be two or three segments separated by slashes", spec)
+	// It's not enough just to use the reference.ParseNamed(). We have the fill


nit: s/We have the/We have to/

Yes, typo. I'll fix it.

ncdc · 2016-10-07T13:21:07Z

pkg/image/utils/utils.go

@@ -0,0 +1,53 @@
+package utils


In kube we try to avoid generic "utils" packages. Not a deal breaker, but I'd recommend a more specific package name.

github.com/openshift/origin/pkg/image/reference ?

ncdc · 2016-10-07T14:34:58Z

pkg/image/api/validation/validation_test.go

@@ -488,7 +488,7 @@ func TestValidateImageStream(t *testing.T) {
 				},
 			},
 			expected: field.ErrorList{
-				field.Invalid(field.NewPath("spec", "tags").Key("badid").Child("from", "name"), "abc@badid", "only tags can be scheduled for import"),


It looks like we lost a test case where we validate that you can't specify a digest for import where the error returned is "only tags can be scheduled for import"

"abc@badid" was always wrong, but previously the error was ignored.
what do you mean "we lost" ?

There is no test case that validates it receives "only tags can be scheduled for import" as an error any more. Could you please add a test case that tries to schedule a valid pull-by-digest spec?

@ncdc Fixed

ncdc · 2016-10-07T14:36:15Z

pkg/image/utils/utils.go

+		return ref, err
+	}
+
+	if named, ok := dockerRef.(reference.Named); ok {


Doesn't reference.ParseNamed return a reference.Named?

Good catch! thanks!

ncdc · 2016-10-07T14:40:43Z

pkg/image/utils/utils_test.go

+			Tag:       "tag",
+		},
+		{
+			// docker.io/namespace/name == 255 chars without explicit namespace


s/without/with

legionus · 2016-10-11T10:19:22Z

@ncdc please review again.

ncdc · 2016-10-11T11:51:03Z

It sure if I'll have time to review today. Feel free to find another
reviewer if needed.

On Tuesday, October 11, 2016, Alexey Gladkov [email protected]
wrote:

@ncdc https://github.com/ncdc please review again.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11245 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAABYnjlCnFiwLxFPyxYkAfxPfVl43xrks5qy2KzgaJpZM4KP7r-
.

legionus · 2016-10-11T11:58:51Z

It sure if I'll have time to review today. Feel free to find another reviewer if needed.

@ncdc Then please take off lock on this PR.

legionus · 2016-10-11T12:00:15Z

@liggitt @smarterclayton please review

ncdc

Did another pass

ncdc · 2016-10-11T12:17:17Z

pkg/build/graph/analysis/bc.go

@@ -346,6 +347,8 @@ func validImageStreamImage(imageNode *imagegraph.ImageStreamImageNode, imageStre
 			}
 		}
 		return false, dockerImageReference.ID
+	} else {
+		fmt.Fprintf(os.Stderr, "ERR(%s): %#+v\n", imageNode.Name, err)


Is this for debugging purposes? If not, maybe something a bit gentler than "ERR" would be better?

@ncdc I forgot to remove debugging. Fixed.

ncdc · 2016-10-11T12:23:55Z

pkg/image/reference/reference.go

+)
+
+// NamedDockerImageReference points to a Docker image.
+type NamedDockerImageReference struct {


This type is identical in everything except name to DockerImageReference in pkg/image/api/types.go. And ParseNamedDockerImageReference below is effectively wrapped by pkg/image/api/helpers.go#ParseDockerImageReference. Can we collapse the two types and functions into just 1 type and 1 function?

I split it to able use this parser in S2I as mentioned in #11245 (comment). S2I can't use ParseDockerImageReference directly because origin imports it. To avoid differences in the parser behavior we need to use one parser everywhere. So I move it into separate module with minimal dependencies.

mfojtik · 2016-10-12T09:52:22Z

flake: #8571

We can't remove ParseDockerImageReference and just use docker/distribution/reference to parse DockerImageReference because we have to support few special cases related to docker.io. Keeping it in mind, we can reimplement ParseDockerImageReference to use new parser. In addition we got new limits on the length of the DockerImageReference. So, after refactoring our restrictions are pretty the same as in docker/distribution. Signed-off-by: Gladkov Alexey <[email protected]>

openshift-bot · 2016-10-13T15:02:29Z

Evaluated for origin test up to caa9083

openshift-bot · 2016-10-13T16:28:41Z

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10015/) (Base Commit: 6b0457c)

legionus · 2016-10-13T17:13:18Z

@mfojtik merge ?

mfojtik · 2016-10-13T18:28:46Z

[merge]

Michal Fojtik

On 13 October 2016 at 19:13:19, Alexey Gladkov ([email protected])
wrote:

@mfojtik https://github.com/mfojtik merge ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11245 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AACsaHG9UWTQLfJW8fnIgawCIjyL8RgSks5qzmavgaJpZM4KP7r-
.

openshift-bot · 2016-10-13T18:32:35Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10015/) (Image: devenv-rhel7_5179)

openshift-bot · 2016-10-13T18:32:35Z

Evaluated for origin merge up to caa9083

legionus self-assigned this Oct 6, 2016

legionus added the component/imageregistry label Oct 6, 2016

miminar suggested changes Oct 6, 2016

View reviewed changes

miminar reviewed Oct 6, 2016

View reviewed changes

legionus force-pushed the refactor-parse-dockerimagereference branch from a1b8c4b to e3afe0b Compare October 7, 2016 09:44

legionus force-pushed the refactor-parse-dockerimagereference branch from e3afe0b to 2ea0593 Compare October 7, 2016 10:40

miminar suggested changes Oct 7, 2016

View reviewed changes

legionus force-pushed the refactor-parse-dockerimagereference branch 6 times, most recently from 0613aae to d9b7feb Compare October 7, 2016 13:25

miminar approved these changes Oct 7, 2016

View reviewed changes

ncdc suggested changes Oct 7, 2016

View reviewed changes

legionus force-pushed the refactor-parse-dockerimagereference branch 3 times, most recently from 78c9a44 to 148e83a Compare October 7, 2016 15:41

legionus force-pushed the refactor-parse-dockerimagereference branch 4 times, most recently from bf5259e to 8b073e8 Compare October 11, 2016 10:18

legionus force-pushed the refactor-parse-dockerimagereference branch from 8b073e8 to a9c2463 Compare October 11, 2016 11:34

legionus force-pushed the refactor-parse-dockerimagereference branch from a9c2463 to e9843b1 Compare October 11, 2016 12:16

ncdc suggested changes Oct 11, 2016

View reviewed changes

legionus force-pushed the refactor-parse-dockerimagereference branch from e9843b1 to 0cf14aa Compare October 11, 2016 12:34

ncdc approved these changes Oct 11, 2016

View reviewed changes

mfojtik added the lgtm Indicates that a PR is ready to be merged. label Oct 12, 2016

mfojtik added this to the 1.4.0 milestone Oct 12, 2016

legionus force-pushed the refactor-parse-dockerimagereference branch from 0cf14aa to 2c9ba48 Compare October 12, 2016 14:54

legionus mentioned this pull request Oct 13, 2016

Refactor the func 'ParseDockerImageReference', and clear the duplicate code #11331

Closed

legionus force-pushed the refactor-parse-dockerimagereference branch from 2c9ba48 to caa9083 Compare October 13, 2016 14:59

openshift-bot merged commit 23bc588 into openshift:master Oct 14, 2016

legionus mentioned this pull request Oct 14, 2016

Use common parser for DockerImageReference openshift/source-to-image#614

Closed

mfojtik mentioned this pull request Oct 16, 2016

oc get istag/mysql:5.5 fails #11381

Closed

mmilata mentioned this pull request Oct 18, 2016

oc new-app --list fails with ImageStreamImport error #11418

Closed


		repoParts := strings.Split(ref.Name, "/")

		if !isRegistryName(ref.Registry) && len(repoParts) == 1 {

ParseDockerImageReference use docker/distribution reference parser #11245

ParseDockerImageReference use docker/distribution reference parser #11245

Uh oh!

Conversation

legionus commented Oct 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

legionus commented Oct 6, 2016

Uh oh!

legionus commented Oct 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miminar commented Oct 6, 2016 • edited by legionus Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

legionus commented Oct 6, 2016

Uh oh!

miminar commented Oct 6, 2016

Uh oh!

legionus commented Oct 7, 2016

Uh oh!

mfojtik commented Oct 7, 2016

Uh oh!

miminar commented Oct 7, 2016

Uh oh!

legionus commented Oct 7, 2016

Uh oh!

legionus commented Oct 7, 2016

Uh oh!

miminar commented Oct 7, 2016

Uh oh!

legionus commented Oct 7, 2016

Uh oh!

miminar Oct 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miminar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

legionus commented Oct 6, 2016 •

edited

Loading

miminar commented Oct 6, 2016 •

edited by legionus

Loading

miminar Oct 7, 2016 •

edited

Loading