schemas: add remote-manifest-v0

[lucab]

This introduces a new schema to describe a remote via JSON manifest,
adding support for remote sources for addons.
This will be consumed in the future to automatically populate
the local store from a remote source.

[lucab]

Assuming #105 is at its final round, this is the next one queued to get in.

@squeed @euank rebased on latest profile-v1, PTAL.

[euank]

I can understand if you don't want to add a dependency on a small unknown project, but I do think https://github.com/euank/gotmpl would be less repetitive feeling and fit this almost perfectly.

I think it would just end up as:

_, err := gotmpl.TemplateString(r.TemplateURL, gotmpl.MapLookup(map[string]string{}))
if err == nil {
   return r.TemplateURL, nil // no variables
}
if _, ok := err.(gotmpl.UnresolvedVariableError); !ok {
    return err // something wrong besides needing vars
}

return gotmpl.TemplateString(r.TemplateURL, gotmpl.MapLookup(templateVariableMap))

It also has the benefit of allowing users to escape $ characters if that's desirable, though realistically few urls will contain them anyways.

If you don't want to pull in a dependency that can do simple substitution, I do think this code would be shorter and more readable if it were generic (e.g. split into a pkg and just templated the k/v of a map, didn't go through the trouble of only substitution detected variables, etc).

[lucab]

You are right, it's silly to keep this generic logic in here.

[euank]

This looks reasonable to me, though I'm not much a fan of the templating code.

I think it may end up being easier to review this in conjunction with the 'fetch' logic for the well-known manifest at base url, especially if the majority template code were moved into its own package and handled separately.

[squeed]

would really like to see a template test here.

[lucab]

@squeed un-hardcoded the os-release path logic and added a test to exercise all template variables.

@euank vendored gotmpl and got rid of the previous poor substitution logic.

Rebased, PTAL.

[lucab]

@euank ping, I'm leaving this open for a chance in case you want to have a final pass on this. Let me know if otherwise.

[euank]

I don't understand the value of exposing the usrMountpoint to the template logic.

I understand it's needed by this function to find the appropriate version (e.g. if /usr is mounted by update_engine at /tmp/au_postint_mount.xxx, it obviously needs to read os-release from there), but I don't see how the remote url would ever need to care about this detail.

Did you have any specific cases or use in mind? If there's not a specific use-case compelling this, I'd rather not support it.

[lucab]

This is something that I missed in the original design doc, so here is the full rationale. The current and upcoming USR partition both carry a local remote (com.coreos.cl). The manifest for that will contain a URL like file:///usr/share/torcx/remotes/.... Such URL works for the current USR, but at update-engine time we also need to inspect the remote coming with next-OS, which is mounted under a temporary dir. Options we have are:

• introduce a template variable, and point the URL to file://${COREOS_USR}/share/torcx/...
• manually replace /usr when processing the URL from com.coreos.cl

Both are a bit of corner cases, but I felt that the first one would be a bit less surprising.

[euank]

Thanks for explaining, I think it makes sense to have that template variable now.

However, I think the base_url bit needs to also include file as a supported protocol, and ideally should have an example explaining this possibility (if only to say 'this is supported, but generally should only be used by pseudo-remotes distributed by the creator/vendor of /usr').

I think having that clarification could have also resolved my confusion.

[lucab]

Indeed. I'll fix and add a couple of explicit notes to remote-manifest-v0.md.

[euank]

Other than the question above, this looks good to me, but I would like to resolve that question before merging.

[lucab]

@euank I've clarified the docs in that regard.

[euank]

LGTM.

A few more tests might be nice, but I think once the fetch logic is added it may be easier to test this code via testing fetch, so adding those in upcoming PRs seems fine to me.