Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: tar bin/helm-s3 Not found in archive #466

Merged
merged 3 commits into from
Jan 31, 2025

Conversation

arahmangulov
Copy link
Contributor

@arahmangulov arahmangulov commented Oct 5, 2024

What type of PR is this?
/kind fix

What this PR does / why we need it:
This PR fixes helm-s3 plugin installation on Windows (CYGWIN, MINGW, MSYS_NT) platforms.
Can be reproduced via installing any version above v0.14.0, example:

$ helm plugin install https://github.com/hypnoglow/helm-s3.git --version v0.16.2
Downloading and installing helm-s3 v0.16.2 ...
Checksum is valid.
tar: bin/helm-s3: Not found in archive
tar: Exiting with failure status due to previous errors
helm-s3 install hook failed. Please remove the plugin using 'helm plugin remove s3' and install again.
Error: plugin install hook for "s3" exited with error

This issue started happening after chaning the way of extracting the binary from the tarball: e9e5382#diff-3bac97673cfc352e120c81f8812245c509aa6dfe24f5eeeb07162159fe82aef5L99
While mv command is good with moving binary without explicit extension definition, the tar is pretty restrictive on this, so it's failing on the extraction stage. So, I just added binary_extension variable which would be empty for UNIX/Linux systems, and .exe suffix will be appended for Windows installations.

Also this MR fixes os variable usage, the previous initOS function had os definition with uname -s value and there are unecessary plain uname calls happening later on.

Fixed <4-spaces indentations as well.

Which issue(s) this PR fixes:

Special notes for your reviewer:
NONE

Does this PR introduce a user-facing change?:

Fixes helm-s3 plugin installation on Windows platforms

@arahmangulov arahmangulov changed the title fix: tar bin/helm-s3: Not found in archive fix: tar bin/helm-s3 Not found in archive Oct 5, 2024
@hypnoglow hypnoglow added this to the Next milestone Jan 31, 2025
@hypnoglow hypnoglow force-pushed the fix/windows-installation branch from f12a5ee to 4209d9d Compare January 31, 2025 14:07
@hypnoglow hypnoglow self-assigned this Jan 31, 2025
@hypnoglow hypnoglow enabled auto-merge January 31, 2025 14:15
@hypnoglow hypnoglow merged commit 6fbd687 into hypnoglow:master Jan 31, 2025
9 checks passed
@hypnoglow
Copy link
Owner

Thank you!

esac
os=$(uname -s)
binary_extension=""
case "$(os)" in

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect variable reference. Should be ${os} instead of $(os).

Same below.

@stefangluszek
Copy link

This change broke the plugin:

> helm plugin install https://github.com/hypnoglow/helm-s3.git --version master
Downloading and installing helm-s3 v0.16.2 ...
./hack/install.sh: line 37: os: command not found
./hack/install.sh: line 42: os: command not found
OS '' not supported!

@gli-vijay-lingesh
Copy link

This change broke the plugin:

> helm plugin install https://github.com/hypnoglow/helm-s3.git --version master
Downloading and installing helm-s3 v0.16.2 ...
./hack/install.sh: line 37: os: command not found
./hack/install.sh: line 42: os: command not found
OS '' not supported!

Facing the same issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.