On Fri, 2015-05-15 at 15:54 +0200, Lennart Poettering wrote: > On Thu, 07.05.15 17:47, Pavel Odvody (podv...@redhat.com) wrote: > > > > > #define HEADER_TOKEN "X-Do" /* the HTTP header for the auth token */ > > "cker-Token:" > > -#define HEADER_REGISTRY "X-Do" /*the HTTP header for the registry */ > > "cker-Endpoints:" > > +#define HEADER_REGISTRY "X-Do" /* the HTTP header for the registry */ > > "cker-Endpoints:" > > +#define HEADER_DIGEST "Do" /* the HTTP header for the manifest digest */ > > "cker-Content-Digest:" > > +#define HEADER_USER_AGENT_V2 "User-Agent: do" /* otherwise we get > > load-balanced(!) to a V1 registyry */ "cker/1.6.0" > > I hope we can get rid of this. I'd prefer if we wouldn't have to > pretend to be other software.
As of today we can, kudos to Vincent for pushing this. > > > +#define HEADER_BEARER_REALM "https://auth.doc"; /* URL which we query for a > > bearer token */ "ker.io/token" > > +#define HEADER_BEARER_SERVICE "registry.doc" /* the service we > > query the token for */ "ker.io" > > These two are problematic, we really shouldn't hardcode them. First of > all users might want to use other servers here. More importantly > though, to my knowledge the service agreements of the company > providing those server does not allow usage like this. IANAL, but I > really would prefer staying out of any legal issues here. We can make > the defaults for this configurable via ./configure switches but we > should not carry these URLs by default. If downstream distros are > willing to take the legal risk of encoding the URLs by default then, > they are welcome to via the configure switches, but upstream I really > don't want to see that... > > Also, can you quickly explain how the three servers relate to each > other? Is this documented anywhere? > I'm currently in the process of rewriting these to use the same domain name as is passed in --dkr-index-url, so they'll look like: ${PROTOCOL}auth.${ADDRESS}/[v2]/token and registry.${ADDRESS} respectively. These are somehow documented here: https://docs.docker.com/registry/spec/auth/token/ > > > > + if (strategy == PULL_V2) { > > + char **k = NULL; > > + STRV_FOREACH(k, i->ancestry) { > > + char *d = strjoina(i->image_root, "/.dkr-", > > *k, NULL); > > Using alloca() (which strjoina() uses internally) within loops is not > OK, since deallacation happens at the end of the function call, not at > the end of the {} block. Oh, didn't know that, will change. > > + > > + r = json_parse((const char *)j->payload, &doc); > > Why the cast? void, and non-const automatically upgrade to const and > any type... I think that j->payload is uint8_t* and the function expects const char*, what do you thing about changing the signature of json_parse to int json_parse(const void* payload, size_t size, json_variant **rv); ? > > > > > +enum PullStrategy { PULL_V1, PULL_V2 }; > > So far we typedef'ed enums for exported types, and prefixed both with > a namespacing prefix. Also, why call this a "strategy"? Does dkr > upstream call it that way? Shouldn't this be "protocol" or so? > > typedef enum DkrProtocolVersion { > DKR_PROTOCOL_V1, > DKR_PROTOCOL_V2, > } DkrProtocolVersion; Nope, entirely my choice of a name. Protocol sounds more like switching between HTTP/S, FTP or something, while the underlying network protocols has stayed the same, only the "strategy" how the image is pulled has changed. But I have no problem with changing that. > > > + > > +typedef struct DkrSignature { > > + char *curve; > > + > > + char *key_id; > > + char *key_type; > > + > > + char *x; > > + char *y; > > + > > + char *algorithm; > > + > > + char *signature; > > + char *protected; > > +} DkrSignature; > > This is not used so far, is it? > > > + > > +//typedef struct DkrHistory { > > +// char *image_id; > > +// char *parent_id; > > +//} DkrHistory; > > > And neither is this? Also, we don't use C++ comments in our code, > i.e. /* this */ is preferred over // this... Yep, I'll get rid of these. > > Lennart > -- Pavel Odvody <podv...@redhat.com> Software Engineer - EMEA ENG Developer Experience 5EC1 95C1 8E08 5BD9 9BBF 9241 3AFA 3A66 024F F68D Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno
signature.asc
Description: This is a digitally signed message part
_______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
