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

imdsclient: return options for all IMDS fetch functions #1598

Merged
merged 1 commit into from
May 26, 2021

Conversation

jpculp
Copy link
Member

@jpculp jpculp commented May 25, 2021

Issue number:

N/A

Description of changes:

This commit changes the return type for the IMDS fetch functions to
options returning None when a 404 is encountered. This makes it easier
for the caller to decide what to do when a 404 occurs and is closer to
the behavior before imdsclient was written.

fetch_identity_document and its associated IdentityDocument struct
were removed in favor of separate fetch_region and
fetch_instance_type functions. Instance-type is now fetched via IMDS
meta-data as opposed to the identity document. We are returning to the
original behavior as it is possible that the identity document can be
absent in certain situations.

Changes were also made to shibaken, pluto, and early-boot-config to
accommodate the options detailed above.

Testing done:

  • Built aws-k8s-1.19 ami and launched instance.
  • Instance connected to eks cluster.
  • Connected to control container via ssm session.
  • Verified that host-containers.admin.user-data contained a base64-encoded block.
  • Connected to admin container via ssh.
  • Verified that /.bottlerocket/host-containers/admin/user-data contained JSON.
  • Ran sudo sheltie to verify root shell was still available.
  • Checked for failed systemd units.
  • Ran pluto with it's sub-commands to verify functionality.

Other testing:

  • aws-ecs-1 with no user data set.
  • aws-dev with no user data set. (tested by @arnaldo2792)
  • vmware-k8s-1.20 with user data from guestinfo interface.

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.

@jpculp jpculp requested review from bcressey, zmrow and webern May 25, 2021 18:53
@zmrow
Copy link
Contributor

zmrow commented May 25, 2021

Could we also boot variants without user data to check that behavior as well?

@jpculp
Copy link
Member Author

jpculp commented May 25, 2021

  • Small tweak in shibaken based on @webern's feedback.
  • Added more details to commit message and PR description.

@jpculp
Copy link
Member Author

jpculp commented May 25, 2021

Fixed typo in commit message.

@arnaldo2792
Copy link
Contributor

I tested the change and it works:

[  OK  ] Started docker engine.
[  OK  ] Reached target Multi-User System.
[  OK  ] Finished Isolates multi-user.target.
Bottlerocket OS 1.1.1
Kernel 5.10.29 on an x86_64 (ttyS0)
ip-172-31-61-1 login:

@jpculp jpculp marked this pull request as ready for review May 26, 2021 00:24
@jpculp jpculp requested review from zmrow and arnaldo2792 May 26, 2021 00:24
sources/imdsclient/src/lib.rs Show resolved Hide resolved
sources/imdsclient/src/lib.rs Outdated Show resolved Hide resolved
sources/imdsclient/src/lib.rs Outdated Show resolved Hide resolved
sources/imdsclient/src/lib.rs Outdated Show resolved Hide resolved
sources/imdsclient/src/lib.rs Outdated Show resolved Hide resolved
@jpculp
Copy link
Member Author

jpculp commented May 26, 2021

  • Addressed @zmrow's feedback.
  • Retested by running by launching a new build of aws-k8s-1.19 and running through pluto.

@jpculp jpculp requested a review from zmrow May 26, 2021 17:15
This commit changes the return type for the IMDS fetch functions to
options returning None when a 404 is encountered. This makes it easier
for the caller to decide what to do when a 404 occurs and is closer to
the behavior before imdsclient was written.

`fetch_identity_document` and its associated `IdentityDocument` struct
were removed in favor of separate `fetch_region` and
`fetch_instance_type` functions. Instance-type is now fetched via IMDS
meta-data as opposed to the identity document. We are returning to the
original behavior as it is possible that the identity document can be
absent in certain situations.

Changes were also made to shibaken, pluto, and early-boot-config to
accommodate the options detailed above.
@jpculp
Copy link
Member Author

jpculp commented May 26, 2021

Rebased and re-tested aws-k8s-1.19, aws-ecs-1, and vmware-k8s-1.20 to verify everything still works with #1576.

sources/imdsclient/src/lib.rs Show resolved Hide resolved
sources/imdsclient/src/lib.rs Show resolved Hide resolved
@jpculp jpculp merged commit 0fe7e8f into bottlerocket-os:develop May 26, 2021
@jpculp jpculp deleted the optional-imds branch May 26, 2021 21:44
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.

4 participants