[DRAFT] mmds: Smoother migration from v1 to v2 #5290
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5290 +/- ##
==========================================
+ Coverage 82.86% 82.93% +0.07%
==========================================
Files 250 250
Lines 26897 26855 -42
==========================================
- Hits 22288 22272 -16
+ Misses 4609 4583 -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:
|
zulinx86
force-pushed
the
mmds_compat
branch
13 times, most recently
from
b1d6593 to
d7021ce
Compare
* 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 <[email protected]>
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. To line up with EC2 IMDS, breaks `TokenHeaders` down to dedicated structs (`XMetadataToken` and `XMetadataTokenTtlSeconds`) and parses "X-metadata-token" only for GET requests and "X-metadata-token-ttl-seconds" only for PUT requests to "latest/api/token". Signed-off-by: Takahiro Itazuri <[email protected]>
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 <[email protected]>
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 <[email protected]>
The constructor for TokenAuthority can fail and returns Result. Signed-off-by: Takahiro Itazuri <[email protected]>
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 <[email protected]>
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 <[email protected]>
The metric is useful for users to track whether v1-style requests are issued or not inside a guest. Signed-off-by: Takahiro Itazuri <[email protected]>
* 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 <[email protected]>
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 <[email protected]>
zulinx86
force-pushed
the
mmds_compat
branch
2 times, most recently
from
d1f34a0 to
fc485e6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reason / Changes
To allow easier migration from MMDS v1 to v2, made the following changes:
X-aws-ec2-metadata-tokenandX-aws-ec2-metadata-token-ttl-seconds).mmds.rx_invalid_tokenandmmds.rx_no_tokenmetrics 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-secondsheader only if it is aPUTrequest to/latest/api/token.X-Forwarded-Forheader 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 checkstyleto 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.