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

Allow to customize publish.Namer #476

Closed
cardil opened this issue Oct 13, 2021 · 2 comments · Fixed by #477
Closed

Allow to customize publish.Namer #476

cardil opened this issue Oct 13, 2021 · 2 comments · Fixed by #477

Comments

@cardil
Copy link
Contributor

cardil commented Oct 13, 2021

Problem

Ko has a number of built-in publish.Namer's, but despite its public type, the code doesn't allow to influence it. Also, the built-in namers, all, use the path.Join which is uses / char, which often prevents using google/ko with container registries that don't allow nested directories - a majority.

https://github.com/google/ko/blob/6447264ff8b5d48aff64000f81bb0847aefc7bac/pkg/commands/options/publish.go#L119-L128

Possible solutions

I think a specific function should be used instead of path.Join, and maybe it should use some environmental variable to allow overriding default delimiter (/ char). It also seems to be good idea to allow providing completely custom implementation for users.

@imjasonh
Copy link
Member

imjasonh commented Oct 13, 2021

The / path separator is used because it's the only one that's valid in a container image name. We want to use that separator regardless of what platform is being used to run ko.

There have been some discussions in the past about making the image name templatized/scriptable from the commandline, but I don't think we've ever landed on something we're happy with. If you have ideas about how you'd like this to look, that could be helpful.

In the meantime, you could pass your own Namer implementation to publish.NewDefault using publish.WithNamer, rather than using the options provided via PublishOptions.

edit to add:

which often prevents using google/ko with container registries that don't allow nested directories - a majority

This is largely why the default naming option for ko is to push to an image reference that doesn't rely on nested directories -- e.g., registry.io/username/app-[md5]. The --preserve-import-path flag is the only one that depends on the registry supporting nested directories.

cardil added a commit to cardil/google-ko that referenced this issue Oct 13, 2021
@cardil
Copy link
Contributor Author

cardil commented Oct 13, 2021

That's not true, that / is the only viable option. Users could like to use - or _ as a separator.

By default, ko builds image name $KO_DOCKER_REPO/app-<md5>. That prevents from using KO, or forces people to create small organizations to keep everything organized.

Imagine you have an app called foo who has 3 different images, for ex.: sender, receiver, pipe. And the other app called bar with 3 images called the same. You'd like to use a single organization to serve them on docker hub (or quay.io, ghcr), as they are part of the same project for ex.: "awesome". In this example, you'll set KO_DOCKER_REPO=docker.io/awesome and you'll end up with the following list of images:

docker.io/awesome/sender-17c0ebc6dcdd02d82b046fd6352c0bda
docker.io/awesome/sender-2ecc83ec58bfab019a2fc204663af8e5
docker.io/awesome/receiver-29e0aa969da0e157fdb05e3aa0ead686
docker.io/awesome/receiver-e03ffa047a8d6d8d636e44324a141ac3
docker.io/awesome/pipe-75a8f51b34c90496cd742e62bf7f83d6
docker.io/awesome/pipe-6b9a56782ffad9cf85f35641df4c4ace

It's really hard to tell which ones belong to which project. Also, you can't use --base-import-paths because of naming conflicts.

Of course, you could walk around the issue by managing 2 organizations on docker hub: awesome-foo, awesome-bar, but that's cumbersome - not viable in practice.

With my proposal, (see #477) you could use different separator, and set KO_DOCKER_REPO appropriately while deploying. In the example above, that might be:

$ KO_DOCKER_REPO=docker.io/awesome/foo ko publish --image-name-separator='-' --base-import-paths ./foo/...
$ KO_DOCKER_REPO=docker.io/awesome/bar ko publish --image-name-separator='-' --base-import-paths ./bar/...

You would end up with the following images (much better IMHO):

docker.io/awesome/foo-sender
docker.io/awesome/foo-receiver
docker.io/awesome/foo-pipe
docker.io/awesome/bar-sender
docker.io/awesome/bar-receiver
docker.io/awesome/bar-pipe

cardil added a commit to wavesoftware/go-magetasks that referenced this issue Oct 14, 2021
cardil added a commit to wavesoftware/go-magetasks that referenced this issue Oct 14, 2021
* A rework of the library structure
* Project builds and passes golangci-lint
* Binary builds are working
* In fact build for various platforms.
* Tweaking the output
* KO build works, tests as well
* Basic KO publisher
* Publishing works
* LDflags for images works
* Minor UI fixes
* Remove changes for ko-build/ko#476
cardil added a commit to wavesoftware/go-magetasks that referenced this issue Oct 14, 2021
jonjohnsonjr pushed a commit that referenced this issue Dec 14, 2021
* Allow to customize publish.Namer

Fixes #476

* Removing a CLI flag for path separator

* Removing ImageNameSeparator option after review
cardil added a commit to wavesoftware/go-magetasks that referenced this issue Dec 15, 2021
* Apply changes for ko-build/ko#476

This reverts commit 6b07449.

* Adjusting code to  outcome of ko-build/ko#477
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 a pull request may close this issue.

2 participants