search

stashed / project

Enhancements & Issues
7 stars 2 forks source link

Already released chart versions should not be republished #22

Open whage opened 1 year ago

whage commented 1 year ago

We have been testing v2023.05.31 for weeks now while preparing our AKS clusters for a 1.25 upgrade. A colleague of mine recently reported a bug about the readOnlyRootFilesystem setting which was fixed and then the v2023.05.31 release was republished. Today, during testing we observed that it was republished again (a few hours ago) after these changes were merged: https://github.com/stashed/installer/pull/308 The above changes broke our deployment because the generated yaml manifests included an invalid, half-empty object that should have been created by this snippet: https://github.com/stashed/installer/pull/308#diff-f50f84488dc414cd53a6d5d841526d57ecdffc00517925e746b423eb52a11314R97-R105 Our templated manifest now looks like this (showing only the relevant part for brevity, notice the empty lines above metadata):

---
# Source: stash/charts/stash-enterprise/templates/apiregistration.yaml
# copy kube-system/extension-apiserver-authentication into stash operator namespace

metadata:
  namespace: kube-system
---

This was the error reported by Helm:

Error: Failed to render chart: exit status 1: Error: unable to build kubernetes objects from release manifest: error validating "": error validating data: [apiVersion not set, kind not set]

We solved the above problem by setting security.copyAuthenticationConfigMap=false but the true case (which is the default) should work or it should be false by default.

If this version of the chart is still under development then please don't publish it with a release tag, this is dangerous for us, such a new, updated release can break our deployments if it contains bugs. If you consider v2023.05.31 to be correct and working, then please do not change it but instead publish the chart with a new tag if you add changes.

tamalsaha commented 1 year ago

@whage , sorry for the inconvenience. Are you using ArgoCD or do you use helm template | kubectl apply -f - to apply helm charts? Because helm install or helm upgrade produces correct result even with the option security.copyAuthenticationConfigMap=true. helm template on the other hand won't work, as the lookup function is not supported by the helm template command.

I have switched the default value security.copyAuthenticationConfigMap=false and republished the charts https://github.com/stashed/installer/commit/3cf0dd91e48ceb16bfd97885e84947c4a25398bf

whage commented 1 year ago

Thanks for changing the default to false. We are using Helmsman to deploy Helm charts and AFAIK it issues helm upgrade --install ... command. I'm not sure why you saw that helm upgrade doesn't produce this error. Are you sure?

Can you please also reflect on the republication issue? Why do you keep republishing v2023.05.31 with new changes? It should stay constant especially since it is tagged with a date.