Merge pull request #1172 from equinor/master

release operator
equinor · Aug 21, 2024 · b8252ff · b8252ff
2 parents 4de4697 + 52aa6da
commit b8252ff
Show file tree

Hide file tree

Showing 22 changed files with 411 additions and 285 deletions.
diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml
@@ -77,8 +77,7 @@ jobs:
     - name: golangci-lint
       uses: golangci/golangci-lint-action@v6
       with:
-        version: v1.58.2
-        working-directory: './radix-operator'
+        version: v1.59.1
 
   verify-code-generation:
     name: Verify Code Generation

diff --git a/charts/radix-operator/Chart.yaml b/charts/radix-operator/Chart.yaml
@@ -1,7 +1,7 @@
 apiVersion: v2
 name: radix-operator
-version: 1.37.5
-appVersion: 1.57.13
+version: 1.37.8
+appVersion: 1.57.16
 kubeVersion: ">=1.24.0"
 description: Radix Operator
 keywords:

diff --git a/pkg/apis/alert/secrets.go b/pkg/apis/alert/secrets.go
@@ -5,37 +5,51 @@ import (
 	"fmt"
 
 	"github.com/equinor/radix-operator/pkg/apis/kube"
-	v1 "k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func (syncer *alertSyncer) createOrUpdateSecret(ctx context.Context) error {
-	secretName, ns := GetAlertSecretName(syncer.radixAlert.Name), syncer.radixAlert.Namespace
+	current, desired, err := syncer.getCurrentAndDesiredAlertSecret(ctx)
+	if err != nil {
+		return err
+	}
 
-	secret, err := syncer.kubeUtil.GetSecret(ctx, ns, secretName)
-	if err != nil && !errors.IsNotFound(err) {
+	if current != nil {
+		_, err = syncer.kubeUtil.UpdateSecret(ctx, current, desired)
 		return err
 	}
 
-	if secret == nil {
-		secret = &v1.Secret{
-			Type: v1.SecretType("Opaque"),
+	_, err = syncer.kubeUtil.CreateSecret(ctx, desired.Namespace, desired)
+	return err
+}
+
+func (syncer *alertSyncer) getCurrentAndDesiredAlertSecret(ctx context.Context) (current, desired *corev1.Secret, err error) {
+	secretName, ns := GetAlertSecretName(syncer.radixAlert.Name), syncer.radixAlert.Namespace
+	currentInternal, err := syncer.kubeUtil.GetSecret(ctx, ns, secretName)
+	if err != nil {
+		if !errors.IsNotFound(err) {
+			return nil, nil, err
+		}
+		desired = &corev1.Secret{
+			Type: corev1.SecretType("Opaque"),
 			ObjectMeta: metav1.ObjectMeta{
-				Name: secretName,
+				Name:      secretName,
+				Namespace: ns,
 			},
 		}
 	} else {
-		syncer.removedOrphanedSecretKeys(secret)
+		desired = currentInternal.DeepCopy()
+		current = currentInternal
 	}
 
-	syncer.setSecretCommonProps(secret)
-
-	_, err = syncer.kubeUtil.ApplySecret(ctx, ns, secret)
-	return err
+	syncer.removedOrphanedSecretKeys(desired)
+	syncer.setSecretCommonProps(desired)
+	return current, desired, nil
 }
 
-func (syncer *alertSyncer) setSecretCommonProps(secret *v1.Secret) {
+func (syncer *alertSyncer) setSecretCommonProps(secret *corev1.Secret) {
 	secret.OwnerReferences = syncer.getOwnerReference()
 
 	labels := map[string]string{}
@@ -45,7 +59,7 @@ func (syncer *alertSyncer) setSecretCommonProps(secret *v1.Secret) {
 	secret.Labels = labels
 }
 
-func (syncer *alertSyncer) removedOrphanedSecretKeys(secret *v1.Secret) {
+func (syncer *alertSyncer) removedOrphanedSecretKeys(secret *corev1.Secret) {
 	expectedKeys := map[string]interface{}{}
 
 	// Secret keys related to receiver configuration

diff --git a/pkg/apis/application/application_test.go b/pkg/apis/application/application_test.go
@@ -123,8 +123,9 @@ func TestOnSync_RegistrationCreated_AppNamespaceWithResourcesCreated(t *testing.
 	assert.Equal(t, 1, len(appAdminRoleBinding.Subjects))
 
 	secrets, _ := client.CoreV1().Secrets("any-app-app").List(context.Background(), metav1.ListOptions{})
-	assert.Equal(t, 1, len(secrets.Items))
-	assert.Equal(t, "git-ssh-keys", secrets.Items[0].Name)
+	secretNames := slice.Map(secrets.Items, func(s corev1.Secret) string { return s.Name })
+	expectedSecrets := []string{defaults.GitPrivateKeySecretName, defaults.AzureACRServicePrincipleBuildahSecretName, defaults.AzureACRServicePrincipleSecretName, defaults.AzureACRTokenPasswordAppRegistrySecretName}
+	assert.ElementsMatch(t, expectedSecrets, secretNames)
 
 	serviceAccounts, _ := client.CoreV1().ServiceAccounts("any-app-app").List(context.Background(), metav1.ListOptions{})
 	assert.Equal(t, 2, len(serviceAccounts.Items))

diff --git a/pkg/apis/application/secrets.go b/pkg/apis/application/secrets.go
@@ -15,97 +15,149 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+const (
+	knownHostsSecretKey = "known_hosts"
+)
+
 // ApplySecretsForPipelines creates secrets needed by pipeline to run
 func (app *Application) applySecretsForPipelines(ctx context.Context) error {
-	radixRegistration := app.registration
-
 	log.Ctx(ctx).Debug().Msg("Apply secrets for pipelines")
-	buildNamespace := utils.GetAppNamespace(radixRegistration.Name)
 
-	err := app.applyGitDeployKeyToBuildNamespace(ctx, buildNamespace)
-	if err != nil {
-		return err
+	if err := app.applyGitDeployKeyToBuildNamespace(ctx); err != nil {
+		return fmt.Errorf("failed to apply pipeline git deploy keys: %w", err)
 	}
-	err = app.applyServicePrincipalACRSecretToBuildNamespace(ctx, buildNamespace)
-	if err != nil {
-		log.Ctx(ctx).Warn().Msgf("Failed to apply service principle acr secrets (%s, %s) to namespace %s", defaults.AzureACRServicePrincipleSecretName, defaults.AzureACRServicePrincipleBuildahSecretName, buildNamespace)
+
+	if err := app.applyServicePrincipalACRSecretToBuildNamespace(ctx); err != nil {
+		return fmt.Errorf("failed to apply pipeline service principal ACR secrets: %w", err)
 	}
+
 	return nil
 }
 
-func (app *Application) applyGitDeployKeyToBuildNamespace(ctx context.Context, namespace string) error {
-	radixRegistration := app.registration
-	secret, err := app.kubeutil.GetSecret(ctx, namespace, defaults.GitPrivateKeySecretName)
+func (app *Application) applyGitDeployKeyToBuildNamespace(ctx context.Context) error {
+	currentSecret, desiredSecret, derivedPublicKey, err := app.getCurrentAndDesiredGitPrivateDeployKeySecret(ctx)
 	if err != nil {
-		if !k8serrors.IsNotFound(err) {
+		return err
+	}
+
+	currentCm, desiredCm, err := app.getCurrentAndDesiredGitPublicDeployKeyConfigMap(ctx, derivedPublicKey)
+	if err != nil {
+		return err
+	}
+
+	if currentSecret != nil {
+		if _, err := app.kubeutil.UpdateSecret(ctx, currentSecret, desiredSecret); err != nil {
+			return err
+		}
+	} else {
+		if _, err := app.kubeutil.CreateSecret(ctx, desiredSecret.Namespace, desiredSecret); err != nil {
 			return err
 		}
 	}
 
-	deployKey := &utils.DeployKey{}
-
-	privateKeyExists := app.gitPrivateKeyExists(secret) // checking if private key exists
-	if privateKeyExists {
-		deployKey, err = deriveKeyPairFromSecret(secret) // if private key exists, retrieve public key from secret
-		if err != nil {
+	if currentCm != nil {
+		if _, err = app.kubeutil.UpdateConfigMap(ctx, currentCm, desiredCm); err != nil {
 			return err
 		}
 	} else {
-		// if private key secret does not exist, check if RR has private key
-		deployKeyString := radixRegistration.Spec.DeployKey
-		if deployKeyString == "" {
-			// if RR does not have private key, generate new key pair
-			deployKey, err = utils.GenerateDeployKey()
-			if err != nil {
-				return err
-			}
-		} else {
-			// if RR has private key, retrieve both public and private key from RR
-			deployKey.PublicKey = radixRegistration.Spec.DeployKeyPublic
-			deployKey.PrivateKey = radixRegistration.Spec.DeployKey
+		if _, err := app.kubeutil.CreateConfigMap(ctx, desiredCm.Namespace, desiredCm); err != nil {
+			return err
 		}
 	}
 
-	// create corresponding secret with private key
-	secret, err = app.createNewGitDeployKey(ctx, namespace, deployKey.PrivateKey, radixRegistration)
+	return nil
+}
+
+func (app *Application) getCurrentAndDesiredGitPrivateDeployKeySecret(ctx context.Context) (current, desired *corev1.Secret, derivedPublicKey string, err error) {
+	namespace := utils.GetAppNamespace(app.registration.Name)
+	// Cannot assign `current` directly from GetSecret since kube client returns a non-nil value even when an error is returned
+	currentInternal, err := app.kubeutil.GetSecret(ctx, namespace, defaults.GitPrivateKeySecretName)
 	if err != nil {
-		return err
+		if !k8serrors.IsNotFound(err) {
+			return nil, nil, "", err
+		}
+		desired = &corev1.Secret{
+			Type: "Opaque",
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      defaults.GitPrivateKeySecretName,
+				Namespace: namespace,
+			},
+		}
+	} else {
+		desired = currentInternal.DeepCopy()
+		current = currentInternal
 	}
-	_, err = app.kubeutil.ApplySecret(ctx, namespace, secret)
+
+	knownHostsSecret, err := app.kubeutil.GetSecret(ctx, corev1.NamespaceDefault, "radix-known-hosts-git")
 	if err != nil {
-		return err
+		return nil, nil, "", fmt.Errorf("failed to get known hosts secret: %w", err)
 	}
 
-	newCm := app.createGitPublicKeyConfigMap(namespace, deployKey.PublicKey)
-	_, err = app.kubeutil.CreateConfigMap(ctx, namespace, newCm)
+	deployKey, err := getExistingOrGenerateNewDeployKey(current, app.registration)
 	if err != nil {
-		if k8serrors.IsAlreadyExists(err) {
-			existingCm, err := app.kubeutil.GetConfigMap(ctx, namespace, defaults.GitPublicKeyConfigMapName)
-			if err != nil {
-				return err
-			}
-			err = app.kubeutil.ApplyConfigMap(ctx, namespace, existingCm, newCm)
-			if err != nil {
-				return err
-			}
-		} else {
-			return err
-		}
+		return nil, nil, "", fmt.Errorf("failed to get deploy key: %w", err)
 	}
 
-	return nil
+	desired.ObjectMeta.Labels = labels.ForApplicationName(app.registration.Name) // Required when restoring with Velero. We only restore secrets with the "radix-app" label
+	desired.Data = map[string][]byte{
+		defaults.GitPrivateKeySecretKey: []byte(deployKey.PrivateKey),
+		knownHostsSecretKey:             knownHostsSecret.Data[knownHostsSecretKey],
+	}
+
+	return current, desired, deployKey.PublicKey, nil
+}
+
+func getExistingOrGenerateNewDeployKey(fromSecret *corev1.Secret, fromRadixRegistration *v1.RadixRegistration) (*utils.DeployKey, error) {
+	switch {
+	case fromSecret != nil && secretHasGitPrivateDeployKey(fromSecret):
+		privateKey := fromSecret.Data[defaults.GitPrivateKeySecretKey]
+		keypair, err := utils.DeriveDeployKeyFromPrivateKey(string(privateKey))
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse deploy key from existing secret: %w", err)
+		}
+		return keypair, nil
+	case len(fromRadixRegistration.Spec.DeployKey) > 0:
+		return &utils.DeployKey{
+			PrivateKey: fromRadixRegistration.Spec.DeployKey,
+			PublicKey:  fromRadixRegistration.Spec.DeployKeyPublic,
+		}, nil
+	default:
+		keypair, err := utils.GenerateDeployKey()
+		if err != nil {
+			return nil, fmt.Errorf("failed to generate new git deploy key: %w", err)
+		}
+		return keypair, nil
+	}
 }
 
-func deriveKeyPairFromSecret(secret *corev1.Secret) (*utils.DeployKey, error) {
-	privateKey := string(secret.Data[defaults.GitPrivateKeySecretKey])
-	deployKey, err := utils.DeriveDeployKeyFromPrivateKey(privateKey)
+func (app *Application) getCurrentAndDesiredGitPublicDeployKeyConfigMap(ctx context.Context, publicKey string) (current, desired *corev1.ConfigMap, err error) {
+	namespace := utils.GetAppNamespace(app.registration.Name)
+	// Cannot assign `current` directly from GetConfigMap since kube client returns a non-nil value even when an error is returned
+	currentInternal, err := app.kubeutil.GetConfigMap(ctx, namespace, defaults.GitPublicKeyConfigMapName)
 	if err != nil {
-		return nil, err
+		if !k8serrors.IsNotFound(err) {
+			return nil, nil, err
+		}
+		desired = &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      defaults.GitPublicKeyConfigMapName,
+				Namespace: namespace,
+			},
+		}
+	} else {
+		desired = currentInternal.DeepCopy()
+		current = currentInternal
 	}
-	return deployKey, nil
+
+	desired.Data = map[string]string{
+		defaults.GitPublicKeyConfigMapKey: publicKey,
+	}
+
+	return current, desired, nil
 }
 
-func (app *Application) applyServicePrincipalACRSecretToBuildNamespace(ctx context.Context, buildNamespace string) error {
+func (app *Application) applyServicePrincipalACRSecretToBuildNamespace(ctx context.Context) error {
+	buildNamespace := utils.GetAppNamespace(app.registration.Name)
 	servicePrincipalSecretForBuild, err := app.createNewServicePrincipalACRSecret(ctx, buildNamespace, defaults.AzureACRServicePrincipleSecretName)
 	if err != nil {
 		return err
@@ -122,7 +174,7 @@ func (app *Application) applyServicePrincipalACRSecretToBuildNamespace(ctx conte
 	}
 
 	for _, secret := range []*corev1.Secret{servicePrincipalSecretForBuild, servicePrincipalSecretForBuildahBuild, tokenSecretForAppRegistry} {
-		_, err = app.kubeutil.ApplySecret(ctx, buildNamespace, secret)
+		_, err = app.kubeutil.ApplySecret(ctx, buildNamespace, secret) //nolint:staticcheck // must be updated to use UpdateSecret or CreateSecret
 		if err != nil {
 			return err
 		}
@@ -131,28 +183,6 @@ func (app *Application) applyServicePrincipalACRSecretToBuildNamespace(ctx conte
 	return nil
 }
 
-func (app *Application) createNewGitDeployKey(ctx context.Context, namespace, deployKey string, registration *v1.RadixRegistration) (*corev1.Secret, error) {
-	knownHostsSecret, err := app.kubeutil.GetSecret(ctx, corev1.NamespaceDefault, "radix-known-hosts-git")
-	if err != nil {
-		return nil, fmt.Errorf("failed to get known hosts secret: %w", err)
-	}
-
-	secret := corev1.Secret{
-		Type: "Opaque",
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      defaults.GitPrivateKeySecretName,
-			Namespace: namespace,
-			// Required when restoring with Velero. We only restore secrets with the "radix-app" label
-			Labels: labels.ForApplicationName(registration.GetName()),
-		},
-		Data: map[string][]byte{
-			defaults.GitPrivateKeySecretKey: []byte(deployKey),
-			"known_hosts":                   knownHostsSecret.Data["known_hosts"],
-		},
-	}
-	return &secret, nil
-}
-
 func (app *Application) createNewServicePrincipalACRSecret(ctx context.Context, namespace, secretName string) (*corev1.Secret, error) {
 	servicePrincipalSecret, err := app.kubeutil.GetSecret(ctx, corev1.NamespaceDefault, secretName)
 	if err != nil {
@@ -175,19 +205,9 @@ func (app *Application) createNewServicePrincipalACRSecret(ctx context.Context,
 	return &secret, nil
 }
 
-func (app *Application) gitPrivateKeyExists(secret *corev1.Secret) bool {
+func secretHasGitPrivateDeployKey(secret *corev1.Secret) bool {
 	if secret == nil {
 		return false
 	}
 	return len(strings.TrimSpace(string(secret.Data[defaults.GitPrivateKeySecretKey]))) > 0
 }
-func (app *Application) createGitPublicKeyConfigMap(namespace string, key string) *corev1.ConfigMap {
-	// Create a configmap with the public key
-	cm := &corev1.ConfigMap{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      defaults.GitPublicKeyConfigMapName,
-			Namespace: namespace,
-		}, Data: map[string]string{defaults.GitPublicKeyConfigMapKey: key}}
-
-	return cm
-}