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

Add AWS CLI to system install tools #993

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

VariableExp0rt
Copy link

Description

This PR attempts to add support for downloading and installing the AWS CLI for supported platforms. This turned out to be a bit more difficult with the special requirements for each platform supported (Linux, MacOS, Windows).

Motivation and Context

Fixes a currently open issue - #941

How Has This Been Tested?

Changes tested with:

gofmt -w -s ./pkg
gofmt -w -s ./cmd

go build

make test
make e2e

If updating or adding a new CLI to arkade get, run:

go build && ./hack/test-tool.sh TOOL_NAME

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Documentation

  • I have updated the list of tools in README.md if (required) with ./arkade get --format markdown
  • I have updated the list of apps in README.md if (required) with ./arkade install --help

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have tested this on arm, or have added code to prevent deployment

(I've added a boolean flag --run-installer which defaults to false)

@alexellis
Copy link
Owner

@Jasstkn would you be able to take a look at the approach?

@Jasstkn
Copy link
Contributor

Jasstkn commented Oct 25, 2023

@Jasstkn would you be able to take a look at the approach?

sure, will take a look. In the meantime, @VariableExp0rt can I ask you to add a confirmation how you tested the installation via arkade?

@VariableExp0rt
Copy link
Author

VariableExp0rt commented Oct 25, 2023

@Jasstkn I tested via arkade system install aws-cli (I haven't tested on the other platforms, as I'm primarily operating on a Mac and don't have access to a Windows box). If the approach is a bit wrong, I can re-do it, the installer stuff AWS provide is a bit messy

@VariableExp0rt
Copy link
Author

Hey @Jasstkn, let me know if there is anything else you need from me :)

Copy link
Contributor

@Shikachuu Shikachuu left a comment

Choose a reason for hiding this comment

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

Please see my comments above and do a rebase!

Overall really nice work, I really like the tool unit test for this too, it is a cool idea!

cmd/system/aws_cli.go Outdated Show resolved Hide resolved
cmd/system/aws_cli.go Show resolved Hide resolved
cmd/system/aws_cli.go Outdated Show resolved Hide resolved
cmd/system/aws_cli.go Outdated Show resolved Hide resolved
cmd/system/aws_cli.go Outdated Show resolved Hide resolved
cmd/system/aws_cli.go Outdated Show resolved Hide resolved
@derek derek bot added the no-dco label Jan 3, 2024
Copy link

derek bot commented Jan 3, 2024

Thank you for your contribution. It seems that one or more of your commits have a "Signed-off-by" statement with an anonymous email address. The Developer Certificate of Origin (DCO) requires all commits to be signed off by genuine, contactable individuals. Please see our contributing guide.

### :bulb: Shall we fix this?

This will only take a few moments.

First, clone your fork and checkout this branch using the git CLI.

Next, set up your real name and email address:

git config --global user.name "Your Full Name"
git config --global user.email "you@domain.com"

Finally, run one of these commands to add the "Signed-off-by" line to your commits.

If you only have one commit so far then run: git commit --amend --signoff and then git push --force.
If you have multiple commits, watch this video.

Check that the message has been added properly by running "git log".

Copy link
Contributor

@Shikachuu Shikachuu left a comment

Choose a reason for hiding this comment

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

Looks promising.

Please don't forget to update the README.md and squash the commits in to single one, we like to keep the git history clean for single tools like this.

Thanks!

cmd/system/aws_cli.go Show resolved Hide resolved
cmd/system/aws_cli.go Outdated Show resolved Hide resolved
@VariableExp0rt
Copy link
Author

Looks promising.

Please don't forget to update the README.md and squash the commits in to single one, we like to keep the git history clean for single tools like this.

Thanks!

Have updated the README.md here 👍 .

@VariableExp0rt
Copy link
Author

VariableExp0rt commented Jan 3, 2024

@Shikachuu I'm having quite a bit of trouble squashing into a single commit right now, is there a correct way I should be doing that other than git rebase -i HEAD~7 and squash-ing my commits? I think the previous rebase is making things awkward

Signed-off-by: liam.baker <liam.baker@sage.com>
@VariableExp0rt
Copy link
Author

@Shikachuu I think I've managed to fix it, sorry about that! Let me know if there are any more comments to address 👍

@VariableExp0rt
Copy link
Author

For posterity:

./hack/test-tool.sh awscli
+ ./arkade get awscli --arch arm64 --os darwin --quiet
+ file /Users/liam.baker/.arkade/bin/awscli
/Users/liam.baker/.arkade/bin/awscli: xar archive compressed TOC: 4792, SHA-1 checksum
+ rm /Users/liam.baker/.arkade/bin/awscli
+ echo

+ ./arkade get awscli --arch x86_64 --os darwin --quiet
+ file /Users/liam.baker/.arkade/bin/awscli
/Users/liam.baker/.arkade/bin/awscli: xar archive compressed TOC: 4792, SHA-1 checksum
+ rm /Users/liam.baker/.arkade/bin/awscli
+ echo

+ ./arkade get awscli --arch x86_64 --os linux --quiet
+ file /Users/liam.baker/.arkade/bin/awscli
/Users/liam.baker/.arkade/bin/awscli: Zip archive data, at least v2.0 to extract, compression method=store
+ rm /Users/liam.baker/.arkade/bin/awscli
+ echo

+ ./arkade get awscli --arch aarch64 --os linux --quiet
+ file /Users/liam.baker/.arkade/bin/awscli
/Users/liam.baker/.arkade/bin/awscli: Zip archive data, at least v2.0 to extract, compression method=store
+ rm /Users/liam.baker/.arkade/bin/awscli
+ echo

+ ./arkade get awscli --arch x86_64 --os mingw --quiet
+ file /Users/liam.baker/.arkade/bin/awscli.exe
/Users/liam.baker/.arkade/bin/awscli.exe: Composite Document File V2 Document, Little Endian, Os: Windows, Version 6.2, MSI Installer, Code page: 1252, Title: Installation Database, Subject: AWS Command Line Interface v2, Author: Amazon Web Services, Keywords: Installer, Comments: This installer database contains the logic and data required to install AWS Command Line Interface v2., Template: x64;1033, Revision Number: {568A8D9E-6206-47B9-8AF8-EA26143A52B8}, Create Time/Date: Fri Oct 20 19:03:24 2023, Last Saved Time/Date: Fri Oct 20 19:03:24 2023, Number of Pages: 200, Number of Words: 2, Name of Creating Application: Windows Installer XML Toolset (3.11.1.2318), Security: 2
+ rm /Users/liam.baker/.arkade/bin/awscli.exe
+ echo

@VariableExp0rt
Copy link
Author

@Shikachuu is there anything else I should do here?

}

