-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Semi-automatic egress IP #16561
Semi-automatic egress IP #16561
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,6 +68,9 @@ type HostSubnet struct { | |
HostIP string `json:"hostIP" protobuf:"bytes,3,opt,name=hostIP"` | ||
// Subnet is the CIDR range of the overlay network assigned to the node for its pods | ||
Subnet string `json:"subnet" protobuf:"bytes,4,opt,name=subnet"` | ||
|
||
// EgressIPs is the list of automatic egress IP addresses currently hosted by this node | ||
EgressIPs []string `json:"egressIPs" protobuf:"bytes,5,rep,name=egressIPs"` | ||
} | ||
|
||
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object | ||
|
@@ -97,6 +100,10 @@ type NetNamespace struct { | |
NetName string `json:"netname" protobuf:"bytes,2,opt,name=netname"` | ||
// NetID is the network identifier of the network namespace assigned to each overlay network packet. This can be manipulated with the "oc adm pod-network" commands. | ||
NetID uint32 `json:"netid" protobuf:"varint,3,opt,name=netid"` | ||
|
||
// EgressIPs is a list of reserved IPs that will be used as the source for external traffic coming from pods in this namespace. (If empty, external traffic will be masqueraded to Node IPs.) | ||
// +optional | ||
EgressIPs []string `json:"egressIPs" protobuf:"bytes,4,rep,name=egressIPs"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same omitempty? |
||
} | ||
|
||
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,8 +102,6 @@ func ValidateClusterNetworkUpdate(obj *networkapi.ClusterNetwork, old *networkap | |
return allErrs | ||
} | ||
|
||
// ValidateHostSubnet tests fields for the host subnet, the host should be a network resolvable string, | ||
// and subnet should be a valid CIDR | ||
func ValidateHostSubnet(hs *networkapi.HostSubnet) field.ErrorList { | ||
allErrs := validation.ValidateObjectMeta(&hs.ObjectMeta, false, path.ValidatePathSegmentName, field.NewPath("metadata")) | ||
|
||
|
@@ -125,6 +123,13 @@ func ValidateHostSubnet(hs *networkapi.HostSubnet) field.ErrorList { | |
if net.ParseIP(hs.HostIP) == nil { | ||
allErrs = append(allErrs, field.Invalid(field.NewPath("hostIP"), hs.HostIP, "invalid IP address")) | ||
} | ||
|
||
for i, egressIP := range hs.EgressIPs { | ||
if net.ParseIP(egressIP) == nil { | ||
allErrs = append(allErrs, field.Invalid(field.NewPath("egressIPs").Index(i), egressIP, "invalid IP address")) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we expect egress IP to work for IPV6 as well? If not, may be we should add ipv4 check here and in netnamespace.EgressIPs validation. |
||
} | ||
|
||
return allErrs | ||
} | ||
|
||
|
@@ -150,6 +155,13 @@ func ValidateNetNamespace(netnamespace *networkapi.NetNamespace) field.ErrorList | |
if err := network.ValidVNID(netnamespace.NetID); err != nil { | ||
allErrs = append(allErrs, field.Invalid(field.NewPath("netid"), netnamespace.NetID, err.Error())) | ||
} | ||
|
||
for i, ip := range netnamespace.EgressIPs { | ||
if net.ParseIP(ip) == nil { | ||
allErrs = append(allErrs, field.Invalid(field.NewPath("egressIPs").Index(i), ip, "invalid IP address")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this allowed to be localhost? Multicast? Any implications of those? (extra validation is needed only if it there is no clear use case for ever allowing those) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no use case for setting it to either of those, but there's no danger either. (It would just fail, just like if they tried to claim a random far-away IP address; EgressIPs have to be on the same subnet as some node's primary IP.) We don't check for semantically bad IPs in validation anywhere else, so I think just letting it through here and then failing later makes sense. |
||
} | ||
} | ||
|
||
return allErrs | ||
} | ||
|
||
|
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.
omitempty?