Skip to content
This repository was archived by the owner on Aug 14, 2020. It is now read-only.

Commit 8d0262c

Browse files
sgottisgotti
committed
discovery: return only best matching templates.
Actually the spec doesn't clarify when an endpoint template should be accepted or not. Now, the appc/spec implementation returns only endpoints that can be fully rendered. This means that it will also accept a template if it doesn't contain some of the required discovery labels. This looks good, but, trying to implement some meta discovery ideas it will bring to unwanted endpoints. Example 1: Suppose I want to implement the "latest" pattern behavior: ```html <!-- ACIs with specific version --> <meta name="ac-discovery" content="example.com https://storage.example.com/{name}-{version}-{os}-{arch}.{ext}"> <!-- Latest ACIs --> <meta name="ac-discovery" content="example.com https://storage.example.com/{name}-latest-{os}-{arch}.{ext}"> ``` If, on discovery, I ask for the _name_, _os_ and _arch_ labels only the second template will be rendered (since the first cannot be fully rendered due to missing _version_ label). So I achieved latest pattern. But if I'm asking for a specific _version_ both templates will be rendered. Example 2: As an extension of the first example, suppose I want to create a global meta discovery for all my images on example.com. So on the root of my server I'll put some meta tags using example.com as prefix: ```html <!-- ACIs with specific version --> <meta name="ac-discovery" content="example.com https://storage.example.com/{name}-{version}-{os}-{arch}.{ext}"> <meta name="ac-discovery" content="example.com https://mirror.storage.example.com/{name}-{version}-{os}-{arch}.{ext}"> <!-- noarch ACIs --> <meta name="ac-discovery" content="example.com https://storage.example.com/{name}-{version}-noarch.{ext}"> <meta name="ac-discovery" content="example.com https://mirror.storage.example.com/{name}-{version}-noarch.{ext}"> <!-- Latest ACIs --> <meta name="ac-discovery" content="example.com https://storage.example.com/{name}-latest-{os}-{arch}.{ext}"> <meta name="ac-discovery" content="example.com https://mirror.storage.example.com/{name}-latest-{os}-{arch}.{ext}"> <!-- Latest noarch ACIs --> <meta name="ac-discovery" content="example.com https://storage.example.com/{name}-latest-noarch.{ext}"> <meta name="ac-discovery" content="example.com https://mirror.storage.example.com/{name}-latest-noarch.{ext}"> ``` With this tags I want to implement global "latest" and "noarch" patterns and also return multiple mirrors. If, on discovery, I ask only for the _name_ and _version_ labels the template 3 and 4 will be rendered. So I achieved a versioned noarch pattern. But, unfortunately, also the last two templates will be rendered. And, as the first example, if I'm asking for a specific _name_, _version_, _os_ and _arch_ ALL the templates will be rendered. Since the spec says: ``` Note that multiple `ac-discovery` tags MAY be returned for a given prefix-match [snip] In this case, the client implementation MAY choose which to use at its own discretion. ``` If an implementation chooses the second it can download the wrong image version. This patch makes template matching stricter choosing only best matching templates. Best matching templates are the one where all the templates labels can be substituted and with the highest number of substituted labels. With this we can obtain various patterns like latest and noarch and also keeping the ability to return multiple mirror urls. See also the tests added in this commit. It also documents this behavior. It should not BREAK many of the current meta tags implementation (it depends on how the client uses the returned endpoints, for example rkt, currently, only uses the first one)
1 parent 80ab01d commit 8d0262c

File tree

5 files changed

+195
-14
lines changed

5 files changed

+195
-14
lines changed

‎discovery/discovery.go‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -107,20 +107,26 @@ func extractACMeta(r io.Reader) []acMeta {
107107
}
108108
}
109109

