Get the default version from vendored cilium/charts repo

[michi-covalent]

Get the default Cilium version from vendored cilium/charts repo so that the build is reproducible.

Fixes: #2870

[michi-covalent]

output from make:

% make
CGO_ENABLED=0 go build  \
	-ldflags "-w -s -X 'github.com/cilium/cilium/cilium-cli/defaults.CLIVersion=v0.16.20-21-gbed0f4506' -X 'github.com/cilium/cilium/cilium-cli/defaults.Version=v1.16.3'" \
	-o cilium \
	./cmd/cilium

[tklauser]

Makes sense 👍

Now that we rely on cilium/charts for the version, I think we should also update the instructions in RELEASE.md to update that dependency before the release in order to make sure it will ship with the latest Cilium release baked in.

[Foxboron]

Why can't this version be manually set as part of the release process?

[tklauser]

Why can't this version be manually set as part of the release process?

Updating github.com/cilium/charts as part of the release process will be a manual action as well and extracting the latest version from there is IMO is less error prone than requiring the developer to manually updating the version. Do you see any reason why this won't work?

[Foxboron]

Do you see any reason why this won't work?

It will. You just have an implied dependency on how vendor/github.com/cilium/charts structure their repository. Is the team aware of this inter-dependency, and what if it breaks?

This also doesn't fix the manual install process which still recommends the "curl from master" approach, so if you want to be consistent and clear to the users I don't see how you can solve this without having to manually type the version during release.

EDIT: that's stable.txt from cilium-cli, not cilium, sorry :)

[tklauser]

Do you see any reason why this won't work?

It will. You just have an implied dependency on how vendor/github.com/cilium/charts structure their repository. Is the team aware of this inter-dependency, and what if it breaks?

Yes, but that (indirect) dependency already exists anyway. And it's the Cilium community (i.e. us) controlling that repository, so I don't see a big risk in depending on its structure. I actually think the chance of forgetting to manually update the version or getting it wrong on cilium-cli release is much higher.

This also doesn't fix the manual install process which still recommends the "curl from master" approach, so if you want to be consistent and clear to the users I don't see how you can solve this without having to manually type the version during release.

EDIT: that's stable.txt from cilium-cli, not cilium, sorry :)

👍

[tklauser]

That being said, I'm not strongly opposed to changing to a manual approach as sugested. I just think it has the risk of being slightly more error-prone. But I'll let @michi-covalent decide on what approach to take here.

[Foxboron]

Yes, but that (indirect) dependency already exists anyway. And it's the Cilium community (i.e. us) controlling that repository, so I don't see a big risk in depending on its structure. I actually think the chance of forgetting to manually update the version or getting it wrong on cilium-cli release is much higher.

Generally, overly clever things like this in build systems introduces errors and edge-cases, and the time spent fixing them once bugs happen, does not end up saving you a lot of time.

This is code that I would need to bring into the Arch downstream package as well, as the correct value can't be easily obtained.

Ref: https://gitlab.archlinux.org/archlinux/packaging/packages/cilium-cli/-/merge_requests/1

That being said, I'm not strongly opposed to changing to a manual approach as sugested.

You all know the projects better than me, I'm just speaking from a downstream perspective. Feel free to do what you think is best for you project.

[michi-covalent]

thanks for the pointer https://gitlab.archlinux.org/archlinux/packaging/packages/cilium-cli/-/merge_requests/1/diffs, it helps to see how it's being built downstream.

let's just stop setting github.com/cilium/cilium/cilium-cli/defaults.Version using -ldflags. cilium-cli can just figure this out from the vendored charts, and i do want to avoid adding extra steps in the release process 🚀🙏

[Foxboron]

Here is a collection of different build scripts from other distros, for reference.

• https://build.opensuse.org/projects/openSUSE:Factory/packages/cilium-cli/files/cilium-cli.spec?expand=1
• https://github.com/NixOS/nixpkgs/blob/nixos-unstable/pkgs/by-name/ci/cilium-cli/package.nix
• https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-cluster/cilium-cli/cilium-cli-0.16.16.ebuild
• https://gitlab.alpinelinux.org/alpine/aports/-/blob/master/testing/cilium-cli/APKBUILD

[michi-covalent]

closing this one. will fix this in the cilium-cli code