Repository: uber/makisu (go)

Issue:
Base image pull by digest doesn't work
Apparently, makisu doesn't work with format like this (tag + digest):
```
FROM gcr.io/makisu-project/makisu-alpine:v0.1.11@sha256:fc80186c3b46b19d3fc2df062250a85c95dd57380ac4fe19ad20d26358b9a3a5
```
or even like this (digest only):
```
FROM gcr.io/makisu-project/makisu-alpine@sha256:fc80186c3b46b19d3fc2df062250a85c95dd57380ac4fe19ad20d26358b9a3a5
```
I think it's crucial to be able to pull base images via digest since this way is defined as a [best practice](https://success.docker.com/article/images-tagging-vs-digests) and supported by at least Docker and [Kubernetes](https://github.com/kubernetes/kubernetes/blob/9946d92e167ec56b7fd7aa2324573fa75c177b98//pkg/kubelet/dockershim/libdocker/helpers.go#L40).

Pull request description:
Implement pulling base images by digest.
This will allow base images that are formatted like:

```
FROM gcr.io/makisu-project/makisu-alpine:v0.1.11@sha256:fc80186c3b46b19d3fc2df062250a85c95dd57380ac4fe19ad20d26358b9a3a5
```

or

```
FROM gcr.io/makisu-project/makisu-alpine@sha256:fc80186c3b46b19d3fc2df062250a85c95dd57380ac4fe19ad20d26358b9a3a5
```

Which allows a base image to be specified by an immutable digest instead
of a, potentially floating, tag.

Closes #235.

