-
Notifications
You must be signed in to change notification settings - Fork 522
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 "shibaken" generator to populate user-data with public keys from IMDS #1331
Conversation
21b15d4
to
6894374
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also - could we update the testing of this PR to reflect that the migration up/down was tested and the settings appeared as we expected them to and the admin host container works in both versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
🪂
sources/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
sources/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
sources/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
sources/api/migration/migrations/v1.0.6/add-shibaken/src/main.rs
Outdated
Show resolved
Hide resolved
} | ||
|
||
/// Helper to fetch an IMDSv2 session token that is valid for 60 seconds. | ||
fn fetch_imds_session_token(client: &Client) -> Result<String> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tangent: It is a bit unfortunate to have three implementations of this now, each slightly different. We should consider making an imds crate as a follow-up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely agree. I can take that on in another PR if folks are interested.
925e9f1
to
11e5885
Compare
For some context on the recent pushes that squashed together... Rebased and addressed comments/concerns with the shibaken crate itself: Replaced instances of host container with admin container and enhanced the AddMetadataMigration to accept multiple settings (+ unit test): Attempt to manually fix merge conflict: Went back and just rebased instead: |
sources/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
sources/api/migration/migration-helpers/src/common_migrations.rs
Outdated
Show resolved
Hide resolved
|
…IMDS This populates the admin container's user-data setting with public keys available from IMDS in the event that user-data has not been populated by the user.
Adds new migration helper `AddMetadataMigration` to remove setting metadata (like setting-generators).
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🦺
Adds a new migration for shibaken setting-generator
|
Issue number:
N/A
Description of changes:
Add "shibaken" generator to populate user-data with public keys from IMDS
This populates the host container's user-data setting with public keys
available from IMDS in the event that user-data has not been populated
by the user.
Adds a new migration helper
AddMetadataMigration
to remove settingmetadata (like setting-generators).
Adds a new migration for shibaken setting-generator.
Testing done:
Build aws-ecs-1 image and launched instance.
Verified that
host-containers.admin.user-data
contained a base64-encoded block.Enabled and launched admin container, which started fine.
Verified that
/.bottlerocket/host-containers/admin/user-data
contained JSON with the following structure:Ran
sudo sheltie
to verify root shell was still available.@etungsten verified the migration between (1.0.5, 1.0.6)
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.