From 51194cc6f544f07ce4fe6fae66e227272883afad Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Tue, 25 Jun 2024 15:33:49 +0200 Subject: [PATCH 1/3] chore: remove duplicated logic by function call Signed-off-by: Wen Zhou --- pkg/cluster/cert.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/pkg/cluster/cert.go b/pkg/cluster/cert.go index f704203deef..54a0043b325 100644 --- a/pkg/cluster/cert.go +++ b/pkg/cluster/cert.go @@ -44,12 +44,9 @@ func CreateSelfSignedCertificate(ctx context.Context, c client.Client, secretNam return fmt.Errorf("failed getting certificate secret: %w", err) } } else if existingSecret.Type != certSecret.Type { - // Secret exists but with a different type, delete and recreate it - if err := c.Delete(ctx, existingSecret); err != nil { - return fmt.Errorf("failed deleting existing secret: %w", err) - } - if createErr := c.Create(ctx, certSecret); client.IgnoreAlreadyExists(createErr) != nil { - return fmt.Errorf("failed creating certificate secret: %w", createErr) + // Secret exists but with a different type, delete and create it again + if recreateSecret(ctx, c, existingSecret, certSecret) != nil { + return errors.New("failed to recreate secret with type corrected") } } From 11fb6b6083e5c4e79a11d34581f2d01e1e877ea6 Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Tue, 25 Jun 2024 18:20:47 +0200 Subject: [PATCH 2/3] update:refactor some more Signed-off-by: Wen Zhou --- pkg/cluster/cert.go | 70 ++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 39 deletions(-) diff --git a/pkg/cluster/cert.go b/pkg/cluster/cert.go index 54a0043b325..608f8665734 100644 --- a/pkg/cluster/cert.go +++ b/pkg/cluster/cert.go @@ -32,24 +32,9 @@ func CreateSelfSignedCertificate(ctx context.Context, c client.Client, secretNam if err := ApplyMetaOptions(certSecret, metaOptions...); err != nil { return err } - existingSecret := &corev1.Secret{} - err = c.Get(ctx, client.ObjectKey{Name: secretName, Namespace: namespace}, existingSecret) - if err != nil { - if k8serr.IsNotFound(err) { - // Secret does not exist, create it - if createErr := c.Create(ctx, certSecret); createErr != nil { - return fmt.Errorf("failed creating certificate secret: %w", createErr) - } - } else { - return fmt.Errorf("failed getting certificate secret: %w", err) - } - } else if existingSecret.Type != certSecret.Type { - // Secret exists but with a different type, delete and create it again - if recreateSecret(ctx, c, existingSecret, certSecret) != nil { - return errors.New("failed to recreate secret with type corrected") - } + if err = generateCertSecret(ctx, c, certSecret, secretName, namespace); err != nil { + return fmt.Errorf("failed update self-signed certificate secret: %w", err) } - return nil } @@ -191,29 +176,9 @@ func copySecretToNamespace(ctx context.Context, c client.Client, secret *corev1. Data: secret.Data, Type: secret.Type, } - - existingSecret := &corev1.Secret{} - err := c.Get(ctx, client.ObjectKey{Name: newSecretName, Namespace: namespace}, existingSecret) - if k8serr.IsNotFound(err) { // create if not found - if err = c.Create(ctx, newSecret); err != nil { - return fmt.Errorf("failed to create new secret: %w", err) - } - } else if err != nil { - return fmt.Errorf("failed to get existing secret: %w", err) - } - - if existingSecret.Type != newSecret.Type { // recreate if found with mismatched type - if recreateSecret(ctx, c, existingSecret, newSecret) != nil { - return errors.New("failed to recreate secret with type corrected") - } - } - - if isSecretOutdated(existingSecret.Data, newSecret.Data) { - if err = c.Update(ctx, newSecret); err != nil { // update data if found with same type but outdated content - return fmt.Errorf("failed to update secret: %w", err) - } + if err := generateCertSecret(ctx, c, newSecret, newSecretName, namespace); err != nil { + return fmt.Errorf("failed to deploy default cert secret to namespace %s: %w", namespace, err) } - return nil } @@ -228,6 +193,33 @@ func recreateSecret(ctx context.Context, c client.Client, existingSecret, newSec return nil } +// generateCertSecret creates a secret if it does not exist; recreate this secret if type not match; update data if outdated. +func generateCertSecret(ctx context.Context, c client.Client, certSecret *corev1.Secret, secretName, namespace string) error { + existingSecret := &corev1.Secret{} + err := c.Get(ctx, client.ObjectKey{Name: secretName, Namespace: namespace}, existingSecret) + if err != nil { + if k8serr.IsNotFound(err) { + // Secret does not exist, create it + if createErr := c.Create(ctx, certSecret); createErr != nil { + return fmt.Errorf("failed creating certificate secret: %w", createErr) + } + } else { + return fmt.Errorf("failed getting certificate secret: %w", err) + } + } else if existingSecret.Type != certSecret.Type { + // Secret exists but with a different type, delete and create it again + if recreateSecret(ctx, c, existingSecret, certSecret) != nil { + return errors.New("failed to recreate secret with type corrected") + } + } + if isSecretOutdated(existingSecret.Data, certSecret.Data) { + if err = c.Update(ctx, certSecret); err != nil { // update data if found with same type but outdated content + return fmt.Errorf("failed to update secret: %w", err) + } + } + return nil +} + // isSecretOutdated compares two secret data of type map[string][]byte and returns true if they are not equal. func isSecretOutdated(existingSecretData, newSecretData map[string][]byte) bool { if len(existingSecretData) != len(newSecretData) { From fe4e0e582a7856b3f0707dff1b5743a84966159f Mon Sep 17 00:00:00 2001 From: Wen Zhou Date: Wed, 26 Jun 2024 08:47:01 +0200 Subject: [PATCH 3/3] update: code review - change if-else-if to switch - fast return in recreateSecret case Signed-off-by: Wen Zhou --- pkg/cluster/cert.go | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/pkg/cluster/cert.go b/pkg/cluster/cert.go index 608f8665734..12f75329108 100644 --- a/pkg/cluster/cert.go +++ b/pkg/cluster/cert.go @@ -185,10 +185,10 @@ func copySecretToNamespace(ctx context.Context, c client.Client, secret *corev1. // recreateSecret deletes the existing secret and creates a new one. func recreateSecret(ctx context.Context, c client.Client, existingSecret, newSecret *corev1.Secret) error { if err := c.Delete(ctx, existingSecret); err != nil { - return fmt.Errorf("failed to delete existing secret: %w", err) + return fmt.Errorf("failed to delete existing secret before recreating new one: %w", err) } if err := c.Create(ctx, newSecret); err != nil { - return fmt.Errorf("failed to create new secret: %w", err) + return fmt.Errorf("failed to create new secret after existing one has been deleted: %w", err) } return nil } @@ -197,26 +197,27 @@ func recreateSecret(ctx context.Context, c client.Client, existingSecret, newSec func generateCertSecret(ctx context.Context, c client.Client, certSecret *corev1.Secret, secretName, namespace string) error { existingSecret := &corev1.Secret{} err := c.Get(ctx, client.ObjectKey{Name: secretName, Namespace: namespace}, existingSecret) - if err != nil { - if k8serr.IsNotFound(err) { - // Secret does not exist, create it - if createErr := c.Create(ctx, certSecret); createErr != nil { - return fmt.Errorf("failed creating certificate secret: %w", createErr) - } - } else { - return fmt.Errorf("failed getting certificate secret: %w", err) - } - } else if existingSecret.Type != certSecret.Type { + switch { + case err == nil: // Secret exists but with a different type, delete and create it again - if recreateSecret(ctx, c, existingSecret, certSecret) != nil { - return errors.New("failed to recreate secret with type corrected") + if existingSecret.Type != certSecret.Type { + return recreateSecret(ctx, c, existingSecret, certSecret) } - } - if isSecretOutdated(existingSecret.Data, certSecret.Data) { - if err = c.Update(ctx, certSecret); err != nil { // update data if found with same type but outdated content - return fmt.Errorf("failed to update secret: %w", err) + // update data if found with same type but outdated content + if isSecretOutdated(existingSecret.Data, certSecret.Data) { + if err = c.Update(ctx, certSecret); err != nil { + return fmt.Errorf("failed to update existing secret: %w", err) + } + } + case k8serr.IsNotFound(err): + // Secret does not exist, create it + if err := c.Create(ctx, certSecret); err != nil { + return fmt.Errorf("failed creating new certificate secret: %w", err) } + default: + return fmt.Errorf("failed getting certificate secret: %w", err) } + return nil }