Simplifies the image options parser. Closes #49.

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]

docker/resource_docker_image_funcs.go

@@ -185,45 +185,52 @@ type internalPullImageOptions struct {
 
 func parseImageOptions(image string) internalPullImageOptions {
 	pullOpts := internalPullImageOptions{}
-	splitImageName := strings.Split(image, ":")
+	// splitImageName := strings.Split(image, ":")
-	switch len(splitImageName) {
+	// switch len(splitImageName) {
 
-	// It's in registry:port/username/repo:tag or registry:port/repo:tag format
+	// // It's in registry:port/username/repo:tag or registry:port/repo:tag format
-	case 3:
+	// case 3:
-		splitPortRepo := strings.Split(splitImageName[1], "/")
+	// 	splitPortRepo := strings.Split(splitImageName[1], "/")
-		pullOpts.Registry = splitImageName[0] + ":" + splitPortRepo[0]
+	// 	pullOpts.Registry = splitImageName[0] + ":" + splitPortRepo[0]
-		pullOpts.Tag = splitImageName[2]
+	// 	pullOpts.Tag = splitImageName[2]
-		pullOpts.Repository = pullOpts.Registry + "/" + strings.Join(splitPortRepo[1:], "/")
+	// 	pullOpts.Repository = pullOpts.Registry + "/" + strings.Join(splitPortRepo[1:], "/")
 
-	// It's either registry:port/username/repo, registry:port/repo
+	// // It's either registry:port/username/repo, registry:port/repo
-	// or repo:tag with default registry
+	// // or repo:tag with default registry
-	case 2:
+	// case 2:
-		splitPortRepo := strings.Split(splitImageName[1], "/")
+	// 	splitPortRepo := strings.Split(splitImageName[1], "/")
-		switch len(splitPortRepo) {
+	// 	switch len(splitPortRepo) {
-		// repo:tag
+	// 	// repo:tag
-		case 1:
+	// 	case 1:
-			pullOpts.Repository = splitImageName[0]
+	// 		pullOpts.Repository = splitImageName[0]
-			pullOpts.Tag = splitImageName[1]
+	// 		pullOpts.Tag = splitImageName[1]
 
-		// registry:port/username/repo or registry:port/repo
+	// 	// registry:port/username/repo or registry:port/repo
-		default:
+	// 	default:
-			pullOpts.Registry = splitImageName[0] + ":" + splitPortRepo[0]
+	// 		pullOpts.Registry = splitImageName[0] + ":" + splitPortRepo[0]
-			pullOpts.Repository = pullOpts.Registry + "/" + strings.Join(splitPortRepo[1:], "/")
+	// 		pullOpts.Repository = pullOpts.Registry + "/" + strings.Join(splitPortRepo[1:], "/")
-			pullOpts.Tag = "latest"
+	// 		pullOpts.Tag = "latest"
-		}
+	// 	}
 
-	// Registry/username/repo or plain username/repo or repo
+	// Pre-fill with image by default, update later if tag found
-	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
+
+	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

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)
+	}
+}