Address comments

Signed-off-by: Joe Nathan Abellard <contact@jabellard.com>
This commit is contained in:
Joe Nathan Abellard 2024-11-20 07:41:31 -05:00
parent 620638d0f4
commit 4ff99ebbd9
5 changed files with 58 additions and 59 deletions

View File

@ -212,25 +212,29 @@ func KarmadaCertEtcdClient() *CertConfig {
// KarmadaCert is karmada certificate, it includes certificate basic message. // KarmadaCert is karmada certificate, it includes certificate basic message.
// we can directly get the byte array of certificate key and cert from the object. // we can directly get the byte array of certificate key and cert from the object.
type KarmadaCert struct { type KarmadaCert struct {
PairName string pairName string
CAName string caName string
Cert []byte cert []byte
Key []byte key []byte
}
func NewKarmadaCert(pairName, caName string, cert, key []byte) *KarmadaCert {
return &KarmadaCert{pairName: pairName, caName: caName, cert: cert, key: key}
} }
// CertData returns certificate cert data. // CertData returns certificate cert data.
func (cert *KarmadaCert) CertData() []byte { func (cert *KarmadaCert) CertData() []byte {
return cert.Cert return cert.cert
} }
// KeyData returns certificate key data. // KeyData returns certificate key data.
func (cert *KarmadaCert) KeyData() []byte { func (cert *KarmadaCert) KeyData() []byte {
return cert.Key return cert.key
} }
// CertName returns cert file name. its default suffix is ".crt". // CertName returns cert file name. its default suffix is ".crt".
func (cert *KarmadaCert) CertName() string { func (cert *KarmadaCert) CertName() string {
pair := cert.PairName pair := cert.pairName
if len(pair) == 0 { if len(pair) == 0 {
pair = "cert" pair = "cert"
} }
@ -239,7 +243,7 @@ func (cert *KarmadaCert) CertName() string {
// KeyName returns cert key file name. its default suffix is ".key". // KeyName returns cert key file name. its default suffix is ".key".
func (cert *KarmadaCert) KeyName() string { func (cert *KarmadaCert) KeyName() string {
pair := cert.PairName pair := cert.pairName
if len(pair) == 0 { if len(pair) == 0 {
pair = "cert" pair = "cert"
} }
@ -282,10 +286,10 @@ func NewCertificateAuthority(cc *CertConfig) (*KarmadaCert, error) {
} }
return &KarmadaCert{ return &KarmadaCert{
PairName: cc.Name, pairName: cc.Name,
CAName: cc.CAName, caName: cc.CAName,
Cert: EncodeCertPEM(cert), cert: EncodeCertPEM(cert),
Key: encoded, key: encoded,
}, nil }, nil
} }
@ -329,10 +333,10 @@ func CreateCertAndKeyFilesWithCA(cc *CertConfig, caCertData, caKeyData []byte) (
} }
return &KarmadaCert{ return &KarmadaCert{
PairName: cc.Name, pairName: cc.Name,
CAName: cc.CAName, caName: cc.CAName,
Cert: EncodeCertPEM(cert), cert: EncodeCertPEM(cert),
Key: encoded, key: encoded,
}, nil }, nil
} }

View File

