Add more restrictions on VPC ip range

CentaurusInfra · Mar 3, 2022 · 5d8b724 · 5d8b724
1 parent 5d8567d
commit 5d8b724
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 25 deletions.
diff --git a/pkg/controller/mizar/mizar-arktos-network-controller.go b/pkg/controller/mizar/mizar-arktos-network-controller.go
@@ -256,8 +256,15 @@ func createVpcAndSubnet(vpc, subnet string, dynamicClient dynamic.Interface) err
 func generateVPCSpec(vpcName string) (int, *MizarVPC) {
 	// randomize ip start segment:
 	ipStart := ran.Intn(255) + 1 // IpStart range [1, 255]
+
+	// 224.x.x.x - 239.x.x.x is reserved for IPv4 multicast
+	// https://www.iana.org/assignments/multicast-addresses/multicast-addresses.xhtml
+	if ipStart >= 224 && ipStart <= 239 {
+		ipStart %= 224
+	}
+
 	// Simply not allow ip start from 10, 172, 192, 100 for well-known private range
-	if ipStart == 10 || ipStart == 172 || ipStart == 192 || ipStart == 100 {
+	if ipStart == 0 || ipStart == 10 || ipStart == 172 || ipStart == 192 || ipStart == 100 {
 		ipStart++
 	}
 

diff --git a/pkg/controller/mizar/mizar-arktos-network-controller_test.go b/pkg/controller/mizar/mizar-arktos-network-controller_test.go
@@ -23,31 +23,34 @@ import (
 )
 
 func TestGenerateVPCSpec(t *testing.T) {
-	ipStart, vpcSpec := generateVPCSpec("vpc1")
-	verifyIpStart(t, ipStart)
-	verifyVPCSpec(t, vpcSpec)
+	for i := 0; i < 1000; i++ {
+		ipStart, vpcSpec := generateVPCSpec("vpc1")
+		verifyIpStart(t, ipStart)
+		verifyVPCSpec(t, vpcSpec)
 
-	vpcJsonData, err := json.Marshal(vpcSpec)
-	assert.Nil(t, err, "Unexpected marshalling error")
-	var unmarshallData MizarVPC
-	err = json.Unmarshal(vpcJsonData, &unmarshallData)
-	assert.Nil(t, err, "Unexpected unmarshalling error")
-	assert.Equal(t, vpcSpec.APIVersion, unmarshallData.APIVersion)
-	assert.Equal(t, vpcSpec.Kind, unmarshallData.Kind)
-	assert.Equal(t, vpcSpec.Metadata.Name, unmarshallData.Metadata.Name)
-	assert.Equal(t, vpcSpec.Spec.IP, unmarshallData.Spec.IP)
-	assert.Equal(t, vpcSpec.Spec.Prefix, unmarshallData.Spec.Prefix)
-	assert.Equal(t, vpcSpec.Spec.Divider, unmarshallData.Spec.Divider)
-	assert.Equal(t, vpcSpec.Spec.Status, unmarshallData.Spec.Status)
+		vpcJsonData, err := json.Marshal(vpcSpec)
+		assert.Nil(t, err, "Unexpected marshalling error")
+		var unmarshallData MizarVPC
+		err = json.Unmarshal(vpcJsonData, &unmarshallData)
+		assert.Nil(t, err, "Unexpected unmarshalling error")
+		assert.Equal(t, vpcSpec.APIVersion, unmarshallData.APIVersion)
+		assert.Equal(t, vpcSpec.Kind, unmarshallData.Kind)
+		assert.Equal(t, vpcSpec.Metadata.Name, unmarshallData.Metadata.Name)
+		assert.Equal(t, vpcSpec.Spec.IP, unmarshallData.Spec.IP)
+		assert.Equal(t, vpcSpec.Spec.Prefix, unmarshallData.Spec.Prefix)
+		assert.Equal(t, vpcSpec.Spec.Divider, unmarshallData.Spec.Divider)
+		assert.Equal(t, vpcSpec.Spec.Status, unmarshallData.Spec.Status)
+	}
 }
 
 func verifyIpStart(t *testing.T, ipStart int) {
-	assert.True(t, ipStart >= 1, "VPC started should be in range [1, 255]")
-	assert.True(t, ipStart <= 255, "VPC started should be in range [1, 255]")
-	assert.True(t, ipStart != 10, "VPC cannot start with 10.x.x.x")
-	assert.True(t, ipStart != 172, "VPC cannot start with 172.x.x.x")
-	assert.True(t, ipStart != 192, "VPC cannot start with 192.x.x.x")
-	assert.True(t, ipStart != 100, "VPC cannot start with 100.x.x.x")
+	assert.True(t, ipStart >= 1, "VPC started should be in range [1, 255], got %d", ipStart)
+	assert.True(t, ipStart <= 255, "VPC started should be in range [1, 255], got %d", ipStart)
+	assert.True(t, ipStart != 10, "VPC cannot start with 10.x.x.x, got %d", ipStart)
+	assert.True(t, ipStart != 172, "VPC cannot start with 172.x.x.x, got %d", ipStart)
+	assert.True(t, ipStart != 192, "VPC cannot start with 192.x.x.x, got %d", ipStart)
+	assert.True(t, ipStart != 100, "VPC cannot start with 100.x.x.x, got %d", ipStart)
+	assert.True(t, ipStart < 224 || ipStart > 239, "VPC cannot start with 224-239.x.x.x, got %d", ipStart)
 }
 
 func verifyVPCSpec(t *testing.T, vpcSpec *MizarVPC) {

diff --git a/pkg/controller/mizar/mizar-service-controller.go b/pkg/controller/mizar/mizar-service-controller.go
@@ -245,7 +245,7 @@ func (c *MizarServiceController) processServiceCreateOrUpdate(service *v1.Servic
 		service.Annotations[mizarAnnotationsVpcKey] = getVPC(tenantDefaultNetwork)
 		service.Annotations[mizarAnnotationsSubnetKey] = getSubnetNameFromVPC(tenantDefaultNetwork.Spec.VPCID)
 		_, err := c.kubeClientset.CoreV1().ServicesWithMultiTenancy(service.Namespace, service.Tenant).Update(service)
-		klog.V(4).Infof("Add mizar annotation for service %s/%s/%s. error %v", key, err)
+		klog.V(4).Infof("Add mizar annotation for service %s. error %v", key, err)
 		if err != nil {
 			return errors.New(fmt.Sprintf("update service %s mizar annotation got error (%v)", key, err))
 		}
@@ -303,7 +303,7 @@ func (c *MizarServiceController) processServiceCreateOrUpdate(service *v1.Servic
 			}
 		}
 	case CodeType_TEMP_ERROR:
-		klog.Warningf("Mizar hit temporary error for service creation for service: %s.")
+		klog.Warningf("Mizar hit temporary error for service creation for service: %s.", key)
 		return errors.New("Service creation failed on mizar side, will try again.....")
 	case CodeType_PERM_ERROR:
 		klog.Errorf("Mizar hit permanent error for service creation for service: %s.", key)
@@ -323,7 +323,7 @@ func (c *MizarServiceController) processServiceCreateOrUpdate(service *v1.Servic
 			return err
 		}
 	} else if service.Spec.ClusterIP != ip {
-		klog.Warningf("Service %s cluster ip %s is different from mizar assigned ip %s", key, ip)
+		klog.Warningf("Service %s cluster ip %s is different from mizar assigned ip %s", key, service.Spec.ClusterIP, ip)
 	}
 
 	return nil