-
Notifications
You must be signed in to change notification settings - Fork 2k
mmds: Smoother migration from v1 to v2 #5290
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5290 +/- ##
==========================================
+ Coverage 82.95% 83.02% +0.06%
==========================================
Files 250 250
Lines 26879 26828 -51
==========================================
- Hits 22298 22273 -25
+ Misses 4581 4555 -26
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d1f34a0
to
fc485e6
Compare
067665a
to
189c487
Compare
8eedccb
to
a9ff226
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.
LGTM!
.
_\____
|_===__`. ==/
\/ '---"\ _ _ _ _/
______/_______/_|_|_|_|_|
_|--------------------==."
\____________________.' LGB
------------------------------------------------
Clippy is angry at you 🫨
|
Fixed! Sorry about that :( |
* Insert a line break with "\" after "\r\n" if another line follows * Make multi-line strings start at the same column. * Eliminate unneeded variable assignments (`request_bytes`). * Use test utility function if possible * Remove duplicate test cases Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
We checked that "X-metadata-token-ttl-seconds" has a valid positive number regardless of the request type. On the oher hand, EC2 IMDS only validates it only if it is a PUT request to "/latest/api/token". Such a behavioral difference from EC2 IMDS could result in unexpected issues. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
The custom headers supported by MMDS (X-metadata-token and X-metadata-token-ttl-seconds) are different from what EC2 IMDS supports (X-aws-ec2-metadata-token and X-aws-ec2-metadata-token-ttl-seconds). Supporting EC2 IMDS-compatible custom headers in MMDS, users become able to make their workloads work with MMDS out of the box. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Add an integration test that checks AWS SDK for Python (boto3) is able to get credentials via MMDS without modification. CI artifacts (guest rootfs) update was needed to install AWS SDK for Python. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
The constructor for TokenAuthority can fail and returns Result. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
To enable smoother migration from v1 to v2, make MMDS v1 support PUT request to /latest/api/token for token generation. We do NOT make MMDS v1 deny GET requests even with invalid tokens not to break existing workloads. It does not validate a given toekn at this point, but it will validate to increment a metric that will be added to count such requests having invalid tokens. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
In the previous commit, MMDS v1 was made to support token generation but a given token to GET request was not validated. Validates the token and increments a new metric `rx_invalid_token` if it is not valid. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
The metric is useful for users to track whether v1-style requests are issued or not inside a guest. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
* Move out version-independent parts to dedicated tests * Test happy path first, and then test error cases No test cases are dropped. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
As EC2 IMDS does, MMDS denies PUT requests if X-Forwarded-For header is included, but it was validated only in a case-sensitive way. However, it is defined that HTTP headers are case-insensitive. We should deny it regardless of its case. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Reason / Changes
To allow easier migration from MMDS v1 to v2, made the following changes:
X-aws-ec2-metadata-token
andX-aws-ec2-metadata-token-ttl-seconds
).mmds.rx_invalid_token
andmmds.rx_no_token
metrics to allow users to keep track of the number of GET requests that were rejected in MMDS version 2. Even with MMDS version 1 configured, they also count GET requests that would be rejected under MMDS version 1.Fixed some minor things
X-metadata-token-ttl-seconds
header only if it is aPUT
request to/latest/api/token
.X-Forwarded-For
header in a case-insensitive way.License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md
.PR Checklist
tools/devtool checkstyle
to verify that the PR passes theautomated style checks.
how they are solving the problem in a clear and encompassing way.
in the PR.
CHANGELOG.md
.[ ] If a specific issue led to this PR, this PR closes the issue.[ ] When making API changes, I have followed theRunbook for Firecracker API changes.
integration tests.
[ ] I have linked an issue to every newTODO
.rust-vmm
.