diff --git a/CHANGELOG.md b/CHANGELOG.md index ebdef26d..55bbe3ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ IMPROVEMENTS * Adds container healthcheck[[#93](https://github.com/terraform-providers/terraform-provider-docker/pull/93)] * Adds the docker container start flag [GH-62] and [[#94](https://github.com/terraform-providers/terraform-provider-docker/pull/94)] * Adds `cpu_set` to docker container [[#41](https://github.com/terraform-providers/terraform-provider-docker/pull/41)] +* Simplifies the image options parser and adds missing registry combinations [[#49](https://github.com/terraform-providers/terraform-provider-docker/pull/49)] BUG FIXES * Fixes that new network were appended to the default bridge [GH-10] diff --git a/docker/resource_docker_image_funcs.go b/docker/resource_docker_image_funcs.go index f8cbcb04..1a3f171e 100644 --- a/docker/resource_docker_image_funcs.go +++ b/docker/resource_docker_image_funcs.go @@ -185,45 +185,52 @@ type internalPullImageOptions struct { func parseImageOptions(image string) internalPullImageOptions { pullOpts := internalPullImageOptions{} - splitImageName := strings.Split(image, ":") - switch len(splitImageName) { + // splitImageName := strings.Split(image, ":") + // switch len(splitImageName) { - // 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:], "/") + // // 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:], "/") - // 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] + // // 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] - // 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" - } + // // 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" + // } - // 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 - } + // Pre-fill with image by default, update later if tag found + pullOpts.Repository = image + + firstSlash := strings.Index(image, "/") + + // 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] + } + + prefixLength := len(pullOpts.Registry) + tagIndex := strings.Index(image[prefixLength:], ":") + + 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) + } +}