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: don't require Infra.toml to get started #1166

Merged
merged 1 commit into from
Oct 16, 2020

Conversation

tjkirch
Copy link
Contributor

@tjkirch tjkirch commented Oct 16, 2020

Issue number:

Found by @izaakschroeder in #1162 (issue is unrelated)

Description of changes:

If the user has no Infra.toml, we can continue with defaults most of the time.
For making an AMI, only passing PUBLISH_REGIONS is required.  For making a
repo, we can use a default configuration and write it under 'default'. SSM
still requires Infra.toml.

Testing done:

Tested ami, publish-ami, and repo tasks (and ssm, though it didn't change) with and without an Infra.toml file. They worked as expected:

  • With Infra.toml:
    • ami and publish-ami use region from file
    • repo is built in build/repos/default by default
    • ssm works
  • Without Infra.toml:
    • ami and publish-ami require -e PUBLISH_REGIONS, and work
    • repo is still built in build/repos/default and that section of my config is detected
    • ssm doesn't work, as expected; prefix is required. (This could be a future improvement, passing through prefix from a variable.)

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.

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.

🦓

Copy link

@izaakschroeder izaakschroeder left a comment

Choose a reason for hiding this comment

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

Looks fantastic to me! It might be useful to note which things require/don't require the Infra.toml file in your PUBLISHING.md. 🎉

@tjkirch
Copy link
Contributor Author

tjkirch commented Oct 16, 2020

It might be useful to note which things require/don't require the Infra.toml file in your PUBLISHING.md.

That's a good point. It's covered for AMIs, where we have the longer command that doesn't rely on Infra.toml in BUILDING.md, and move to a simpler command that relies on Infra.toml in PUBLISHING.md. I guess that's the theme of PUBLISHING.md - we're expecting that most users going through the steps beyond creating an AMI are a bit more invested, and so creating an Infra.toml makes sense. Then again, you seem invested and you're asking for it, so theory disproven I guess :)

If the user has no Infra.toml, we can continue with defaults most of the time.
For making an AMI, only passing PUBLISH_REGIONS is required.  For making a
repo, we can use a default configuration and write it under 'default'. SSM
still requires Infra.toml.
@tjkirch
Copy link
Contributor Author

tjkirch commented Oct 16, 2020

^ This push adds a couple notes to PUBLISHING.md describing what you can do without an Infra.toml, per @izaakschroeder. (There's already a note in the SSM section that it's required there.)

@izaakschroeder
Copy link

Thanks for being so prompt and open to feedback! 😄

@tjkirch tjkirch merged commit bfb0d04 into bottlerocket-os:develop Oct 16, 2020
@tjkirch tjkirch deleted the no-infra-toml branch October 16, 2020 23:05
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