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

Define the representation for a tag name. #256

Merged
merged 2 commits into from
Jun 17, 2021

Conversation

briandealwis
Copy link
Contributor

A tag is defined in the spec as a manifest identifier, but the spec does not define the acceptable representations for a tag. This PR provides a definition for the tag name based on the historical description from docker tag.

There was no reference to definitions for regular expressions, and other examples used basic regexs, so I didn't use character classes or bounds limits.

Motivated by #52

@jonjohnsonjr
Copy link
Contributor

We might want to be a little more explicit around what the audience of the limitation. Clients need to know the allowed character set and maximum safe length for tags. Registries need to know the minimum length they need to support.

Clients: (1,128]

Registries: [128,Inf)

So... in order for clients to be able to use at most 128 characters, registries should support at least 128 characters.

We could also sidestep some of these questions with something like:

Throughout this document, `<reference>` must match the [grammar for digests](https://github.com/opencontainers/image-spec/blob/v1.0.1/descriptor.md#digests) or the following regular expression for tags:

`[\w][\w.-]{0,127}`

Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

see comment regarding tag name as a term.

@briandealwis
Copy link
Contributor Author

Oops sorry: these comments ended up in my hidden in my personal email.

@jonjohnsonjr I'm a bit hesitant:

  • character set: should be brought in by reference to other standards documents such as HTTP (which defines that the URI and header block must be processed as octets encoded as a superset of US-ASCII).
  • regex: there needs to be a reference to a specific form of regexs as POSIX regular expressions don't have \w, and RFCs usually use Augmented Backus-Naur Form (ABNF) notation specified in RFC2234. But your regexp showed me that my own regex was incorrect (ok for a tag to start with underscore).
  • registries should support at least 128 characters: I'm a bit confused as 128,inf seems to imply that a registry must support tags that have a minimum of 128 characters?

@jonjohnsonjr
Copy link
Contributor

seems to imply that a registry must support tags that have a minimum of 128 characters?

If registries don't support at least 128 characters in a tag, then they will be "broken" from the perspective of a client that expects to be able to use up to 128 characters.

It's basically Postel's Law: clients need to be conservative in what they do, and registries should be liberal in what they accept.

Is that a good idea? Who knows. The bare minimum, IMO, is that registries should support up to 128 characters. Given that clients are probably not prepared to deal with tags longer than 128 characters, maybe registries should also not support longer tags than that, but it seems a little bit limiting...

Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM

@jonjohnsonjr
Copy link
Contributor

LGTM but I'm not a maintainer :)

Copy link
Contributor

@waynr waynr left a comment

Choose a reason for hiding this comment

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

Looks good to me since it answers a question I brought up in slack. Not sure if I'm a maintainer on this project though -- probably not.

@vbatts
Copy link
Member

vbatts commented Jun 16, 2021

@briandealwis your commit are missing DCO signature

Co-authored-by: João Pereira <[email protected]>
Signed-off-by: Brian de Alwis <[email protected]>
@briandealwis
Copy link
Contributor Author

Added a sign-off.

Copy link
Member

@vbatts vbatts left a comment

Choose a reason for hiding this comment

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

🐙

@briandealwis
Copy link
Contributor Author

Approval required by 1 of: @caniszczyk, @crosbymichael, @dmcgowan, @jdolitsky, @jzelinskie, @mikebrow, @stevvooe

Copy link
Member

@jzelinskie jzelinskie left a comment

Choose a reason for hiding this comment

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

Looks really good in terms of keeping things surgical.

@vbatts vbatts merged commit ef28f81 into opencontainers:main Jun 17, 2021
@jdolitsky jdolitsky mentioned this pull request Sep 15, 2022
@sudo-bmitch sudo-bmitch mentioned this pull request Feb 1, 2024
8 tasks
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.

8 participants