Fix: failed to create envoy-oidc-hmac secret when upgrading EG (envoy…

…proxy#2835)

try to create every secret instead of returning eraly

Signed-off-by: huabing zhao <[email protected]>
Signed-off-by: zirain <[email protected]>

internal/cmd/certgen.go

@@ -74,12 +74,11 @@ func outputCerts(ctx context.Context, cli client.Client, cfg *config.Server, cer
 	log := cfg.Logger
 
 	if err != nil {
-		if errors.Is(err, kubernetes.ErrSecretExists) {
-			log.Info("exiting early", "reason", err)
-			return nil
+		if !errors.Is(err, kubernetes.ErrSecretExists) {
+			log.Info(err.Error())
+		} else {
+			return fmt.Errorf("failed to create or update secrets: %w", err)
 		}
-
-		return fmt.Errorf("failed to create or update secrets: %w", err)
 	}
 
 	for i := range secrets {

internal/provider/kubernetes/secrets.go

@@ -92,7 +92,11 @@ func CertsToSecret(namespace string, certs *crypto.Certificates) []corev1.Secret
 // CreateOrUpdateSecrets creates the provided secrets if they don't exist or updates
 // them if they do.
 func CreateOrUpdateSecrets(ctx context.Context, client client.Client, secrets []corev1.Secret, update bool) ([]corev1.Secret, error) {
-	var tidySecrets []corev1.Secret
+	var (
+		tidySecrets     []corev1.Secret
+		existingSecrets []string
+	)
+
 	for i := range secrets {
 		secret := secrets[i]
 		current := new(corev1.Secret)
@@ -109,10 +113,10 @@ func CreateOrUpdateSecrets(ctx context.Context, client client.Client, secrets []
 			// Update if current value is different and update arg is set.
 		} else {
 			if !update {
-				return nil, fmt.Errorf("%s/%s: %w;"+
-					"Either update it manually or set overwriteControlPlaneCerts "+
-					"in the EnvoyGateway config", secret.Namespace, secret.Name, ErrSecretExists)
+				existingSecrets = append(existingSecrets, fmt.Sprintf("%s/%s", secret.Namespace, secret.Name))
+				continue
 			}
+			fmt.Println()
 
 			if !reflect.DeepEqual(secret.Data, current.Data) {
 				if err := client.Update(ctx, &secret); err != nil {
@@ -123,5 +127,11 @@ func CreateOrUpdateSecrets(ctx context.Context, client client.Client, secrets []
 		tidySecrets = append(tidySecrets, secret)
 	}
 
+	if len(existingSecrets) > 0 {
+		return tidySecrets, fmt.Errorf("%v: %w;"+
+			"Either update the secrets manually or set overwriteControlPlaneCerts "+
+			"in the EnvoyGateway config", existingSecrets, ErrSecretExists)
+	}
+
 	return tidySecrets, nil
 }

internal/provider/kubernetes/secrets_test.go

@@ -0,0 +1,97 @@
+// Copyright Envoy Gateway Authors
+// SPDX-License-Identifier: Apache-2.0
+// The full text of the Apache license is available in the LICENSE file at
+// the root of the repo.
+
+package kubernetes
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
+)
+
+var (
+	envoyGatewaySecret = corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "envoy-gateway",
+			Namespace: "envoy-gateway-system",
+		},
+	}
+
+	envoySecret = corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "envoy",
+			Namespace: "envoy-gateway-system",
+		},
+	}
+
+	envoyRateLimitSecret = corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "envoy-rate-limit",
+			Namespace: "envoy-gateway-system",
+		},
+	}
+
+	oidcHMACSecret = corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "envoy-oidc-hmac",
+			Namespace: "envoy-gateway-system",
+		},
+	}
+
+	existingSecretsWithoutHMAC = []client.Object{
+		&envoyGatewaySecret,
+		&envoySecret,
+		&envoyRateLimitSecret,
+	}
+
+	existingSecretsWithHMAC = []client.Object{
+		&envoyGatewaySecret,
+		&envoySecret,
+		&envoyRateLimitSecret,
+		&oidcHMACSecret,
+	}
+
+	SecretsToCreate = []corev1.Secret{
+		envoyGatewaySecret,
+		envoySecret,
+		envoyRateLimitSecret,
+		oidcHMACSecret,
+	}
+)
+
+func TestCreateSecretsWhenUpgrade(t *testing.T) {
+	t.Run("create HMAC secret when it does not exist", func(t *testing.T) {
+		cli := fakeclient.NewClientBuilder().WithObjects(existingSecretsWithoutHMAC...).Build()
+
+		created, err := CreateOrUpdateSecrets(context.Background(), cli, SecretsToCreate, false)
+		require.ErrorIs(t, err, ErrSecretExists)
+		require.Len(t, created, 1)
+		require.Equal(t, "envoy-oidc-hmac", created[0].Name)
+
+		err = cli.Get(context.Background(), client.ObjectKeyFromObject(&oidcHMACSecret), &corev1.Secret{})
+		require.NoError(t, err)
+	})
+
+	t.Run("skip HMAC secret when it exist", func(t *testing.T) {
+		cli := fakeclient.NewClientBuilder().WithObjects(existingSecretsWithHMAC...).Build()
+
+		created, err := CreateOrUpdateSecrets(context.Background(), cli, SecretsToCreate, false)
+		require.ErrorIs(t, err, ErrSecretExists)
+		require.Emptyf(t, created, "expected no secrets to be created, got %v", created)
+	})
+
+	t.Run("update secrets when they exist", func(t *testing.T) {
+		cli := fakeclient.NewClientBuilder().WithObjects(existingSecretsWithHMAC...).Build()
+
+		created, err := CreateOrUpdateSecrets(context.Background(), cli, SecretsToCreate, true)
+		require.NoError(t, err)
+		require.Len(t, created, 4)
+	})
+}