refactor(docker): respect image settings, use vals, customizable registry protocol#128
Conversation
56e5b83 to
aa9b5d6
Compare
…stry protocol
This is a bunch of fixes together to cover a single use case, the one expressed in the included test: multiple images pushed to different destinations with a single script.
Image registry, name and tag can use `vals`, just like the rest of Kubenix, to expand dynamic or secret references on run time.
BREAKING CHANGE: `docker.copyScript` now puts a binary inside `$out/bin/kubenix-push-images`. The benefit is that now you can just `nix run .#kubenix.config.docker.copyScript`, which is more ergonomic.
BREAKING CHANGE: Registry definitions, which were never a URL, are no longer set in a `url` option. Instead of using `docker.registry.url` and `docker.images.*.registry` options are now consolidated into `[docker.registry|docker.images.*.registry].registry.{protocol,host}`, which behave as you'd expect.
BREAKING CHANGE: `docker.copyScript` push images to `docker.images.*.uri` instead of a handmade combination of the `imageName` and `imageTag` passthru attributes of the `docker.images.*.image` derivation.
@moduon MT-1075
adrian-gierakowski
left a comment
There was a problem hiding this comment.
BREAKING CHANGE: docker.copyScript now puts a binary inside $out/bin/kubenix-push-images. The benefit is that now you can just nix run .#kubenix.config.docker.copyScript, which is more ergonomic.
What did it do previously?
Can we avoid the other breaking changes?
| runtimeInputs = [ | ||
| pkgs.gzip | ||
| pkgs.skopeo | ||
| pkgs.vals |
There was a problem hiding this comment.
Can we hide the use of vals behind a flag? I personally don't use them and would prefer not to be force to download this dep
There was a problem hiding this comment.
I do use vals to be able to generate a URI that:
- Can be generated in pure nix.
- Can be escaped in Bash.
- Can be filled at runtime automatically.
My use case is a docker image pushed with a variable name that depends on the pipeline I'm running to push it. Example: docker.io/img:pipeline-1234
If you have another suggestion to get that, I can change it. But if the concern is just disk space, keep in mind you already have vals if you use kubenix for k8s:
Line 30 in 4b46e69
So you won't be saving any disk space by removing vals, unless you don't use kubenix for k8s.
There was a problem hiding this comment.
unless you don't use kubenix for k8s
Yeah, I don't use kubenix to deploy, I render manifests into a repo and use agrocd to deploy. Would appreciate if you could add a flag which could be used to disable vals, thanks!
| registryUrl = mkOption { | ||
| description = "Docker registry url"; | ||
| registryHost = mkOption { | ||
| description = "Docker registry host"; |
There was a problem hiding this comment.
Can we keep url for backwards compat. Make it optional, and use it with default protocol ("docker://") when set?
There was a problem hiding this comment.
please document this in change-log, thanks!
yajo
left a comment
There was a problem hiding this comment.
BREAKING CHANGE: docker.copyScript now puts a binary inside $out/bin/kubenix-push-images. The benefit is that now you can just nix run .#kubenix.config.docker.copyScript, which is more ergonomic.
What did it do previously?
Before:
nix build .#kubenix.config.docker.copyScript && ./result
After:
nix run .#kubenix.config.docker.copyScript
Which is equivalent to:
nix build .#kubenix.config.docker.copyScript && ./result/bin/kubenix-push-images
Can we avoid the other breaking changes?
It's a bit difficult because the feature was broken before. Even if you set the image and tag in kubenix, those would be ignored when pushing, and instead the imageName and imageTag attrs of the package were used.
| runtimeInputs = [ | ||
| pkgs.gzip | ||
| pkgs.skopeo | ||
| pkgs.vals |
There was a problem hiding this comment.
I do use vals to be able to generate a URI that:
- Can be generated in pure nix.
- Can be escaped in Bash.
- Can be filled at runtime automatically.
My use case is a docker image pushed with a variable name that depends on the pipeline I'm running to push it. Example: docker.io/img:pipeline-1234
If you have another suggestion to get that, I can change it. But if the concern is just disk space, keep in mind you already have vals if you use kubenix for k8s:
Line 30 in 4b46e69
So you won't be saving any disk space by removing vals, unless you don't use kubenix for k8s.
thanks, makes sense |
| runtimeInputs = [ | ||
| pkgs.gzip | ||
| pkgs.skopeo | ||
| pkgs.vals |
There was a problem hiding this comment.
unless you don't use kubenix for k8s
Yeah, I don't use kubenix to deploy, I render manifests into a repo and use agrocd to deploy. Would appreciate if you could add a flag which could be used to disable vals, thanks!
Summary of changes: 1. **CHANGELOG.md** - Added migration instructions for breaking changes 2. **modules/docker-image-from-package.nix** - Migration module that: - Maps `docker.registry.url` to `docker.registry.host` 3. **modules/docker.nix** - Added `useVals` and `copyScriptArgs` options 4. **lib/docker/default.nix** - Added `useVals` parameter to conditionally skip `vals` expansion, and added SC2046 to excluded shell checks 5. **tests/docker/image-from-package.nix** - Test for the migration module 6. **flake.nix** - Added the new test to the checks
|
Please check now. |
|
Thanks @yajo! |
refactor(docker): respect image settings, use vals, customizable registry protocol
This is a bunch of fixes together to cover a single use case, the one expressed in the included test: multiple images pushed to different destinations with a single script.
Image registry, name and tag can use
vals, just like the rest of Kubenix, to expand dynamic or secret references on run time.BREAKING CHANGE:
docker.copyScriptnow puts a binary inside$out/bin/kubenix-push-images. The benefit is that now you can justnix run .#kubenix.config.docker.copyScript, which is more ergonomic.BREAKING CHANGE: Registry definitions, which were never a URL, are no longer set in a
urloption. Instead of usingdocker.registry.urlanddocker.images.*.registryoptions are now consolidated into[docker.registry|docker.images.*.registry].registry.{protocol,host}, which behave as you'd expect.BREAKING CHANGE:
docker.copyScriptpush images todocker.images.*.uriinstead of a handmade combination of theimageNameandimageTagpassthru attributes of thedocker.images.*.imagederivation.@moduon MT-1075