discovery: return only best matching templates. #596
Conversation
sgotti
force-pushed
the
discovery_return_only_best_matching_templates
branch
2 times, most recently
from
5b7134f
to
8d0262c
Compare
| &mockHTTPDoer{ | ||
| doer: fakeHTTPGet( | ||
| []meta{ | ||
| {"/myapp", |
There was a problem hiding this comment.
formatting is a little weird (elsewhere too) - newline before " ?
|
Great write-up 👏. I mostly agree with your diagnosis and suggestion, just added a few comments on the details. |
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)
sgotti
force-pushed
the
discovery_return_only_best_matching_templates
branch
from
8d0262c
to
d5f8a46
Compare
|
@jonboulle Thanks. I'm on the fence between:
IMHO the former is stricter and better (since it'll fail discovery if a template doesn't contain some client provided labels) but I've the fear that it'll break some current implementations (perhaps this is a pointless fear). I'm open to change it back to #580 behavior (changing this PR since it provides more tests) if you are ok. |
|
@sgotti If we were to expand the We could supply optional default values for missing variables. This allows for #580 (which I agree is better than a "best match" which could easily result in invalid URLs) while also avoiding the need to maintain many different tags to cover all the permutations. Additionally, while I agree in principle that all client supplied labels should be substituted, there's no way to know from the tag what those variables are (without implementing the RFC and parsing the URL). Another useful extension would be: Where the required fields would just be ones present and without default values. Taken together, here's Example 2 above: This reduces the eight required tags to two, plus gives you OS-independent permutations. It also allows the client to quickly find usable URLs just by checking the |
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:
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:
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:
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)
This is also needed to implement #575 second proposal (remove the "hidden" behavior that sets a version label to "latest" if not provided)