@ -428,23 +428,23 @@ func TestNewCertificateAuthority(t *testing.T) {
t.Fatal("NewCertificateAuthority() returned nil cert") t.Fatal("NewCertificateAuthority() returned nil cert")
} }
if cert.PairName != cc.Name { if cert.pairName != cc.Name {
t.Errorf("expected pairName to be %s, got %s", cc.Name, cert.PairName) t.Errorf("expected pairName to be %s, got %s", cc.Name, cert.pairName)
} }
if cert.CAName != cc.CAName { if cert.caName != cc.CAName {
t.Errorf("expected caName to be %s, got %s", cc.CAName, cert.CAName) t.Errorf("expected caName to be %s, got %s", cc.CAName, cert.caName)
} }
if cert.Cert == nil { if cert.cert == nil {
t.Error("expected cert to be non-nil") t.Error("expected cert to be non-nil")
} }
if cert.Key == nil { if cert.key == nil {
t.Error("expected key to be non-nil") t.Error("expected key to be non-nil")
} }
block, _ := pem.Decode(cert.Cert) block, _ := pem.Decode(cert.cert)
if block == nil || block.Type != CertificateBlockType { if block == nil || block.Type != CertificateBlockType {
t.Errorf("expected PEM block type to be %s, got %v", CertificateBlockType, block) t.Errorf("expected PEM block type to be %s, got %v", CertificateBlockType, block)
} }
@ -524,19 +524,19 @@ func TestCreateCertAndKeyFilesWithCA(t *testing.T) {
t.Fatal("CreateCertAndKeyFilesWithCA() returned nil cert") t.Fatal("CreateCertAndKeyFilesWithCA() returned nil cert")
} }
if cert.Cert == nil || cert.Key == nil { if cert.cert == nil || cert.key == nil {
t.Error("Expected cert and key to be non-nil") t.Error("Expected cert and key to be non-nil")
} }
if cert.PairName != certConfig.Name { if cert.pairName != certConfig.Name {
t.Errorf("expected pairName to be %s, got %s", certConfig.Name, cert.PairName) t.Errorf("expected pairName to be %s, got %s", certConfig.Name, cert.pairName)
} }
if cert.CAName != certConfig.CAName { if cert.caName != certConfig.CAName {
t.Errorf("expected caName to be %s, got %s", certConfig.CAName, cert.CAName) t.Errorf("expected caName to be %s, got %s", certConfig.CAName, cert.caName)
} }
block, _ := pem.Decode(cert.Cert) block, _ := pem.Decode(cert.cert)
if block == nil || block.Type != CertificateBlockType { if block == nil || block.Type != CertificateBlockType {
t.Errorf("expected PEM block type to be %s, got %v", CertificateBlockType, block) t.Errorf("expected PEM block type to be %s, got %v", CertificateBlockType, block)
} }
@ -566,7 +566,7 @@ func TestNewSignedCert_Success(t *testing.T) {
} }
caCert := caCerts[0] caCert := caCerts[0]
caKey, err := ParsePrivateKeyPEM(caKarmadaCert.Key) caKey, err := ParsePrivateKeyPEM(caKarmadaCert.key)
if err != nil { if err != nil {
t.Error(err) t.Error(err)
} }

View File

