Bugfix: fix handle poolcoordinator certificates in case of restarting… (

#1187)

* Bugfix: fix handle poolcoordinator certificates in case of restarting yurt-controller-manager

* Test: unittests for poolcoordinator ip address comparison

* Test: unittests for poolcoordinator ip address comparison

pkg/controller/poolcoordinator/cert/certificate.go

@@ -98,13 +98,8 @@ func NewSignedCert(client client.Interface, cfg *CertConfig, key crypto.Signer,
 			return nil, errors.Wrapf(err, "init cert %s fail", cfg.CertName)
 		}
 
-		for _, ip := range ips {
-			cfg.IPs = append(cfg.IPs, ip)
-		}
-		for _, dnsName := range dnsNames {
-			cfg.DNSNames = append(cfg.DNSNames, dnsName)
-		}
-
+		cfg.IPs = append(cfg.IPs, ips...)
+		cfg.DNSNames = append(cfg.DNSNames, dnsNames...)
 	}
 
 	// prepare cert serial number

pkg/controller/poolcoordinator/cert/poolcoordinator_cert_manager.go

@@ -21,7 +21,6 @@ import (
 	"crypto/x509"
 	"fmt"
 	"net"
-	"reflect"
 	"time"
 
 	"github.com/pkg/errors"
@@ -36,6 +35,7 @@ import (
 	"k8s.io/klog/v2"
 
 	certfactory "github.com/openyurtio/openyurt/pkg/util/certmanager/factory"
+	ip "github.com/openyurtio/openyurt/pkg/util/ip"
 )
 
 const (
@@ -107,16 +107,6 @@ type CertConfig struct {
 	certInit certInitFunc
 }
 
-func (c *CertConfig) init(clientSet client.Interface, stopCh <-chan struct{}) (err error) {
-	if c.certInit != nil {
-		c.IPs, c.DNSNames, err = c.certInit(clientSet, stopCh)
-		if err != nil {
-			return errors.Wrapf(err, "fail to init cert %s", c.CertName)
-		}
-	}
-	return nil
-}
-
 var allSelfSignedCerts []CertConfig = []CertConfig{
 	{
 		CertName:     "apiserver-etcd-client",
@@ -296,13 +286,16 @@ func initPoolCoordinator(clientSet client.Interface, stopCh <-chan struct{}) err
 
 			// 1.3 check has dynamic attrs changed
 			if certConf.certInit != nil {
-				if err := certConf.init(clientSet, stopCh); err != nil {
+				// receive dynamic IP addresses
+				ips, _, err := certConf.certInit(clientSet, stopCh)
+				if err != nil {
 					// if cert init failed, skip this cert
-					klog.Errorf("fail to init cert when checking dynamic attrs: %v", err)
+					klog.Errorf("fail to init cert %s when checking dynamic attrs: %v", certConf.CertName, err)
 					continue
 				} else {
-					// check if dynamic IP address has changed
-					if !reflect.DeepEqual(certConf.IPs, cert.IPAddresses) {
+					// check if dynamic IP addresses already exist in cert
+					changed := ip.SearchAllIP(cert.IPAddresses, ips)
+					if changed {
 						klog.Infof("cert %s IP has changed", certConf.CertName)
 						selfSignedCerts = append(selfSignedCerts, certConf)
 						continue

pkg/util/ip/ip.go

@@ -88,3 +88,23 @@ func ParseIPList(ips []string) []net.IP {
 	}
 	return results
 }
+
+// searchIP returns true if ip is in ipList
+func SearchIP(ipList []net.IP, ip net.IP) bool {
+	for _, ipItem := range ipList {
+		if ipItem.Equal(ip) {
+			return true
+		}
+	}
+	return false
+}
+
+// searchAllIP returns true if all ips are in ipList
+func SearchAllIP(ipList []net.IP, ips []net.IP) bool {
+	for _, ip := range ips {
+		if !SearchIP(ipList, ip) {
+			return false
+		}
+	}
+	return true
+}

pkg/util/ip/ip_test.go

@@ -20,6 +20,8 @@ import (
 	"net"
 	"reflect"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestGetLoopbackIP(t *testing.T) {
@@ -135,3 +137,47 @@ func TestParseIPList(t *testing.T) {
 		})
 	}
 }
+
+func TestSearchIPs(t *testing.T) {
+
+	// empty ip list
+	ipList := []net.IP{}
+	assert.False(t, SearchIP(ipList, net.ParseIP("10.0.0.1")))
+
+	// ip list with multiple ips
+	ipList = []net.IP{
+		net.ParseIP("10.0.0.1"),
+		net.ParseIP("10.0.0.2"),
+	}
+	assert.True(t, SearchIP(ipList, net.ParseIP("10.0.0.1")))
+	assert.True(t, SearchIP(ipList, net.ParseIP("10.0.0.2")))
+	assert.False(t, SearchIP(ipList, net.ParseIP("10.0.0.3")))
+
+	// search one exiting ip
+	ips := []net.IP{
+		net.ParseIP("10.0.0.1"),
+	}
+	assert.True(t, SearchAllIP(ipList, ips))
+
+	// search multiple existing ips
+	ips = []net.IP{
+		net.ParseIP("10.0.0.1"),
+		net.ParseIP("10.0.0.2"),
+	}
+	assert.True(t, SearchAllIP(ipList, ips))
+
+	// search multiple existing ips with one missing ip
+	ips = []net.IP{
+		net.ParseIP("10.0.0.3"),
+	}
+	assert.False(t, SearchAllIP(ipList, ips))
+
+	// search multiple existing ips with multiple missing ips
+	ips = []net.IP{
+		net.ParseIP("10.0.0.1"),
+		net.ParseIP("10.0.0.4"),
+		net.ParseIP("10.0.0.3"),
+		net.ParseIP("10.0.0.2"),
+	}
+	assert.False(t, SearchAllIP(ipList, ips))
+}