-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(ec2): support network interface L2 #28901
base: main
Are you sure you want to change the base?
Conversation
This PR has been in the BUILD FAILING state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
I suggest we hold off on this PR, as there is a more comprehensive one in progress right now. For more information, check out the RFC. |
This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
@@ -554,6 +556,13 @@ export class Instance extends Resource implements IInstance { | |||
groupSet: securityGroupsToken, | |||
privateIpAddress: props.privateIpAddress, | |||
}] : undefined; | |||
this.attachedNetworkInterfaces = [ | |||
new class DummyNetworkInterface extends Resource implements INetworkInterface { |
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.
could you help me understand why do we need a dummy class here ?
/** | ||
* Adds a network interface to the instance. | ||
*/ | ||
public addNetworkInterface(networkInterface: INetworkInterface, options: AddNetworkInterfaceOptions = {}) { |
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.
can a same network interface be attached to two instance? i think not.. let me know if that's not the case.
instead of taking a network interface as an argument, create a new one with and take in the parameters that are required to create n/w interface
What change?
Add
ec2.NetworkInterface
L2 constructs andec2.Instance.addNetworkInterface()
method.Why need this change?
There are three reasons.
ec2.Instance
doesn't support additional IPv4 address. It is hard in the cace of advanced network architects (e.g. Multi-VPC ENI Attacements). To support network interface L2 is very usefull.ec2.Instance
can not get ID of network interface. Customers who need a network interface ID wants configure network interface at initializeec2.Instance
.How to use this?
Following document is README.md.
Network Interface
You can attach additional network interfaces to an EC2 instance. Attaching multiple network interfaces to an instance is useful when you want to:
The following code how to add additional network interfaces for an EC2 instance.
You can also assign private IPv4 address from prefixes or specific IPv4 addresses.
The following code assigns IPv4 address from prefixes.
For more information see Scenarios for network interfaces.
Design decision-making
Additional network interfaces are not allow modify security group rules by
ec2.Instance.connections.allowXxx()
. For allow network interface security group rules, customers must explicitly allow it in the security group of the network interface. This decision because, I think customers such as using network interface are need advanced networking architects, they wants specify security groups every network interfaces.What to do and what not to do in this PR
For minimize this PR size, I implement part of features. Not implemented features will implement at another PR after marged this.
Do
ec2.NetworkInterface
constructec2.INetworkInterface
construct interfaceec2.Instance.addNetworkInterface()
methodNetworkInterface.fromNetworkInterfaceAttributes()
)Do not
Ref
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license