From 145bbd1df0405944d358c9401ac77c5beeeea55c Mon Sep 17 00:00:00 2001 From: Robert Cerven Date: Tue, 2 Jul 2024 22:03:46 +0200 Subject: [PATCH] Refactor secrets linking 1. when linking secret to SA, don't add it if already present 2. unlink secret from SA upon imageRepository deletion 3. don't link secret anymore to imagePullSecrets (used for the image pod is using, task/pipeline bundle image) 4. new option to clean up secret links via spec.credentials.verify-linking - It will link secret to service account if link is missing. - It will remove duplicate links of secret in service account. - It will remove secret from imagePullSecrets in service account. - It will unlink secret from service account, if secret doesn't exist (can recreated by using 'regenerate-token'). STONEBLD-2540 Signed-off-by: Robert Cerven --- README.md | 23 +- api/v1alpha1/imagerepository_types.go | 4 + api/v1alpha1/zz_generated.deepcopy.go | 5 + ...ppstudio.redhat.com_imagerepositories.yaml | 5 + controllers/imagerepository_controller.go | 293 +++++++++++++++--- 5 files changed, 281 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index 446ac6c..5cdf268 100644 --- a/README.md +++ b/README.md @@ -111,7 +111,28 @@ spec: regenerate-token: true ... ``` -After token rotation, the `spec.credentials` section will be deleted and `status.credentials.generationTimestamp` updated. +After token rotation, the `spec.credentials.regenerate-token` section will be deleted and `status.credentials.generationTimestamp` updated. + +--- + +### Verify and fix secrets links + +It will link secret to service account if link is missing. +It will remove duplicate links of secret in service account. +It will remove secret from imagePullSecrets in service account. +It will unlink secret from service account, if secret doesn't exist (you can recreate secret using 'regenerate-token'). +It's possible to request verification and fixing of secrets linking to service account by adding: +```yaml +... +spec: + ... + credentials: + verify-linking: true + ... +``` +After verification, the `spec.credentials.verify-linking` section will be deleted. + +--- ### Error handling diff --git a/api/v1alpha1/imagerepository_types.go b/api/v1alpha1/imagerepository_types.go index ae5d53c..adae0a7 100644 --- a/api/v1alpha1/imagerepository_types.go +++ b/api/v1alpha1/imagerepository_types.go @@ -64,6 +64,10 @@ type ImageCredentials struct { // Refreshes both, push and pull tokens. // The field gets cleared after the refresh. RegenerateToken *bool `json:"regenerate-token,omitempty"` + // VerifyLinking defines a request to verify and fix + // secret linking in pipeline service account. + // The field gets cleared after fixing. + VerifyLinking *bool `json:"verify-linking,omitempty"` } type Notifications struct { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index c26f491..28e993c 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -52,6 +52,11 @@ func (in *ImageCredentials) DeepCopyInto(out *ImageCredentials) { *out = new(bool) **out = **in } + if in.VerifyLinking != nil { + in, out := &in.VerifyLinking, &out.VerifyLinking + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ImageCredentials. diff --git a/config/crd/bases/appstudio.redhat.com_imagerepositories.yaml b/config/crd/bases/appstudio.redhat.com_imagerepositories.yaml index 6d14fb1..19638fd 100644 --- a/config/crd/bases/appstudio.redhat.com_imagerepositories.yaml +++ b/config/crd/bases/appstudio.redhat.com_imagerepositories.yaml @@ -50,6 +50,11 @@ spec: accessing credentials. Refreshes both, push and pull tokens. The field gets cleared after the refresh. type: boolean + verify-linking: + description: VerifyLinking defines a request to verify and fix + secret linking in pipeline service account. The field gets cleared + after fixing. + type: boolean type: object image: description: Requested image repository configuration. diff --git a/controllers/imagerepository_controller.go b/controllers/imagerepository_controller.go index 51c8d23..6ce6f36 100644 --- a/controllers/imagerepository_controller.go +++ b/controllers/imagerepository_controller.go @@ -94,7 +94,7 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ // The object is deleted, nothing to do return ctrl.Result{}, nil } - log.Error(err, "failed to get image repository", l.Action, l.ActionView) + log.Error(err, "Failed to get image repository", "ImageRepositoryName", imageRepository.Name, l.Action, l.ActionView) return ctrl.Result{}, err } @@ -107,13 +107,18 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ // Reread quay token r.QuayClient = r.BuildQuayClient(log) + if err := r.unlinkSecretFromServiceAccount(ctx, imageRepository.Status.Credentials.PushSecretName, imageRepository.Namespace); err != nil { + log.Error(err, "Failed to unlink secret from service account", "SecretName", imageRepository.Status.Credentials.PushSecretName, l.Action, l.ActionUpdate) + return ctrl.Result{}, err + } + if controllerutil.ContainsFinalizer(imageRepository, ImageRepositoryFinalizer) { // Do not block deletion on failures r.CleanupImageRepository(ctx, imageRepository) controllerutil.RemoveFinalizer(imageRepository, ImageRepositoryFinalizer) if err := r.Client.Update(ctx, imageRepository); err != nil { - log.Error(err, "failed to remove image repository finalizer", l.Action, l.ActionUpdate) + log.Error(err, "Failed to remove image repository finalizer", "ImageRepositoryName", imageRepository.Name, l.Action, l.ActionUpdate) return ctrl.Result{}, err } log.Info("Image repository finalizer removed", l.Action, l.ActionDelete) @@ -140,7 +145,7 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ if !controllerutil.ContainsFinalizer(imageRepository, ImageRepositoryFinalizer) { setMetricsTime(repositoryIdForMetrics, reconcileStartTime) if err := r.ProvisionImageRepository(ctx, imageRepository); err != nil { - log.Error(err, "provision of image repository failed") + log.Error(err, "provision of image repository failed", "ImageRepositoryName", imageRepository.Name) return ctrl.Result{}, err } return ctrl.Result{}, nil @@ -160,21 +165,21 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, nil } - log.Error(err, "failed to get component", "ComponentName", componentName) + log.Error(err, "Failed to get component", "ComponentName", componentName, l.Action, l.ActionView) return ctrl.Result{}, err } component.Spec.ContainerImage = imageRepository.Status.Image.URL if err := r.Client.Update(ctx, component); err != nil { - log.Error(err, "failed to update Component after provision", "ComponentName", componentName) + log.Error(err, "Failed to update Component after provision", "ComponentName", componentName, l.Action, l.ActionUpdate) return ctrl.Result{}, err } log.Info("Updated component's ContainerImage", "ComponentName", componentName) delete(imageRepository.Annotations, updateComponentAnnotationName) if err := r.Client.Update(ctx, imageRepository); err != nil { - log.Error(err, "failed to update imageRepository annotation") + log.Error(err, "Failed to update imageRepository annotation", "ImageRepositoryName", imageRepository.Name, l.Action, l.ActionUpdate) return ctrl.Result{}, err } log.Info("Updated image repository annotation") @@ -192,7 +197,7 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ oldName := imageRepository.Spec.Image.Name imageRepository.Spec.Image.Name = imageRepositoryName if err := r.Client.Update(ctx, imageRepository); err != nil { - log.Error(err, "failed to revert image repository name", "OldName", oldName, "ExpectedName", imageRepositoryName, l.Action, l.ActionUpdate) + log.Error(err, "Failed to revert image repository name", "OldName", oldName, "ExpectedName", imageRepositoryName, l.Action, l.ActionUpdate) return ctrl.Result{}, err } log.Info("reverted image repository name", "OldName", oldName, "ExpectedName", imageRepositoryName, l.Action, l.ActionUpdate) @@ -216,6 +221,15 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ } return ctrl.Result{}, nil } + + // Check and fix linking is requested + verifyLinking := imageRepository.Spec.Credentials.VerifyLinking + if verifyLinking != nil && *verifyLinking { + if err := r.VerifyAndFixSecretsLinking(ctx, imageRepository); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil + } } if err = r.HandleNotifications(ctx, imageRepository); err != nil { @@ -223,7 +237,7 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ } if err := r.Client.Status().Update(ctx, imageRepository); err != nil { - log.Error(err, "failed to update image repository status") + log.Error(err, "Failed to update imageRepository status", "ImageRepositoryName", imageRepository.Name, l.Action, l.ActionUpdate) return ctrl.Result{}, err } @@ -241,7 +255,7 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ // ProvisionImageRepository creates image repository, robot account(s) and secret(s) to access the image repository. // If labels with Application and Component name are present, robot account with pull only access -// will be created and pull token will be propagated to all environments via Remote Secret. +// will be created and pull token will be propagated Secret. func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context, imageRepository *imagerepositoryv1alpha1.ImageRepository) error { log := ctrllog.FromContext(ctx).WithName("ImageRepositoryProvision") ctx = ctrllog.IntoContext(ctx, log) @@ -256,13 +270,13 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context imageRepository.Status.State = imagerepositoryv1alpha1.ImageRepositoryStateFailed imageRepository.Status.Message = fmt.Sprintf("Component '%s' does not exist", componentName) if err := r.Client.Status().Update(ctx, imageRepository); err != nil { - log.Error(err, "failed to update image repository status") + log.Error(err, "Failed to update imageRepository status", "ImageRepositoryName", imageRepository.Name, l.Action, l.ActionUpdate) return err } log.Info("attempt to create image repository related to non existing component", "Component", componentName) return nil } - log.Error(err, "failed to get component", "ComponentName", componentName) + log.Error(err, "Failed to get component", "ComponentName", componentName, l.Action, l.ActionView) return err } } @@ -299,7 +313,7 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context Description: "AppStudio repository for the user", }) if err != nil { - log.Error(err, "failed to create image repository", l.Action, l.ActionAdd, l.Audit, "true") + log.Error(err, "Failed to create image repository", l.Action, l.ActionAdd, l.Audit, "true") imageRepository.Status.State = imagerepositoryv1alpha1.ImageRepositoryStateFailed if err.Error() == "payment required" { imageRepository.Status.Message = "Number of private repositories exceeds current quay plan limit" @@ -307,7 +321,7 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context imageRepository.Status.Message = err.Error() } if err := r.Client.Status().Update(ctx, imageRepository); err != nil { - log.Error(err, "failed to update image repository status") + log.Error(err, "Failed to update imageRepository status", "ImageRepositoryName", imageRepository.Name, l.Action, l.ActionUpdate) } return nil } @@ -352,21 +366,20 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context controllerutil.AddFinalizer(imageRepository, ImageRepositoryFinalizer) if isComponentLinked(imageRepository) { if err := controllerutil.SetOwnerReference(component, imageRepository, r.Scheme); err != nil { - log.Error(err, "failed to set component as owner", "ComponentName", component.Name) - // Do not brake provision because of failed owner reference + log.Error(err, "Failed to set component as owner", "ComponentName", component.Name) + // Do not fail provision because of failed owner reference } } if err := r.Client.Update(ctx, imageRepository); err != nil { - log.Error(err, "failed to update CR after provision") + log.Error(err, "Failed to update imageRepository after provision", "ImageRepositoryName", imageRepository.Name, l.Action, l.ActionUpdate) return err - } else { - log.Info("Finished provision of image repository and added finalizer") } + log.Info("Finished provision of image repository and added finalizer", "ImageRepositoryName", imageRepository.Name) imageRepository.Status = status if err := r.Client.Status().Update(ctx, imageRepository); err != nil { - log.Error(err, "failed to update CR status after provision") + log.Error(err, "Failed to update imageRepository status after provision", "ImageRepositoryName", imageRepository.Name, l.Action, l.ActionUpdate) return err } @@ -379,7 +392,7 @@ type imageRepositoryAccessData struct { } // ProvisionImageRepositoryAccess makes existing quay image repository accessible -// by creating robot account and storing its token in a RemoteSecret. +// by creating robot account and storing its token in a Secret. func (r *ImageRepositoryReconciler) ProvisionImageRepositoryAccess(ctx context.Context, imageRepository *imagerepositoryv1alpha1.ImageRepository, isPullOnly bool) (*imageRepositoryAccessData, error) { log := ctrllog.FromContext(ctx).WithName("ProvisionImageRepositoryAccess").WithValues("IsPullOnly", isPullOnly) ctx = ctrllog.IntoContext(ctx, log) @@ -390,7 +403,7 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepositoryAccess(ctx context.C robotAccountName := generateQuayRobotAccountName(imageRepositoryName, isPullOnly) robotAccount, err := r.QuayClient.CreateRobotAccount(r.QuayOrganization, robotAccountName) if err != nil { - log.Error(err, "failed to create robot account", "RobotAccountName", robotAccountName, l.Action, l.ActionAdd, l.Audit, "true") + log.Error(err, "Failed to create robot account", "RobotAccountName", robotAccountName, l.Action, l.ActionAdd, l.Audit, "true") return nil, err } if robotAccount == nil { @@ -401,7 +414,7 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepositoryAccess(ctx context.C err = r.QuayClient.AddPermissionsForRepositoryToRobotAccount(r.QuayOrganization, imageRepositoryName, robotAccount.Name, !isPullOnly) if err != nil { - log.Error(err, "failed to add permissions to robot account", "RobotAccountName", robotAccountName, l.Action, l.ActionUpdate, l.Audit, "true") + log.Error(err, "Failed to add permissions to robot account", "RobotAccountName", robotAccountName, l.Action, l.ActionUpdate, l.Audit, "true") return nil, err } @@ -433,20 +446,20 @@ func (r *ImageRepositoryReconciler) RegenerateImageRepositoryCredentials(ctx con imageRepository.Spec.Credentials.RegenerateToken = nil if err := r.Client.Update(ctx, imageRepository); err != nil { - log.Error(err, "failed to update image repository", l.Action, l.ActionUpdate) + log.Error(err, "Failed to update imageRepository", "ImageRepositoryName", imageRepository.Name, l.Action, l.ActionUpdate) return err } imageRepository.Status.Credentials.GenerationTimestamp = &metav1.Time{Time: time.Now()} if err := r.Client.Status().Update(ctx, imageRepository); err != nil { - log.Error(err, "failed to update image repository status", l.Action, l.ActionUpdate) + log.Error(err, "Failed to update imageRepository status", "ImageRepositoryName", imageRepository.Name, l.Action, l.ActionUpdate) return err } return nil } -// RegenerateImageRepositoryAccessToken rotates robot account token and updates new one to the corresponding Remote Secret. +// RegenerateImageRepositoryAccessToken rotates robot account token and updates new one to the corresponding Secret. func (r *ImageRepositoryReconciler) RegenerateImageRepositoryAccessToken(ctx context.Context, imageRepository *imagerepositoryv1alpha1.ImageRepository, isPullOnly bool) error { log := ctrllog.FromContext(ctx).WithName("RegenerateImageRepositoryAccessToken").WithValues("IsPullOnly", isPullOnly) ctx = ctrllog.IntoContext(ctx, log) @@ -459,7 +472,7 @@ func (r *ImageRepositoryReconciler) RegenerateImageRepositoryAccessToken(ctx con } robotAccount, err := r.QuayClient.RegenerateRobotAccountToken(r.QuayOrganization, robotAccountName) if err != nil { - log.Error(err, "failed to refresh robot account token") + log.Error(err, "Failed to refresh robot account token") return err } else { log.Info("Refreshed quay robot account token") @@ -482,7 +495,7 @@ func (r *ImageRepositoryReconciler) CleanupImageRepository(ctx context.Context, robotAccountName := imageRepository.Status.Credentials.PushRobotAccountName isRobotAccountDeleted, err := r.QuayClient.DeleteRobotAccount(r.QuayOrganization, robotAccountName) if err != nil { - log.Error(err, "failed to delete push robot account", l.Action, l.ActionDelete, l.Audit, "true") + log.Error(err, "Failed to delete push robot account", l.Action, l.ActionDelete, l.Audit, "true") } if isRobotAccountDeleted { log.Info("Deleted push robot account", "RobotAccountName", robotAccountName, l.Action, l.ActionDelete) @@ -492,7 +505,7 @@ func (r *ImageRepositoryReconciler) CleanupImageRepository(ctx context.Context, pullRobotAccountName := imageRepository.Status.Credentials.PullRobotAccountName isPullRobotAccountDeleted, err := r.QuayClient.DeleteRobotAccount(r.QuayOrganization, pullRobotAccountName) if err != nil { - log.Error(err, "failed to delete pull robot account", l.Action, l.ActionDelete, l.Audit, "true") + log.Error(err, "Failed to delete pull robot account", l.Action, l.ActionDelete, l.Audit, "true") } if isPullRobotAccountDeleted { log.Info("Deleted pull robot account", "RobotAccountName", pullRobotAccountName, l.Action, l.ActionDelete) @@ -502,7 +515,7 @@ func (r *ImageRepositoryReconciler) CleanupImageRepository(ctx context.Context, imageRepositoryName := imageRepository.Spec.Image.Name isImageRepositoryDeleted, err := r.QuayClient.DeleteRepository(r.QuayOrganization, imageRepositoryName) if err != nil { - log.Error(err, "failed to delete image repository", l.Action, l.ActionDelete, l.Audit, "true") + log.Error(err, "Failed to delete image repository", l.Action, l.ActionDelete, l.Audit, "true") } if isImageRepositoryDeleted { log.Info("Deleted image repository", "ImageRepository", imageRepositoryName, l.Action, l.ActionDelete) @@ -523,25 +536,25 @@ func (r *ImageRepositoryReconciler) ChangeImageRepositoryVisibility(ctx context. imageRepository.Status.Image.Visibility = imageRepository.Spec.Image.Visibility imageRepository.Status.Message = "" if err := r.Client.Status().Update(ctx, imageRepository); err != nil { - log.Error(err, "failed to update image repository name", l.Action, l.ActionUpdate) + log.Error(err, "Failed to update imageRepository status", "ImageRepositoryName", imageRepository.Name, l.Action, l.ActionUpdate) return err } - log.Info("changed image repository visibility", "visibility", imageRepository.Spec.Image.Visibility) + log.Info("changed image repository visibility", "ImageRepositoryName", imageRepository.Name, "visibility", imageRepository.Spec.Image.Visibility) return nil } if err.Error() == "payment required" { - log.Info("failed to make image repository private due to quay plan limit", l.Audit, "true") + log.Info("Failed to make image repository private due to quay plan limit", l.Audit, "true") imageRepository.Spec.Image.Visibility = imageRepository.Status.Image.Visibility if err := r.Client.Update(ctx, imageRepository); err != nil { - log.Error(err, "failed to update image repository", l.Action, l.ActionUpdate) + log.Error(err, "Failed to update imageRepository", "ImageRepositoryName", imageRepository.Name, l.Action, l.ActionUpdate) return err } imageRepository.Status.Message = "Quay organization plan private repositories limit exceeded" if err := r.Client.Status().Update(ctx, imageRepository); err != nil { - log.Error(err, "failed to update image repository", l.Action, l.ActionUpdate) + log.Error(err, "Failed to update imageRepository status", "ImageRepositoryName", imageRepository.Name, l.Action, l.ActionUpdate) return err } @@ -549,7 +562,7 @@ func (r *ImageRepositoryReconciler) ChangeImageRepositoryVisibility(ctx context. return nil } - log.Error(err, "failed to change image repository visibility") + log.Error(err, "Failed to change image repository visibility") return err } @@ -560,7 +573,7 @@ func (r *ImageRepositoryReconciler) EnsureSecret(ctx context.Context, imageRepos secretKey := types.NamespacedName{Namespace: imageRepository.Namespace, Name: secretName} if err := r.Client.Get(ctx, secretKey, secret); err != nil { if !errors.IsNotFound(err) { - log.Error(err, "failed to get remote secret", l.Action, l.ActionView) + log.Error(err, "Failed to get secret", "SecretName", secretName, l.Action, l.ActionView) return err } @@ -577,32 +590,216 @@ func (r *ImageRepositoryReconciler) EnsureSecret(ctx context.Context, imageRepos } if err := controllerutil.SetOwnerReference(imageRepository, secret, r.Scheme); err != nil { - log.Error(err, "failed to set owner for image repository secret") + log.Error(err, "Failed to set owner for image repository secret") return err } if err := r.Client.Create(ctx, secret); err != nil { - log.Error(err, "failed to create image repository secret", l.Action, l.ActionAdd, l.Audit, "true") + log.Error(err, "Failed to create image repository secret", l.Action, l.ActionAdd, l.Audit, "true") return err } else { log.Info("Image repository secret created") } if !isPull { - serviceAccount := &corev1.ServiceAccount{} - serviceAccountKey := types.NamespacedName{Namespace: imageRepository.Namespace, Name: buildPipelineServiceAccountName} - if err := r.Client.Get(ctx, serviceAccountKey, serviceAccount); err != nil { - log.Error(err, "failed to get service account", l.Action, l.ActionView) + if err := r.linkSecretToServiceAccount(ctx, secretName, imageRepository.Namespace); err != nil { + log.Error(err, "Failed to link secret to service account", "SecretName", secretName, l.Action, l.ActionUpdate) return err } - serviceAccount.Secrets = append(serviceAccount.Secrets, corev1.ObjectReference{Name: secretName}) - serviceAccount.ImagePullSecrets = append(serviceAccount.ImagePullSecrets, corev1.LocalObjectReference{Name: secretName}) - if err := r.Client.Update(ctx, serviceAccount); err != nil { - log.Error(err, "failed to update service account", l.Action, l.ActionUpdate) - return err + } + } + return nil +} + +// linkSecretToServiceAccount ensures that the given secret is linked with the provided service account. +func (r *ImageRepositoryReconciler) linkSecretToServiceAccount(ctx context.Context, secretNameToAdd, namespace string) error { + log := ctrllog.FromContext(ctx) + + serviceAccount := &corev1.ServiceAccount{} + err := r.Client.Get(ctx, types.NamespacedName{Name: buildPipelineServiceAccountName, Namespace: namespace}, serviceAccount) + if err != nil { + if errors.IsNotFound(err) { + return nil + } + log.Error(err, "Failed to read pipeline service account", "ServiceAccountName", buildPipelineServiceAccountName, "NamespaceName", namespace, l.Action, l.ActionView) + return err + } + + // check if secret is already linked and add it only if it isn't to avoid duplication + secretLinked := false + for _, serviceAccountSecret := range serviceAccount.Secrets { + if serviceAccountSecret.Name == secretNameToAdd { + secretLinked = true + break + } + } + if !secretLinked { + // add it only to Secrets and not in imagePullSecrets which is used only for the image pod itself needs to run + serviceAccount.Secrets = append(serviceAccount.Secrets, corev1.ObjectReference{Name: secretNameToAdd}) + + if err := r.Client.Update(ctx, serviceAccount); err != nil { + log.Error(err, "Failed to update pipeline service account", "ServiceAccountName", buildPipelineServiceAccountName, "NamespaceName", namespace, l.Action, l.ActionUpdate) + return err + } + log.Info("Added secret link to pipeline service account", "SecretName", secretNameToAdd, "ServiceAccountName", buildPipelineServiceAccountName, l.Action, l.ActionUpdate) + } + return nil +} + +// unlinkSecretFromServiceAccount ensures that the given secret is not linked with the provided service account. +func (r *ImageRepositoryReconciler) unlinkSecretFromServiceAccount(ctx context.Context, secretNameToRemove, namespace string) error { + log := ctrllog.FromContext(ctx) + + serviceAccount := &corev1.ServiceAccount{} + err := r.Client.Get(ctx, types.NamespacedName{Name: buildPipelineServiceAccountName, Namespace: namespace}, serviceAccount) + if err != nil { + if errors.IsNotFound(err) { + return nil + } + log.Error(err, "Failed to read pipeline service account", "ServiceAccountName", buildPipelineServiceAccountName, "NamespaceName", namespace, l.Action, l.ActionView) + return err + } + + isSecretUnlinked := false + // Remove secret from secrets list + pushSecrets := []corev1.ObjectReference{} + for _, credentialSecret := range serviceAccount.Secrets { + // don't break and search for duplicities + if credentialSecret.Name == secretNameToRemove { + isSecretUnlinked = true + continue + } + pushSecrets = append(pushSecrets, credentialSecret) + } + serviceAccount.Secrets = pushSecrets + + // Remove secret from pull secrets list + // we aren't adding them anymore in imagePullSecrets but just to be sure check and remove them from there + imagePullSecrets := []corev1.LocalObjectReference{} + for _, pullSecret := range serviceAccount.ImagePullSecrets { + // don't break and search for duplicities + if pullSecret.Name == secretNameToRemove { + isSecretUnlinked = true + continue + } + imagePullSecrets = append(imagePullSecrets, pullSecret) + } + serviceAccount.ImagePullSecrets = imagePullSecrets + + if isSecretUnlinked { + if err := r.Client.Update(ctx, serviceAccount); err != nil { + log.Error(err, "Failed to update pipeline service account", "ServiceAccountName", buildPipelineServiceAccountName, "NamespaceName", namespace, l.Action, l.ActionUpdate) + return err + } + log.Info("Removed secret link from pipeline service account", "SecretName", secretNameToRemove, "ServiceAccountName", buildPipelineServiceAccountName, l.Action, l.ActionUpdate) + } + return nil +} + +// cleanUpSecretInServiceAccount ensures that the given secret is linked with the provided service account just once +// and remove the secret from ImagePullSecrets +func (r *ImageRepositoryReconciler) cleanUpSecretInServiceAccount(ctx context.Context, secretName, namespace string) error { + log := ctrllog.FromContext(ctx) + + serviceAccount := &corev1.ServiceAccount{} + err := r.Client.Get(ctx, types.NamespacedName{Name: buildPipelineServiceAccountName, Namespace: namespace}, serviceAccount) + if err != nil { + if errors.IsNotFound(err) { + return nil + } + log.Error(err, "Failed to read pipeline service account", "ServiceAccountName", buildPipelineServiceAccountName, "NamespaceName", namespace, l.Action, l.ActionView) + return err + } + + linksModified := false + + // Check for duplicates for the secret and remove them + pushSecrets := []corev1.ObjectReference{} + foundSecret := false + for _, credentialSecret := range serviceAccount.Secrets { + if credentialSecret.Name == secretName { + if !foundSecret { + pushSecrets = append(pushSecrets, credentialSecret) + foundSecret = true + } else { + linksModified = true } + } else { + pushSecrets = append(pushSecrets, credentialSecret) + } + } + serviceAccount.Secrets = pushSecrets + + // Remove secret from pull secrets list + imagePullSecrets := []corev1.LocalObjectReference{} + for _, pullSecret := range serviceAccount.ImagePullSecrets { + // don't break and search for duplicities + if pullSecret.Name == secretName { + linksModified = true + continue + } + imagePullSecrets = append(imagePullSecrets, pullSecret) + } + serviceAccount.ImagePullSecrets = imagePullSecrets + + if linksModified { + if err := r.Client.Update(ctx, serviceAccount); err != nil { + log.Error(err, "Failed to update pipeline service account", "ServiceAccountName", buildPipelineServiceAccountName, "NamespaceName", namespace, l.Action, l.ActionUpdate) + return err + } + log.Info("Cleaned up secret links in pipeline service account", "SecretName", secretName, "ServiceAccountName", buildPipelineServiceAccountName, l.Action, l.ActionUpdate) + } + return nil +} + +// VerifyAndFixSecretsLinking ensures that the given secret is linked to the provided service account, and also removes duplicated link for the secret. +// when secret doesn't exist unlink it from service account +func (r *ImageRepositoryReconciler) VerifyAndFixSecretsLinking(ctx context.Context, imageRepository *imagerepositoryv1alpha1.ImageRepository) error { + log := ctrllog.FromContext(ctx) + + secretName := imageRepository.Status.Credentials.PushSecretName + + // check secret existence and if secret doesn't exist unlink it from service account + // users can recreate it by using regenerate-token + secret := &corev1.Secret{} + secretKey := types.NamespacedName{Namespace: imageRepository.Namespace, Name: secretName} + secretExists := true + if err := r.Client.Get(ctx, secretKey, secret); err != nil { + if !errors.IsNotFound(err) { + log.Error(err, "Failed to get secret", "SecretName", secretName, l.Action, l.ActionView) + return err + } + + secretExists = false + log.Info("Secret doesn't exist, will unlink secret from service account", "SecretName", secretName) + log.Info("To recreate secret use regenerate-token") + + if err := r.unlinkSecretFromServiceAccount(ctx, secretName, imageRepository.Namespace); err != nil { + log.Error(err, "Failed to unlink secret from service account", "SecretName", secretName, l.Action, l.ActionUpdate) + return err } } + + if !secretExists { + // link secret to service account if isn't linked already + if err := r.linkSecretToServiceAccount(ctx, secretName, imageRepository.Namespace); err != nil { + log.Error(err, "Failed to link secret to service account", "SecretName", secretName, l.Action, l.ActionUpdate) + return err + } + + // clean duplicate secret links and remove secret from ImagePullSecrets + if err := r.cleanUpSecretInServiceAccount(ctx, secretName, imageRepository.Namespace); err != nil { + log.Error(err, "Failed to clean up secret in service account", "SecretName", secretName, l.Action, l.ActionUpdate) + return err + } + } + + imageRepository.Spec.Credentials.VerifyLinking = nil + if err := r.Client.Update(ctx, imageRepository); err != nil { + log.Error(err, "failed to update imageRepository", "ImageRepositoryName", imageRepository.Name, l.Action, l.ActionUpdate) + return err + } + return nil } @@ -656,7 +853,7 @@ func (r *ImageRepositoryReconciler) UpdateImageRepositoryStatusMessage(ctx conte log := ctrllog.FromContext(ctx) imageRepository.Status.Message = statusMessage if err := r.Client.Status().Update(ctx, imageRepository); err != nil { - log.Error(err, "failed to update image repository status") + log.Error(err, "Failed to update imageRepository status", "ImageRepositoryName", imageRepository.Name, l.Action, l.ActionUpdate) return err }