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 }