Support the Semver2 standard

[wqld]

Description

What problem are you trying to solve?

Attempts to get the latest tags utilizing semver ranges via Helm or FluxCD's HelmRelease resource will fail.

This is caused by all versions of karpenter helm chart being prefixed with v.
In fact, if you look at public.ecr.aws/karpenter, you'll see the following tags.

➜  tmp TOKEN=$(curl -k https://public.ecr.aws/token/ | jq -r '.token')

➜  tmp curl -k -H "Authorization: Bearer $TOKEN" https://public.ecr.aws/v2/karpenter/karpenter/tags/list  | jq . | head
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 42196    0 42196    0     0  16310      0 --:--:--  0:00:02 --:--:-- 16304
{
  "name": "karpenter/karpenter",
  "tags": [
    "v0-69bd00df79db89a1171ef950537230e5493cd60e",
    "v0-31de5dcf837359033cf40b72db989e3b96fa46f9",
    "v0-ba600d8038747224dd55d89156e4bd65512a6000",
    "20220911",
    "20220912",
    "20220910",
    "v0-dcbb2d8de30e7c4f13670cf3bffd3d47a324d662",

Due to this version format, the helm pull command will fail to get the latest tags using semver range.

➜  tmp helm pull oci://public.ecr.aws/karpenter/karpenter --version="0.*" --debug 
Error: Unable to locate any tags in provided repository: oci://public.ecr.aws/karpenter/karpenter
helm.go:84: [debug] Unable to locate any tags in provided repository: oci://public.ecr.aws/karpenter/karpenter
helm.sh/helm/v3/pkg/downloader.(*ChartDownloader).getOciURI
	helm.sh/helm/v3/pkg/downloader/chart_downloader.go:159
helm.sh/helm/v3/pkg/downloader.(*ChartDownloader).ResolveChartVersion
	helm.sh/helm/v3/pkg/downloader/chart_downloader.go:199
helm.sh/helm/v3/pkg/downloader.(*ChartDownloader).DownloadTo
	helm.sh/helm/v3/pkg/downloader/chart_downloader.go:90
helm.sh/helm/v3/pkg/action.(*Pull).Run
	helm.sh/helm/v3/pkg/action/pull.go:131
main.newPullCmd.func2
	helm.sh/helm/v3/cmd/helm/pull.go:74
github.com/spf13/cobra.(*Command).execute
	github.com/spf13/[email protected]/command.go:916
github.com/spf13/cobra.(*Command).ExecuteC
	github.com/spf13/[email protected]/command.go:1044
github.com/spf13/cobra.(*Command).Execute
	github.com/spf13/[email protected]/command.go:968
main.main
	helm.sh/helm/v3/cmd/helm/helm.go:83
runtime.main
	runtime/proc.go:250
runtime.goexit
	runtime/asm_amd64.s:1598

The tag list we get via the tags/list endpoint is 1576,

registryTags - []string len: 1576, cap: 2048, ["v0-69bd00df79db89a1171ef950537230e5493cd60e","v0-31de5dcf837359033cf40b72db989e3b96fa46f9","v0-ba600d8038747224dd55d89156e4bd65512a6000","20220911","20220912","20220910","v0-dcbb2d8de30e7c4f13670cf3bffd3d47a324d662","v0-49fac0ca5a5d4b98e7e5801df2890c9e044fe99c","v0-80e60d8a9a737b2a8dc0aaf43140f9928db8c337","v0-49a318ecfbc59e79d69115ac8dc1c9429616b671","v0-5fc55df4f570d8ccbb6b503dca37bc35d6d2d405","v0-a255b82814e6c94e0e933d23ed5cd76126408770","v0-cdf4fa69b339742ae0429267845fa16918627338","v0-9cbaa76d3c6c0ecbc23d46ebc1693bb1d2ad5418","v0-6217d62d1d199bd61e252b9a55f9539430efaeff","20221116","v0-3ead1d92f7b827a30b851929e3832761cde9641e","v0-b828a83db3fce5c5ad7c28581d68cd6c7aa6314d","v0-36ebd55249f736f2bb5c13777dc0a5656e338639","v0-111c114b6dfe7e8403a2c16e98d656ba6ad807c3","v0-62b1069f25b30280d22e8aa99c4d7695b4fb38e7","v0-558393be817833ed7b4b9359259e76027bf6e522","v0-33bfe9e960dd65c808ec410256b9b170a43e9e53","20220808","20220807","20220806","v0-b1c1a9da8b34a68801e5c6b5c15fc24054c09983","20220923","v0-1e6c5d9b3bf87c21eaaeb7a78248c2db407dee96","v0-8e92f731f0bd16894e949fd0254f7c9631579360","20221107","v0-30ce02a5a20398bf9b624fdd899a48f0987c27b9","v0-d4ec73075788514eaa7a6d6ea8957a7204ca5cec","v0-5a49e01bba57fa1ba82733b87853c9644ed5e56b","v0-582ca837b7b976689ca33b100bb531aa9de6097f","v0-c5a2044907258346afc879f7bf7867f9e15a1bb4","v0-6bdd21387902c14cb4f7be252558fbffbe654a6b","v0-fa3a89accab4f7f7d3973078f1f513ec85022674","v0-471148a26513e25b02aa3f32a51116490ed6feab","v0-4ff98b06de9d27a0ec8cd7ff6a5d65b5050d90bf","v0-109295bb3be995f1487afc402298bf278f768433","v0-5333fb247531c60038d5b3b9aae45a406b257b28","v0-5661ebef509dbfca0bc2e8529311e2ebcdc9adb5","v0-2a93373ed6d3f8717b336060f78a699655a8be3a","v0-a35dd1e56418e9ac92aaff15096197ab748450fb","v0-470aa83476d121f9215d7a209d6725b2d0e11738","v0-125b96fce06ab4e10b8f56f7c6cca5cfc9eb4556","v0-a34da16c8e3b60ba6c380b7a03248d3557ea24b9","v0-2e34488038bdf346dd407051c617f2954a517f66","v0-0b25c60640757741d733c45eb9bbc7160a7d62b1","v0-4300212384bb7a051d82e11b61d1b0831e33868c","v0-117f007c50646d57825387d47ebcea9b10f769b0","v0-698f22f5c07ba1cef4ffb5d80dee482d37b717c0","v0-7657e823b38af57393ddf1ede4447055779787ca","v0-e9b6a72e003b81da3ee51ce7850947b3ea19391a","v0-d43c9b2cac2b4ae98199e80e4660244646690284","v0-2f949e5a1be3877fb6a932598054638f406164e2","v0-448cb6662c5d53e5cfd6751c8da696ac9faca9eb","v0-958b40484560affe9883c838d80cc8891f212f91","v0-f2a49d45b76ac113e74adbf6a7cb778083a71b9a","v0.18.0","v0-1eeb010f80397e2f8ff412a377f9b18a782bc681","v0-717adebe9c6aa6294a3f52ae34f097af90a2cddf","v0-653b456f6b0156009f82db3389d46dd24802bf3c",...+1512 more]

However, since HELM's implementation looks like this, none of the valid semvers are filtered out.

var tagVersions []*semver.Version
for _, tag := range registryTags {
	// Change underscore (_) back to plus (+) for Helm
	// See https://github.com/helm/helm/issues/10166
	tagVersion, err := semver.StrictNewVersion(strings.ReplaceAll(tag, "_", "+"))
	if err == nil {
		tagVersions = append(tagVersions, tagVersion)
	}
}

"Invalid characters in version."

tagVersions - []*github.com/Masterminds/semver/v3.Version len: 0, cap: 0, nil

How important is this feature to you?

https://helm.sh/docs/topics/charts/#charts-and-versioning

Every chart must have a version number. A version must follow the SemVer 2 standard. Unlike Helm Classic, Helm v2 and later uses version numbers as release markers. Packages in repositories are identified by name plus version.

To keep track of the latest tags using semver ranges, why not use the available semvers for the StrictNewVersion method and upload them to the public ecr repo?

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

[stevehipwell]

Looks like a duplicate of #4248. We'd definitely support an update where we push both tags. I'm not sure that we can completely drop the old version. Does helm not work if we support both tags in our ECR?

@jonathan-innis (reply from #5305) I'm specifically talking about fixing the release process so future releases are correctly versioned. AFAIK the OCI tag needs to match the chart version, which should be a valid SemVer 2 version; so you could release each chart twice but I'm not sure what the value would be considering the current pattern is actually incorrect (and doesn't match the actual chart file in the repo).

[jonathan-innis]

so you could release each chart twice but I'm not sure what the value would be considering the current pattern is actually incorrect (and doesn't match the actual chart file in the repo).

I'm fine with swapping this over on newer versions of karpenter with some sort of breaking change notice, but the important thing to note here is that this will absolutely break anyone who has any logic that is relying on the handling of that v prefix in the convention.

Another important question to me here (that I haven't had a chance to test yet) is if updating to using this new convention would fix the helm version parsing logic. To my question from the other ticket, will helm parsing work fine if there are other tags in the repo that hold the v prefix and don't follow SemVer while there are some that do.

[mikestef9]

Updating to the strict format does work in an existing repo. ACK made this switch a couple months back. You can see example by looking at history of example chart here https://gallery.ecr.aws/aws-controllers-k8s/eks-chart.

Edit: Looking closer at the tag history, it looks like we backported releases in the correct format, so this needs some more investigation.

[stevehipwell]

I'm fine with swapping this over on newer versions of karpenter with some sort of breaking change notice, but the important thing to note here is that this will absolutely break anyone who has any logic that is relying on the handling of that v prefix in the convention.

@jonathan-innis that's the benefit of fixing this while karpenter is still at 0.x.x, SemVer breaking changes should be expected.

Another important question to me here (that I haven't had a chance to test yet) is if updating to using this new convention would fix the helm version parsing logic. To my question from the other ticket, will helm parsing work fine if there are other tags in the repo that hold the v prefix and don't follow SemVer while there are some that do.

I can confirm that helm pull oci://public.ecr.aws/aws-controllers-k8s/eks-chart works correctly and there are other tags present there (thanks @mikestef9).

I think it'd be worth backporting tags for Karpenter versions using the beta APIs.

[jonathan-innis]

Makes sense. I agree that we should ideally do this during v1 with a breaking change warning in our Upgrade Guide docs about the change.

I'm not sure this is something that someone on our team would have immediate bandwidth to do. @stevehipwell is this something that you would be willing to help out with? I don't suspect that the changes here would be too large.

[stevehipwell]

@jonathan-innis I'd be happy to pick this up.

[jonathan-innis]

Sounds great @stevehipwell. I've assigned you to it! Looking forward to a PR!

[jonathan-innis]

Linking an issue where someone hit a problem with this when using Flux: fluxcd/helm-controller#670