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.

Sign up for GitHub

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

Jump to bottom

Chart versions prefixed with v aren't compatible with Flux semver ranges #1715

Closed
RedbackThomson opened this issue Mar 2, 2023 · 2 comments
Closed

Chart versions prefixed with v aren't compatible with Flux semver ranges #1715

RedbackThomson opened this issue Mar 2, 2023 · 2 comments
Assignees
@RedbackThomson
Labels
priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. service/all Indicates issues or PRs related to all the service controllers.

Comments

@RedbackThomson
Copy link
Contributor

Describe the bug
All of our controller charts are versioned, prefixed with a v before the semver. eg. public.ecr.aws/aws-controllers-k8s/ec2-chart:v1.0.1

The Flux Helm controller has support for Semver ranges:

The chart.spec.version can be a fixed semver, or any semver range (i.e. >=4.0.0 <5.0.0). It is ignored for HelmRelease resources that reference a GitRepository or Bucket source.

This support is not compatible with the v. Instead, the controller complains that it cannot find any images within that range.

Steps to reproduce
Create a Flux HelmRelease for one of the ACK OCI Helm charts, and use the chart.spec.version with a range (eg. >=1.0.0 <2.0.0).

Expected outcome
The controller should understand the chart versions and be able to use proper semver ranges and filtering.
@RedbackThomson RedbackThomson mentioned this issue Mar 2, 2023
Merged
@jljaco jljaco added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Mar 2, 2023
@a-hilaly a-hilaly mentioned this issue Apr 3, 2023
Closed
@jljaco jljaco added service/all Indicates issues or PRs related to all the service controllers. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Apr 12, 2023
@stefanprodan
Copy link

This is not a Flux issue, we conform with the Helm spec and we use the Helm Go SDK. Helm itself does not support the v prefix, charts should be versioned using strict semver format. Using the v prefix also breaks the Helm CLI helm/helm#11107.

@wallrj
Copy link

wallrj commented Apr 27, 2023

I regard it as a Helm bug. Helm does support the v prefix when interacting with non-oci registry. For example, the cert-manager Helm chart has a v prefix and helm install can calculate the latest semver release just fine.

$ helm repo add jetstack https://charts.jetstack.io
$ helm repo update jetstack

$ helm search repo jetstack
NAME                                    CHART VERSION   APP VERSION     DESCRIPTION
jetstack/cert-manager                   v1.11.1         v1.11.1         A Helm chart for cert-manager

$ helm install cert-manager jetstack/cert-manager --namespace cert-manager --set
installCRDs=true
NAME: cert-manager
LAST DEPLOYED: Thu Apr 27 16:58:08 2023
NAMESPACE: cert-manager
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
cert-manager v1.11.1 has been deployed successfully!

This was referenced May 3, 2023
Merged
Merged
Merged
@RedbackThomson RedbackThomson mentioned this issue May 15, 2023
Closed
ack-prow bot pushed a commit that referenced this issue May 15, 2023
@RedbackThomson
Remove references to v prefix in Helm charts (#1794)
7404eaf 
Issue #, if available:
#1715

Description of changes:
This pull request removes all references to Helm charts and images prefixed with `v`. Some parts of the release documentation still reference mono-repo style image tags. For those parts, I have partially updated them where applicable, but honestly they need to be entirely rewritten to match the current structure.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
ack-prow bot pushed a commit to aws-controllers-k8s/test-infra that referenced this issue May 15, 2023
@RedbackThomson
Remove v prefix from Helm charts and container images (#341)
2544a22 
Issue #, if available:
aws-controllers-k8s/community#1715

Description of changes:
This pull requests updates all CD scripts that release Helm charts, or rely on the versions defined within Helm charts, to remove or disregard the `v` prefix at the beginning of the Git tag. 

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
ack-prow bot pushed a commit to aws-controllers-k8s/code-generator that referenced this issue May 15, 2023
@RedbackThomson
Trim possible v prefixes in release code (#445)
8f3ba42 
Issue #, if available:
aws-controllers-k8s/community#1715

Description of changes:
This pull request updates the release Go code paths and ensures that all `ReleaseVersion` template variables do not contain the `v` prefix.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
@jljaco jljaco closed this as completed May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@RedbackThomson RedbackThomson

Labels
priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. service/all Indicates issues or PRs related to all the service controllers.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@wallrj @RedbackThomson @stefanprodan @jljaco