This repository was archived by the owner on Aug 14, 2020. It is now read-only.
Open
Conversation
sgotti
force-pushed
the
discovery_do_not_set_default_version
branch
3 times, most recently
from
2fe5633 to
b9859ac
Compare
This was referenced Mar 25, 2016
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 template 5, 6, 7, and 8 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)
Actually, if the required discovery labels doesn't contain a version label, it'll be set to a default of "latest". This behavior isn't documented in the spec and can create unwanted behaviors. This patch removes this and also adds to the spec an example on how to obtain the latest behavior without this "hidden" default.
sgotti
force-pushed
the
discovery_do_not_set_default_version
branch
from
b9859ac to
13ab4e0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The relevant commit is the latest one.
This is the second part that tries to implement #575 second proposal (remove the "hidden" behavior that sets a version label to "latest" if not provided)
discovery: do not set version to latest if not provide
Actually, if the required discovery labels doesn't contain a version
label, it'll be set to a default of "latest". This behavior isn't
documented in the spec and can create unwanted behaviors.
This patch removes this and also adds to the spec an example on how to
obtain the latest behavior without this "hidden" default.