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

Option to not inject defaults in objects defined in the pkg/name package #527

Closed
smukherj1 opened this issue Sep 13, 2019 · 5 comments · Fixed by #532
Closed

Option to not inject defaults in objects defined in the pkg/name package #527

smukherj1 opened this issue Sep 13, 2019 · 5 comments · Fixed by #532
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@smukherj1
Copy link
Contributor

For Tag and Reference created in the pkg/name package using NewTag and ParseReference, for image strings that don't specify the registry name or name the repository in a certain way, some default values defined here and here get injected into the generated objects. I would like the ability to specify an option to the Tag & Reference creator functions such that they never insert any defaults. So if an image name like foo/bar:latest is specified, I would prefer the Registry entry in the created object to be left blank instead of defaulting it to docker.io. The default behavior of these functions can still be inserting these values if necessary.

@smukherj1
Copy link
Contributor Author

This is currently blocking bazelbuild/rules_docker#580

@jonjohnsonjr
Copy link
Collaborator

A couple options:

  1. Add a name.Option to not add the defaulted values when we call String()
  2. Expose something like func Original() string on Reference that returns the input string, so you can avoid throwing away the input.

Either one of these should also apply to references of the form <repo>:<tag>@<digest>. These are currently transformed into just <repo>@<digest> (the tag is thrown away), but we should be able to pass through the tag.

@jonjohnsonjr jonjohnsonjr added good first issue Good for newcomers help wanted Extra attention is needed labels Sep 13, 2019
@smukherj1
Copy link
Contributor Author

A couple options:

  1. Add a name.Option to not add the defaulted values when we call String()
  2. Expose something like func Original() string on Reference that returns the input string, so you can avoid throwing away the input.

Either one of these should also apply to references of the form <repo>:<tag>@<digest>. These are currently transformed into just <repo>@<digest> (the tag is thrown away), but we should be able to pass through the tag.

If we decide to go with 2, the v1.tarball package will need an option to use the Original function instead of the String function when generating the image manifest for the image tarball.

@jonjohnsonjr
Copy link
Collaborator

If we decide to go with 2, the v1.tarball package will need an option to use the Original function instead of the String function when generating the image manifest for the image tarball.

I think that's reasonable default behavior, since we should be able to parse either form.

@jonjohnsonjr
Copy link
Collaborator

Alright, so looking at the interface for name.Reference:

// Reference defines the interface that consumers use when they can
// take either a tag or a digest.
type Reference interface {
	fmt.Stringer

	// Context accesses the Repository context of the reference.
	Context() Repository

	// Identifier accesses the type-specific portion of the reference.
	Identifier() string

	// Name is the fully-qualified reference name.
	Name() string

	// Scope is the scope needed to access this reference.
	Scope(string) string
}

It seems like Name should be the defaulted thing, and String should be the original input. Right now Name and String are just the same thing. We could add a new method that preserves the input or just change String to return the original input. We might be able to fix this as a happy side-effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants