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

Commit d2793c7

Browse files
committed
discovery: match only templates that satisfy all required labels
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}"> <!-- noarch ACIs --> <meta name="ac-discovery" content="example.com https://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://storage.example.com/{name}-latest.{ext}"> ``` With this tags I want to implement a "latest" and also "noarch" pattern. If, on discovery, I ask only for the _name_ and _version_ labels the second template will be rendered. So I achieved noarch pattern. But, unfortunately, also the last template 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 template that fully satisfy the required labels. It can probably BREAK some of the current meta tags implementations. It also documents this behavior.
1 parent 80ab01d commit d2793c7

File tree

4 files changed

+70
-11
lines changed

4 files changed

+70
-11
lines changed

discovery/discovery.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,16 +111,21 @@ func renderTemplate(tpl string, kvs ...string) (string, bool) {
111111
for i := 0; i < len(kvs); i += 2 {
112112
k := kvs[i]
113113
v := kvs[i+1]
114+
if k != "{name}" && !strings.Contains(tpl, k) {
115+
return tpl, false
116+
}
114117
tpl = strings.Replace(tpl, k, v, -1)
115118
}
116119
return tpl, !templateExpression.MatchString(tpl)
117120
}
118121

119122
func createTemplateVars(app App) []string {
120123
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
124+
// Ignore labels called "name"
123125
for n, v := range app.Labels {
126+
if n == "name" {
127+
continue
128+
}
124129
tplVars = append(tplVars, fmt.Sprintf("{%s}", n), v)
125130
}
126131
return tplVars
@@ -151,13 +156,14 @@ func doDiscover(pre string, hostHeaders map[string]http.Header, app App, insecur
151156

152157
switch m.name {
153158
case "ac-discovery":
154-
// Ignore not handled variables as {ext} isn't already rendered.
155-
uri, _ := renderTemplate(m.uri, tplVars...)
156-
asc, ok := renderTemplate(uri, "{ext}", "aci.asc")
159+
// Ignore not handled variables as only {ext} has been rendered
160+
aci, _ := renderTemplate(m.uri, "{ext}", "aci")
161+
asc, _ := renderTemplate(m.uri, "{ext}", "aci.asc")
162+
aci, ok := renderTemplate(aci, tplVars...)
157163
if !ok {
158164
continue
159165
}
160-
aci, ok := renderTemplate(uri, "{ext}", "aci")
166+
asc, ok = renderTemplate(asc, tplVars...)
161167
if !ok {
162168
continue
163169
}

discovery/discovery_test.go

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,9 @@ func TestDiscoverEndpoints(t *testing.T) {
460460
[]string{"https://example.com/pubkeys.gpg"},
461461
nil,
462462
},
463-
// Test multiple ACIEndpoints.
463+
// Test multiple endpoints.
464+
// Should render two endpoint, since '<meta name="ac-discovery" content="example.com https://storage.example.com/{name}-{version}.{ext}">'
465+
// doesn't contain all the required labels
464466
{
465467
&mockHTTPDoer{
466468
doer: fakeHTTPGet(
@@ -487,10 +489,6 @@ func TestDiscoverEndpoints(t *testing.T) {
487489
ACI: "https://storage.example.com/example.com/myapp-1.0.0-linux-amd64.aci",
488490
ASC: "https://storage.example.com/example.com/myapp-1.0.0-linux-amd64.aci.asc",
489491
},
490-
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",
493-
},
494492
ACIEndpoint{
495493
ACI: "hdfs://storage.example.com/example.com/myapp-1.0.0-linux-amd64.aci",
496494
ASC: "hdfs://storage.example.com/example.com/myapp-1.0.0-linux-amd64.aci.asc",
@@ -499,6 +497,37 @@ func TestDiscoverEndpoints(t *testing.T) {
499497
[]string{"https://example.com/pubkeys.gpg"},
500498
nil,
501499
},
500+
// Test a discovery string that has an hardcoded app name instead of using the provided {name}
501+
{
502+
&mockHTTPDoer{
503+
doer: fakeHTTPGet(
504+
[]meta{
505+
{"/myapp",
506+
"meta07.html",
507+
},
508+
},
509+
nil,
510+
),
511+
},
512+
true,
513+
true,
514+
App{
515+
Name: "example.com/myapp",
516+
Labels: map[types.ACIdentifier]string{
517+
"version": "1.0.0",
518+
"os": "linux",
519+
"arch": "amd64",
520+
},
521+
},
522+
[]ACIEndpoint{
523+
ACIEndpoint{
524+
ACI: "https://storage.example.com/myapp-1.0.0-linux-amd64.aci",
525+
ASC: "https://storage.example.com/myapp-1.0.0-linux-amd64.aci.asc",
526+
},
527+
},
528+
[]string{"https://example.com/pubkeys.gpg"},
529+
nil,
530+
},
502531

503532
// Test with an auth header
504533
{

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: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,17 @@ 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 templates that can be fully substituted and that satisfy all the required 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://storage.example.com/{os}/{arch}/{name}-latest.{ext}">
68+
```
69+
70+
If the client requires the labels _name_, _version_, _os_, _arch_ only the first template will be rendered since the second doesn't satisfy the _version_ label.
71+
If the client requires the labels _name_, _os_, _arch_ only the second template will be rendered since in the first template '{version}' cannot be substituted.
72+
6273
Note that multiple `ac-discovery` tags MAY be returned for a given prefix-match (for example, with different scheme names representing different transport mechanisms).
6374
In this case, the client implementation MAY choose which to use at its own discretion.
6475
Public discovery implementations SHOULD always provide at least one HTTPS URL template.

0 commit comments

Comments
 (0)