Skip to content

Commit

Permalink
improve ca data for yurthub component
Browse files Browse the repository at this point in the history
  • Loading branch information
rambohe-ch committed Nov 21, 2023
1 parent 754ccf8 commit e05b7e5
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 55 deletions.
7 changes: 7 additions & 0 deletions pkg/util/certmanager/pki.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,10 @@ func GenCertPoolUseCA(caFile string) (*x509.CertPool, error) {
certPool.AppendCertsFromPEM(caData)
return certPool, nil
}

// GenCertPoolUseCAData generates a x509 CertPool based on the given CA data
func GenCertPoolUseCAData(caData []byte) (*x509.CertPool, error) {
certPool := x509.NewCertPool()
certPool.AppendCertsFromPEM(caData)
return certPool, nil

Check warning on line 193 in pkg/util/certmanager/pki.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/certmanager/pki.go#L190-L193

Added lines #L190 - L193 were not covered by tests
}
1 change: 1 addition & 0 deletions pkg/yurthub/certificate/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type YurtClientCertificateManager interface {
Stop()
UpdateBootstrapConf(joinToken string) error
GetHubConfFile() string
GetCAData() []byte
GetCaFile() string
GetAPIServerClientCert() *tls.Certificate
}
Expand Down
11 changes: 11 additions & 0 deletions pkg/yurthub/certificate/kubeletcertificate/kubelet_certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"crypto/x509"
"errors"
"fmt"
"os"
"time"

"k8s.io/klog/v2"
Expand All @@ -40,6 +41,7 @@ type kubeletCertManager struct {
kubeletCAFile string
kubeletPemFile string
cert *tls.Certificate
caData []byte
}

func NewKubeletCertManager(kubeConfFile, kubeletCAFile, kubeletPemFile string) (certificate.YurtClientCertificateManager, error) {
Expand All @@ -50,6 +52,10 @@ func NewKubeletCertManager(kubeConfFile, kubeletCAFile, kubeletPemFile string) (
if exist, _ := util.FileExists(kubeletCAFile); !exist {
return nil, KubeletCANotExistErr
}
caData, err := os.ReadFile(kubeletCAFile)
if err != nil {
return nil, err
}

Check warning on line 58 in pkg/yurthub/certificate/kubeletcertificate/kubelet_certificate.go

View check run for this annotation

Codecov / codecov/patch

pkg/yurthub/certificate/kubeletcertificate/kubelet_certificate.go#L57-L58

Added lines #L57 - L58 were not covered by tests

if exist, _ := util.FileExists(kubeletPemFile); !exist {
return nil, KubeletPemNotExistErr
Expand All @@ -65,6 +71,7 @@ func NewKubeletCertManager(kubeConfFile, kubeletCAFile, kubeletPemFile string) (
kubeletCAFile: kubeletCAFile,
kubeletPemFile: kubeletPemFile,
cert: cert,
caData: caData,
}, nil
}

Expand All @@ -84,6 +91,10 @@ func (kcm *kubeletCertManager) GetHubConfFile() string {
return kcm.kubeConfFile
}

func (kcm *kubeletCertManager) GetCAData() []byte {
return kcm.caData

Check warning on line 95 in pkg/yurthub/certificate/kubeletcertificate/kubelet_certificate.go

View check run for this annotation

Codecov / codecov/patch

pkg/yurthub/certificate/kubeletcertificate/kubelet_certificate.go#L94-L95

Added lines #L94 - L95 were not covered by tests
}

func (kcm *kubeletCertManager) GetCaFile() string {
return kcm.kubeletCAFile
}
Expand Down
9 changes: 2 additions & 7 deletions pkg/yurthub/certificate/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
"github.com/openyurtio/openyurt/pkg/yurthub/certificate/kubeletcertificate"
hubServerCert "github.com/openyurtio/openyurt/pkg/yurthub/certificate/server"
"github.com/openyurtio/openyurt/pkg/yurthub/certificate/token"
"github.com/openyurtio/openyurt/pkg/yurthub/util"
)

const (
Expand Down Expand Up @@ -123,12 +122,8 @@ func (hcm *yurtHubCertManager) Ready() bool {
errs = append(errs, apiServerClientCertNotReadyError)
}

if exist, err := util.FileExists(hcm.YurtClientCertificateManager.GetCaFile()); !exist {
if err == nil {
errs = append(errs, caCertIsNotReadyError)
} else {
errs = append(errs, err)
}
if len(hcm.YurtClientCertificateManager.GetCAData()) == 0 {
errs = append(errs, caCertIsNotReadyError)

Check warning on line 126 in pkg/yurthub/certificate/manager/manager.go

View check run for this annotation

Codecov / codecov/patch

pkg/yurthub/certificate/manager/manager.go#L126

Added line #L126 was not covered by tests
}

if hcm.GetHubServerCert() == nil {
Expand Down
42 changes: 8 additions & 34 deletions pkg/yurthub/certificate/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,11 @@ import (
"testing"
"time"

"github.com/pkg/errors"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/tools/clientcmd"
certutil "k8s.io/client-go/util/cert"

"github.com/openyurtio/openyurt/cmd/yurthub/app/options"
"github.com/openyurtio/openyurt/pkg/projectinfo"
kubeconfigutil "github.com/openyurtio/openyurt/pkg/util/kubeconfig"
"github.com/openyurtio/openyurt/pkg/yurthub/certificate/testdata"
"github.com/openyurtio/openyurt/pkg/yurthub/util"
)

func TestGetHubServerCertFile(t *testing.T) {
Expand Down Expand Up @@ -108,33 +103,6 @@ func TestReady(t *testing.T) {
if mgr.Ready() {
return true, nil
}

if exist, err := util.FileExists(mgr.GetCaFile()); !exist {
if err != nil {
t.Logf("could not get ca file(%s), %v", mgr.GetCaFile(), err)
return false, err
}

if exist, err := util.FileExists(mgr.GetHubConfFile()); err != nil {
t.Logf("could not get hub conf file(%s), %v", mgr.GetHubConfFile(), err)
return false, nil
} else if exist {
t.Logf("%s file already exists, so use it to create ca file", mgr.GetHubConfFile())
hubKubeConfig, err := clientcmd.LoadFromFile(mgr.GetHubConfFile())
if err != nil {
return false, err
}

cluster := kubeconfigutil.GetClusterFromKubeConfig(hubKubeConfig)
if cluster != nil {
if err := certutil.WriteCert(mgr.GetCaFile(), cluster.CertificateAuthorityData); err != nil {
return false, errors.Wrap(err, "couldn't save the CA certificate to disk")
}
} else {
return false, errors.Errorf("couldn't prepare ca.crt(%s) file", mgr.GetCaFile())
}
}
}
return false, nil
})

Expand All @@ -161,8 +129,14 @@ func TestReady(t *testing.T) {
return
}
newMgr.Start()
if !newMgr.Ready() {
t.Errorf("certificates can not be reused")
err = wait.PollImmediate(2*time.Second, 1*time.Minute, func() (done bool, err error) {
if mgr.Ready() {
return true, nil
}
return false, nil
})
if err != nil {
t.Errorf("certificates are not reused, %v", err)
}
newMgr.Stop()

Expand Down
17 changes: 17 additions & 0 deletions pkg/yurthub/certificate/token/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ type yurtHubClientCertManager struct {
joinToken string
bootstrapFile string
dialer *util.Dialer
caData []byte
}

// NewYurtHubClientCertManager new a YurtCertificateManager instance
Expand Down Expand Up @@ -203,11 +204,17 @@ func (ycm *yurtHubClientCertManager) prepareConfigAndCaFile() error {
if err := certutil.WriteCert(ycm.GetCaFile(), cluster.CertificateAuthorityData); err != nil {
return errors.Wrap(err, "couldn't save the CA certificate to disk")
}
ycm.caData = cluster.CertificateAuthorityData

Check warning on line 207 in pkg/yurthub/certificate/token/token.go

View check run for this annotation

Codecov / codecov/patch

pkg/yurthub/certificate/token/token.go#L207

Added line #L207 was not covered by tests
} else {
return errors.Errorf("couldn't prepare ca.crt(%s) file", ycm.GetCaFile())
}
} else {
klog.V(2).Infof("%s file already exists, so reuse it", ycm.GetCaFile())
caData, err := os.ReadFile(ycm.GetCaFile())
if err != nil {
return err
}
ycm.caData = caData

Check warning on line 217 in pkg/yurthub/certificate/token/token.go

View check run for this annotation

Codecov / codecov/patch

pkg/yurthub/certificate/token/token.go#L213-L217

Added lines #L213 - L217 were not covered by tests
}
return nil
}
Expand Down Expand Up @@ -250,11 +257,17 @@ func (ycm *yurtHubClientCertManager) prepareConfigAndCaFile() error {
if err := certutil.WriteCert(ycm.GetCaFile(), cluster.CertificateAuthorityData); err != nil {
return errors.Wrap(err, "couldn't save the CA certificate to disk")
}
ycm.caData = cluster.CertificateAuthorityData

Check warning on line 260 in pkg/yurthub/certificate/token/token.go

View check run for this annotation

Codecov / codecov/patch

pkg/yurthub/certificate/token/token.go#L260

Added line #L260 was not covered by tests
} else {
return errors.Errorf("couldn't prepare ca.crt(%s) file", ycm.GetCaFile())
}
} else {
klog.V(2).Infof("%s file already exists, so reuse it", ycm.GetCaFile())
caData, err := os.ReadFile(ycm.GetCaFile())
if err != nil {
return err
}
ycm.caData = caData

Check warning on line 270 in pkg/yurthub/certificate/token/token.go

View check run for this annotation

Codecov / codecov/patch

pkg/yurthub/certificate/token/token.go#L266-L270

Added lines #L266 - L270 were not covered by tests
}

return nil
Expand Down Expand Up @@ -284,6 +297,10 @@ func (ycm *yurtHubClientCertManager) getBootstrapConfFile() string {
return filepath.Join(ycm.hubRunDir, bootstrapConfigFileName)
}

func (ycm *yurtHubClientCertManager) GetCAData() []byte {
return ycm.caData

Check warning on line 301 in pkg/yurthub/certificate/token/token.go

View check run for this annotation

Codecov / codecov/patch

pkg/yurthub/certificate/token/token.go#L300-L301

Added lines #L300 - L301 were not covered by tests
}

// GetCaFile returns the path of ca file
func (ycm *yurtHubClientCertManager) GetCaFile() string {
return filepath.Join(ycm.getPkiDir(), hubCaFileName)
Expand Down
19 changes: 9 additions & 10 deletions pkg/yurthub/transport/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ type CertGetter interface {
// GetAPIServerClientCert returns the currently selected certificate, as well as
// the associated certificate and key data in PEM format.
GetAPIServerClientCert() *tls.Certificate
// Return CA file path.
GetCaFile() string
// GetCAData returns CA file data.
GetCAData() []byte
}

// Interface is an transport interface for managing clients that used to connecting kube-apiserver
Expand All @@ -60,13 +60,12 @@ type transportManager struct {

// NewTransportManager create a transport interface object.
func NewTransportManager(certGetter CertGetter, stopCh <-chan struct{}) (Interface, error) {
caFile := certGetter.GetCaFile()
if len(caFile) == 0 {
return nil, fmt.Errorf("ca cert file was not prepared when new transport")
caData := certGetter.GetCAData()
if len(caData) == 0 {
return nil, fmt.Errorf("ca cert data was not prepared when new transport")
}
klog.V(2).Infof("use %s ca cert file to access remote server", caFile)

cfg, err := tlsConfig(certGetter.GetAPIServerClientCert, caFile)
cfg, err := tlsConfig(certGetter.GetAPIServerClientCert, caData)
if err != nil {
klog.Errorf("could not get tls config when new transport, %v", err)
return nil, err
Expand All @@ -81,7 +80,7 @@ func NewTransportManager(certGetter CertGetter, stopCh <-chan struct{}) (Interfa
DialContext: d.DialContext,
})

bearerTLSCfg, err := tlsConfig(nil, caFile)
bearerTLSCfg, err := tlsConfig(nil, caData)
if err != nil {
klog.Errorf("could not get tls config when new bearer transport, %v", err)
return nil, err
Expand Down Expand Up @@ -151,9 +150,9 @@ func (tm *transportManager) start() {
}, 10*time.Second, tm.stopCh)
}

func tlsConfig(current func() *tls.Certificate, caFile string) (*tls.Config, error) {
func tlsConfig(current func() *tls.Certificate, caData []byte) (*tls.Config, error) {
// generate the TLS configuration based on the latest certificate
rootCert, err := certmanager.GenCertPoolUseCA(caFile)
rootCert, err := certmanager.GenCertPoolUseCAData(caData)
if err != nil {
klog.Errorf("could not generate a x509 CertPool based on the given CA file, %v", err)
return nil, err
Expand Down
6 changes: 6 additions & 0 deletions pkg/yurthub/yurtcoordinator/certmanager/certmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ type CertManager struct {
coordinatorCert *tls.Certificate
nodeLeaseProxyCert *tls.Certificate
store fs.FileSystemOperator
caData []byte

// Used for unit test.
secret *corev1.Secret
Expand All @@ -115,6 +116,10 @@ func (c *CertManager) GetNodeLeaseProxyClientCert() *tls.Certificate {
return c.nodeLeaseProxyCert
}

func (c *CertManager) GetCAData() []byte {
return c.caData

Check warning on line 120 in pkg/yurthub/yurtcoordinator/certmanager/certmanager.go

View check run for this annotation

Codecov / codecov/patch

pkg/yurthub/yurtcoordinator/certmanager/certmanager.go#L119-L120

Added lines #L119 - L120 were not covered by tests
}

func (c *CertManager) GetCaFile() string {
return c.GetFilePath(RootCA)
}
Expand Down Expand Up @@ -162,6 +167,7 @@ func (c *CertManager) updateCerts(secret *corev1.Secret) {
if err := c.createOrUpdateFile(c.GetFilePath(RootCA), ca); err != nil {
klog.Errorf("could not update ca, %v", err)

Check warning on line 168 in pkg/yurthub/yurtcoordinator/certmanager/certmanager.go

View check run for this annotation

Codecov / codecov/patch

pkg/yurthub/yurtcoordinator/certmanager/certmanager.go#L168

Added line #L168 was not covered by tests
}
c.caData = ca
}

if cook {
Expand Down
8 changes: 4 additions & 4 deletions pkg/yurthub/yurtcoordinator/coordinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ type coordinator struct {
statusInfoChan chan statusInfo
isPoolCacheSynced bool
certMgr *certmanager.CertManager
// cloudCAFilePath is the file path of cloud kubernetes cluster CA cert.
cloudCAFilePath string
// cloudCAFileData is the file data of cloud kubernetes cluster CA cert.
cloudCAFileData []byte
// cloudHealthChecker is health checker of cloud APIServers. It is used to
// pick a healthy cloud APIServer to proxy heartbeats.
cloudHealthChecker healthchecker.MultipleBackendsHealthChecker
Expand Down Expand Up @@ -153,7 +153,7 @@ func NewCoordinator(

coordinator := &coordinator{
ctx: ctx,
cloudCAFilePath: cfg.CertManager.GetCaFile(),
cloudCAFileData: cfg.CertManager.GetCAData(),

Check warning on line 156 in pkg/yurthub/yurtcoordinator/coordinator.go

View check run for this annotation

Codecov / codecov/patch

pkg/yurthub/yurtcoordinator/coordinator.go#L156

Added line #L156 was not covered by tests
cloudHealthChecker: cloudHealthChecker,
etcdStorageCfg: etcdStorageCfg,
restConfigMgr: restMgr,
Expand Down Expand Up @@ -425,7 +425,7 @@ func (coordinator *coordinator) newNodeLeaseProxyClient() (coordclientset.LeaseI
restCfg := &rest.Config{
Host: healthyCloudServer.String(),
TLSClientConfig: rest.TLSClientConfig{
CAFile: coordinator.cloudCAFilePath,
CAData: coordinator.cloudCAFileData,

Check warning on line 428 in pkg/yurthub/yurtcoordinator/coordinator.go

View check run for this annotation

Codecov / codecov/patch

pkg/yurthub/yurtcoordinator/coordinator.go#L428

Added line #L428 was not covered by tests
CertFile: coordinator.certMgr.GetFilePath(certmanager.NodeLeaseProxyClientCert),
KeyFile: coordinator.certMgr.GetFilePath(certmanager.NodeLeaseProxyClientKey),
},
Expand Down

0 comments on commit e05b7e5

Please sign in to comment.