Repository: spidernet-io/spiderpool
Pull request #3068 (2024-01-09 07:20:58)

add IPPool and Subnet property subnet field format validation
Refer to https://github.com/spidernet-io/spiderpool/issues/2134

Signed-off-by: Icarus9913 [icaruswu66@qq.com](mailto:icaruswu66@qq.com)

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3068?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io) Report
Attention: `6 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`7f5a0c6`)](https://app.codecov.io/gh/spidernet-io/spiderpool/commit/7f5a0c6d60a2f96f69c72f4006044a5b96059eb5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io) 81.15% compared to head [(`2493b8e`)](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3068?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io) 81.05%.

<details><summary>Additional details and impacted files</summary>


[![Impacted file tree graph](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3068/graphs/tree.svg?width=650&height=150&src=pr&token=YKXY2E4Q8G&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io)](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3068?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io)

```diff
@@            Coverage Diff             @@
##             main    #3068      +/-   ##
==========================================
- Coverage   81.15%   81.05%   -0.11%     
==========================================
  Files          49       49              
  Lines        5334     5351      +17     
==========================================
+ Hits         4329     4337       +8     
- Misses        850      856       +6     
- Partials      155      158       +3     
```

| [Flag](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3068/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io) | Coverage Δ | |
|---|---|---|
| [unittests](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3068/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io) | `81.05% <64.70%> (-0.11%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Files](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3068?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io) | Coverage Δ | |
|---|---|---|
| [pkg/ip/cidr.go](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3068?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io#diff-cGtnL2lwL2NpZHIuZ28=) | `100.00% <100.00%> (ø)` | |
| [pkg/ippoolmanager/ippool\_validate.go](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3068?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io#diff-cGtnL2lwcG9vbG1hbmFnZXIvaXBwb29sX3ZhbGlkYXRlLmdv) | `94.35% <0.00%> (-0.90%)` | :arrow_down: |
| [pkg/subnetmanager/subnet\_validate.go](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3068?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io#diff-cGtnL3N1Ym5ldG1hbmFnZXIvc3VibmV0X3ZhbGlkYXRlLmdv) | `91.90% <0.00%> (-1.13%)` | :arrow_down: |

... and [1 file with indirect coverage changes](https://app.codecov.io/gh/spidernet-io/spiderpool/pull/3068/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spidernet-io)

</details>

Patch:
diff --git a/pkg/ip/cidr.go b/pkg/ip/cidr.go
index 68c01bdb5..cb050e284 100644
--- a/pkg/ip/cidr.go
+++ b/pkg/ip/cidr.go
@@ -102,3 +102,18 @@ func IsIPv6CIDR(subnet string) bool {
 
 	return ip.To4() == nil
 }
+
+func IsFormatCIDR(subnet string) error {
+	_, ipNet, err := net.ParseCIDR(subnet)
+	if nil != err {
+		return err
+	}
+
+	formatCIDR := ipNet.String()
+
+	if formatCIDR != subnet {
+		return fmt.Errorf("unformatted subnet %s, please change it to %s", subnet, formatCIDR)
+	}
+
+	return nil
+}
diff --git a/pkg/ippoolmanager/ippool_validate.go b/pkg/ippoolmanager/ippool_validate.go
index 92b4af093..a68ed3903 100644
--- a/pkg/ippoolmanager/ippool_validate.go
+++ b/pkg/ippoolmanager/ippool_validate.go
@@ -188,6 +188,9 @@ func (iw *IPPoolWebhook) validateIPPoolCIDR(ctx context.Context, ipPool *spiderp
 			err.Error(),
 		)
 	}
+	if err := spiderpoolip.IsFormatCIDR(ipPool.Spec.Subnet); err != nil {
+		return field.Invalid(subnetField, ipPool.Spec.Subnet, err.Error())
+	}
 
 	// TODO(iiiceoo): Use label selector.
 	var ipPoolList spiderpoolv2beta1.SpiderIPPoolList
diff --git a/pkg/subnetmanager/subnet_validate.go b/pkg/subnetmanager/subnet_validate.go
index 84effa05c..f4dbf8e19 100644
--- a/pkg/subnetmanager/subnet_validate.go
+++ b/pkg/subnetmanager/subnet_validate.go
@@ -184,6 +184,9 @@ func (sw *SubnetWebhook) validateSubnetCIDR(ctx context.Context, subnet *spiderp
 			err.Error(),
 		)
 	}
+	if err := spiderpoolip.IsFormatCIDR(subnet.Spec.Subnet); err != nil {
+		return field.Invalid(subnetField, subnet.Spec.Subnet, err.Error())
+	}
 
 	subnetList := spiderpoolv2beta1.SpiderSubnetList{}
 	if err := sw.APIReader.List(ctx, &subnetList); err != nil {


Test patch:
diff --git a/pkg/ip/cidr_test.go b/pkg/ip/cidr_test.go
index 898aca571..cb8c19350 100644
--- a/pkg/ip/cidr_test.go
+++ b/pkg/ip/cidr_test.go
@@ -183,4 +183,31 @@ var _ = Describe("CIDR", Label("cidr_test"), func() {
 			Expect(spiderpoolip.IsIPv6CIDR("abcd:1234::/120")).To(BeTrue())
 		})
 	})
+
+	Describe("Test IsFormatCIDR", func() {
+		It("invalid cidr", func() {
+			err := spiderpoolip.IsFormatCIDR("0.0.0.0.0/16")
+			Expect(err).To(HaveOccurred())
+		})
+
+		It("unformatted IPv4 CIDR", func() {
+			err := spiderpoolip.IsFormatCIDR("172.10.5.0/16")
+			Expect(err).To(HaveOccurred())
+		})
+
+		It("unformatted IPv6 CIDR", func() {
+			err := spiderpoolip.IsFormatCIDR("fc00:f853:ccd:e793::ee/64")
+			Expect(err).To(HaveOccurred())
+		})
+
+		It("format IPv4 CIDR", func() {
+			err := spiderpoolip.IsFormatCIDR("172.10.0.0/16")
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It("format IPv6 CIDR", func() {
+			err := spiderpoolip.IsFormatCIDR("fc00:f853:ccd:e793::/64")
+			Expect(err).NotTo(HaveOccurred())
+		})
+	})
 })
diff --git a/test/e2e/common/tools.go b/test/e2e/common/tools.go
index 4dc38a4e1..a4c6b3383 100644
--- a/test/e2e/common/tools.go
+++ b/test/e2e/common/tools.go
@@ -28,7 +28,7 @@ func GenerateString(lenNum int, isHex bool) string {
 	var chars []string
 	chars = []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z", "A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "N", "O", "P", "Q", "R", "S", "T", "U", "V", "W", "X", "Y", "Z", "1", "2", "3", "4", "5", "6", "7", "8", "9", "0"}
 	if isHex {
-		chars = []string{"0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "a", "b", "c", "d", "e", "f"}
+		chars = []string{"1", "2", "3", "4", "5", "6", "7", "8", "9", "a", "b", "c", "d", "e", "f"}
 	}
 	str := strings.Builder{}
 	length := len(chars)