Skip to content

Comments

Added all current available AWS regions#172855

Merged
roberth merged 1 commit intoNixOS:masterfrom
bolt12:bol12-all-regions
May 18, 2022
Merged

Added all current available AWS regions#172855
roberth merged 1 commit intoNixOS:masterfrom
bolt12:bol12-all-regions

Conversation

@bolt12
Copy link
Contributor

@bolt12 bolt12 commented May 13, 2022

Description of changes

Added all currently available AWS regions to create-amis.sh file.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label May 13, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels May 13, 2022
Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

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

Seems like a good idea to sort.
Would be good to get this in soon, as it may be used at release time.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# > aws ec2 describe-regions --all-regions --query "Regions[].{Name:RegionName}" --output text
# > aws ec2 describe-regions --all-regions --query "Regions[].{Name:RegionName}" --output text | sort

I don't know what determines the current order, but we'll regret (slightly) not sorting if it changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed!

@bolt12 bolt12 force-pushed the bol12-all-regions branch from 242a2bf to 5a26a22 Compare May 17, 2022 08:58
@bolt12 bolt12 requested a review from roberth May 17, 2022 08:58
@roberth
Copy link
Member

roberth commented May 17, 2022

Please change the commit message to use imperative mood, as is standard in git. Some context is also helpful. Example

maintainers/create-amis.sh: Add all currently available AWS regions

Otherwise, lgtm!

@roberth roberth requested a review from AmineChikhaoui May 17, 2022 09:12
Add all currently available AWS regions
@bolt12 bolt12 force-pushed the bol12-all-regions branch from 5a26a22 to fdf74c7 Compare May 17, 2022 09:21
@bolt12
Copy link
Contributor Author

bolt12 commented May 17, 2022

How about this @roberth ? Thank you very much for your review!

Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

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

👍

@bolt12
Copy link
Contributor Author

bolt12 commented May 18, 2022

Ping @AmineChikhaoui

@AmineChikhaoui
Copy link
Member

Hi, iirc the regions need to be enabled through the AWS account first. Since the diff isn't very clear, can you list what regions where added in this PR ?

@bolt12
Copy link
Contributor Author

bolt12 commented May 18, 2022

@AmineChikhaoui you'd have to enable all regions since this PR adds all regions currently available. There's also something else that might be needed to do which has to do with this warning on the Enable Regions dashboard:

AWS recommends using regional AWS Security Token Service (STS) endpoints to reduce latency. Session tokens from regional STS endpoints are valid in all AWS Regions. If you use regional STS endpoints, no action is required.

Session tokens from the global STS endpoint (https://sts.amazonaws.com) are valid only in AWS Regions that are enabled by default. If you intend to enable a new Region for your account, you can either use session tokens from regional STS endpoints or activate the global STS endpoint to issue session tokens that are valid in all AWS Regions. You can do this in Account Settings in the IAM console.

Session tokens that are valid in all AWS Regions are larger. If you store session tokens, these larger tokens might affect your systems. Learn more

So this might be need enabling as well

@roberth roberth merged commit 36fb966 into NixOS:master May 18, 2022
@Janik-Haag Janik-Haag added the 12.first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
@johnalotoski johnalotoski mentioned this pull request Nov 8, 2023
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.first-time contribution This PR is the author's first one; please be gentle!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants