feat: check configuration #1832

lut777 · 2022-08-21T11:20:30Z

Make sure you have followed Kube-OVN Code Style.

What type of this PR

Examples of user facing changes:

Features

Now controller will check network CIDRs before initialization, and illegal CIDR will lead to fatal error.

Which issue(s) this PR fixes:

Fixes #1813

oilbeater · 2022-08-22T12:06:40Z

util/validator.go has similar logical and should reuse it.

lut777 · 2022-08-23T06:44:39Z

util/validator.go has similar logical and should reuse it.

It's done.

oilbeater · 2022-08-24T02:00:38Z

pkg/controller/config.go

@@ -249,6 +249,10 @@ func ParseFlags() (*Configuration, error) {
 		return nil, err
 	}

+	if err := util.CIDRConflict([]string{config.NodeSwitchCIDR, config.DefaultCIDR, config.ServiceClusterIPRange}); err != nil {


cidrCheck 和 conflictCheck 要拆开

两个功能性函数已经拆开, 并且已经根据之前的函数功能重新简化命名并使用.

oilbeater · 2022-08-24T02:02:46Z

pkg/util/net.go

+		if err != nil {
+			return err
+		}
+		if v4 != nil {


不要在 cidrconflict 里检查 cidr 是否正确。否则一个子网非法，其他正常子网都会冲突无法创建

oilbeater · 2022-08-24T02:03:38Z

pkg/util/net.go

+	return nil
+}
+
+func ReturnCidr(address string) (*net.IPNet, *net.IPNet, error) {


Suggested change

func ReturnCidr(address string) (*net.IPNet, *net.IPNet, error) {

func ReturnCidr(cidrBlock string) (*net.IPNet, *net.IPNet, error) {

oilbeater · 2022-08-24T02:04:21Z

pkg/util/net.go

+		return nil, acidr, nil
+	}
+
+	return nil, nil, fmt.Errorf("ip %v unresolvable", address)


Suggested change

return nil, nil, fmt.Errorf("ip %v unresolvable", address)

return nil, nil, fmt.Errorf("cidr %v unresolvable", address)

oilbeater · 2022-08-24T02:04:36Z

pkg/util/net.go

+	return nil, nil, fmt.Errorf("ip %v unresolvable", address)
+}
+
+func checkIPV4(ip *net.IPNet) error {


Suggested change

func checkIPV4(ip *net.IPNet) error {

func checkIPV4Cidr(cidr *net.IPNet) error {

oilbeater · 2022-08-24T02:05:09Z

pkg/util/net.go

+	return nil
+}
+
+func checkIPV6(ip *net.IPNet) error {


Suggested change

func checkIPV6(ip *net.IPNet) error {

func checkIPV6Cidr(cidr *net.IPNet) error {

zhangzujian · 2022-08-30T07:20:51Z

.github/workflows/build-x86-image.yaml

+      - name: Setup upterm session
+        uses: lhotari/action-upterm@v1
+


should be removed

deleted after debug

oilbeater · 2022-08-30T13:22:27Z

pkg/util/net.go

@@ -434,3 +428,77 @@ func GatewayContains(gatewayNodeStr, gateway string) bool {
 func JoinHostPort(host string, port int32) string {
 	return net.JoinHostPort(host, strconv.FormatInt(int64(port), 10))
 }
+
+func CheckSupCIDROverlap(a, b string) bool {


CheckSupCIDROverlap -> CIDROverlap

oilbeater · 2022-08-30T13:22:59Z

pkg/util/net.go

+	return false
+}
+
+func CheckCIDRSpec(cidr string) error {


CheckCIDRSpec -> CIDRGlobalUnicast

oilbeater · 2022-08-30T13:23:54Z

pkg/util/net.go

+const (
+	IPV4Multicast        = "224.0.0.0/4"
+	IPV4Loopback         = "127.0.0.1/8"
+	IPv4bcast            = "255.255.255.255/32"


IPv4Broadcast

oilbeater · 2022-08-30T13:24:16Z

pkg/util/net.go

+	IPV4Multicast        = "224.0.0.0/4"
+	IPV4Loopback         = "127.0.0.1/8"
+	IPv4bcast            = "255.255.255.255/32"
+	IPv4zero             = "0.0.0.0/32"


oilbeater · 2022-08-30T13:25:11Z

pkg/util/net.go

+	IPv4LinkLocalUnicast = "169.254.0.0/16"
+
+	IPv6unspecified      = "::/128"
+	IPv6loopback         = "::1/128"
+	IPV6Multicast        = "ff00::/8"
+	IPv6linkLocalUnicast = "FE80::/10"


All the variables should use camelcase

oilbeater · 2022-08-30T13:26:54Z

pkg/util/net.go

+func CheckCIDRSpec(cidr string) error {
+	for _, cidrBlock := range strings.Split(cidr, ",") {
+		if CheckSupCIDROverlap(cidrBlock, IPv4bcast) {
+			return fmt.Errorf("%s conflict with v4 boardcast cidr %s", cidr, IPv4bcast)


oilbeater · 2022-08-30T13:28:17Z

pkg/util/net.go

+			return fmt.Errorf("%s conflict with v4 localnet cidr %s", cidr, IPv4zero)
+		}
+		if CheckSupCIDROverlap(cidrBlock, IPv4LinkLocalUnicast) {
+			return fmt.Errorf("%s conflict with v4 locallink cidr %s", cidr, IPv4LinkLocalUnicast)


oilbeater · 2022-08-30T13:29:06Z

pkg/util/net.go

+			return fmt.Errorf("%s conflict with vv6 multicast cidr %s", cidr, IPV6Multicast)
+		}
+		if CheckSupCIDROverlap(cidrBlock, IPv6linkLocalUnicast) {
+			return fmt.Errorf("%s conflict with v6 locallink cidr %s", cidr, IPv6linkLocalUnicast)


oilbeater · 2022-08-30T13:30:13Z

pkg/util/net.go

+	return nil
+}
+
+func CheckCIDRsAll(cidrs []string) error {


CheckCIDRsAll -> CheckSystemCIDR

oilbeater · 2022-08-30T13:34:14Z

pkg/util/net.go

+		if i == (lenth - 1) {
+			break
+		}
+		c := i + 1
+		for c < lenth {
+			if CheckSupCIDROverlap(cidr, cidrs[c]) {
+				err := fmt.Errorf("cidr %s is conflict with cidr %s", cidr, cidrs[c])
+				return err
+			}
+			c += 1
+		}


Suggested change

if i == (lenth - 1) {

break

}

c := i + 1

for c < lenth {

if CheckSupCIDROverlap(cidr, cidrs[c]) {

err := fmt.Errorf("cidr %s is conflict with cidr %s", cidr, cidrs[c])

return err

}

c += 1

}

for j := range cidrs {

if j == i {

continue

}

if CheckSupCIDROverlap(cidr, cidrs[c]) {

err := fmt.Errorf("cidr %s is conflict with cidr %s", cidr, cidrs[c])

return err

}

}

fixed and done

oilbeater · 2022-08-31T08:40:12Z

pkg/util/net.go

+	IPV4Multicast        = "224.0.0.0/4"
+	IPV4Loopback         = "127.0.0.1/8"


oilbeater · 2022-08-31T08:40:26Z

pkg/util/net.go

+
+	IPv6Unspecified      = "::/128"
+	IPv6Loopback         = "::1/128"
+	IPV6Multicast        = "ff00::/8"


oilbeater · 2022-08-31T08:41:03Z

pkg/util/net.go

+			return fmt.Errorf("%s conflict with v6 loopback cidr %s", cidr, IPv6Loopback)
+		}
+		if CIDROverlap(cidrBlock, IPV6Multicast) {
+			return fmt.Errorf("%s conflict with vv6 multicast cidr %s", cidr, IPV6Multicast)


oilbeater · 2022-08-31T08:42:32Z

pkg/controller/config.go

@@ -249,6 +249,10 @@ func ParseFlags() (*Configuration, error) {
 		return nil, err
 	}

+	if err := util.CheckSystemCIDR([]string{config.NodeSwitchCIDR, config.DefaultCIDR, config.ServiceClusterIPRange}); err != nil {
+		return nil, err


klog.Errorf("check system cidr failed, %v", err)

lut777 force-pushed the master branch 2 times, most recently from fdec3b2 to cf4863c Compare August 23, 2022 04:04

lut777 force-pushed the master branch 2 times, most recently from 75b922d to 88a43d7 Compare August 23, 2022 12:16

oilbeater requested changes Aug 24, 2022

View reviewed changes

lut777 force-pushed the master branch 4 times, most recently from c09f492 to f2252bd Compare August 30, 2022 06:56

zhangzujian reviewed Aug 30, 2022

View reviewed changes

lut777 force-pushed the master branch 5 times, most recently from 77a3654 to f7b6973 Compare August 30, 2022 10:46

oilbeater requested changes Aug 30, 2022

View reviewed changes

lut777 force-pushed the master branch from f7b6973 to 4b12e8a Compare August 31, 2022 08:23

oilbeater requested changes Aug 31, 2022

View reviewed changes

feat: check configuration

d56c581

lut777 force-pushed the master branch from 4b12e8a to d56c581 Compare August 31, 2022 08:59

oilbeater approved these changes Aug 31, 2022

View reviewed changes

lut777 merged commit dedd5aa into kubeovn:master Aug 31, 2022

lut777 added a commit that referenced this pull request Aug 31, 2022

feat: check configuration (#1832)

e52d347

	func ReturnCidr(address string) (net.IPNet, net.IPNet, error) {
	func ReturnCidr(cidrBlock string) (net.IPNet, net.IPNet, error) {

	return nil, nil, fmt.Errorf("ip %v unresolvable", address)
	return nil, nil, fmt.Errorf("cidr %v unresolvable", address)

	func checkIPV4(ip *net.IPNet) error {
	func checkIPV4Cidr(cidr *net.IPNet) error {

	func checkIPV6(ip *net.IPNet) error {
	func checkIPV6Cidr(cidr *net.IPNet) error {

feat: check configuration #1832

feat: check configuration #1832

Conversation

lut777 commented Aug 21, 2022 • edited Loading

What type of this PR

Which issue(s) this PR fixes:

oilbeater commented Aug 22, 2022

lut777 commented Aug 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lut777 commented Aug 21, 2022 •

edited

Loading