oauth2-proxy / manifests

For hosting manifests to allow for the deployment of OAuth2-Proxy/OAuth2-Proxy
Apache License 2.0
170 stars 153 forks source link

removed v from version of oauth2-proxy deployment.yaml #189

Closed RadOctocode closed 7 months ago

RadOctocode commented 8 months ago

Reasoning: assuming all versions for the deployment image for oauth2-proxy will start with v is bad practice.

pierluigilenoci commented 8 months ago

@RadOctocode Thanks for your contribution, but some work remains to be considered. Simply removing that character isn't enough. The code in other parts of the chart must also be changed. Furthermore, the chart needs a version bump.

pierluigilenoci commented 8 months ago

@RadOctocode Eventually, can you link me to some documents explaining the rationale behind this change and the technical reasons for this best practice?

RadOctocode commented 8 months ago

Hi Pierluigi, sorry about the wait. I'm happy to make the changes to the other files and bumping the version to remove the v from the helm chart. As far as the reasoning for removing the implicit v from the helm chart, many companies host their images in private docker repositories that have rules. As part of a financial institution one of those rules for our private registry is that image semver flags are not prefixed with a v.

RadOctocode commented 7 months ago

Hey @pierluigilenoci I removed the stripping of v from the helper function as well, please let me know if there is anything else I should do.

pierluigilenoci commented 7 months ago

@RadOctocode, the chart needs a version bump.

RadOctocode commented 7 months ago

@pierluigilenoci I bumped the chart up by one minor version, let me know if I should change this to be a patch bump instead or if I should add something to artifacthub.io/changes

pierluigilenoci commented 7 months ago

@RadOctocode, could you please also edit the annotation for ArtifactHub? https://github.com/oauth2-proxy/manifests/blob/main/helm/oauth2-proxy/Chart.yaml#L36-L41

RadOctocode commented 7 months ago

@pierluigilenoci I have edited the annotation for artifacthub