110-
func renderTemplate(tpl string, kvs ...string) (string, bool) {
110+
func renderTemplate(tpl string, kvs ...string) (string, int, bool) {
111+
count := 0
111112
for i := 0; i < len(kvs); i += 2 {
112113
k := kvs[i]
113114
v := kvs[i+1]
115+
if k != "{name}" && strings.Contains(tpl, k) {
116+
count++
117+
}
114118
tpl = strings.Replace(tpl, k, v, -1)
115119
}
116-
return tpl, !templateExpression.MatchString(tpl)
120+
return tpl, count, !templateExpression.MatchString(tpl)
117121
}
118122

119123
func createTemplateVars(app App) []string {
120124
tplVars := []string{"{name}", app.Name.String()}
121-
// If a label is called "name", it will be ignored as it appears after
122-
// in the slice
125+
// Ignore labels called "name"
123126
for n, v := range app.Labels {
127+
if n == "name" {
128+
continue
129+
}
124130
tplVars = append(tplVars, fmt.Sprintf("{%s}", n), v)
125131
}
126132
return tplVars
@@ -144,6 +150,7 @@ func doDiscover(pre string, hostHeaders map[string]http.Header, app App, insecur
144150

145151
dd := &discoveryData{}
146152

153+
bestCount := 0
147154
for _, m := range meta {
148155
if !strings.HasPrefix(app.Name.String(), m.prefix) {
149156
continue
@@ -152,16 +159,22 @@ func doDiscover(pre string, hostHeaders map[string]http.Header, app App, insecur
152159
switch m.name {
153160
case "ac-discovery":
154161
// Ignore not handled variables as {ext} isn't already rendered.
155-
uri, _ := renderTemplate(m.uri, tplVars...)
156-
asc, ok := renderTemplate(uri, "{ext}", "aci.asc")
162+
uri, count, _ := renderTemplate(m.uri, tplVars...)
163+
asc, _, ok := renderTemplate(uri, "{ext}", "aci.asc")
157164
if !ok {
158165
continue
159166
}
160-
aci, ok := renderTemplate(uri, "{ext}", "aci")
167+
aci, _, ok := renderTemplate(uri, "{ext}", "aci")
161168
if !ok {
162169
continue
163170
}
164-
dd.ACIEndpoints = append(dd.ACIEndpoints, ACIEndpoint{ACI: aci, ASC: asc})
171+
172+
if count > bestCount {
173+
dd.ACIEndpoints = ACIEndpoints{ACIEndpoint{ACI: aci, ASC: asc}}
174+
bestCount = count
175+
} else if count == bestCount {
176+
dd.ACIEndpoints = append(dd.ACIEndpoints, ACIEndpoint{ACI: aci, ASC: asc})
177+
}
165178

166179
case "ac-discovery-pubkeys":
167180
dd.PublicKeys = append(dd.PublicKeys, m.uri)

‎discovery/discovery_test.go‎

Lines changed: 137 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,7 @@ func TestDiscoverEndpoints(t *testing.T) {
461461
nil,
462462
},
463463
// Test multiple ACIEndpoints.
464+
// Should render the first two endpoint, since the others match less labels
464465
{
465466
&mockHTTPDoer{
466467
doer: fakeHTTPGet(
@@ -488,12 +489,144 @@ func TestDiscoverEndpoints(t *testing.T) {
488489
ASC: "https://storage.example.com/example.com/myapp-1.0.0-linux-amd64.aci.asc",
489490
},
490491
ACIEndpoint{
491-
ACI: "https://storage.example.com/example.com/myapp-1.0.0.aci",
492-
ASC: "https://storage.example.com/example.com/myapp-1.0.0.aci.asc",
492+
ACI: "https://mirror.storage.example.com/example.com/myapp-1.0.0-linux-amd64.aci",
493+
ASC: "https://mirror.storage.example.com/example.com/myapp-1.0.0-linux-amd64.aci.asc",
494+
},
495+
},
496+
[]string{"https://example.com/pubkeys.gpg"},
497+
nil,
498+
},
499+
// Test multiple ACIEndpoints.
500+
// Should render endpoint 3 and 4, since the 1 and 2 doesn't fully render and the others match less labels
501+
// Example noarch versioned matching
502+
{
503+
&mockHTTPDoer{
504+
doer: fakeHTTPGet(
505+
[]meta{
506+
{"/myapp",
507+
"meta06.html",
508+
},
509+
},
510+
nil,
511+
),
512+
},
513+
true,
514+
true,
515+
App{
516+
Name: "example.com/myapp",
517+
Labels: map[types.ACIdentifier]string{
518+
"version": "1.0.0",
519+
},
520+
},
521+
[]ACIEndpoint{
522+
ACIEndpoint{
523+
ACI: "https://storage.example.com/example.com/myapp-1.0.0-noarch.aci",
524+
ASC: "https://storage.example.com/example.com/myapp-1.0.0-noarch.aci.asc",
525+
},
526+
ACIEndpoint{
527+
ACI: "https://mirror.storage.example.com/example.com/myapp-1.0.0-noarch.aci",
528+
ASC: "https://mirror.storage.example.com/example.com/myapp-1.0.0-noarch.aci.asc",
529+
},
530+
},
531+
[]string{"https://example.com/pubkeys.gpg"},
532+
nil,
533+
},
534+
// Test multiple ACIEndpoints.
535+
// Should render endpoint 5 and 6, since the 1, 2, 3, 4 doesn't fully render and the others match less labels
536+
// Example latest matching
537+
{
538+
&mockHTTPDoer{
539+
doer: fakeHTTPGet(
540+
[]meta{
541+
{"/myapp",
542+
"meta06.html",
543+
},
544+
},
545+
nil,
546+
),
547+
},
548+
true,
549+
true,
550+
App{
551+
Name: "example.com/myapp",
552+
Labels: map[types.ACIdentifier]string{
553+
"os": "linux",
554+
"arch": "amd64",
555+
},
556+
},
557+
[]ACIEndpoint{
558+
ACIEndpoint{
559+
ACI: "https://storage.example.com/example.com/myapp-latest-linux-amd64.aci",
560+
ASC: "https://storage.example.com/example.com/myapp-latest-linux-amd64.aci.asc",
561+
},
562+
ACIEndpoint{
563+
ACI: "https://mirror.storage.example.com/example.com/myapp-latest-linux-amd64.aci",
564+
ASC: "https://mirror.storage.example.com/example.com/myapp-latest-linux-amd64.aci.asc",
565+
},
566+
},
567+
[]string{"https://example.com/pubkeys.gpg"},
568+
nil,
569+
},
570+
// Test multiple ACIEndpoints.
571+
// Should render endpoint 7 and 8, since the others don't fully render.
572+
// Example noarch latest matching
573+
{
574+
&mockHTTPDoer{
575+
doer: fakeHTTPGet(
576+
[]meta{
577+
{"/myapp",
578+
"meta06.html",
579+
},
580+
},
581+
nil,
582+
),
583+
},
584+
true,
585+
true,
586+
App{
587+
Name: "example.com/myapp",
588+
Labels: map[types.ACIdentifier]string{},
589+
},
590+
[]ACIEndpoint{
591+
ACIEndpoint{
592+
ACI: "https://storage.example.com/example.com/myapp-latest-noarch.aci",
593+
ASC: "https://storage.example.com/example.com/myapp-latest-noarch.aci.asc",
493594
},
494595
ACIEndpoint{
495-
ACI: "hdfs://storage.example.com/example.com/myapp-1.0.0-linux-amd64.aci",
496-
ASC: "hdfs://storage.example.com/example.com/myapp-1.0.0-linux-amd64.aci.asc",
596+
ACI: "https://mirror.storage.example.com/example.com/myapp-latest-noarch.aci",
597+
ASC: "https://mirror.storage.example.com/example.com/myapp-latest-noarch.aci.asc",
598+
},
599+
},
600+
[]string{"https://example.com/pubkeys.gpg"},
601+
nil,
602+
},
603+
604+
// Test a discovery string that has an hardcoded app name instead of using the provided {name}
605+
{
606+
&mockHTTPDoer{
607+
doer: fakeHTTPGet(
608+
[]meta{
609+
{"/myapp",
610+
"meta07.html",
611+
},
612+
},
613+
nil,
614+
),
615+
},
616+
true,
617+
true,
618+
App{
619+
Name: "example.com/myapp",
620+
Labels: map[types.ACIdentifier]string{
621+
"version": "1.0.0",
622+
"os": "linux",
623+
"arch": "amd64",
624+
},
625+
},
626+
[]ACIEndpoint{
627+
ACIEndpoint{
628+
ACI: "https://storage.example.com/myapp-1.0.0-linux-amd64.aci",
629+
ASC: "https://storage.example.com/myapp-1.0.0-linux-amd64.aci.asc",
497630
},
498631
},
499632
[]string{"https://example.com/pubkeys.gpg"},

‎discovery/testdata/meta06.html‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,19 @@
33
<html>
44
<head>
55
<title>My app</title>
6+
<!-- ACIs with specific version -->
67
<meta name="ac-discovery" content="example.com https://storage.example.com/{name}-{version}-{os}-{arch}.{ext}">
7-
<meta name="ac-discovery" content="example.com https://storage.example.com/{name}-{version}.{ext}">
8-
<meta name="ac-discovery" content="example.com hdfs://storage.example.com/{name}-{version}-{os}-{arch}.{ext}">
8+
<meta name="ac-discovery" content="example.com https://mirror.storage.example.com/{name}-{version}-{os}-{arch}.{ext}">
9+
<!-- noarch ACIs -->
10+
<meta name="ac-discovery" content="example.com https://storage.example.com/{name}-{version}-noarch.{ext}">
11+
<meta name="ac-discovery" content="example.com https://mirror.storage.example.com/{name}-{version}-noarch.{ext}">
12+
<!-- Latest ACIs -->
13+
<meta name="ac-discovery" content="example.com https://storage.example.com/{name}-latest-{os}-{arch}.{ext}">
14+
<meta name="ac-discovery" content="example.com https://mirror.storage.example.com/{name}-latest-{os}-{arch}.{ext}">
15+
<!-- Latest noarch ACIs -->
16+
<meta name="ac-discovery" content="example.com https://storage.example.com/{name}-latest-noarch.{ext}">
17+
<meta name="ac-discovery" content="example.com https://mirror.storage.example.com/{name}-latest-noarch.{ext}">
18+
919
<meta name="ac-discovery-pubkeys" content="example.com https://example.com/pubkeys.gpg">
1020
</head>
1121

‎discovery/testdata/meta07.html‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<!DOCTYPE html>
2+
3+
<html>
4+
<head>
5+
<title>My app</title>
6+
<meta name="ac-discovery" content="example.com/myapp https://storage.example.com/myapp-{version}-{os}-{arch}.{ext}">
7+
<meta name="ac-discovery-pubkeys" content="example.com https://example.com/pubkeys.gpg">
8+
</head>
9+
10+
<body>
11+
<h1>My App</h1>
12+
</body>
13+
</html>

‎spec/discovery.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,18 @@ curl $(echo "$urltmpl" | sed -e "s/{name}/$name/" -e "s/{version}/$version/" -e
5959

6060
where _name_, _version_, _os_, and _arch_ are set to their respective values for the image, and _ext_ is either `aci` or `aci.asc` for retrieving an App Container Image or signature respectively.
6161

62+
The client MUST accept only best matching templates. Best matching templates are the one where all the templates labels can be substituted and with the highest number of substituted labels.
63+
64+
For example given these meta tags:
65+
```html
66+
<meta name="ac-discovery" content="example.com https://storage.example.com/{os}/{arch}/{name}-{version}.{ext}">
67+
<meta name="ac-discovery" content="example.com https://mirror.storage.example.com/{os}/{arch}/{name}-{version}.{ext}">
68+
<meta name="ac-discovery" content="example.com https://storage.example.com/{os}/{arch}/{name}-latest.{ext}">
69+
```
70+
71+
If the client requires the image name _name_ and labels _version_, _os_, _arch_ only the first two templates will be rendered since they match 3 labels (while the the last matches only 2 labels since it doesn't match the _version_ label).
72+
If the client requires the image name _name_ and labels _os_, _arch_ only the last template will be rendered since in the first template '{version}' cannot be substituted.
73+
6274
Note that multiple `ac-discovery` tags MAY be returned for a given prefix-match (for example, with different scheme names representing different transport mechanisms).
6375
In this case, the client implementation MAY choose which to use at its own discretion.
6476
Public discovery implementations SHOULD always provide at least one HTTPS URL template.

0 commit comments

Comments
 (0)