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

pubsys: grant AMI permissions to target account before copying #1065

Merged
merged 1 commit into from
Aug 24, 2020

Conversation

tjkirch
Copy link
Contributor

@tjkirch tjkirch commented Aug 23, 2020

If you specify regional roles in Infra.toml that refer to different accounts,
those accounts need access to the AMI and its snapshots before it can copy
the AMI.  This change adds the missing grants:

* If we find an existing AMI, we describe its snapshots so we can grant them
  * publish_ami functions were split so we can handle single regions or lists
    of regions more easily
* Ask STS for the account IDs of the given roles (removing the original account
  ID that already has access)
* modify_snapshots and modify_images were updated with user/group parameters
  rather than hardcoding "all"
* A new RegisteredIds struct is used to pass around the image and snapshot IDs
  to grant

Sorry this looks so long, I don't think it's as bad as it seems; the changes to publish_ami/mod.rs are basically just splitting methods so we can get/modify snapshots/images individually or in groups, rather than having to construct artificial groups for the single cases.

Testing done:

  • Confirmed normal ami / ami-public / ami-private path.
  • Confirmed that AMI copy now works across account boundaries.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@tjkirch tjkirch requested review from bcressey and zmrow August 23, 2020 22:37
Copy link
Contributor

@zmrow zmrow left a comment

Choose a reason for hiding this comment

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

🌾

tools/pubsys/src/aws/ami/mod.rs Show resolved Hide resolved
@tjkirch tjkirch changed the base branch from pubsys-ssm to develop August 24, 2020 23:26
If you specify regional roles in Infra.toml that refer to different accounts,
those accounts need access to the AMI and its snapshots before it can copy
the AMI.  This change adds the missing grants:

* If we find an existing AMI, we describe its snapshots so we can grant them
  * publish_ami functions were split so we can handle single regions or lists
    of regions more easily
* Ask STS for the account IDs of the given roles (removing the original account
  ID that already has access)
* modify_snapshots and modify_images were updated with user/group parameters
  rather than hardcoding "all"
* A new RegisteredIds struct is used to pass around the image and snapshot IDs
  to grant

Co-authored-by: Zac Mrowicki <mrowicki@amazon.com>
Co-authored-by: Tom Kirchner <tjk@amazon.com>
@tjkirch
Copy link
Contributor Author

tjkirch commented Aug 24, 2020

^ trivial rebase on develop.

@tjkirch tjkirch merged commit 2f40216 into bottlerocket-os:develop Aug 24, 2020
@tjkirch tjkirch deleted the copy-permissions branch August 24, 2020 23:31
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.

5 participants