Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add some restriction to "org.opencontainers.ref.name" #591

Closed

Conversation

coolljt0725
Copy link
Member

A common use case of org.opencontainers.ref.name
is representing a tag for a container image. In docker,
there is some restriction of the tag see
https://github.com/docker/docker/blob/master/vendor/github.com/docker/distribution/reference/regexp.go#L37.
Without these restrictions, I'm afraid the tag of
oci image is invalid for docker. These restrictions used to exist but removed in
#533, removed it by accidents or by intention?

Signed-off-by: Lei Jitang [email protected]

ping @stevvooe @vbatts

A common use case of `org.opencontainers.ref.name`
is representing a `tag` for a container image. In docker,
there is some restriction of the name see
https://github.com/docker/docker/blob/master/vendor/github.com/docker/distribution/reference/regexp.go#L37.
Without these restrictions, I'm afraid the tag of
oci image is invalid for docker. These restrictions used to exist but removed in
opencontainers#533.

Signed-off-by: Lei Jitang <[email protected]>
@lucab
Copy link
Contributor

lucab commented Mar 8, 2017

Cross-reference #581 (comment)

@@ -148,6 +148,7 @@ The [image index](image-index.md) is a multi-descriptor entry point.
This index provides an established path (`/index.json`) to have an entry point for an image-layout and to discover auxiliary descriptors.

No semantic restriction is given for the "org.opencontainers.ref.name" annotation of descriptors.
The value of "org.opencontainers.ref.name" annotation is a string which MUST NOT include characters outside of the set of "A" to "Z", "a" to "z", "0" to "9", the hyphen `-`, the dot `.`, and the underscore `_`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original regexp also has some more constraints on corner-cases which are allowed by your wording:

  • empty string
  • arbitrarily-long string (> 128 chars)
  • symbol-only string (eg. ..)
  • string starting with a symbol (eg. .hidden)

@vbatts
Copy link
Member

vbatts commented Mar 8, 2017

I'm not sure this is always wanted though. It had been a request in the past for nested names, like including the "/" but not possible due to file names. It there are hard requirement for these restrictions?

@stevvooe
Copy link
Contributor

stevvooe commented Mar 8, 2017

We'll have to rethink restrictions here. These were really to support mapping to arbitrary filesystems. We have a lot more options, now that this is just a json string.

Recently, I have done some research in specification names in the context of containered. This format provides a superset of docker+rkt(based on examples) and is intended to be a lot less opinionated. It is based around schema-less URIs. Now, I am not suggesting we adopt this, but I think the approach of restricting this to the URI charset may be viable for this use case.

@stevvooe
Copy link
Contributor

stevvooe commented Mar 8, 2017

@coolljt0725 Could you please open an issue so we can discuss the way forward before we go ahead with a PR? I'll do it! :)

@stevvooe
Copy link
Contributor

stevvooe commented Mar 8, 2017

Closing in favor of addressing full scope of problem in #599.

@stevvooe stevvooe closed this Mar 8, 2017
@coolljt0725 coolljt0725 deleted the ref_name_restrict branch March 9, 2017 01:11
taqtiqa-mark added a commit to taqtiqa-mark/image-spec that referenced this pull request Apr 17, 2018
Arises where a container is a composition of several containers. See Buildah issue opencontainers#591 specifically the current use case given in [comment](containers/buildah#591 (comment))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants