From 3090e33ca05a614129d9805267a4a007d4cb4bf7 Mon Sep 17 00:00:00 2001 From: Michal Prihoda Date: Fri, 23 Mar 2018 10:58:14 +0100 Subject: [PATCH] Simplify the image options parser. Adding support for missing variants of registry/foo/bar/baz and registry/foo/bar/baz:tag. --- docker/resource_docker_image_funcs.go | 50 +++------ docker/resource_docker_image_funcs_test.go | 117 +++++++++++++++++++++ 2 files changed, 132 insertions(+), 35 deletions(-) create mode 100644 docker/resource_docker_image_funcs_test.go diff --git a/docker/resource_docker_image_funcs.go b/docker/resource_docker_image_funcs.go index 5bffbf13..dbaede29 100644 --- a/docker/resource_docker_image_funcs.go +++ b/docker/resource_docker_image_funcs.go @@ -154,45 +154,25 @@ func pullImage(data *Data, client *dc.Client, authConfig *dc.AuthConfigurations, func parseImageOptions(image string) dc.PullImageOptions { pullOpts := dc.PullImageOptions{} - splitImageName := strings.Split(image, ":") - switch len(splitImageName) { + // Pre-fill with image by default, update later if tag found + pullOpts.Repository = image - // It's in registry:port/username/repo:tag or registry:port/repo:tag format - case 3: - splitPortRepo := strings.Split(splitImageName[1], "/") - pullOpts.Registry = splitImageName[0] + ":" + splitPortRepo[0] - pullOpts.Tag = splitImageName[2] - pullOpts.Repository = pullOpts.Registry + "/" + strings.Join(splitPortRepo[1:], "/") + firstSlash := strings.Index(image, "/") - // It's either registry:port/username/repo, registry:port/repo - // or repo:tag with default registry - case 2: - splitPortRepo := strings.Split(splitImageName[1], "/") - switch len(splitPortRepo) { - // repo:tag - case 1: - pullOpts.Repository = splitImageName[0] - pullOpts.Tag = splitImageName[1] + // Detect the registry name - it should either contain port, be fully qualified or be localhost + // If the image contains more than 2 path components, or at least one and the prefix looks like a hostname + if strings.Count(image, "/") > 1 || firstSlash != -1 && (strings.ContainsAny(image[:firstSlash], ".:") || image[:firstSlash] == "localhost") { + // registry/repo/image + pullOpts.Registry = image[:firstSlash] + } - // registry:port/username/repo or registry:port/repo - default: - pullOpts.Registry = splitImageName[0] + ":" + splitPortRepo[0] - pullOpts.Repository = pullOpts.Registry + "/" + strings.Join(splitPortRepo[1:], "/") - pullOpts.Tag = "latest" - } + prefixLength := len(pullOpts.Registry) + tagIndex := strings.Index(image[prefixLength:], ":") - // Registry/username/repo or plain username/repo or repo - default: - splitRegistryRepo := strings.Split(image, "/") - switch len(splitRegistryRepo) { - // registry/username/repo - case 3: - pullOpts.Registry = splitRegistryRepo[0] - pullOpts.Repository = pullOpts.Registry + "/" + strings.Join(splitRegistryRepo[1:], "/") - // plain username/repo or repo - default: - pullOpts.Repository = image - } + if tagIndex != -1 { + // we have the tag, strip it + pullOpts.Repository = image[:prefixLength+tagIndex] + pullOpts.Tag = image[prefixLength+tagIndex+1:] } return pullOpts diff --git a/docker/resource_docker_image_funcs_test.go b/docker/resource_docker_image_funcs_test.go new file mode 100644 index 00000000..f17dc47b --- /dev/null +++ b/docker/resource_docker_image_funcs_test.go @@ -0,0 +1,117 @@ +package docker + +import ( + "github.com/fsouza/go-dockerclient" + "testing" +) + +func TestParseRegistryPortTagFormat(t *testing.T) { + validatePullOpts(t, + "registry.gitlab.com:443/foo/bar:v1", + &docker.PullImageOptions{ + Registry: "registry.gitlab.com:443", + Repository: "registry.gitlab.com:443/foo/bar", + Tag: "v1", + }) + validatePullOpts(t, + "registry.gitlab.com:443/foo:v1", + &docker.PullImageOptions{ + Registry: "registry.gitlab.com:443", + Repository: "registry.gitlab.com:443/foo", + Tag: "v1", + }) +} + +func TestParseRegistryPortFormat(t *testing.T) { + validatePullOpts(t, + "registry.gitlab.com:443/foo/bar", + &docker.PullImageOptions{ + Registry: "registry.gitlab.com:443", + Repository: "registry.gitlab.com:443/foo/bar", + Tag: "", + }) + validatePullOpts(t, + "registry.gitlab.com:443/foo", + &docker.PullImageOptions{ + Registry: "registry.gitlab.com:443", + Repository: "registry.gitlab.com:443/foo", + Tag: "", + }) +} + +func TestParseRepoTagFormat(t *testing.T) { + validatePullOpts(t, + "foo:bar", + &docker.PullImageOptions{ + Registry: "", + Repository: "foo", + Tag: "bar", + }) +} + +func TestParseRegistryRepoFormat(t *testing.T) { + validatePullOpts(t, + "registry.gitlab.com/foo/bar", + &docker.PullImageOptions{ + Registry: "registry.gitlab.com", + Repository: "registry.gitlab.com/foo/bar", + Tag: "", + }) +} + +func TestParsePlainRepoFormat(t *testing.T) { + validatePullOpts(t, + "foo/bar", + &docker.PullImageOptions{ + Registry: "", + Repository: "foo/bar", + Tag: "", + }) +} + +func TestParseGitlabComThreePartImageOptions(t *testing.T) { + validatePullOpts(t, + "registry.gitlab.com:443/foo/bar/baz:v1", + &docker.PullImageOptions{ + Registry: "registry.gitlab.com:443", + Repository: "registry.gitlab.com:443/foo/bar/baz", + Tag: "v1", + }) + validatePullOpts(t, + "registry.gitlab.com:443/foo/bar/baz", + &docker.PullImageOptions{ + Registry: "registry.gitlab.com:443", + Repository: "registry.gitlab.com:443/foo/bar/baz", + Tag: "", + }) + validatePullOpts(t, + "registry.gitlab.com/foo/bar/baz:v1", + &docker.PullImageOptions{ + Registry: "registry.gitlab.com", + Repository: "registry.gitlab.com/foo/bar/baz", + Tag: "v1", + }) + validatePullOpts(t, + "registry.gitlab.com/foo/bar/baz", + &docker.PullImageOptions{ + Registry: "registry.gitlab.com", + Repository: "registry.gitlab.com/foo/bar/baz", + Tag: "", + }) +} + +func validatePullOpts(t *testing.T, inputString string, expected *docker.PullImageOptions) { + pullOpts := parseImageOptions(inputString) + + if pullOpts.Registry != expected.Registry { + t.Fatalf("For '%s' expected registry '%s', got '%s'", inputString, expected.Registry, pullOpts.Registry) + } + + if pullOpts.Repository != expected.Repository { + t.Fatalf("For '%s' expected repository '%s', got '%s'", inputString, expected.Repository, pullOpts.Repository) + } + + if pullOpts.Tag != expected.Tag { + t.Fatalf("For '%s' expected tag '%s', got '%s'", inputString, expected.Tag, pullOpts.Tag) + } +}