Allow multiple segments in DockerImageReference

We need to be able to handle a repository with more than one component in the name (separated by slash). This necessity follows from the fact that docker/distribution supports such names, but we can't parse them. https://bugzilla.redhat.com/show_bug.cgi?id=1373281 Signed-off-by: Gladkov Alexey <agladkov@redhat.com>
openshift · Oct 3, 2016 · 1a43a2e · 1a43a2e
1 parent b3ec794
commit 1a43a2e
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 28 deletions.
diff --git a/pkg/build/api/validation/validation_test.go b/pkg/build/api/validation/validation_test.go
@@ -1269,7 +1269,7 @@ func TestValidateCommonSpec(t *testing.T) {
 				Output: buildapi.BuildOutput{
 					To: &kapi.ObjectReference{
 						Kind: "DockerImage",
-						Name: "some/long/value/with/no/meaning",
+						Name: "index.docker.io/some/long/value/with/no/meaning",
 					},
 				},
 			},
@@ -1290,7 +1290,7 @@ func TestValidateCommonSpec(t *testing.T) {
 				Output: buildapi.BuildOutput{
 					To: &kapi.ObjectReference{
 						Kind: "ImageStream",
-						Name: "some/long/value/with/no/meaning",
+						Name: "index.docker.io/some/long/value/with/no/meaning",
 					},
 				},
 			},
@@ -1311,7 +1311,7 @@ func TestValidateCommonSpec(t *testing.T) {
 				Output: buildapi.BuildOutput{
 					To: &kapi.ObjectReference{
 						Kind: "ImageStreamTag",
-						Name: "some/long/value/with/no/meaning",
+						Name: "index.docker.io/some/long/value/with/no/meaning",
 					},
 				},
 			},
@@ -1332,7 +1332,7 @@ func TestValidateCommonSpec(t *testing.T) {
 				Output: buildapi.BuildOutput{
 					To: &kapi.ObjectReference{
 						Kind: "ImageStreamTag",
-						Name: "some/long/value/with/no/meaning:latest",
+						Name: "index.docker.io/some/long/value/with/no/meaning:latest",
 					},
 				},
 			},
@@ -2051,6 +2051,54 @@ func TestValidateCommonSpecSuccess(t *testing.T) {
 				},
 			},
 		},
