Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Joe Nathan Abellard <[email protected]>
  • Loading branch information
@jabellard
jabellard committed Nov 20, 2024
1 parent 620638d commit 4ff99eb
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 59 deletions.
36 changes: 20 additions & 16 deletions operator/pkg/certs/certs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,25 +212,29 @@ func KarmadaCertEtcdClient() *CertConfig {
// KarmadaCert is karmada certificate, it includes certificate basic message.
// we can directly get the byte array of certificate key and cert from the object.
type KarmadaCert struct {
PairName string
CAName string
Cert []byte
Key []byte
pairName string
caName string
cert []byte
key []byte
}

func NewKarmadaCert(pairName, caName string, cert, key []byte) *KarmadaCert {

Check failure on line 221 in operator/pkg/certs/certs.go

View workflow job for this annotation

GitHub Actions / lint 

exported: exported function NewKarmadaCert should have comment or be unexported (revive)
return &KarmadaCert{pairName: pairName, caName: caName, cert: cert, key: key}
}

// CertData returns certificate cert data.
func (cert *KarmadaCert) CertData() []byte {
return cert.Cert
return cert.cert
}

// KeyData returns certificate key data.
func (cert *KarmadaCert) KeyData() []byte {
return cert.Key
return cert.key
}

// CertName returns cert file name. its default suffix is ".crt".
func (cert *KarmadaCert) CertName() string {
pair := cert.PairName
pair := cert.pairName
if len(pair) == 0 {
pair = "cert"
}
Expand All @@ -239,7 +243,7 @@ func (cert *KarmadaCert) CertName() string {

// KeyName returns cert key file name. its default suffix is ".key".
func (cert *KarmadaCert) KeyName() string {
pair := cert.PairName
pair := cert.pairName
if len(pair) == 0 {
pair = "cert"
}
Expand Down Expand Up @@ -282,10 +286,10 @@ func NewCertificateAuthority(cc *CertConfig) (*KarmadaCert, error) {
}

return &KarmadaCert{
PairName: cc.Name,
CAName: cc.CAName,
Cert: EncodeCertPEM(cert),
Key: encoded,
pairName: cc.Name,
caName: cc.CAName,
cert: EncodeCertPEM(cert),
key: encoded,
}, nil
}

Expand Down Expand Up @@ -329,10 +333,10 @@ func CreateCertAndKeyFilesWithCA(cc *CertConfig, caCertData, caKeyData []byte) (
}

return &KarmadaCert{
PairName: cc.Name,
CAName: cc.CAName,
Cert: EncodeCertPEM(cert),
Key: encoded,
pairName: cc.Name,
caName: cc.CAName,
cert: EncodeCertPEM(cert),
key: encoded,
}, nil
}

Expand Down
28 changes: 14 additions & 14 deletions operator/pkg/certs/certs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -428,23 +428,23 @@ func TestNewCertificateAuthority(t *testing.T) {
t.Fatal("NewCertificateAuthority() returned nil cert")
}

if cert.PairName != cc.Name {
t.Errorf("expected pairName to be %s, got %s", cc.Name, cert.PairName)
if cert.pairName != cc.Name {
t.Errorf("expected pairName to be %s, got %s", cc.Name, cert.pairName)
}

if cert.CAName != cc.CAName {
t.Errorf("expected caName to be %s, got %s", cc.CAName, cert.CAName)
if cert.caName != cc.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")
}

if cert.Key == nil {
if cert.key == 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 {
t.Errorf("expected PEM block type to be %s, got %v", CertificateBlockType, block)
}
Expand Down Expand Up @@ -524,19 +524,19 @@ func TestCreateCertAndKeyFilesWithCA(t *testing.T) {
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")
}

if cert.PairName != certConfig.Name {
t.Errorf("expected pairName to be %s, got %s", certConfig.Name, cert.PairName)
if cert.pairName != certConfig.Name {
t.Errorf("expected pairName to be %s, got %s", certConfig.Name, cert.pairName)
}

if cert.CAName != certConfig.CAName {
t.Errorf("expected caName to be %s, got %s", certConfig.CAName, cert.CAName)
if cert.caName != certConfig.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 {
t.Errorf("expected PEM block type to be %s, got %v", CertificateBlockType, block)
}
Expand Down Expand Up @@ -566,7 +566,7 @@ func TestNewSignedCert_Success(t *testing.T) {
}
caCert := caCerts[0]

caKey, err := ParsePrivateKeyPEM(caKarmadaCert.Key)
caKey, err := ParsePrivateKeyPEM(caKarmadaCert.key)
if err != nil {
t.Error(err)
}
Expand Down
14 changes: 7 additions & 7 deletions operator/pkg/certs/store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
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 {
for _, c := range store.certs {
if c.PairName == name {
if c.pairName == name {
return c
}
}
Expand Down Expand Up @@ -105,15 +105,15 @@ func (store *KarmadaCertStore) LoadCertFromSecret(secret *corev1.Secret) error {
kc := store.GetCert(pairName)
if kc == nil {
kc = &KarmadaCert{
PairName: pairName,
pairName: pairName,
}
}

if strings.Contains(name, certExtension) {
kc.Cert = data
kc.cert = data
}
if strings.Contains(name, keyExtension) {
kc.Key = data
kc.key = data
}

store.AddCert(kc)
Expand Down
32 changes: 16 additions & 16 deletions operator/pkg/certs/store_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ import (
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 {
return &KarmadaCert{
PairName: pairName,
Cert: certData,
Key: keyData,
pairName: pairName,
cert: certData,
key: keyData,
}
}

Expand All @@ -51,11 +51,11 @@ func TestAddAndGetCert(t *testing.T) {
if retrievedCert == nil {
t.Fatalf("expected to retrieve cert but got nil")
}
if string(retrievedCert.Cert) != "certData" {
t.Errorf("expected certData but got %s", string(retrievedCert.Cert))
if string(retrievedCert.cert) != "certData" {
t.Errorf("expected certData but got %s", string(retrievedCert.cert))
}
if string(retrievedCert.Key) != "keyData" {
t.Errorf("expected keyData but got %s", string(retrievedCert.Key))
if string(retrievedCert.key) != "keyData" {
t.Errorf("expected keyData but got %s", string(retrievedCert.key))
}
}

Expand Down Expand Up @@ -98,13 +98,13 @@ func TestLoadCertFromSecret(t *testing.T) {
}

cert1 := store.GetCert("cert1")
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))
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))
}

cert2 := store.GetCert("cert2")
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))
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))
}
}

Expand Down Expand Up @@ -144,10 +144,10 @@ func TestLoadCertFromSecret_InvalidFormat(t *testing.T) {
}

karmadaCert := store.GetCert(pairName)
if len(karmadaCert.Key) != 0 {
t.Errorf("expected the cert data content to be empty but got %v", karmadaCert.Cert)
if len(karmadaCert.key) != 0 {
t.Errorf("expected the cert data content to be empty but got %v", karmadaCert.cert)
}
if len(karmadaCert.Key) != 0 {
t.Errorf("expected the key data content to be empty but got %v", karmadaCert.Key)
if len(karmadaCert.key) != 0 {
t.Errorf("expected the key data content to be empty but got %v", karmadaCert.key)
}
}
7 changes: 1 addition & 6 deletions operator/pkg/tasks/init/cert.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

customKarmadaCert := &certs.KarmadaCert{
PairName: kc.Name,
CAName: kc.CAName,
Cert: certData,
Key: keyData,
}
customKarmadaCert := certs.NewKarmadaCert(kc.Name, kc.CAName, certData, keyData)

data.AddCert(customKarmadaCert)
klog.V(2).InfoS("[certs] Successfully added custom CA certificate to cert store", "certName", kc.Name)
Expand Down

0 comments on commit 4ff99eb

Please sign in to comment.