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 cargo make ami-public and ami-private targets #1033

Merged
merged 4 commits into from Aug 19, 2020

Conversation

zmrow
Copy link
Contributor

@zmrow zmrow commented Aug 12, 2020

Description of changes:

dc4d53b5 Add `cargo make ami-public` and `ami-private` targets
1d806774 pubsys: Move region_from_string to common aws module
e0988d67 Allow JSON output of registered AMIs
293d5b24 rustfmt pubsys client module

Testing done:

  • Made multiple AMIs public and private
    • @tjkirch launched an AMI made public from @zmrow 's account
    • Checked permissions on all AMIs before and after cargo make ami-public/private
  • Verified that a missing AMI will cause an error

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.

@zmrow zmrow requested review from tjkirch and bcressey August 12, 2020 22:42
zmrow and others added 3 commits August 13, 2020 16:54
Co-authored-by: Zac Mrowicki <mrowicki@amazon.com>
Co-authored-by: Tom Kirchner <tjk@amazon.com>
Co-authored-by: Zac Mrowicki <mrowicki@amazon.com>
Co-authored-by: Tom Kirchner <tjk@amazon.com>
Co-authored-by: Zac Mrowicki <mrowicki@amazon.com>
Co-authored-by: Tom Kirchner <tjk@amazon.com>
@tjkirch
Copy link
Contributor

tjkirch commented Aug 13, 2020

^ This push rebases to fix the conflict from #1034, and rewords the commit messages per @bcressey.

@tjkirch
Copy link
Contributor

tjkirch commented Aug 14, 2020

^ Addresses @bcressey's comment above; per offline discussion, rather than an exact match, we check that the requested regions are a subset of the ones in the JSON file, in case a user only wants to publish in some of their regions at first.

Copy link
Member

@webern webern left a comment

Choose a reason for hiding this comment

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

Nice!

tools/pubsys/src/aws/publish_ami/mod.rs Outdated Show resolved Hide resolved
tools/pubsys/src/aws/publish_ami/mod.rs Show resolved Hide resolved
tools/pubsys/src/aws/publish_ami/mod.rs Show resolved Hide resolved
Co-authored-by: Zac Mrowicki <mrowicki@amazon.com>
Co-authored-by: Tom Kirchner <tjk@amazon.com>
@tjkirch
Copy link
Contributor

tjkirch commented Aug 18, 2020

^ This push addresses @webern's comment about the inaccurate description of the regions parameter. I think the other comments are OK.

@tjkirch tjkirch requested a review from webern August 18, 2020 16:05
@tjkirch tjkirch merged commit 4d55376 into bottlerocket-os:develop Aug 19, 2020
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.

None yet

4 participants