HIP for helm release locking #175
Conversation
| It will take a release name and chart version to lock to. | ||
| ``` | ||
| helm lock autoscaler --version '8.*' | ||
| ``` |
There was a problem hiding this comment.
Why does lock to 8.* create a lock to 8.0.* (example above)?
There was a problem hiding this comment.
I didn't mean that 8.* would become 8.0.* I was simply providing different examples of how a user could lock their chart to a major or minor version of the chart.
| NAME NAMESPACE REVISION UPDATED STATUS CHART APP VERSION | ||
| autoscaler default 1 2021-02-23 17:58:07.479425206 +0000 UTC deployed cluster-autoscaler-8.0.0 (locked 8.*) 1.17.1 | ||
| ``` | ||
| Current chart versions can be seen from the `helm show chart [CHART]` command. |
There was a problem hiding this comment.
Note that under current backward compatibility rules, this is a breaking change to add a new field to helm list's output. Would need a flag in Helm 3, and could become default in Helm 4.
There was a problem hiding this comment.
I was hoping the change would be backwards compatible but I am not familiar enough with breaking changes with helm v2. If you think it requires waiting for helm 4 that is fine. I hope it can be considered and included at some point.
|
This would probably be a candidate for Helm 4. I think there are too many backward compatibility edge cases for doing this in Helm 3. (Any changes to the release object's format or content are considered breaking b/c they are incompatible with older versions of Helm. There was discussion on why field additions are breaking here in the security report from ToB.) |
|
@rothgar the DCO bot failed. Can you sign the DCO and re-push the PR? |
| ``` | ||
| helm lock autoscaler --version '8.*' | ||
| ``` | ||
| _The version is a free form field but should be validated aganist SemVer standards with `*` wildcards_ |
There was a problem hiding this comment.
Should this use the existing range format in the SemVer handling? Adding a new wildcard format to support ranges means that we can't use the existing library for this and users have two different ways to specify ranges.
There was a problem hiding this comment.
If there's an existing library used for SemVer range handling we should use that. I was unaware of one in helm so I used examples from other package managers.
| kind: Secret | ||
| metadata: | ||
| creationTimestamp: "2021-02-23T18:07:25Z" | ||
| desiredChartVersion: "8.0.*" <---- this field is added |
There was a problem hiding this comment.
The field desiredChartVersion is on the secret and we can't have it here. The release data can be stored in secrets, configmaps, and SQL. Release information is pluggable. Just this week I was talking with someone about a new place to store release information and I know some folks are using the SQL storage back-end. See the drivers.
It should be on the release object. But, this brings in issues when older versions of Helm v3 try to work with the release object... they won't know about the new field to respect it. How would that work or does this need to wait for Helm v4?
There was a problem hiding this comment.
I'm ok if this change doesn't make it into helm 3. It would be nice but I am not familiar with all of the backwards compatibility concerns. I also wasn't sure where exactly data like this should be placed in the release. If it is more suited to be put in the secret that is fine too. I assumed putting it in the metadata would make it easier to view if a release is locked outside of helm tooling (e.g. kubectl)
Signed-off-by: Justin Garrison <justinleegarrison@gmail.com>
Signed-off-by: Justin Garrison <justinleegarrison@gmail.com>
Signed-off-by: Justin Garrison <justinleegarrison@gmail.com>
Signed-off-by: Justin Garrison <justinleegarrison@gmail.com>
Signed-off-by: Justin Garrison <justinleegarrison@gmail.com>
I repushed the changes after signing off. Do I need to re-run the checks or completely close the PR and open a new one? |
|
Hey @rothgar. Thanks for taking the time to write up a HIP. When planning for Helm 4 (see #179), we discussed whether we should accept HIPs based on whether the author who wrote said HIP is willing to perform the necessary work required to implement it. Are you interested in implementing the work proposed here (assuming it's accepted for Helm 4), or are you looking for someone else to implement this? |
|
Yes, I'd be willing to write the code to implement this. I would need some guidance on the best way to implement it (I've never contributed to core helm before) but depending on the timeline for Helm 4 I should be able to tackle the work and get some help where needed. |
|
I am assuming that the "release manager" for Helm 4 will devise a process for marking this HIP as "accepted for Helm 4". In my mind, though, this is ready for consideration as a Helm 4 feature. Thank you for working so hard on this HIP. |
This HIP is a follow-up from an email thread to allow helm releases to be locked within helm so that they will not be automatically or accidentally upgraded.