Fix generation of repo url for longer urls #18
Conversation
Upgrade action step to use Helm 3.8.2
| @@ -11,7 +10,7 @@ Using Token Auth with OCI Registry: | |||
| ```yaml | |||
| steps: | |||
| - name: Push Helm chart to OCI compatible registry (Github) | |||
| uses: bsord/helm-push@v4 | |||
| uses: oodlefinance/helm-push@5.0.0 | |||
There was a problem hiding this comment.
looks like you forgot to clean this up after testing.
| @@ -5,11 +5,8 @@ ENV XDG_CACHE_HOME=/opt/xdg | |||
|
|
|||
| RUN apk add curl tar bash --no-cache | |||
| RUN set -ex \ | |||
| && curl -sSL https://get.helm.sh/helm-v3.3.1-linux-amd64.tar.gz | tar xz \ | |||
| && curl -sSL https://get.helm.sh/helm-v3.8.2-linux-amd64.tar.gz | tar xz \ | |||
There was a problem hiding this comment.
In helm 3.8, OCI is no longer experimental so the line in entrypoint.sh exporting that envvar can be removed.
| && mv linux-amd64/helm /usr/local/bin/helm \ | ||
| && rm -rf linux-amd64 | ||
| RUN apk add --virtual .helm-build-deps git make \ |
There was a problem hiding this comment.
The chart museum plugin is still needed if the user does not want to use OCI and instead wants to push a tar.gz file. version 0.9.0 conflicts with the new helm push command, but 0.10.0 renames it to helm cm-push so there should be no conflict. A simple if statement should suffice in entrypoint.sh
Actually I also think it's worthwhile to add the helm s3 plugin, so that this action would support s3: urls https://github.com/hypnoglow/helm-s3 but that's probably a different issue and PR.
|  | ||
|  | ||
|  | ||
|  |
There was a problem hiding this comment.
Did this need to be removed? I imagine PRs are still welcome
|  | ||
|  | ||
|  | ||
|  |
There was a problem hiding this comment.
Seems like there's some cleanup needed for usernames
There was a problem hiding this comment.
Good spot, I think as I worked on making it work in my org it's carried these over to this PR.
I'll rebase this PR onto a separate branch specific to this repo
|
Thanks folks. Much appreciated. Will test this here shortly and get merged ASAP. |
This PR fixes the issue reported in #16
It will instead grab the version it needs for
helm chart publishfrom the output ofhelm chart save, and build the required URL for the OCI registry from that.We've tested this with our own workflow and it now works. Feel free to use our fix, or do it in a cleaner way.
Thanks :)