func getAWSCLIVersion(owner, repo string) (string, error) {
url := fmt.Sprintf("https://api.github.com/repos/%s/%s/git/refs/tags", owner, repo)
Copy link
Owner

Choose a reason for hiding this comment

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

We cannot use the API because it's rate-limited.

I'm curious, why did you add this when we already have code in the project (everywhere) that already determines the version?

Copy link
Author

Choose a reason for hiding this comment

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

The reason for this is that tags exist that are for all releases, but there are not releases to accompany them. I might be wrong, but I couldn't see specifically anywhere that only looks at the refs/tags (which is all that AWS provide). More info here #993 (comment)

return latest.String(), nil
}

func runBundledInstaller(osVer string, workDir string, filename string, installPath string) error {
Copy link
Owner

Choose a reason for hiding this comment

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

I don't understand why this option is being included?

What format is the AWS CLI in?

Copy link
Author

Choose a reason for hiding this comment

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

The CLI comes in different formats for different platforms, for instance OSx is a .pkg, Linux is a .zip, and similar for Windows. There isn't just a binary.

}

func installationInstructions(osVer string, workDir string, filename string, installPath string) ([]byte, error) {
t := template.New("Installation Instructions")
Copy link
Owner

Choose a reason for hiding this comment

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

Is there no binary package for the aws cli?

Copy link
Author

Choose a reason for hiding this comment

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

There is not, just zip, pkg, or the msi installer for Windows.

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

Successfully merging this pull request may close these issues.

None yet

4 participants