@ -66,15 +66,15 @@ func NewCertStore() CertStore {
} }
} }
// AddCert adds a cert to cert store, the cache key is cert PairName by default. // AddCert adds a cert to cert store, the cache key is cert pairName by default.
func (store *KarmadaCertStore) AddCert(cert *KarmadaCert) { func (store *KarmadaCertStore) AddCert(cert *KarmadaCert) {
store.certs[cert.PairName] = cert store.certs[cert.pairName] = cert
} }
// GetCert get cert from store by cert PairName. // GetCert get cert from store by cert pairName.
func (store *KarmadaCertStore) GetCert(name string) *KarmadaCert { func (store *KarmadaCertStore) GetCert(name string) *KarmadaCert {
for _, c := range store.certs { for _, c := range store.certs {
if c.PairName == name { if c.pairName == name {
return c return c
} }
} }
@ -105,15 +105,15 @@ func (store *KarmadaCertStore) LoadCertFromSecret(secret *corev1.Secret) error {
kc := store.GetCert(pairName) kc := store.GetCert(pairName)
if kc == nil { if kc == nil {
kc = &KarmadaCert{ kc = &KarmadaCert{
PairName: pairName, pairName: pairName,
} }
} }
if strings.Contains(name, certExtension) { if strings.Contains(name, certExtension) {
kc.Cert = data kc.cert = data
} }
if strings.Contains(name, keyExtension) { if strings.Contains(name, keyExtension) {
kc.Key = data kc.key = data
} }
store.AddCert(kc) store.AddCert(kc)

View File

@ -22,12 +22,12 @@ import (
corev1 "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1"
) )
// Helper function to create a new KarmadaCert with given PairName. // Helper function to create a new KarmadaCert with given pairName.
func newKarmadaCert(pairName string, certData, keyData []byte) *KarmadaCert { func newKarmadaCert(pairName string, certData, keyData []byte) *KarmadaCert {
return &KarmadaCert{ return &KarmadaCert{
PairName: pairName, pairName: pairName,
Cert: certData, cert: certData,
Key: keyData, key: keyData,
} }
} }
@ -51,11 +51,11 @@ func TestAddAndGetCert(t *testing.T) {
if retrievedCert == nil { if retrievedCert == nil {
t.Fatalf("expected to retrieve cert but got nil") t.Fatalf("expected to retrieve cert but got nil")
} }
if string(retrievedCert.Cert) != "certData" { if string(retrievedCert.cert) != "certData" {
t.Errorf("expected certData but got %s", string(retrievedCert.Cert)) t.Errorf("expected certData but got %s", string(retrievedCert.cert))
} }
if string(retrievedCert.Key) != "keyData" { if string(retrievedCert.key) != "keyData" {
t.Errorf("expected keyData but got %s", string(retrievedCert.Key)) t.Errorf("expected keyData but got %s", string(retrievedCert.key))
} }
} }
@ -98,13 +98,13 @@ func TestLoadCertFromSecret(t *testing.T) {
} }
cert1 := store.GetCert("cert1") cert1 := store.GetCert("cert1")
if cert1 == nil || string(cert1.Cert) != "cert1CertData" || string(cert1.Key) != "cert1KeyData" { if cert1 == nil || string(cert1.cert) != "cert1CertData" || string(cert1.key) != "cert1KeyData" {
t.Errorf("cert1 content is incorrect expected cert %s key %s, got cert %s key %s", "cert1CertData", "cert1KeyData", string(cert1.Cert), string(cert1.Key)) t.Errorf("cert1 content is incorrect expected cert %s key %s, got cert %s key %s", "cert1CertData", "cert1KeyData", string(cert1.cert), string(cert1.key))
} }
cert2 := store.GetCert("cert2") cert2 := store.GetCert("cert2")
if cert2 == nil || string(cert2.Cert) != "cert2CertData" || string(cert2.Key) != "cert2KeyData" { if cert2 == nil || string(cert2.cert) != "cert2CertData" || string(cert2.key) != "cert2KeyData" {
t.Errorf("cert2 content is incorrect expected cert %s key %s, got cert %s key %s", "cert2CertData", "cert2KeyData", string(cert2.Cert), string(cert2.Key)) t.Errorf("cert2 content is incorrect expected cert %s key %s, got cert %s key %s", "cert2CertData", "cert2KeyData", string(cert2.cert), string(cert2.key))
} }
} }
@ -144,10 +144,10 @@ func TestLoadCertFromSecret_InvalidFormat(t *testing.T) {
} }
karmadaCert := store.GetCert(pairName) karmadaCert := store.GetCert(pairName)
if len(karmadaCert.Key) != 0 { if len(karmadaCert.key) != 0 {
t.Errorf("expected the cert data content to be empty but got %v", karmadaCert.Cert) t.Errorf("expected the cert data content to be empty but got %v", karmadaCert.cert)
} }
if len(karmadaCert.Key) != 0 { if len(karmadaCert.key) != 0 {
t.Errorf("expected the key data content to be empty but got %v", karmadaCert.Key) t.Errorf("expected the key data content to be empty but got %v", karmadaCert.key)
} }
} }

View File

@ -117,12 +117,7 @@ func runCATask(kc *certs.CertConfig) func(d workflow.RunData) error {
klog.V(2).InfoS("[certs] Successfully loaded custom CA certificate", "secret", secretRef.Name) klog.V(2).InfoS("[certs] Successfully loaded custom CA certificate", "secret", secretRef.Name)
customKarmadaCert := &certs.KarmadaCert{ customKarmadaCert := certs.NewKarmadaCert(kc.Name, kc.CAName, certData, keyData)
PairName: kc.Name,
CAName: kc.CAName,
Cert: certData,
Key: keyData,
}
data.AddCert(customKarmadaCert) data.AddCert(customKarmadaCert)
klog.V(2).InfoS("[certs] Successfully added custom CA certificate to cert store", "certName", kc.Name) klog.V(2).InfoS("[certs] Successfully added custom CA certificate to cert store", "certName", kc.Name)