+		// 6
+		{
+			CommonSpec: buildapi.CommonSpec{
+				Source: buildapi.BuildSource{
+					Git: &buildapi.GitBuildSource{
+						URI: "http://github.com/my/repository",
+					},
+				},
+				Strategy: buildapi.BuildStrategy{
+					SourceStrategy: &buildapi.SourceBuildStrategy{
+						From: kapi.ObjectReference{
+							Kind: "DockerImage",
+							Name: "reponame",
+						},
+					},
+				},
+				Output: buildapi.BuildOutput{
+					To: &kapi.ObjectReference{
+						Kind: "DockerImage",
+						Name: "registry/project/repository/data",
+					},
+				},
+			},
+		},
+		// 7
+		{
+			CommonSpec: buildapi.CommonSpec{
+				Source: buildapi.BuildSource{
+					Git: &buildapi.GitBuildSource{
+						URI: "http://github.com/my/repository",
+					},
+				},
+				Strategy: buildapi.BuildStrategy{
+					SourceStrategy: &buildapi.SourceBuildStrategy{
+						From: kapi.ObjectReference{
+							Kind: "DockerImage",
+							Name: "registry/project/repository/data",
+						},
+					},
+				},
+				Output: buildapi.BuildOutput{
+					To: &kapi.ObjectReference{
+						Kind: "DockerImage",
+						Name: "repository/data",
+					},
+				},
+			},
+		},
 	}
 	for count, config := range testCases {
 		errors := validateCommonSpec(&config.CommonSpec, nil)

diff --git a/pkg/image/api/helper.go b/pkg/image/api/helper.go
@@ -184,7 +184,41 @@ func ParseDockerImageReference(spec string) (DockerImageReference, error) {
 		ref.ID = id
 		break
 	default:
-		// TODO: this is no longer true with V2
+		// Handle multiple segments in form: registry/namespace/namesegment1/namesegment2/.../name
+		// but not this form: http://registry/namespace/name. We don't support using the schema in
+		// the DockerImageReference.
+		if len(repoParts) > 3 && !strings.HasSuffix(repoParts[0], ":") {
+			if IsRegistryDockerHub(repoParts[0]) {
+				// DockerHub doesn't support multiple segments.
+				return ref, fmt.Errorf("the docker pull spec %q must be two or three segments separated by slashes", spec)
+			}
+			if len(repoParts[0]) == 0 {
+				return ref, fmt.Errorf("the docker pull spec %q must contain non-empty registry name", spec)
+			}
+			ref.Registry = repoParts[0]
+
+			if len(repoParts[1]) == 0 {
+				return ref, fmt.Errorf("the docker pull spec %q must contain non-empty namespace", spec)
+			}
+			ref.Namespace = repoParts[1]
+
+			for i, part := range repoParts[2:] {
+				if len(part) == 0 {
+					continue
+				}
+				if i > 0 {
+					ref.Name += "/"
+				}
+				ref.Name += part
+			}
+			if len(ref.Name) == 0 {
+				return ref, fmt.Errorf("the docker pull spec %q must be three or more segments separated by slashes", spec)
+			}
+
+			ref.Tag = tag
+			ref.ID = id
+			break
+		}
 		return ref, fmt.Errorf("the docker pull spec %q must be two or three segments separated by slashes", spec)
 	}
 

diff --git a/pkg/image/api/helper_test.go b/pkg/image/api/helper_test.go
@@ -291,8 +291,16 @@ func TestParseDockerImageReference(t *testing.T) {
 			Err:  true,
 		},
 		{
-			From: "bar/foo/baz/biz",
-			Err:  true,
+			From:      "bar/foo/baz/biz",
+			Registry:  "bar",
+			Namespace: "foo",
+			Name:      "baz/biz",
+		},
+		{
+			From:      "bar/foo/baz////biz",
+			Registry:  "bar",
+			Namespace: "foo",
+			Name:      "baz/biz",
 		},
 		{
 			From: "ftp://baz/baz/biz",

diff --git a/pkg/image/api/validation/validation_test.go b/pkg/image/api/validation/validation_test.go
@@ -315,24 +315,6 @@ func TestValidateImageStreamMappingNotOK(t *testing.T) {
 			field.ErrorTypeRequired,
 			"image.metadata.name",
 		},
-		"invalid repository pull spec": {
-			api.ImageStreamMapping{
-				ObjectMeta: kapi.ObjectMeta{
-					Namespace: "default",
-				},
-				DockerImageRepository: "registry/extra/openshift/ruby-19-centos",
-				Tag: api.DefaultImageTag,
-				Image: api.Image{
-					ObjectMeta: kapi.ObjectMeta{
-						Name:      "foo",
-						Namespace: "default",
-					},
-					DockerImageReference: "openshift/ruby-19-centos",
-				},
-			},
-			field.ErrorTypeInvalid,
-			"dockerImageRepository",
-		},
 	}
 
 	for k, v := range errorCases {
@@ -416,7 +398,7 @@ func TestValidateImageStream(t *testing.T) {
 			name:      "foo",
 			dockerImageRepository: "a-|///bbb",
 			expected: field.ErrorList{
-				field.Invalid(field.NewPath("spec", "dockerImageRepository"), "a-|///bbb", "the docker pull spec \"a-|///bbb\" must be two or three segments separated by slashes"),
+				field.Invalid(field.NewPath("spec", "dockerImageRepository"), "a-|///bbb", "the docker pull spec \"a-|///bbb\" must contain non-empty namespace"),
 			},
 		},
 		"invalid dockerImageRepository with tag": {
@@ -719,7 +701,7 @@ func TestValidateImageStreamImport(t *testing.T) {
 		"invalid dockerImageRepository": {
 			isi: &api.ImageStreamImport{ObjectMeta: validMeta, Spec: repoFn("a-|///bbb")},
 			expected: field.ErrorList{
-				field.Invalid(field.NewPath("spec", "repository", "from", "name"), "a-|///bbb", "the docker pull spec \"a-|///bbb\" must be two or three segments separated by slashes"),
+				field.Invalid(field.NewPath("spec", "repository", "from", "name"), "a-|///bbb", "the docker pull spec \"a-|///bbb\" must contain non-empty namespace"),
 			},
 		},
 		"invalid dockerImageRepository with tag": {

diff --git a/test/integration/imageimporter_test.go b/test/integration/imageimporter_test.go
@@ -54,7 +54,7 @@ func TestImageStreamImport(t *testing.T) {
 		},
 		Spec: api.ImageStreamImportSpec{
 			Images: []api.ImageImportSpec{
-				{From: kapi.ObjectReference{Kind: "DockerImage", Name: "a/a/a/a/a/redis:latest"}, To: &kapi.LocalObjectReference{Name: "tag"}},
+				{From: kapi.ObjectReference{Kind: "DockerImage", Name: "index.docker.io/a/a/a/a/a/redis:latest"}, To: &kapi.LocalObjectReference{Name: "tag"}},
 				{From: kapi.ObjectReference{Kind: "DockerImage", Name: "redis:latest"}},
 			},
 		},