Skip to content

Commit

Permalink
Merge pull request #4604 from arghosh93/SDN-5138
Browse files Browse the repository at this point in the history 
Add subnet overlap check for POD and join subnets in net-attach-def
  • Loading branch information
@tssurya
tssurya committed Sep 3, 2024
2 parents 22727db + 5115b72 commit 834fd00
Show file tree
Hide file tree
Showing 6 changed files with 250 additions and 74 deletions.
22 changes: 11 additions & 11 deletions go-controller/pkg/clustermanager/endpointslicemirror/endpointslice_mirror_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ var _ = ginkgo.Describe("Cluster manager EndpointSlice mirror controller", func(
ObjectMeta: metav1.ObjectMeta{
Name: "test-pod",
Namespace: namespaceT.Name,
Annotations: map[string]string{util.OvnPodAnnotationName: `{"default":{"mac_address":"0a:58:0a:f4:02:03","ip_address":"10.244.2.3/24","role":"infrastructure-locked"},"testns/l3-network":{"mac_address":"0a:58:0a:80:02:04","ip_address":"10.128.2.4/24","role":"primary"}}`},
Annotations: map[string]string{util.OvnPodAnnotationName: `{"default":{"mac_address":"0a:58:0a:f4:02:03","ip_address":"10.244.2.3/24","role":"infrastructure-locked"},"testns/l3-network":{"mac_address":"0a:58:0a:84:02:04","ip_address":"10.132.2.4/24","role":"primary"}}`},
},
Status: v1.PodStatus{Phase: v1.PodRunning},
}
Expand Down Expand Up @@ -122,7 +122,7 @@ var _ = ginkgo.Describe("Cluster manager EndpointSlice mirror controller", func(

_, err := fakeClient.NetworkAttchDefClient.K8sCniCncfIoV1().NetworkAttachmentDefinitions(namespaceT.Name).Create(
context.TODO(),
testing.GenerateNAD("l3-network", "l3-network", namespaceT.Name, types.Layer3Topology, "10.128.2.0/16/24", types.NetworkRolePrimary),
testing.GenerateNAD("l3-network", "l3-network", namespaceT.Name, types.Layer3Topology, "10.132.2.0/16/24", types.NetworkRolePrimary),
metav1.CreateOptions{})
gomega.Expect(err).ToNot(gomega.HaveOccurred())

Expand Down Expand Up @@ -163,7 +163,7 @@ var _ = ginkgo.Describe("Cluster manager EndpointSlice mirror controller", func(
gomega.Expect(mirroredEndpointSlices.Items[0].Endpoints).To(gomega.HaveLen(1))
gomega.Expect(mirroredEndpointSlices.Items[0].Endpoints[0].Addresses).To(gomega.HaveLen(1))
// check if the Address is set to the primary IP
gomega.Expect(mirroredEndpointSlices.Items[0].Endpoints[0].Addresses[0]).To(gomega.BeEquivalentTo("10.128.2.4"))
gomega.Expect(mirroredEndpointSlices.Items[0].Endpoints[0].Addresses[0]).To(gomega.BeEquivalentTo("10.132.2.4"))

return nil
}
Expand All @@ -182,7 +182,7 @@ var _ = ginkgo.Describe("Cluster manager EndpointSlice mirror controller", func(
ObjectMeta: metav1.ObjectMeta{
Name: "test-pod",
Namespace: namespaceT.Name,
Annotations: map[string]string{util.OvnPodAnnotationName: `{"default":{"mac_address":"0a:58:0a:f4:02:03","ip_address":"10.244.2.3/24","role":"primary"},"testns/l3-network":{"mac_address":"0a:58:0a:80:02:04","ip_address":"10.128.2.4/24","role":"secondary}}`},
Annotations: map[string]string{util.OvnPodAnnotationName: `{"default":{"mac_address":"0a:58:0a:f4:02:03","ip_address":"10.244.2.3/24","role":"primary"},"testns/l3-network":{"mac_address":"0a:58:0a:84:02:04","ip_address":"10.132.2.4/24","role":"secondary}}`},
},
Status: v1.PodStatus{Phase: v1.PodRunning},
}
Expand Down Expand Up @@ -230,7 +230,7 @@ var _ = ginkgo.Describe("Cluster manager EndpointSlice mirror controller", func(

_, err := fakeClient.NetworkAttchDefClient.K8sCniCncfIoV1().NetworkAttachmentDefinitions(namespaceT.Name).Create(
context.TODO(),
testing.GenerateNAD("l3-network", "l3-network", namespaceT.Name, types.Layer3Topology, "10.128.2.0/16/24", types.NetworkRoleSecondary),
testing.GenerateNAD("l3-network", "l3-network", namespaceT.Name, types.Layer3Topology, "10.132.2.0/16/24", types.NetworkRoleSecondary),
metav1.CreateOptions{})
gomega.Expect(err).ToNot(gomega.HaveOccurred())

Expand Down Expand Up @@ -274,7 +274,7 @@ var _ = ginkgo.Describe("Cluster manager EndpointSlice mirror controller", func(
ObjectMeta: metav1.ObjectMeta{
Name: "test-pod",
Namespace: namespaceT.Name,
Annotations: map[string]string{util.OvnPodAnnotationName: `{"default":{"mac_address":"0a:58:0a:f4:02:03","ip_address":"10.244.2.3/24","role":"infrastructure-locked"},"testns/l3-network":{"mac_address":"0a:58:0a:80:02:04","ip_address":"10.128.2.4/24","role":"primary"}}`},
Annotations: map[string]string{util.OvnPodAnnotationName: `{"default":{"mac_address":"0a:58:0a:f4:02:03","ip_address":"10.244.2.3/24","role":"infrastructure-locked"},"testns/l3-network":{"mac_address":"0a:58:0a:84:02:04","ip_address":"10.132.2.4/24","role":"primary"}}`},
},
Status: v1.PodStatus{Phase: v1.PodRunning},
}
Expand Down Expand Up @@ -324,7 +324,7 @@ var _ = ginkgo.Describe("Cluster manager EndpointSlice mirror controller", func(

_, err := fakeClient.NetworkAttchDefClient.K8sCniCncfIoV1().NetworkAttachmentDefinitions(namespaceT.Name).Create(
context.TODO(),
testing.GenerateNAD("l3-network", "l3-network", namespaceT.Name, types.Layer3Topology, "10.128.2.0/16/24", types.NetworkRolePrimary),
testing.GenerateNAD("l3-network", "l3-network", namespaceT.Name, types.Layer3Topology, "10.132.2.0/16/24", types.NetworkRolePrimary),
metav1.CreateOptions{})
gomega.Expect(err).ToNot(gomega.HaveOccurred())

Expand Down Expand Up @@ -361,14 +361,14 @@ var _ = ginkgo.Describe("Cluster manager EndpointSlice mirror controller", func(
return nil
}).WithTimeout(5 * time.Second).ShouldNot(gomega.HaveOccurred())
gomega.Expect(mirroredEndpointSlices.Items[0].Endpoints[0].Addresses).To(gomega.HaveLen(1))
gomega.Expect(mirroredEndpointSlices.Items[0].Endpoints[0].Addresses).To(gomega.BeEquivalentTo([]string{"10.128.2.4"}))
gomega.Expect(mirroredEndpointSlices.Items[0].Endpoints[0].Addresses).To(gomega.BeEquivalentTo([]string{"10.132.2.4"}))

ginkgo.By("when the EndpointSlice changes the mirrored one gets updated")
newPod := v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test-pod-new",
Namespace: namespaceT.Name,
Annotations: map[string]string{util.OvnPodAnnotationName: `{"default":{"mac_address":"0a:58:0a:f4:02:04","ip_address":"10.244.2.4/24","primary":false},"testns/l3-network":{"mac_address":"0a:58:0a:80:02:05","ip_address":"10.128.2.5/24","primary":true}}`},
Annotations: map[string]string{util.OvnPodAnnotationName: `{"default":{"mac_address":"0a:58:0a:f4:02:04","ip_address":"10.244.2.4/24","primary":false},"testns/l3-network":{"mac_address":"0a:58:0a:84:02:05","ip_address":"10.132.2.5/24","primary":true}}`},
},
Status: v1.PodStatus{Phase: v1.PodRunning},
}
Expand Down Expand Up @@ -419,8 +419,8 @@ var _ = ginkgo.Describe("Cluster manager EndpointSlice mirror controller", func(
return nil
}).WithTimeout(5 * time.Second).ShouldNot(gomega.HaveOccurred())

gomega.Expect(mirroredEndpointSlices.Items[0].Endpoints[0].Addresses[0]).To(gomega.BeEquivalentTo("10.128.2.4"))
gomega.Expect(mirroredEndpointSlices.Items[0].Endpoints[1].Addresses[0]).To(gomega.BeEquivalentTo("10.128.2.5"))
gomega.Expect(mirroredEndpointSlices.Items[0].Endpoints[0].Addresses[0]).To(gomega.BeEquivalentTo("10.132.2.4"))
gomega.Expect(mirroredEndpointSlices.Items[0].Endpoints[1].Addresses[0]).To(gomega.BeEquivalentTo("10.132.2.5"))

ginkgo.By("when the default EndpointSlice is removed the mirrored one follows")
err = fakeClient.KubeClient.DiscoveryV1().EndpointSlices(newPod.Namespace).Delete(context.TODO(), defaultEndpointSlice.Name, metav1.DeleteOptions{})
Expand Down
32 changes: 16 additions & 16 deletions go-controller/pkg/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1752,15 +1752,15 @@ func buildKubernetesConfig(exec kexec.Interface, cli, file *config, saPath strin

// completeKubernetesConfig completes the Kubernetes config by parsing raw values
// into their final form.
func completeKubernetesConfig(allSubnets *configSubnets) error {
func completeKubernetesConfig(allSubnets *ConfigSubnets) error {
Kubernetes.ServiceCIDRs = []*net.IPNet{}
for _, cidrString := range strings.Split(Kubernetes.RawServiceCIDRs, ",") {
_, serviceCIDR, err := net.ParseCIDR(cidrString)
if err != nil {
return fmt.Errorf("kubernetes service network CIDR %q invalid: %v", cidrString, err)
}
Kubernetes.ServiceCIDRs = append(Kubernetes.ServiceCIDRs, serviceCIDR)
allSubnets.append(configSubnetService, serviceCIDR)
allSubnets.Append(ConfigSubnetService, serviceCIDR)
}
if len(Kubernetes.ServiceCIDRs) > 2 {
return fmt.Errorf("kubernetes service-cidrs must contain either a single CIDR or else an IPv4/IPv6 pair")
Expand Down Expand Up @@ -1858,7 +1858,7 @@ func buildGatewayConfig(ctx *cli.Context, cli, file *config) error {
return nil
}

func completeGatewayConfig(allSubnets *configSubnets, masqueradeIPs *MasqueradeIPsConfig) error {
func completeGatewayConfig(allSubnets *ConfigSubnets, masqueradeIPs *MasqueradeIPsConfig) error {
// Validate v4 and v6 join subnets
v4IP, v4JoinCIDR, err := net.ParseCIDR(Gateway.V4JoinSubnet)
if err != nil || utilnet.IsIPv6(v4IP) {
Expand All @@ -1869,8 +1869,8 @@ func completeGatewayConfig(allSubnets *configSubnets, masqueradeIPs *MasqueradeI
if err != nil || !utilnet.IsIPv6(v6IP) {
return fmt.Errorf("invalid gateway v6 join subnet specified, subnet: %s: error: %v", Gateway.V6JoinSubnet, err)
}
allSubnets.append(configSubnetJoin, v4JoinCIDR)
allSubnets.append(configSubnetJoin, v6JoinCIDR)
allSubnets.Append(ConfigSubnetJoin, v4JoinCIDR)
allSubnets.Append(ConfigSubnetJoin, v6JoinCIDR)

//validate v4 and v6 masquerade subnets
v4MasqueradeIP, v4MasqueradeCIDR, err := net.ParseCIDR(Gateway.V4MasqueradeSubnet)
Expand All @@ -1889,8 +1889,8 @@ func completeGatewayConfig(allSubnets *configSubnets, masqueradeIPs *MasqueradeI
return fmt.Errorf("unable to allocate V6MasqueradeIPs: %s", err)
}

allSubnets.append(configSubnetMasquerade, v4MasqueradeCIDR)
allSubnets.append(configSubnetMasquerade, v6MasqueradeCIDR)
allSubnets.Append(ConfigSubnetMasquerade, v4MasqueradeCIDR)
allSubnets.Append(ConfigSubnetMasquerade, v6MasqueradeCIDR)

return nil
}
Expand Down Expand Up @@ -2020,7 +2020,7 @@ func buildHybridOverlayConfig(ctx *cli.Context, cli, file *config) error {

// completeHybridOverlayConfig completes the HybridOverlay config by parsing raw values
// into their final form.
func completeHybridOverlayConfig(allSubnets *configSubnets) error {
func completeHybridOverlayConfig(allSubnets *ConfigSubnets) error {
if !HybridOverlay.Enabled || len(HybridOverlay.RawClusterSubnets) == 0 {
return nil
}
Expand All @@ -2031,7 +2031,7 @@ func completeHybridOverlayConfig(allSubnets *configSubnets) error {
return fmt.Errorf("hybrid overlay cluster subnet invalid: %v", err)
}
for _, subnet := range HybridOverlay.ClusterSubnets {
allSubnets.append(configSubnetHybrid, subnet.CIDR)
allSubnets.Append(ConfigSubnetHybrid, subnet.CIDR)
}

return nil
Expand All @@ -2053,7 +2053,7 @@ func buildClusterManagerConfig(ctx *cli.Context, cli, file *config) error {

// completeClusterManagerConfig completes the ClusterManager config by parsing raw values
// into their final form.
func completeClusterManagerConfig(allSubnets *configSubnets) error {
func completeClusterManagerConfig(allSubnets *ConfigSubnets) error {
// Validate v4 and v6 transit switch subnets
v4IP, v4TransitCIDR, err := net.ParseCIDR(ClusterManager.V4TransitSwitchSubnet)
if err != nil || utilnet.IsIPv6(v4IP) {
Expand All @@ -2064,8 +2064,8 @@ func completeClusterManagerConfig(allSubnets *configSubnets) error {
if err != nil || !utilnet.IsIPv6(v6IP) {
return fmt.Errorf("invalid transit switch v6 subnet specified, subnet: %s: error: %v", ClusterManager.V6TransitSwitchSubnet, err)
}
allSubnets.append(configSubnetTransit, v4TransitCIDR)
allSubnets.append(configSubnetTransit, v6TransitCIDR)
allSubnets.Append(ConfigSubnetTransit, v4TransitCIDR)
allSubnets.Append(ConfigSubnetTransit, v6TransitCIDR)
return nil
}

Expand Down Expand Up @@ -2094,14 +2094,14 @@ func buildDefaultConfig(cli, file *config) error {

// completeDefaultConfig completes the Default config by parsing raw values
// into their final form.
func completeDefaultConfig(allSubnets *configSubnets) error {
func completeDefaultConfig(allSubnets *ConfigSubnets) error {
var err error
Default.ClusterSubnets, err = ParseClusterSubnetEntries(Default.RawClusterSubnets)
if err != nil {
return fmt.Errorf("cluster subnet invalid: %v", err)
}
for _, subnet := range Default.ClusterSubnets {
allSubnets.append(configSubnetCluster, subnet.CIDR)
allSubnets.Append(ConfigSubnetCluster, subnet.CIDR)
}

Default.HostMasqConntrackZone = Default.ConntrackZone + 1
Expand Down Expand Up @@ -2326,7 +2326,7 @@ func initConfigWithPath(ctx *cli.Context, exec kexec.Interface, saPath string, d
}

func completeConfig() error {
allSubnets := newConfigSubnets()
allSubnets := NewConfigSubnets()

if err := completeKubernetesConfig(allSubnets); err != nil {
return err
Expand All @@ -2349,7 +2349,7 @@ func completeConfig() error {
return err
}

if err := allSubnets.checkForOverlaps(); err != nil {
if err := allSubnets.CheckForOverlaps(); err != nil {
return err
}

Expand Down
88 changes: 45 additions & 43 deletions go-controller/pkg/config/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,67 +162,69 @@ func ParseFlowCollectors(flowCollectors string) ([]HostPort, error) {
return parsedFlowsCollectors, nil
}

type configSubnetType string
type ConfigSubnetType string

const (
configSubnetJoin configSubnetType = "built-in join subnet"
configSubnetCluster configSubnetType = "cluster subnet"
configSubnetService configSubnetType = "service subnet"
configSubnetHybrid configSubnetType = "hybrid overlay subnet"
configSubnetMasquerade configSubnetType = "masquerade subnet"
configSubnetTransit configSubnetType = "transit switch subnet"
ConfigSubnetJoin ConfigSubnetType = "built-in join subnet"
ConfigSubnetCluster ConfigSubnetType = "cluster subnet"
ConfigSubnetService ConfigSubnetType = "service subnet"
ConfigSubnetHybrid ConfigSubnetType = "hybrid overlay subnet"
ConfigSubnetMasquerade ConfigSubnetType = "masquerade subnet"
ConfigSubnetTransit ConfigSubnetType = "transit switch subnet"
UserDefinedSubnets ConfigSubnetType = "user defined subnet"
UserDefinedJoinSubnet ConfigSubnetType = "user defined join subnet"
)

type configSubnet struct {
subnetType configSubnetType
subnet *net.IPNet
type ConfigSubnet struct {
SubnetType ConfigSubnetType
Subnet *net.IPNet
}

// configSubnets represents a set of configured subnets (and their names)
type configSubnets struct {
subnets []configSubnet
v4 map[configSubnetType]bool
v6 map[configSubnetType]bool
// ConfigSubnets represents a set of configured subnets (and their names)
type ConfigSubnets struct {
Subnets []ConfigSubnet
V4 map[ConfigSubnetType]bool
V6 map[ConfigSubnetType]bool
}

// newConfigSubnets returns a new configSubnets
func newConfigSubnets() *configSubnets {
return &configSubnets{
v4: make(map[configSubnetType]bool),
v6: make(map[configSubnetType]bool),
// NewConfigSubnets returns a new ConfigSubnets
func NewConfigSubnets() *ConfigSubnets {
return &ConfigSubnets{
V4: make(map[ConfigSubnetType]bool),
V6: make(map[ConfigSubnetType]bool),
}
}

// append adds a single subnet to cs
func (cs *configSubnets) append(subnetType configSubnetType, subnet *net.IPNet) {
cs.subnets = append(cs.subnets, configSubnet{subnetType: subnetType, subnet: subnet})
if subnetType != configSubnetJoin && subnetType != configSubnetMasquerade && subnetType != configSubnetTransit {
func (cs *ConfigSubnets) Append(subnetType ConfigSubnetType, subnet *net.IPNet) {
cs.Subnets = append(cs.Subnets, ConfigSubnet{SubnetType: subnetType, Subnet: subnet})
if subnetType == ConfigSubnetCluster || subnetType == ConfigSubnetService || subnetType == ConfigSubnetHybrid {
if utilnet.IsIPv6CIDR(subnet) {
cs.v6[subnetType] = true
cs.V6[subnetType] = true
} else {
cs.v4[subnetType] = true
cs.V4[subnetType] = true
}
}
}

// checkForOverlaps checks if any of the subnets in cs overlap
func (cs *configSubnets) checkForOverlaps() error {
for i, si := range cs.subnets {
// CheckForOverlaps checks if any of the subnets in cs overlap
func (cs *ConfigSubnets) CheckForOverlaps() error {
for i, si := range cs.Subnets {
for j := 0; j < i; j++ {
sj := cs.subnets[j]
if si.subnet.Contains(sj.subnet.IP) || sj.subnet.Contains(si.subnet.IP) {
sj := cs.Subnets[j]
if si.Subnet.Contains(sj.Subnet.IP) || sj.Subnet.Contains(si.Subnet.IP) {
return fmt.Errorf("illegal network configuration: %s %q overlaps %s %q",
si.subnetType, si.subnet.String(),
sj.subnetType, sj.subnet.String())
si.SubnetType, si.Subnet.String(),
sj.SubnetType, sj.Subnet.String())
}
}
}
return nil
}

func (cs *configSubnets) describeSubnetType(subnetType configSubnetType) string {
ipv4 := cs.v4[subnetType]
ipv6 := cs.v6[subnetType]
func (cs *ConfigSubnets) describeSubnetType(subnetType ConfigSubnetType) string {
ipv4 := cs.V4[subnetType]
ipv6 := cs.V6[subnetType]
var familyType string
switch {
case ipv4 && !ipv6:
Expand All @@ -240,22 +242,22 @@ func (cs *configSubnets) describeSubnetType(subnetType configSubnetType) string
// checkIPFamilies determines if cs contains a valid single-stack IPv4 configuration, a
// valid single-stack IPv6 configuration, a valid dual-stack configuration, or none of the
// above.
func (cs *configSubnets) checkIPFamilies() (usingIPv4, usingIPv6 bool, err error) {
if len(cs.v6) == 0 {
func (cs *ConfigSubnets) checkIPFamilies() (usingIPv4, usingIPv6 bool, err error) {
if len(cs.V6) == 0 {
// Single-stack IPv4
return true, false, nil
} else if len(cs.v4) == 0 {
} else if len(cs.V4) == 0 {
// Single-stack IPv6
return false, true, nil
} else if reflect.DeepEqual(cs.v4, cs.v6) {
} else if reflect.DeepEqual(cs.V4, cs.V6) {
// Dual-stack
return true, true, nil
}

netConfig := cs.describeSubnetType(configSubnetCluster)
netConfig += ", " + cs.describeSubnetType(configSubnetService)
if cs.v4[configSubnetHybrid] || cs.v6[configSubnetHybrid] {
netConfig += ", " + cs.describeSubnetType(configSubnetHybrid)
netConfig := cs.describeSubnetType(ConfigSubnetCluster)
netConfig += ", " + cs.describeSubnetType(ConfigSubnetService)
if cs.V4[ConfigSubnetHybrid] || cs.V6[ConfigSubnetHybrid] {
netConfig += ", " + cs.describeSubnetType(ConfigSubnetHybrid)
}

return false, false, fmt.Errorf("illegal network configuration: %s", netConfig)
Expand Down
8 changes: 4 additions & 4 deletions go-controller/pkg/config/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,15 +300,15 @@ func Test_checkForOverlap(t *testing.T) {
}

for _, tc := range tests {
allSubnets := newConfigSubnets()
allSubnets := NewConfigSubnets()
for _, joinSubnet := range tc.joinSubnetCIDRList {
allSubnets.append(configSubnetJoin, joinSubnet)
allSubnets.Append(ConfigSubnetJoin, joinSubnet)
}
for _, subnet := range tc.cidrList {
allSubnets.append(configSubnetCluster, subnet)
allSubnets.Append(ConfigSubnetCluster, subnet)
}

err := allSubnets.checkForOverlaps()
err := allSubnets.CheckForOverlaps()
if err == nil && tc.shouldError {
t.Errorf("testcase \"%s\" failed to find overlap", tc.name)
} else if err != nil && !tc.shouldError {
Expand Down
Loading

0 comments on commit 834fd00

Please sign in to comment.