projectatomic / atomic

Atomic Run Tool for installing/running/managing container images.
Other
525 stars 139 forks source link

(atomic sign) and (skopeo copy) are inconsistent WRT Docker sigstore configuration #594

Closed mtrmac closed 8 years ago

mtrmac commented 8 years ago

Per https://github.com/projectatomic/atomic/pull/539#discussion_r75361262 , the sigstore configuration is done differently in atomic sign and skopeo copy, i.e. skopeo copy won't see signatures created by atomic sign.

We need to make these consistent; it is not as important how, but we do need to do that.


Currently, AFAICS:

Within the first match (only), which should be a dictionary, it uses the field sigstore-write if writing and it is defined, or sigstore (for either reading or writing). The value of the field is an URL to the top directory for the sigstore to be used for these images (which still uses the standard layout, i.e. $sigstore_dir/hostname/ns1/ns2/...).

If nothing matches, the default is no sigstore

See https://github.com/mtrmac/skopeo/blob/integrate-all-the-things/integration/fixtures/atomic.conf for an example of such a configuration.


Rationale for the more complex containers/image design:

mtrmac commented 8 years ago

cc @baude @rhatdan @aweiteka

aweiteka commented 8 years ago

Seems like a no-brainer to use containers/image mechanism.

aweiteka commented 8 years ago

/etc/atomic.conf should just have global configuration. /etc/containers/registries.d should have yaml files configuring each registry

aweiteka commented 8 years ago

Flat list of yaml files concatenated with registry[/repo] as keys.

mtrmac commented 8 years ago

containers/image would, to be vendor-neutral, perhaps not referring to /etc/atomic.conf (though I wouldn’t particularly mind).

lookaside.go currently accepts an "" key (YAML for empty string) to mean “global default”; that is logically consistent but pretty ugly.

Just using default would clash with the host name default, a confusing design bad for security.

Posible options:

… actually the last one seems pretty attractive. It is the least amount of work and the need for a global default is unclear to me. We can add a solution later if needed.

baude commented 8 years ago

@mtrmac @aweiteka can you drop some example files that would land in /etc/containers/registry.d/. Two or more?

mtrmac commented 8 years ago

@baude AFAICS https://github.com/mtrmac/skopeo/blob/e25d93fe391364602267d451b7f931c1f4c5d0c8/integration/fixtures/atomic.conf would still work. s/localhost:5555/docker.io/g on this to get a second example; (edit) Also test docker.io/ns1/ns2/ns3/ns4/ns5 (and prefixes), and docker.io/ns1/ns2/ns3/ns4/ns5:notlatest (or someone stop me and tell me that per-tag configuration is silly; I kinda feel it is but I like the symmetry)

baude commented 8 years ago

so each yaml file should have a top level registries yes?

baude commented 8 years ago

@mtrmac do you have an example for when it should be http instead of file.

mtrmac commented 8 years ago

@baude just s,@split-read@,http://some.host[:port]/path,? per https://github.com/mtrmac/skopeo/blob/e25d93fe391364602267d451b7f931c1f4c5d0c8/integration/copy_test.go#L285 and the surrounding code.

(Not that I’m saying the test is a canonical reference, we are changing the design now; but sigstore: http://… should be boringly unsurprising.)

mtrmac commented 8 years ago

so each yaml file should have a top level registries yes? I wouldn’t mind but I don’t think it is necessary after all; we can either use "" or make the default a special case with a special config file.

baude commented 8 years ago

for now, can we make that assumption? each file has a top level registries: ? i think that makes the logic simpler; however, I'm open to change. Lets just be definitive.

mtrmac commented 8 years ago

Sure, doesn’t hurt.

(registries/repositories? registry = host name; repository = lowest-level namespace IIUC; the intermediate namespaces are not named in docker/distribution v2. Whatever.)

baude commented 8 years ago

@mtrmac can you clarify this last comment? I dont quite follow, examples are helpful,.

mtrmac commented 8 years ago

(So that I don’t forget, a late thought:

Actually, I think the top-level key in those files should be named docker; we don't have anything else in /etc/containers saying "docker distribution API" and we may need to support other distribution protocols. If /etc/containers is technology-agnostic, something needs to say which technology a specific file or specific sigstore scope is talking about.)

aweiteka commented 8 years ago

I think the top-level key in those files should be named docker

I'm good with that. As I understand it, concatenating these files into a single data structure would throw this key away or create a list of all the "docker" items.

mtrmac commented 8 years ago

So, to make it specific, like this?

These would go into /etc/containers/registries.d as *.yaml.

I will also write a documentation page, to be included in containers/image.

internal-example.com.yaml.txt internet-user.yaml.txt

mtrmac commented 8 years ago

(Disclaimer: I don’t have an implementation, may include stupid typos and unimplementable design )

mtrmac commented 8 years ago

https://github.com/containers/image/pull/52 now implements this, format documentation in https://github.com/mtrmac/image/blob/docker-lookaside/docs/registries.d.md (to become https://github.com/containers/image/blob/master/docs/registries.d.md ).

rhatdan commented 8 years ago

Are we aligned now? Can I close this issue?

rhatdan commented 8 years ago

@baude @mtrmac Please comment?

baude commented 8 years ago

I believe we are