Skip to content

Commit

Permalink
registry: do not cache image metadata for latest and edge tags
Browse files Browse the repository at this point in the history
  • Loading branch information
cezarsa committed Dec 8, 2020
1 parent 508860e commit 28acd97
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 25 deletions.
24 changes: 18 additions & 6 deletions internal/registry/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package registry
import (
"crypto/tls"
"encoding/json"
"fmt"
"net/http"
"strings"
"sync"
Expand Down Expand Up @@ -59,25 +60,27 @@ func (r *imageMetadataRetriever) cachedLabels(image string) (map[string]string,
return r.labelsCache[image], nil
}

labels, err := r.imageLabels(image)
parts := parseImage(image)
labels, err := r.imageLabels(parts)
if err != nil {
return nil, err
}
r.labelsCache[image] = labels
if !parts.isLatest() {
r.labelsCache[image] = labels
}

return labels, nil
}

func (r *imageMetadataRetriever) imageLabels(image string) (map[string]string, error) {
func (r *imageMetadataRetriever) imageLabels(image dockerImage) (map[string]string, error) {
type historyEntry struct {
Config struct {
Labels map[string]string
}
}

parts := parseImage(image)
hub := r.registry(parts.registry)
manifest, err := hub.Manifest(parts.image, parts.tag)
hub := r.registry(image.registry)
manifest, err := hub.Manifest(image.image, image.tag)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -126,9 +129,18 @@ type dockerImage struct {
tag string
}

func (i dockerImage) isLatest() bool {
return i.tag == "latest" || i.tag == "edge"
}

func (i dockerImage) String() string {
return fmt.Sprintf("%s/%s:%s", i.registry, i.image, i.tag)
}

func parseImage(imageName string) dockerImage {
img := dockerImage{
registry: "registry-1.docker.io",
tag: "latest",
}

parts := strings.SplitN(imageName, "/", 3)
Expand Down
99 changes: 80 additions & 19 deletions internal/registry/image_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package registry

import (
"fmt"
"net/http"
"net/http/httptest"
"net/url"
Expand All @@ -12,22 +13,15 @@ import (
)

func TestImageMetadataRetriever_Modules(t *testing.T) {
r := NewImageMetadata()
r.insecure = true
var apiCalls []string
var labels string
srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, "/v2/my/img/manifests/v1", r.URL.Path)
assert.Equal(t, "GET", r.Method)
w.Header().Add("Content-Type", "application/vnd.docker.distribution.manifest.v1+json")
apiCalls = append(apiCalls, r.URL.Path)
sig, err := libtrust.NewJSONSignatureFromMap(map[string]interface{}{
"schemaVersion": 1,
"name": "my/img",
"tag": "v1",
"history": []map[string]interface{}{{
"v1Compatibility": `{
"config":{
"Labels":{"io.tsuru.nginx-modules":"mod1,mod2"}
}
}`,
"v1Compatibility": fmt.Sprintf(`{
"config":{%s}
}`, labels),
}},
})
require.NoError(t, err)
Expand All @@ -42,9 +36,76 @@ func TestImageMetadataRetriever_Modules(t *testing.T) {
defer srv.Close()
u, err := url.Parse(srv.URL)
require.NoError(t, err)
mod, err := r.Modules(u.Host + "/my/img:v1")
require.NoError(t, err)
assert.Equal(t, []string{"mod1", "mod2"}, mod)

tests := []struct {
image string
labels string
expectedModules []string
expectedCalls []string
}{
{
image: "my/img:v1",
expectedModules: nil,
expectedCalls: []string{"/v2/my/img/manifests/v1"},
},
{
image: "my/img:v1",
labels: `"Labels":{"io.tsuru.nginx-modules":"mod1,mod2"}`,
expectedModules: []string{"mod1", "mod2"},
expectedCalls: []string{"/v2/my/img/manifests/v1"},
},
{
image: "my/img:latest",
expectedModules: nil,
expectedCalls: []string{"/v2/my/img/manifests/latest", "/v2/my/img/manifests/latest"},
},
{
image: "my/img:edge",
expectedModules: nil,
expectedCalls: []string{"/v2/my/img/manifests/edge", "/v2/my/img/manifests/edge"},
},
{
image: "my/img:v1",
labels: `"Labels":{}`,
expectedModules: nil,
expectedCalls: []string{"/v2/my/img/manifests/v1"},
},
{
image: "my/img:v1",
labels: `"Labels":null`,
expectedModules: nil,
expectedCalls: []string{"/v2/my/img/manifests/v1"},
},
{
image: "my/img:v1",
labels: `"Labels":{"other":"mod1,mod2"}`,
expectedModules: nil,
expectedCalls: []string{"/v2/my/img/manifests/v1"},
},
{
image: "my/img:v1",
labels: `"Labels":{"io.tsuru.nginx-modules":""}`,
expectedModules: nil,
expectedCalls: []string{"/v2/my/img/manifests/v1"},
},
}

for _, tt := range tests {
t.Run("", func(t *testing.T) {
apiCalls = nil
labels = tt.labels
r := NewImageMetadata()
r.insecure = true
mod, err := r.Modules(u.Host + "/" + tt.image)
require.NoError(t, err)
assert.Equal(t, tt.expectedModules, mod)
mod, err = r.Modules(u.Host + "/" + tt.image)
require.NoError(t, err)
assert.Equal(t, tt.expectedModules, mod)
assert.Equal(t, tt.expectedCalls, apiCalls)

})
}
}

func TestParseImage(t *testing.T) {
Expand All @@ -54,11 +115,11 @@ func TestParseImage(t *testing.T) {
expectedImage string
expectedTag string
}{
{"f064bf4", "registry-1.docker.io", "f064bf4", ""},
{"", "registry-1.docker.io", "", ""},
{"f064bf4", "registry-1.docker.io", "f064bf4", "latest"},
{"", "registry-1.docker.io", "", "latest"},
{"registry.io/tsuru/app-img:v1", "registry.io", "tsuru/app-img", "v1"},
{"tsuru/app-img:v1", "registry-1.docker.io", "tsuru/app-img", "v1"},
{"tsuru/app-img", "registry-1.docker.io", "tsuru/app-img", ""},
{"tsuru/app-img", "registry-1.docker.io", "tsuru/app-img", "latest"},
{"f064bf4:v1", "registry-1.docker.io", "f064bf4", "v1"},
{"registry:5000/app-img:v1", "registry:5000", "app-img", "v1"},
{"registry.io/app-img:v1", "registry.io", "app-img", "v1"},
Expand Down

0 comments on commit 28acd97

Please sign in to comment.