feat: globalEnvs for KO_DOCKER_REPO, annotations and labels for the image config based on build ids #632
Conversation
Codecov Report
@@ Coverage Diff @@
## main #632 +/- ##
==========================================
- Coverage 50.41% 50.17% -0.25%
==========================================
Files 44 44
Lines 3338 3392 +54
==========================================
+ Hits 1683 1702 +19
- Misses 1445 1475 +30
- Partials 210 215 +5
Continue to review full report at Codecov.
|
| Annotations map[string]string `yaml:",omitempty"` | ||
| Labels map[string]string `yaml:",omitempty"` |
There was a problem hiding this comment.
Should these just go in Config alongside Env etc.? It seems like a meaningless separation.
The only problem with that is if goreleaser adds a Labels or Annotations field to its config, we'd have a different meaning. I'm starting to really regret copying goreleaser's config so closely. 😢
There was a problem hiding this comment.
IMHO, we should separate them from a build because these are related to the image we provide at the end of that build. So, it might cause a bit more confusion if we put them into the Config section. In the current design, users should match the id fields of the builds and images sections to be able to define labels and annotations per image related to that build, WDYT?
There was a problem hiding this comment.
Oh I see, I was confusing build.env (what envs to set when we go build) vs image.env (what envs to set in the resulting image).
In that case, yeah, I agree we should have a separate image config struct with labels/annotations.
We should also use this to fix #633, with a top-level defaultImageConfig: { } option, that gets overridden by more specific images: and both get overridden if there's a conflicting --image-label specified.
WDYT?
There was a problem hiding this comment.
Oh yeah, we should apply some kind of prioritization between them, that makes a lot of sense 🙋🏻♂️
Here are the prioritization rules:
- The most important one is --image-label, if there is a conflict between others, we should use the defined one by the --image-label flag.
- The second important one is the images section within the .ko.yaml, so, if we specify the more specific one inside of that section, we should override the defaultImageConfig.
- The defaultImageConfig has the lowest priority among others. We should use the values within the defaultImageConfig unless others (images:, --image-label) do not specify any values at all.
There was a problem hiding this comment.
it should be okay @imjasonh 🙋🏻♂️
pkg/commands/apply.go
Outdated
| @@ -96,7 +96,7 @@ func addApply(topLevel *cobra.Command) { | |||
| if err != nil { | |||
| return fmt.Errorf("error creating builder: %w", err) | |||
| } | |||
| publisher, err := makePublisher(po) | |||
| publisher, err := makePublisher(po, bo.Envs) | |||
There was a problem hiding this comment.
Passing a build option to makePublisher is a bit of a smell... Should it be a publish.Option? 🤔
There was a problem hiding this comment.
Yeah, I've updated the code like this 🙋🏻♂️
pkg/commands/options/build.go
Outdated
| ImageConfigs map[string]build.ImageConfig `yaml:",omitempty"` | ||
|
|
||
| // Envs stores the global environment variables from `.ko.yaml`. | ||
| Envs map[string]string `yaml:",omitempty"` |
There was a problem hiding this comment.
(grr GitHub ate my comment here, twice!)
This might be confusing because it kinda sounds like these envs will end up getting set in the resulting image(s), like builds[id].env does today.
At the same time, we've only gotten the request so far to set KO_DOCKER_REPO if the env is unset. Maybe just make this dockerRepo string and use it if it's set and KO_DOCKER_REPO isn't.
(We'll also need to update docs, and the error message when KO_DOCKER_REPO is not set)
There was a problem hiding this comment.
okay, makes sense to me, I've updated the code according to your comments 🙋🏻♂️
| if bo.DockerRepo != "" { | ||
| po.DockerRepo = bo.DockerRepo | ||
| } |
There was a problem hiding this comment.
Bleh, this also doesn't feel right. Sorry to keep moving things around on you, I'm struggling to find a good way to express this without having multiple copies of this floating around.
There was a problem hiding this comment.
What about creating an overrider function to set and pass some variables from builder to publisher struct?
makeOverrider(bo, po)I'm not sure if overrider is the right word but used it as a placeholder.
| @@ -122,6 +122,14 @@ func gobuildOptions(bo *options.BuildOptions) ([]build.Option, error) { | |||
| opts = append(opts, build.WithConfig(bo.BuildConfigs)) | |||
| } | |||
|
|
|||
| if bo.ImageConfigs != nil { | |||
| opts = append(opts, build.WithImageConfig(bo.ImageConfigs)) | |||
There was a problem hiding this comment.
What if WithImageConfig just handled nil? Then we could opts = append(opts, build.WithImageConfig(bo.ImageConfigs)) no matter what. (same with defaultImageConfig)
…mage config based on build ids Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com> add annts Signed-off-by: Furkan <furkan.turkal@trendyol.com> Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com> add test Signed-off-by: Furkan <furkan.turkal@trendyol.com> Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com>
|
kindly ping 🙋🏻♂️ |
|
kindly ping @imjasonh 🙋🏻♂️ |
|
kindly ping @imjasonh |
There was a problem hiding this comment.
This looks okay to me. I don't terribly love the duplication of DockerRepo in build options and publish options, but 🤷 I don't have any better ideas either.
@jonjohnsonjr how do you feel?
(And as always, sorry this sat for so long, that's entirely my fault)
|
waiting for your feedback @jonjohnsonjr before resolving the conflicts 🙋🏻♂️ |
|
This Pull Request is stale because it has been open for 90 days with |
Fixes #627 #626 #633
Signed-off-by: Batuhan Apaydın batuhan.apaydin@trendyol.com
I tested it with the following .ko.yaml
.ko.yaml
The result of this build is
devopps/ko:latestimage.So, if you run
crane manifest devopps/ko:latest, you will see the following contentcrane manifest devopps/ko:latest
{ "schemaVersion": 2, "mediaType": "application/vnd.docker.distribution.manifest.v2+json", "config": { "mediaType": "application/vnd.docker.container.image.v1+json", "size": 7042, "digest": "sha256:979aad7f56f9b0e1a05d54f9dab13fd3d35bda7791b7a5b7fb3ba161c1cd51c8" }, "layers": [ { "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", "size": 54917063, "digest": "sha256:e4d61adff2077d048c6372d73c41b0bd68f525ad41f5530af05098a876683055" }, { "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", "size": 5153034, "digest": "sha256:4ff1945c672b08a1791df62afaaf8aff14d3047155365f9c3646902937f7ffe6" }, { "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", "size": 10871885, "digest": "sha256:ff5b10aec998344606441aec43a335ab6326f32aae331aab27da16a6bb4ec2be" }, { "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", "size": 54575903, "digest": "sha256:12de8c754e45686ace9e25d11bee372b070eed5b5ab20aa3b4fab8c936496d02" }, { "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", "size": 85811039, "digest": "sha256:8c86ff77a3175ed4d7958578d141a96b5da005855d60ea24067de33cd62e4c36" }, { "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", "size": 134830189, "digest": "sha256:5d440ea1ae12a9af6fe55f378a73940c4c2ce8cb576ab3f1a14a91e3f67ca795" }, { "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", "size": 156, "digest": "sha256:450a2e80688e351e0946f174309b2adc7dfb469e09bdbfa82c5569572c40b3ee" }, { "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", "size": 127, "digest": "sha256:250c06f7c38e52dc77e5c7586c3e40280dc7ff9bb9007c396e06d96736cf8542" }, { "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", "size": 14663122, "digest": "sha256:1f2b8377a8c108670f93f96b0323dfcd4e3b00b03f46ca074d06cd7a8d1d3039" } ], "annotations": { "baz": "qux", "foo": "bar" } }Also, you should see the labels:
crane config devopps/ko:latest
{ "architecture": "amd64", "author": "github.com/google/ko", "container": "7c453da5aaec5bf9219390cd100f2ff53004cb959b0e6dfb441ec4cf35f9ea18", "created": "2022-03-02T17:58:32.365615991Z", "docker_version": "20.10.12", "history": [ { "created": "2022-03-01T02:13:15.223116269Z", "created_by": "/bin/sh -c #(nop) ADD file:9c4db2a9644ee3029a8e9cca58350efef660c3167e59b91f2bee9c303e592664 in / " }, { "created": "2022-03-01T02:13:15.66141909Z", "created_by": "/bin/sh -c #(nop) CMD [\"bash\"]", "empty_layer": true }, { "created": "2022-03-01T06:25:59.186625638Z", "created_by": "/bin/sh -c set -eux; \tapt-get update; \tapt-get install -y --no-install-recommends \t\tca-certificates \t\tcurl \t\tnetbase \t\twget \t; \trm -rf /var/lib/apt/lists/*" }, { "created": "2022-03-01T06:26:07.562013445Z", "created_by": "/bin/sh -c set -ex; \tif ! command -v gpg > /dev/null; then \t\tapt-get update; \t\tapt-get install -y --no-install-recommends \t\t\tgnupg \t\t\tdirmngr \t\t; \t\trm -rf /var/lib/apt/lists/*; \tfi" }, { "created": "2022-03-01T06:26:30.039901037Z", "created_by": "/bin/sh -c apt-get update && apt-get install -y --no-install-recommends \t\tgit \t\tmercurial \t\topenssh-client \t\tsubversion \t\t\t\tprocps \t&& rm -rf /var/lib/apt/lists/*" }, { "created": "2022-03-02T17:51:27.512538204Z", "created_by": "/bin/sh -c set -eux; \tapt-get update; \tapt-get install -y --no-install-recommends \t\tg++ \t\tgcc \t\tlibc6-dev \t\tmake \t\tpkg-config \t; \trm -rf /var/lib/apt/lists/*" }, { "created": "2022-03-02T17:51:28.327282667Z", "created_by": "/bin/sh -c #(nop) ENV PATH=/usr/local/go/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "empty_layer": true }, { "created": "2022-03-02T17:56:57.3112007Z", "created_by": "/bin/sh -c #(nop) ENV GOLANG_VERSION=1.17.7", "empty_layer": true }, { "created": "2022-03-02T17:58:29.610351379Z", "created_by": "/bin/sh -c set -eux; \tarch=\"$(dpkg --print-architecture)\"; arch=\"${arch##*-}\"; \turl=; \tcase \"$arch\" in \t\t'amd64') \t\t\turl='https://dl.google.com/go/go1.17.7.linux-amd64.tar.gz'; \t\t\tsha256='02b111284bedbfa35a7e5b74a06082d18632eff824fd144312f6063943d49259'; \t\t\t;; \t\t'armel') \t\t\texport GOARCH='arm' GOARM='5' GOOS='linux'; \t\t\t;; \t\t'armhf') \t\t\turl='https://dl.google.com/go/go1.17.7.linux-armv6l.tar.gz'; \t\t\tsha256='874774f078b182fa21ffcb3878467eb5cb7e78bbffa6343ea5f0fbe47983433b'; \t\t\t;; \t\t'arm64') \t\t\turl='https://dl.google.com/go/go1.17.7.linux-arm64.tar.gz'; \t\t\tsha256='a5aa1ed17d45ee1d58b4a4099b12f8942acbd1dd09b2e9a6abb1c4898043c5f5'; \t\t\t;; \t\t'i386') \t\t\turl='https://dl.google.com/go/go1.17.7.linux-386.tar.gz'; \t\t\tsha256='5d5472672a2e0252fe31f4ec30583d9f2b320f9b9296eda430f03cbc848400ce'; \t\t\t;; \t\t'mips64el') \t\t\texport GOARCH='mips64le' GOOS='linux'; \t\t\t;; \t\t'ppc64el') \t\t\turl='https://dl.google.com/go/go1.17.7.linux-ppc64le.tar.gz'; \t\t\tsha256='2262fdee9147eb61fd1e719cfd19b9c035009c14890de02b5a77071b0a577405'; \t\t\t;; \t\t's390x') \t\t\turl='https://dl.google.com/go/go1.17.7.linux-s390x.tar.gz'; \t\t\tsha256='24dd117581d592f52b4cf45d75ae68a6a1e42691a8671a2d3c2ddd739894a1e4'; \t\t\t;; \t\t*) echo >&2 \"error: unsupported architecture '$arch' (likely packaging update needed)\"; exit 1 ;; \tesac; \tbuild=; \tif [ -z \"$url\" ]; then \t\tbuild=1; \t\turl='https://dl.google.com/go/go1.17.7.src.tar.gz'; \t\tsha256='c108cd33b73b1911a02b697741df3dea43e01a5c4e08e409e8b3a0e3745d2b4d'; \t\techo >&2; \t\techo >&2 \"warning: current architecture ($arch) does not have a compatible Go binary release; will be building from source\"; \t\techo >&2; \tfi; \t\twget -O go.tgz.asc \"$url.asc\"; \twget -O go.tgz \"$url\" --progress=dot:giga; \techo \"$sha256 *go.tgz\" | sha256sum -c -; \t\tGNUPGHOME=\"$(mktemp -d)\"; export GNUPGHOME; \tgpg --batch --keyserver keyserver.ubuntu.com --recv-keys 'EB4C 1BFD 4F04 2F6D DDCC EC91 7721 F63B D38B 4796'; \tgpg --batch --keyserver keyserver.ubuntu.com --recv-keys '2F52 8D36 D67B 69ED F998 D857 78BD 6547 3CB3 BD13'; \tgpg --batch --verify go.tgz.asc go.tgz; \tgpgconf --kill all; \trm -rf \"$GNUPGHOME\" go.tgz.asc; \t\ttar -C /usr/local -xzf go.tgz; \trm go.tgz; \t\tif [ -n \"$build\" ]; then \t\tsavedAptMark=\"$(apt-mark showmanual)\"; \t\tapt-get update; \t\tapt-get install -y --no-install-recommends golang-go; \t\t\t\t( \t\t\tcd /usr/local/go/src; \t\t\texport GOROOT_BOOTSTRAP=\"$(go env GOROOT)\" GOHOSTOS=\"$GOOS\" GOHOSTARCH=\"$GOARCH\"; \t\t\t./make.bash; \t\t); \t\t\t\tapt-mark auto '.*' > /dev/null; \t\tapt-mark manual $savedAptMark > /dev/null; \t\tapt-get purge -y --auto-remove -o APT::AutoRemove::RecommendsImportant=false; \t\trm -rf /var/lib/apt/lists/*; \t\t\t\trm -rf \t\t\t/usr/local/go/pkg/*/cmd \t\t\t/usr/local/go/pkg/bootstrap \t\t\t/usr/local/go/pkg/obj \t\t\t/usr/local/go/pkg/tool/*/api \t\t\t/usr/local/go/pkg/tool/*/go_bootstrap \t\t\t/usr/local/go/src/cmd/dist/dist \t\t; \tfi; \t\tgo version" }, { "created": "2022-03-02T17:58:31.655597521Z", "created_by": "/bin/sh -c #(nop) ENV GOPATH=/go", "empty_layer": true }, { "created": "2022-03-02T17:58:31.745160845Z", "created_by": "/bin/sh -c #(nop) ENV PATH=/go/bin:/usr/local/go/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "empty_layer": true }, { "created": "2022-03-02T17:58:32.267355753Z", "created_by": "/bin/sh -c mkdir -p \"$GOPATH/src\" \"$GOPATH/bin\" && chmod -R 777 \"$GOPATH\"" }, { "created": "2022-03-02T17:58:32.365615991Z", "created_by": "/bin/sh -c #(nop) WORKDIR /go", "empty_layer": true }, { "author": "ko", "created": "0001-01-01T00:00:00Z", "created_by": "ko build ko://github.com/google/ko", "comment": "kodata contents, at $KO_DATA_PATH" }, { "author": "ko", "created": "0001-01-01T00:00:00Z", "created_by": "ko build ko://github.com/google/ko", "comment": "go build output, at /ko-app/ko" } ], "os": "linux", "rootfs": { "type": "layers", "diff_ids": [ "sha256:89fda00479fc0fe3bf2c411d92432001870e9dad42ddd0c53715ab77ac4f2a97", "sha256:26d5108b2cba762ee9b91c30670091458a0c32b02132620b7f844085af596e22", "sha256:48144a6f44ae89c578bd705dba2ebdb2a086b36215affa5659b854308fa22e4b", "sha256:e3f84a8cee1f3e6a38a463251eb05b87a444dec565a7331217c145e9ef4dd192", "sha256:6c14e994de3a023aafde08b658f3d1ca16f9d27666acc9656a1b58acd532301b", "sha256:c75cdf6ea05c65d3baa91ca77a0401387deebf5804b7ceb3a6170d94c39b9280", "sha256:e4a7c99829b9b2459f75bf259f83bf1ad0e9f3064f8070720624a01bcbe70813", "sha256:ffe56a1c5f3878e9b5f803842adb9e2ce81584b6bd027e8599582aefe14a975b", "sha256:767044b3be88d30738fc7d76988b0632c2ee7d93a5689a96a89ff15532e5ceaf" ] }, "config": { "Entrypoint": [ "/ko-app/ko" ], "Env": [ "PATH=/go/bin:/usr/local/go/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/ko-app", "GOLANG_VERSION=1.17.7", "GOPATH=/go", "KO_DATA_PATH=/var/run/ko" ], "Image": "sha256:5b4a155828610ad5a5fb8c15113cd0365f196acd1e9a7a0951a773bf4f40643c", "Labels": { "io.artifacthub.package.readme-url": "https://raw.githubusercontent.com/sigstore/cosign/main/README.md", "org.opencontainers.image.licenses": "batu", "org.opencontainers.image.source": "https://github.com/google/ko" }, "WorkingDir": "/go" } }