Patch:
diff --git a/lib/docker/image/image_name.go b/lib/docker/image/image_name.go
index 2907707..d8ebf2e 100644
--- a/lib/docker/image/image_name.go
+++ b/lib/docker/image/image_name.go
@@ -82,7 +82,12 @@ func (name Name) IsValid() bool {
 
 // ShortName returns the name of the image without the registry information
 func (name Name) ShortName() string {
-	return fmt.Sprintf("%s:%s", name.GetRepository(), name.tag)
+	separator := ":"
+	// If the tag contains a :, it must be a digest tag of the form <digest-algo>:<digest>. Should stringify as <repository>@<digest-algo>:<digest>
+	if strings.Contains(name.tag, ":") {
+		separator = "@"
+	}
+	return fmt.Sprintf("%s%s%s", name.GetRepository(), separator, name.tag)
 }
 
 // String returns the full name of the image with the registry information if available
@@ -103,6 +108,7 @@ func ParseName(input string) (Name, error) {
 
 	slashIndex := strings.LastIndex(input, "/")
 	sepIndex := strings.LastIndex(input, ":")
+	digestIndex := strings.LastIndex(input, "@")
 	if sepIndex < slashIndex || sepIndex == -1 {
 		// <ip>:<port>/<repo>
 		// <dns>:<port>/<repo>
@@ -110,8 +116,25 @@ func ParseName(input string) (Name, error) {
 		// <repo>
 		result.repository = input
 		result.tag = "latest"
+	} else if digestIndex < sepIndex && digestIndex > slashIndex {
+		result.repository = input[:digestIndex]
+		sepIndex = strings.LastIndex(result.repository, ":")
+		if sepIndex >= slashIndex && sepIndex != -1 {
+			// A tag can still be specified for informational purposes if a digest is specified
+			// <ip>:<port>/<repo>:<tag>@<digest-algo>:<digest>
+			// <dns>:<port>/<repo>:<tag>@<digest-algo>:<digest>
+			// <repo>:<tag>@<digest-algo>:<digest>
+			result.repository = input[:sepIndex]
+		}
+		// else if no tag is specified result.repository = input[:digestIndex]
+		// <ip>:<port>/<repo>@<digest-algo>:<digest>
+		// <dns>:<port>/<repo>@<digest-algo>:<digest>
+		// <repo>@<digest-algo>:<digest>
+
+		// When pulling by digest, the docker API expects <digest-algo>:<digest> to take the place of the tag
+		result.tag = input[digestIndex+1:]
 	} else {
-		// if sepIndex >= slashIndex && sepIndex == -1
+		// if sepIndex >= slashIndex && sepIndex != -1 && (digestIndex < slashIndex || digestIndex > sepIndex) -1)
 		// <ip>:<port>/<repo>:<tag>
 		// <dns>:<port>/<repo>:<tag>
 		// <repo>:<tag>


Test patch:
diff --git a/lib/docker/image/image_name_test.go b/lib/docker/image/image_name_test.go
index ee62dd5..a174729 100644
--- a/lib/docker/image/image_name_test.go
+++ b/lib/docker/image/image_name_test.go
@@ -54,6 +54,30 @@ func TestParseImageName(t *testing.T) {
 	require.True(name.IsValid())
 	require.Equal("127.0.0.1:5002/evanescence-golang-1:latest", name.String())
 
+	name, err = ParseNameForPull("127.0.0.1:5002/king-gizzard-golang-1@sha256:2a3dd484ecfcf9343994e0f6c2af0a6faf1af7f7e499905793643f91e90edcb3")
+	require.NoError(err)
+	require.Equal(name.GetRegistry(), "127.0.0.1:5002")
+	require.Equal(name.GetRepository(), "king-gizzard-golang-1")
+	require.Equal(name.GetTag(), "sha256:2a3dd484ecfcf9343994e0f6c2af0a6faf1af7f7e499905793643f91e90edcb3")
+	require.True(name.IsValid())
+	require.Equal("127.0.0.1:5002/king-gizzard-golang-1@sha256:2a3dd484ecfcf9343994e0f6c2af0a6faf1af7f7e499905793643f91e90edcb3", name.String())
+
+	name, err = ParseNameForPull("127.0.0.1:5002/king-gizzard-golang-1:v1.0.0@sha256:2a3dd484ecfcf9343994e0f6c2af0a6faf1af7f7e499905793643f91e90edcb3")
+	require.NoError(err)
+	require.Equal(name.GetRegistry(), "127.0.0.1:5002")
+	require.Equal(name.GetRepository(), "king-gizzard-golang-1")
+	require.Equal(name.GetTag(), "sha256:2a3dd484ecfcf9343994e0f6c2af0a6faf1af7f7e499905793643f91e90edcb3")
+	require.True(name.IsValid())
+	require.Equal("127.0.0.1:5002/king-gizzard-golang-1@sha256:2a3dd484ecfcf9343994e0f6c2af0a6faf1af7f7e499905793643f91e90edcb3", name.String())
+
+	name, err = ParseNameForPull("127.0.0.1:5002/king-gizzard-golang-1:tag-with-an-@-in-it")
+	require.NoError(err)
+	require.Equal(name.GetRegistry(), "127.0.0.1:5002")
+	require.Equal(name.GetRepository(), "king-gizzard-golang-1")
+	require.Equal(name.GetTag(), "tag-with-an-@-in-it")
+	require.True(name.IsValid())
+	require.Equal("127.0.0.1:5002/king-gizzard-golang-1:tag-with-an-@-in-it", name.String())
+
 	name, err = ParseNameForPull("docker-registry01-sjc1:5055/uber-usi/haproxy-agent:sjc1-produ-0000000027")
 	require.NoError(err)
 	require.Equal(name.GetRegistry(), "docker-registry01-sjc1:5055")
diff --git a/test/python/test_build.py b/test/python/test_build.py
index 5931f9f..fd040f4 100644
--- a/test/python/test_build.py
+++ b/test/python/test_build.py
@@ -31,6 +31,16 @@ def test_build_remove(registry1, storage_dir):
     assert code == 0, err
 
 
+def test_build_from_sha256(registry1, storage_dir):
+    new_image = utils.new_image_name()
+    context_dir = os.path.join(
+        os.getcwd(), 'testdata/build-context/from-sha256')
+    utils.makisu_build_image(
+        new_image, context_dir, storage_dir, registry=registry1.addr)
+    code, err = utils.docker_run_image(registry1.addr, new_image)
+    assert code == 0, err
+
+
 def test_build_symlink(registry1, storage_dir):
     new_image = utils.new_image_name()
     context_dir = os.path.join(
diff --git a/testdata/build-context/from-sha256/Dockerfile b/testdata/build-context/from-sha256/Dockerfile
new file mode 100644
index 0000000..bedb1f5
--- /dev/null
+++ b/testdata/build-context/from-sha256/Dockerfile
@@ -0,0 +1,3 @@
+FROM alpine@sha256:26a8d1303ea6109122ba2df7fafcfbe77ddc3988a2c34e818398b8ed4a20b03d
+
+ENTRYPOINT ["/bin/sh", "-c", "[ \"$(cat /etc/alpine-release)\" == '3.8.4' ]"]