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

Use AWS CLI v2 #214

Closed
wants to merge 2 commits into from
Closed

Use AWS CLI v2 #214

wants to merge 2 commits into from

Conversation

joverlee521
Copy link
Contributor

@joverlee521 joverlee521 commented Jun 6, 2024

Description of proposed changes

Update AWS CLI to v2

We were running into dependency issues with awscli v1, which was
installed via Pip.¹

Update to AWS CLI v2 which is not installed via Pip.
Instead of working around the absolute path symlinks created by running
./aws/install, we are "just installing the files ourselves"² and
creating the symlink for /final/bin/aws.

¹ #213
² #214 (comment)

Related issue(s)

Resolves #213

Checklist

  • Checks pass

@tsibley tsibley self-requested a review June 6, 2024 06:06
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
We were running into dependency issues with awscli v1, which was
installed via Pip.¹

Update to AWS CLI v2 which is _not_ installed via Pip.
Instead of working around the absolute path symlinks created by running
`./aws/install`, we are "just installing the files ourselves"² and
creating the symlink for `/final/bin/aws`.

¹ <#213>
² <#214 (comment)>
AWS CLI v2 runs into errors if the region is not defined¹

The AWS_DEFAULT_REGION variable is available in the GH Action CI after
the aws-actions/configure-aws-credentials@v4 step.

¹ <actions/runner-images#2791 (comment)>
@@ -24,7 +24,7 @@ Files are populated from NEXTSTRAIN_ENVD_URL.

$ aws s3 cp --quiet env.d.zip "$NEXTSTRAIN_ENVD_URL"

$ docker run --rm -e NEXTSTRAIN_ENVD_URL --env=AWS_{ACCESS_KEY_ID,SECRET_ACCESS_KEY,SESSION_TOKEN} "$IMAGE" \
$ docker run --rm -e NEXTSTRAIN_ENVD_URL --env=AWS_{ACCESS_KEY_ID,SECRET_ACCESS_KEY,SESSION_TOKEN,DEFAULT_REGION} "$IMAGE" \
Copy link
Member

Choose a reason for hiding this comment

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

AWS CLI v2 runs into errors if the region is not defined¹

Uh... so does this mean that all our usages of the Docker/AWS Batch/Singularity runtimes which call aws are going to have to ensure they have AWS_DEFAULT_REGION set? That seems like a unexpected breaking change. Hmm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If that's an issue we don't want to deal with now, we can also install AWS CLI v1 using the bundled installer

Copy link
Member

Choose a reason for hiding this comment

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

Maybe? Or maybe we could provide a default AWS_DEFAULT_REGION that's backwards compat with v1? (What does it do if region isn't required?) Or maybe we bite the bullet and fix anything that breaks (although this means breaking other people's uses too).

Maybe we can talk thru this next week? I'm not really sure I have all the info at hand here to help make a decision (and it's Friday afternoon and my brain is fried).

Copy link

Choose a reason for hiding this comment

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

+1 (albeit from deep on the sidelines) for not making this type of decision on Friday afternoon, particularly one with lovely (distractingly) sunny weather...

tsibley added a commit that referenced this pull request Jun 11, 2024
Otherwise, the awscli → botocore dependency is broken by the subsequent
installation of the nextstrain-cli → botocore dependency, which results
in --no-sign-request not working as it should.

This approach seemed like the option of least change.  Alternatives to
and/or additional improvements to this include a) installing Nextstrain
CLI in its own venv instead/as well, b) upgrading to AWS CLI v2 (which
is always isolated) or c) updating the nextstrain-cli → botocore
dependency to not conflict.

Resolves: <#213>
Related-to: <#214>
@joverlee521
Copy link
Contributor Author

Closing in favor of #215 since we don't want to deal with the fallout now while we are dealing with fallout from switch to OIDC in GH Action workflows. We will definitely want to revisit this later though!

@victorlin victorlin deleted the use-aws-cli-v2 branch June 26, 2024 16:01
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.

awscli --no-sign-request option not working
4 participants