From 5d8b7240dd392cf322d939c62007e6f154bd6ac6 Mon Sep 17 00:00:00 2001 From: Ying Huang Date: Thu, 3 Mar 2022 05:00:20 +0000 Subject: [PATCH] Add more restrictions on VPC ip range --- .../mizar/mizar-arktos-network-controller.go | 9 +++- .../mizar-arktos-network-controller_test.go | 45 ++++++++++--------- .../mizar/mizar-service-controller.go | 6 +-- 3 files changed, 35 insertions(+), 25 deletions(-) diff --git a/pkg/controller/mizar/mizar-arktos-network-controller.go b/pkg/controller/mizar/mizar-arktos-network-controller.go index 9ae15914995..ea54278a712 100644 --- 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 index 6c6f7535624..c6ce067cdd0 100644 --- 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 index 6e384b84cc8..701654bb73e 100644 --- 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