Skip to content

Commit 148e83a

Browse files
legionuslegionus
committed
ParseDockerImageReference use docker/distribution reference parser
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]>
1 parent a5ce371 commit 148e83a

File tree

8 files changed

+336
-331
lines changed

8 files changed

+336
-331
lines changed

‎pkg/image/api/helper.go

Lines changed: 11 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import (
1818
"github.com/docker/distribution/manifest/schema1"
1919
"github.com/docker/distribution/manifest/schema2"
2020
"github.com/golang/glog"
21+
22+
"github.com/openshift/origin/pkg/image/reference"
2123
)
2224

2325
const (
@@ -109,16 +111,6 @@ func MakeImageStreamImageName(name, id string) string {
109111
return fmt.Sprintf("%s@%s", name, id)
110112
}
111113

112-
func isRegistryName(str string) bool {
113-
switch {
114-
case strings.Contains(str, ":"),
115-
strings.Contains(str, "."),
116-
str == "localhost":
117-
return true
118-
}
119-
return false
120-
}
121-
122114
// IsRegistryDockerHub returns true if the given registry name belongs to
123115
// Docker hub.
124116
func IsRegistryDockerHub(registry string) bool {
@@ -134,87 +126,18 @@ func IsRegistryDockerHub(registry string) bool {
134126
// DockerImageReference.
135127
func ParseDockerImageReference(spec string) (DockerImageReference, error) {
136128
var ref DockerImageReference
137-
// TODO replace with docker version once docker/docker PR11109 is merged upstream
138-
stream, tag, id := parseRepositoryTag(spec)
139-
140-
repoParts := strings.Split(stream, "/")
141-
switch len(repoParts) {
142-
case 2:
143-
if isRegistryName(repoParts[0]) {
144-
// registry/name
145-
ref.Registry = repoParts[0]
146-
if IsRegistryDockerHub(ref.Registry) {
147-
ref.Namespace = DockerDefaultNamespace
148-
}
149-
if len(repoParts[1]) == 0 {
150-
return ref, fmt.Errorf("the docker pull spec %q must be two or three segments separated by slashes", spec)
151-
}
152-
ref.Name = repoParts[1]
153-
ref.Tag = tag
154-
ref.ID = id
155-
break
156-
}
157-
// namespace/name
158-
ref.Namespace = repoParts[0]
159-
if len(repoParts[1]) == 0 {
160-
return ref, fmt.Errorf("the docker pull spec %q must be two or three segments separated by slashes", spec)
161-
}
162-
ref.Name = repoParts[1]
163-
ref.Tag = tag
164-
ref.ID = id
165-
break
166-
case 3:
167-
// registry/namespace/name
168-
ref.Registry = repoParts[0]
169-
ref.Namespace = repoParts[1]
170-
if len(repoParts[2]) == 0 {
171-
return ref, fmt.Errorf("the docker pull spec %q must be two or three segments separated by slashes", spec)
172-
}
173-
ref.Name = repoParts[2]
174-
ref.Tag = tag
175-
ref.ID = id
176-
break
177-
case 1:
178-
// name
179-
if len(repoParts[0]) == 0 {
180-
return ref, fmt.Errorf("the docker pull spec %q must be two or three segments separated by slashes", spec)
181-
}
182-
ref.Name = repoParts[0]
183-
ref.Tag = tag
184-
ref.ID = id
185-
break
186-
default:
187-
// Handle multiple segments in form: registry/namespace/namesegment1/namesegment2/.../name
188-
// but not this form: http://registry/namespace/name. We don't support using the schema in
189-
// the DockerImageReference.
190-
if len(repoParts) > 3 && !strings.HasSuffix(repoParts[0], ":") {
191-
if len(repoParts[0]) == 0 {
192-
return ref, fmt.Errorf("the docker pull spec %q cannot have empty segments", spec)
193-
}
194-
ref.Registry = repoParts[0]
195129

196-
if len(repoParts[1]) == 0 {
197-
return ref, fmt.Errorf("the docker pull spec %q cannot have empty segments", spec)
198-
}
199-
ref.Namespace = repoParts[1]
200-
201-
for i, part := range repoParts[2:] {
202-
if len(part) == 0 {
203-
return ref, fmt.Errorf("the docker pull spec %q cannot have empty segments", spec)
204-
}
205-
if i > 0 {
206-
ref.Name += "/"
207-
}
208-
ref.Name += part
209-
}
210-
211-
ref.Tag = tag
212-
ref.ID = id
213-
break
214-
}
215-
return ref, fmt.Errorf("the docker pull spec %q must be two or three segments separated by slashes", spec)
130+
namedRef, err := reference.ParseNamedDockerImageReference(spec)
131+
if err != nil {
132+
return ref, err
216133
}
217134

135+
ref.Registry = namedRef.Registry
136+
ref.Namespace = namedRef.Namespace
137+
ref.Name = namedRef.Name
138+
ref.Tag = namedRef.Tag
139+
ref.ID = namedRef.ID
140+
218141
return ref, nil
219142
}
220143

‎pkg/image/api/helper_test.go

Lines changed: 0 additions & 229 deletions
Original file line numberDiff line numberDiff line change
@@ -121,235 +121,6 @@ func TestParseImageStreamTagName(t *testing.T) {
121121
}
122122
}
123123

124-
func TestParseDockerImageReference(t *testing.T) {
125-
testCases := []struct {
126-
From string
127-
Registry, Namespace, Name, Tag, ID string
128-
Err bool
129-
}{
130-
{
131-
From: "foo",
132-
Name: "foo",
133-
},
134-
{
135-
From: "foo:tag",
136-
Name: "foo",
137-
Tag: "tag",
138-
},
139-
{
140-
From: "foo@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25",
141-
Name: "foo",
142-
ID: "sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25",
143-
},
144-
{
145-
From: "bar/foo",
146-
Namespace: "bar",
147-
Name: "foo",
148-
},
149-
{
150-
From: "bar/foo:tag",
151-
Namespace: "bar",
152-
Name: "foo",
153-
Tag: "tag",
154-
},
155-
{
156-
From: "bar/foo@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25",
157-
Namespace: "bar",
158-
Name: "foo",
159-
ID: "sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25",
160-
},
161-
{
162-
From: "bar/foo/baz",
163-
Registry: "bar",
164-
Namespace: "foo",
165-
Name: "baz",
166-
},
167-
{
168-
From: "bar/library/baz",
169-
Registry: "bar",
170-
Namespace: "library",
171-
Name: "baz",
172-
},
173-
{
174-
From: "bar/foo/baz:tag",
175-
Registry: "bar",
176-
Namespace: "foo",
177-
Name: "baz",
178-
Tag: "tag",
179-
},
180-
{
181-
From: "bar/foo/baz@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25",
182-
Registry: "bar",
183-
Namespace: "foo",
184-
Name: "baz",
185-
ID: "sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25",
186-
},
187-
{
188-
From: "bar:5000/foo/baz",
189-
Registry: "bar:5000",
190-
Namespace: "foo",
191-
Name: "baz",
192-
},
193-
{
194-
From: "bar:5000/library/baz",
195-
Registry: "bar:5000",
196-
Namespace: "library",
197-
Name: "baz",
198-
},
199-
{
200-
From: "bar:5000/baz",
201-
Registry: "bar:5000",
202-
Name: "baz",
203-
},
204-
{
205-
From: "bar:5000/foo/baz:tag",
206-
Registry: "bar:5000",
207-
Namespace: "foo",
208-
Name: "baz",
209-
Tag: "tag",
210-
},
211-
{
212-
From: "bar:5000/foo/baz@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25",
213-
Registry: "bar:5000",
214-
Namespace: "foo",
215-
Name: "baz",
216-
ID: "sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25",
217-
},
218-
{
219-
From: "myregistry.io/foo",
220-
Registry: "myregistry.io",
221-
Name: "foo",
222-
},
223-
{
224-
From: "localhost/bar",
225-
Registry: "localhost",
226-
Name: "bar",
227-
},
228-
{
229-
From: "docker.io/library/myapp",
230-
Registry: "docker.io",
231-
Namespace: "library",
232-
Name: "myapp",
233-
},
234-
{
235-
From: "docker.io/myapp",
236-
Registry: "docker.io",
237-
Namespace: DockerDefaultNamespace,
238-
Name: "myapp",
239-
},
240-
{
241-
From: "docker.io/user/myapp",
242-
Registry: "docker.io",
243-
Namespace: "user",
244-
Name: "myapp",
245-
},
246-
{
247-
From: "docker.io/user/project/myapp",
248-
Registry: "docker.io",
249-
Namespace: "user",
250-
Name: "project/myapp",
251-
},
252-
{
253-
From: "index.docker.io/bar",
254-
Registry: "index.docker.io",
255-
Namespace: DockerDefaultNamespace,
256-
Name: "bar",
257-
},
258-
// TODO: test cases if ParseDockerImageReference validates segment length and allowed chars
259-
//
260-
// {
261-
// // namespace/name == 255 chars
262-
// From: fmt.Sprintf("bar:5000/%s/%s:tag", strings.Repeat("a", 63), strings.Repeat("b", 191)),
263-
// Registry: "bar:5000",
264-
// Namespace: strings.Repeat("a", 63),
265-
// Name: strings.Repeat("b", 191),
266-
// Tag: "tag",
267-
// },
268-
// {
269-
// // namespace/name == 255 chars with implicit namespace
270-
// From: fmt.Sprintf("bar:5000/%s:tag", strings.Repeat("b", 247)),
271-
// Registry: "bar:5000",
272-
// Namespace: DockerDefaultNamespace,
273-
// Name: strings.Repeat("b", 247),
274-
// Tag: "tag",
275-
// },
276-
// {
277-
// // namespace/name > 255 chars
278-
// From: fmt.Sprintf("bar:5000/%s/%s:tag", strings.Repeat("a", 63), strings.Repeat("b", 192)),
279-
// Err: true,
280-
// },
281-
// {
282-
// // namespace/name > 255 chars with implicit namespace
283-
// From: fmt.Sprintf("bar:5000/%s:tag", strings.Repeat("b", 248)),
284-
// Err: true,
285-
// },
286-
// {
287-
// // namespace < 2 chars
288-
// From: "bar:5000/a/b:tag",
289-
// Err: true,
290-
// },
291-
{
292-
From: "https://bar:5000/foo/baz",
293-
Err: true,
294-
},
295-
{
296-
From: "http://bar:5000/foo/baz@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25",
297-
Err: true,
298-
},
299-
{
300-
From: "bar/foo/baz/biz",
301-
Registry: "bar",
302-
Namespace: "foo",
303-
Name: "baz/biz",
304-
},
305-
{
306-
From: "bar/foo/baz////biz",
307-
Err: true,
308-
},
309-
{
310-
From: "//foo/baz/biz",
311-
Err: true,
312-
},
313-
{
314-
From: "ftp://baz/baz/biz",
315-
Err: true,
316-
},
317-
{
318-
From: "",
319-
Err: true,
320-
},
321-
}
322-
323-
for _, testCase := range testCases {
324-
ref, err := ParseDockerImageReference(testCase.From)
325-
switch {
326-
case err != nil && !testCase.Err:
327-
t.Errorf("%s: unexpected error: %v", testCase.From, err)
328-
continue
329-
case err == nil && testCase.Err:
330-
t.Errorf("%s: unexpected non-error", testCase.From)
331-
continue
332-
case err != nil && testCase.Err:
333-
continue
334-
}
335-
if e, a := testCase.Registry, ref.Registry; e != a {
336-
t.Errorf("%s: registry: expected %q, got %q", testCase.From, e, a)
337-
}
338-
if e, a := testCase.Namespace, ref.Namespace; e != a {
339-
t.Errorf("%s: namespace: expected %q, got %q", testCase.From, e, a)
340-
}
341-
if e, a := testCase.Name, ref.Name; e != a {
342-
t.Errorf("%s: name: expected %q, got %q", testCase.From, e, a)
343-
}
344-
if e, a := testCase.Tag, ref.Tag; e != a {
345-
t.Errorf("%s: tag: expected %q, got %q", testCase.From, e, a)
346-
}
347-
if e, a := testCase.ID, ref.ID; e != a {
348-
t.Errorf("%s: id: expected %q, got %q", testCase.From, e, a)
349-
}
350-
}
351-
}
352-
353124
func TestDockerImageReferenceAsRepository(t *testing.T) {
354125
testCases := []struct {
355126
Registry, Namespace, Name, Tag, ID string

‎pkg/image/api/validation/validation.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,9 @@ func ValidateImageStreamTagReference(tagRef api.TagReference, fldPath *field.Pat
196196
}
197197
switch tagRef.From.Kind {
198198
case "DockerImage":
199-
if ref, err := api.ParseDockerImageReference(tagRef.From.Name); err == nil && tagRef.ImportPolicy.Scheduled && len(ref.ID) > 0 {
199+
if ref, err := api.ParseDockerImageReference(tagRef.From.Name); err != nil && len(tagRef.From.Name) > 0 {
200+
errs = append(errs, field.Invalid(fldPath.Child("from", "name"), tagRef.From.Name, err.Error()))
201+
} else if len(ref.ID) > 0 && tagRef.ImportPolicy.Scheduled {
200202
errs = append(errs, field.Invalid(fldPath.Child("from", "name"), tagRef.From.Name, "only tags can be scheduled for import"))
201203
}
202204
case "ImageStreamImage", "ImageStreamTag":

0 commit comments

Comments
 (0)