From 27b54d5bf428c5eb70bfdca7821fbc3858fef4a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nils=20Gustav=20Str=C3=A5b=C3=B8?= Date: Tue, 20 Aug 2024 13:57:56 +0200 Subject: [PATCH 1/3] refactor alert syncer to not use ApplySecret --- pkg/apis/alert/secrets.go | 44 ++++++++++++++++++++++++------------ pkg/apis/kube/config_maps.go | 6 +++++ pkg/apis/kube/secrets.go | 6 +++++ 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/pkg/apis/alert/secrets.go b/pkg/apis/alert/secrets.go index ff3a3ab9f..efc55025c 100644 --- 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) //nolint:staticcheck // must be updated to use UpdateSecret or CreateSecret - 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/kube/config_maps.go b/pkg/apis/kube/config_maps.go index 56af46eed..147e47918 100644 --- a/pkg/apis/kube/config_maps.go +++ b/pkg/apis/kube/config_maps.go @@ -39,6 +39,9 @@ func (kubeutil *Kube) ListEnvVarsMetadataConfigMaps(ctx context.Context, namespa // CreateConfigMap Create config map func (kubeutil *Kube) CreateConfigMap(ctx context.Context, namespace string, configMap *corev1.ConfigMap) (*corev1.ConfigMap, error) { created, err := kubeutil.kubeClient.CoreV1().ConfigMaps(namespace).Create(ctx, configMap, metav1.CreateOptions{}) + if err != nil { + return nil, err + } log.Ctx(ctx).Info().Msgf("Created config map %s/%s", created.Namespace, created.Name) return created, err } @@ -52,6 +55,9 @@ func (kubeutil *Kube) UpdateConfigMap(ctx context.Context, original, modified *c } updated, err := kubeutil.kubeClient.CoreV1().ConfigMaps(modified.Namespace).Update(ctx, modified, metav1.UpdateOptions{}) + if err != nil { + return nil, err + } log.Ctx(ctx).Info().Msgf("Updated configmap %s/%s", updated.Namespace, updated.Name) return updated, err } diff --git a/pkg/apis/kube/secrets.go b/pkg/apis/kube/secrets.go index ba680576e..45925dc21 100644 --- a/pkg/apis/kube/secrets.go +++ b/pkg/apis/kube/secrets.go @@ -46,6 +46,9 @@ func (kubeutil *Kube) ListSecretExistsForLabels(ctx context.Context, namespace s func (kubeutil *Kube) CreateSecret(ctx context.Context, namespace string, secret *corev1.Secret) (*corev1.Secret, error) { created, err := kubeutil.kubeClient.CoreV1().Secrets(namespace).Create(ctx, secret, metav1.CreateOptions{}) + if err != nil { + return nil, err + } log.Ctx(ctx).Info().Msgf("Created secret %s/%s", created.Namespace, created.Name) return created, err } @@ -59,6 +62,9 @@ func (kubeutil *Kube) UpdateSecret(ctx context.Context, original, modified *core } updated, err := kubeutil.kubeClient.CoreV1().Secrets(modified.Namespace).Update(ctx, modified, metav1.UpdateOptions{}) + if err != nil { + return nil, err + } log.Ctx(ctx).Info().Msgf("Updated secret %s/%s", updated.Namespace, updated.Name) return updated, err } From efab26ed49a2502b283c54927783d47ed978698b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nils=20Gustav=20Str=C3=A5b=C3=B8?= Date: Tue, 20 Aug 2024 16:41:45 +0200 Subject: [PATCH 2/3] refactor application build secrets to not use ApplySecret --- pkg/apis/applicationconfig/build_secret.go | 132 +++++++++------------ pkg/apis/applicationconfig/role.go | 6 +- 2 files changed, 63 insertions(+), 75 deletions(-) diff --git a/pkg/apis/applicationconfig/build_secret.go b/pkg/apis/applicationconfig/build_secret.go index d5e63c66a..6657fef79 100644 --- a/pkg/apis/applicationconfig/build_secret.go +++ b/pkg/apis/applicationconfig/build_secret.go @@ -2,99 +2,99 @@ package applicationconfig import ( "context" + "fmt" + "slices" - commonUtils "github.com/equinor/radix-common/utils" "github.com/equinor/radix-operator/pkg/apis/defaults" "github.com/equinor/radix-operator/pkg/apis/kube" "github.com/equinor/radix-operator/pkg/apis/utils" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func (app *ApplicationConfig) syncBuildSecrets(ctx context.Context) error { - appNamespace := utils.GetAppNamespace(app.config.Name) - isSecretExist := app.kubeutil.SecretExists(ctx, appNamespace, defaults.BuildSecretsName) - - if app.config.Spec.Build == nil { - if isSecretExist { - // Delete build secret - err := app.kubeutil.DeleteSecret(ctx, appNamespace, defaults.BuildSecretsName) - if err != nil { - return err - } + if app.config.Spec.Build == nil || len(app.config.Spec.Build.Secrets) == 0 { + if err := app.garbageCollectBuildSecrets(ctx); err != nil { + return fmt.Errorf("failed to garbage collect build secret: %w", err) } - err := garbageCollectAccessToBuildSecrets(ctx, app) - if err != nil { - return err + + if err := app.garbageCollectAccessToBuildSecrets(ctx); err != nil { + return fmt.Errorf("failed to garbage collect access to build secret: %w", err) } } else { - buildSecrets := app.config.Spec.Build.Secrets - if !isSecretExist { - // Create build secret - err := app.initializeBuildSecret(ctx, appNamespace, defaults.BuildSecretsName, buildSecrets) - if err != nil { - return err + currentSecret, desiredSecret, err := app.getCurrentAndDesiredBuildSecret(ctx) + if err != nil { + return fmt.Errorf("failed get current and desired build secret: %w", err) + } + + if currentSecret != nil { + if _, err := app.kubeutil.UpdateSecret(ctx, currentSecret, desiredSecret); err != nil { + return fmt.Errorf("failed to update build secret: %w", err) } } else { - // Update build secret if there is any change - err := app.updateBuildSecret(ctx, appNamespace, defaults.BuildSecretsName, buildSecrets) - if err != nil { - return err + if _, err := app.kubeutil.CreateSecret(ctx, desiredSecret.Namespace, desiredSecret); err != nil { + return fmt.Errorf("failed to create build secret: %w", err) } } - // Grant access to build secret (RBAC) - err := app.grantAccessToBuildSecrets(ctx, appNamespace) - if err != nil { - return err + if err := app.grantAccessToBuildSecrets(ctx); err != nil { + return fmt.Errorf("failed to grant access to build secret: %w", err) } } return nil } -func (app *ApplicationConfig) initializeBuildSecret(ctx context.Context, namespace, name string, buildSecrets []string) error { - data := make(map[string][]byte) - defaultValue := []byte(defaults.BuildSecretDefaultData) - - for _, buildSecret := range buildSecrets { - data[buildSecret] = defaultValue - } - - secret := getBuildSecretForData(app.config.Name, namespace, name, data) - _, err := app.kubeutil.ApplySecret(ctx, namespace, secret) //nolint:staticcheck // must be updated to use UpdateSecret or CreateSecret +func (app *ApplicationConfig) getCurrentAndDesiredBuildSecret(ctx context.Context) (current, desired *corev1.Secret, err error) { + ns := utils.GetAppNamespace(app.config.Name) + currentInternal, err := app.kubeutil.GetSecret(ctx, ns, defaults.BuildSecretsName) if err != nil { - return err + if !k8serrors.IsNotFound(err) { + return nil, nil, err + } + desired = &corev1.Secret{ + Type: corev1.SecretTypeOpaque, + ObjectMeta: metav1.ObjectMeta{ + Name: defaults.BuildSecretsName, + Namespace: ns, + }, + } + } else { + desired = currentInternal.DeepCopy() + current = currentInternal } - return nil -} -func (app *ApplicationConfig) updateBuildSecret(ctx context.Context, namespace, name string, buildSecrets []string) error { - secret, err := app.kubeutil.GetSecret(ctx, namespace, name) - if err != nil { - return err + desired.Labels = map[string]string{ + kube.RadixAppLabel: app.config.Name, } - orphanRemoved := removeOrphanedSecrets(secret, buildSecrets) - secretAppended := appendSecrets(secret, buildSecrets) - if !orphanRemoved && !secretAppended { - // Secret definition may have changed, but not data - secret = getBuildSecretForData(app.config.Name, namespace, name, secret.Data) - } + setBuildSecretData(desired, app.config.Spec.Build.Secrets) + + return current, desired, nil +} - _, err = app.kubeutil.ApplySecret(ctx, namespace, secret) //nolint:staticcheck // must be updated to use UpdateSecret or CreateSecret +func (app *ApplicationConfig) garbageCollectBuildSecrets(ctx context.Context) error { + secret, err := app.kubeutil.GetSecret(ctx, utils.GetAppNamespace(app.config.Name), defaults.BuildSecretsName) if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } return err } + return app.kubeutil.DeleteSecret(ctx, secret.Namespace, secret.Name) +} - return nil +func setBuildSecretData(buildSecret *corev1.Secret, buildSecretKeys []string) { + removeOrphanedBuildSecretKeys(buildSecret, buildSecretKeys) + appendMissingBuildSecretKeys(buildSecret, buildSecretKeys) } -func removeOrphanedSecrets(buildSecrets *corev1.Secret, secrets []string) bool { +func removeOrphanedBuildSecretKeys(buildSecret *corev1.Secret, buildSecretKeys []string) bool { orphanRemoved := false - for secretName := range buildSecrets.Data { - if !commonUtils.ContainsString(secrets, secretName) { - delete(buildSecrets.Data, secretName) + for secretName := range buildSecret.Data { + if !slices.Contains(buildSecretKeys, secretName) { + delete(buildSecret.Data, secretName) orphanRemoved = true } } @@ -102,7 +102,7 @@ func removeOrphanedSecrets(buildSecrets *corev1.Secret, secrets []string) bool { return orphanRemoved } -func appendSecrets(buildSecrets *corev1.Secret, secrets []string) bool { +func appendMissingBuildSecretKeys(buildSecrets *corev1.Secret, buildSecretKeys []string) bool { defaultValue := []byte(defaults.BuildSecretDefaultData) if buildSecrets.Data == nil { @@ -111,7 +111,7 @@ func appendSecrets(buildSecrets *corev1.Secret, secrets []string) bool { } secretAppended := false - for _, secretName := range secrets { + for _, secretName := range buildSecretKeys { if _, ok := buildSecrets.Data[secretName]; !ok { buildSecrets.Data[secretName] = defaultValue secretAppended = true @@ -120,17 +120,3 @@ func appendSecrets(buildSecrets *corev1.Secret, secrets []string) bool { return secretAppended } - -func getBuildSecretForData(appName, namespace, name string, data map[string][]byte) *corev1.Secret { - return &corev1.Secret{ - Type: corev1.SecretTypeOpaque, - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - Labels: map[string]string{ - kube.RadixAppLabel: appName, - }, - }, - Data: data, - } -} diff --git a/pkg/apis/applicationconfig/role.go b/pkg/apis/applicationconfig/role.go index b8b6e5396..851371655 100644 --- a/pkg/apis/applicationconfig/role.go +++ b/pkg/apis/applicationconfig/role.go @@ -13,7 +13,8 @@ import ( "k8s.io/apimachinery/pkg/api/errors" ) -func (app *ApplicationConfig) grantAccessToBuildSecrets(ctx context.Context, namespace string) error { +func (app *ApplicationConfig) grantAccessToBuildSecrets(ctx context.Context) error { + namespace := utils.GetAppNamespace(app.config.Name) err := app.grantPipelineAccessToSecret(ctx, namespace, defaults.BuildSecretsName) if err != nil { return err @@ -93,7 +94,8 @@ func (app *ApplicationConfig) garbageCollectAccessToBuildSecretsForRole(ctx cont return nil } -func garbageCollectAccessToBuildSecrets(ctx context.Context, app *ApplicationConfig) error { + +func (app *ApplicationConfig) garbageCollectAccessToBuildSecrets(ctx context.Context) error { appNamespace := utils.GetAppNamespace(app.config.Name) for _, roleName := range []string{ getPipelineRoleNameToSecret(defaults.BuildSecretsName), From ae4e179e840f16daf8545eef297cd4e9cccfeefe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nils=20Gustav=20Str=C3=A5b=C3=B8?= Date: Wed, 21 Aug 2024 09:00:08 +0200 Subject: [PATCH 3/3] update chart version --- charts/radix-operator/Chart.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/charts/radix-operator/Chart.yaml b/charts/radix-operator/Chart.yaml index 00d9550a9..c0938d092 100644 --- a/charts/radix-operator/Chart.yaml +++ b/charts/radix-operator/Chart.yaml @@ -1,7 +1,7 @@ apiVersion: v2 name: radix-operator -version: 1.37.7 -appVersion: 1.57.15 +version: 1.37.8 +appVersion: 1.57.16 kubeVersion: ">=1.24.0" description: Radix Operator keywords: