gravitl / netmaker

Netmaker makes networks with WireGuard. Netmaker automates fast, secure, and distributed virtual networks.
https://netmaker.io
Other
9.5k stars 552 forks source link

VERSION=0.8.5 results in ERROR 404 #403

Closed pete1019 closed 3 years ago

pete1019 commented 3 years ago

curl -sfL https://raw.githubusercontent.com/gravitl/netmaker/develop/scripts/netclient-install.sh | VERSION=0.8.5 KEY=...

Which is copied from the UI 0.8.5 will result in https://github.com/gravitl/netmaker/releases/download/0.8.5/netclient: 2021-10-29 20:09:34 ERROR 404: Not Found.

If i delete VERSION=0.8.5 then it works.

bsherman commented 3 years ago

I ran into this too, so I started digging.

This was caused by a recent update to AccessKeys.js which added VERSTION=${serverVersion || "latest"} to the generated agent install command...

This generally results in a version string like VERSION=0.8.4 which is problematic as the netclient-install.sh script, which uses the VERSION, assumes it will be a valid version string for download from github. However, the github URLs expect the version to match the tags in this repo which would be v0.8.4 for example.

I was thinking of submitting a PR to netmaker-ui to add the v before the version, but thought it would be more expedient to submit a PR for the netclient-install.sh (to add a v before the version if not latest as all the docs and the UI reference the develop branch for install script anyway. Plus, fixing in the script would generally be more reliable for typing v or not.

# was going to add this after the line 10, which starts with [ -z "$VERSION" ]
[ "latest" != "$VERSION" ] && [ "v" != `echo $VERSION | cut -c1` ] && VERSION="v$VERSION"

But finally, I realized yet another problem seems to be in play here.

As I write this, v0.8.5 is the current version, however, even that is not a valid tag for downloading, as instead latest is being used as the tag for this version. This use of a latest tag instead of the actual version further complicates things.

Not sure what you want to do here @afeiszli . I'd generally suggest dropping the use of latest tag in favor of always using valid version tags, but I'm not sure how that fits in with your build workflows, and obviously, it would impact your documentation.

Maybe you could start by always using a proper version tag, but after creating that new version tag, also tag with latest such that the docs aren't broken?

Another idea... I didn't dig deep on the React code, but if it's aware of what is most recent version, it could provide VERSION=latest if running latest version, else provide VERSION=vX.Y.Z for any non current version... so that would be two changes there. But hopefully wouldn't require tweaking workflows or docs.

afeiszli commented 3 years ago

@bsherman I like the idea of the PR. In addition, I believe @pete1019 is working on a PR to fallback to latest if curl returns a 404. Those two things together will solve the issue short term.

However, I agree that the latest tag is an issue. How we do it is give the most recent release the "latest" tag, and then the previous version becomes "v0.X.X". So, for previous versions, the script should work, but not for the latest.

I think that not using the "latest" tag, and instead just using the version tag should work. I believe the latest release still gets "latest" applied anyway. We can try this in the next release.

In the meantime, if you (@bsherman) can submit a PR for the install script semantics, and @pete1019 submits a PR for the "latest" tag error handling, then that should solve the problem.

pete1019 commented 3 years ago

Here would be the "check if url exists" commit:

419

pete1019 commented 3 years ago

Closed since fixed with latest commit.