-
Notifications
You must be signed in to change notification settings - Fork 632
IPv6 default route for IPv6-only awsvpc tasks #4603
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
7434788
to
ba1a2a8
Compare
if eni.IPv6Only() { | ||
// Populate IPv6 Subnet Gateway address only for IPv6-only case as historically it hasn't | ||
// been populated for dual-stack case. | ||
if eni.GetSubnetGatewayIPv6Address() == "" { |
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.
Would it be an error if the IPv4 subnet gatway address is populated here and if so should we report an error?
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.
plus 1 to this comment, I dont see any validation being done for this field not to get an ipv4 address
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.
Would it be an error if the IPv4 subnet gatway address is populated here and if so should we report an error?
Technically yes. The consequence will be that the task network will have an IPv4 default route and no IPv6 default route causing all IPv6 outgoing traffic to not work. However, task payloads are sent to Agent by ECS Backend which is a trusted source, so Agent does minimal validation of task payloads.
Parsing ACS payload for this field is a part of this other PR. That has some validation (not specifically the IP version though). We can discuss the validations there.
ba1a2a8
to
609522f
Compare
Summary
This change makes Agent create an IPv6 default route in task network namespace of awsvpc tasks whose task ENIs are IPv6-only. The default route is via the task subnet's gateway address which will be passed to Agent by ECS backend. Changes to update ECS backend model to include IPv6 subnet gateway address are not a part of this change and will be made separately in the near future.
This change is applicable only to IPv6-only task ENIs. Dual stack task ENIs continue to work as before (without an explicitly created default IPv6 route).
The default routes are created by vpc-eni (for standard awsvpc mode) and vpc-branch-eni (for awsvpc mode with ENI trunking enabled) CNI plugins. The plugins themselves already support creating default IPv6 routes, so we just need Agent to pass the IPv6 subnet gateway address to the plugins.
Implementation details
SubnetGatewayIPV6Address
field toNetworkInterface
type in ecs-agent module. Also add aGetSubnetGatewayIPv6Address
method to get it. This struct is Agent's representation of ENIs.Testing
New tests cover the changes: yes
Made temporary changes to the code so that task ENIs as seen by task engine only have IPv6 addresses (they are already passed by ECS backend) and an IPv6 subnet gateway address. Ran an awsvpc task in dual-stack subnet and verified that the task's network has an IPv6 default route via the subnet gateway and no IPv4 default route.
There are two IPv6 default routes because a default route is automatically configured for IPv6 task ENIs by EC2. However, ECS will be adding an explicit route that will be via the default subnet gateway address and will take priority (as it has no metric).
Did the same testing on an instance enabled for ENI trunking also. Observed the same results.
Description for the changelog
Additional Information
Does this PR include breaking model changes? If so, Have you added transformation functions?
Does this PR include the addition of new environment variables in the README?
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.