diff --git a/api/v1beta1/groupversion_info.go b/api/v1beta1/groupversion_info.go index 2915f32..dd2d78c 100644 --- a/api/v1beta1/groupversion_info.go +++ b/api/v1beta1/groupversion_info.go @@ -16,7 +16,7 @@ limitations under the License. // Package v1beta1 contains API Schema definitions for the appstudio.redhat.com v1beta1 API group // +kubebuilder:object:generate=true -// +groupName=appstudio.redhat.com.appstudio.redhat.com +// +groupName=appstudio.redhat.com package v1beta1 import ( @@ -26,7 +26,7 @@ import ( var ( // GroupVersion is group version used to register these objects - GroupVersion = schema.GroupVersion{Group: "appstudio.redhat.com.appstudio.redhat.com", Version: "v1beta1"} + GroupVersion = schema.GroupVersion{Group: "appstudio.redhat.com", Version: "v1beta1"} // SchemeBuilder is used to add go types to the GroupVersionKind scheme SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} diff --git a/api/v1beta1/imagerepository_types.go b/api/v1beta1/imagerepository_types.go index 0a6ba4f..d1f466a 100644 --- a/api/v1beta1/imagerepository_types.go +++ b/api/v1beta1/imagerepository_types.go @@ -37,7 +37,7 @@ type ImageParameters struct { // If ommited, then defaults to "cr-namespace/cr-name". // This field cannot be changed after the resource creation. // +optional - // +kubebuilder:validation:Pattern="[a-z0-9][.a-z0-9_-]*(/[a-z0-9][.a-z0-9_-]*)*" + // +kubebuilder:validation:Pattern="^[a-z0-9][.a-z0-9_-]*(/[a-z0-9][.a-z0-9_-]*)*$" Name string `json:"name,omitempty"` // Visibility defines whether the image is publicly visible. @@ -94,6 +94,7 @@ type ImageStatus struct { URL string `json:"url,omitempty"` // Visibility shows actual generated image repository visibility. + // +kubebuilder:validation:Enum=public;private Visibility ImageVisibility `json:"visibility,omitempty"` } diff --git a/config/crd/bases/appstudio.redhat.com.appstudio.redhat.com_imagerepositories.yaml b/config/crd/bases/appstudio.redhat.com_imagerepositories.yaml similarity index 94% rename from config/crd/bases/appstudio.redhat.com.appstudio.redhat.com_imagerepositories.yaml rename to config/crd/bases/appstudio.redhat.com_imagerepositories.yaml index 637967c..1093dbb 100644 --- a/config/crd/bases/appstudio.redhat.com.appstudio.redhat.com_imagerepositories.yaml +++ b/config/crd/bases/appstudio.redhat.com_imagerepositories.yaml @@ -5,9 +5,9 @@ metadata: annotations: controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null - name: imagerepositories.appstudio.redhat.com.appstudio.redhat.com + name: imagerepositories.appstudio.redhat.com spec: - group: appstudio.redhat.com.appstudio.redhat.com + group: appstudio.redhat.com names: kind: ImageRepository listKind: ImageRepositoryList @@ -58,7 +58,7 @@ spec: description: Name of the image within configured Quay organization. If ommited, then defaults to "cr-namespace/cr-name". This field cannot be changed after the resource creation. - pattern: '[a-z0-9][.a-z0-9_-]*(/[a-z0-9][.a-z0-9_-]*)*' + pattern: ^[a-z0-9][.a-z0-9_-]*(/[a-z0-9][.a-z0-9_-]*)*$ type: string visibility: description: Visibility defines whether the image is publicly @@ -113,11 +113,15 @@ spec: / pull from. type: string visibility: + allOf: + - enum: + - public + - private + - enum: + - public + - private description: Visibility shows actual generated image repository visibility. - enum: - - public - - private type: string type: object message: diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 5bb484b..8d0fbbb 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -3,7 +3,7 @@ # It should be run by config/default #resources: #- bases/appstudio.redhat.com_components.yaml -- bases/appstudio.redhat.com.appstudio.redhat.com_imagerepositories.yaml +- bases/appstudio.redhat.com_imagerepositories.yaml #+kubebuilder:scaffold:crdkustomizeresource #patchesStrategicMerge: diff --git a/config/crd/patches/cainjection_in_imagerepositories.yaml b/config/crd/patches/cainjection_in_imagerepositories.yaml index a2812dc..bbdae20 100644 --- a/config/crd/patches/cainjection_in_imagerepositories.yaml +++ b/config/crd/patches/cainjection_in_imagerepositories.yaml @@ -4,4 +4,4 @@ kind: CustomResourceDefinition metadata: annotations: cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) - name: imagerepositories.appstudio.redhat.com.appstudio.redhat.com + name: imagerepositories.appstudio.redhat.com diff --git a/config/crd/patches/webhook_in_imagerepositories.yaml b/config/crd/patches/webhook_in_imagerepositories.yaml index 3fee3a4..1b05777 100644 --- a/config/crd/patches/webhook_in_imagerepositories.yaml +++ b/config/crd/patches/webhook_in_imagerepositories.yaml @@ -2,7 +2,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: - name: imagerepositories.appstudio.redhat.com.appstudio.redhat.com + name: imagerepositories.appstudio.redhat.com spec: conversion: strategy: Webhook diff --git a/config/rbac/imagerepository_editor_role.yaml b/config/rbac/imagerepository_editor_role.yaml index 05133c4..f575e0b 100644 --- a/config/rbac/imagerepository_editor_role.yaml +++ b/config/rbac/imagerepository_editor_role.yaml @@ -5,7 +5,7 @@ metadata: name: imagerepository-editor-role rules: - apiGroups: - - appstudio.redhat.com.appstudio.redhat.com + - appstudio.redhat.com resources: - imagerepositories verbs: @@ -17,7 +17,7 @@ rules: - update - watch - apiGroups: - - appstudio.redhat.com.appstudio.redhat.com + - appstudio.redhat.com resources: - imagerepositories/status verbs: diff --git a/config/rbac/imagerepository_viewer_role.yaml b/config/rbac/imagerepository_viewer_role.yaml index 3f3d4e8..38227c1 100644 --- a/config/rbac/imagerepository_viewer_role.yaml +++ b/config/rbac/imagerepository_viewer_role.yaml @@ -5,7 +5,7 @@ metadata: name: imagerepository-viewer-role rules: - apiGroups: - - appstudio.redhat.com.appstudio.redhat.com + - appstudio.redhat.com resources: - imagerepositories verbs: @@ -13,7 +13,7 @@ rules: - list - watch - apiGroups: - - appstudio.redhat.com.appstudio.redhat.com + - appstudio.redhat.com resources: - imagerepositories/status verbs: diff --git a/config/samples/appstudio.redhat.com_v1beta1_imagerepository.yaml b/config/samples/appstudio.redhat.com_v1beta1_imagerepository.yaml index d83b021..03d291f 100644 --- a/config/samples/appstudio.redhat.com_v1beta1_imagerepository.yaml +++ b/config/samples/appstudio.redhat.com_v1beta1_imagerepository.yaml @@ -1,4 +1,4 @@ -apiVersion: appstudio.redhat.com.appstudio.redhat.com/v1beta1 +apiVersion: appstudio.redhat.com/v1beta1 kind: ImageRepository metadata: name: imagerepository-sample diff --git a/controllers/component_image_controller_test.go b/controllers/component_image_controller_test.go index e1b8749..337981a 100644 --- a/controllers/component_image_controller_test.go +++ b/controllers/component_image_controller_test.go @@ -54,7 +54,6 @@ var _ = Describe("Component image controller", func() { Context("Image repository provision flow", func() { It("should prepare environment", func() { - deleteNamespace(defaultNamespace) createNamespace(defaultNamespace) ResetTestQuayClient() diff --git a/controllers/imagerepository_controller.go b/controllers/imagerepository_controller.go index bdeae02..2df9a39 100644 --- a/controllers/imagerepository_controller.go +++ b/controllers/imagerepository_controller.go @@ -115,48 +115,35 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ log.Error(err, "provision of image repository failed") return ctrl.Result{}, err } - if imageRepository.Status.State == imagerepositoryv1beta1.ImageRepositoryStateFailed { - log.Error(err, "provision of image repository failed permanently") - return ctrl.Result{}, nil - } - - // Add finalizer - if err := r.Client.Get(ctx, req.NamespacedName, imageRepository); err != nil { - log.Error(err, "failed to get image repository", l.Action, l.ActionView) - return ctrl.Result{}, err - } - controllerutil.AddFinalizer(imageRepository, ImageRepositoryFinalizer) - if err := r.Client.Update(ctx, imageRepository); err != nil { - log.Error(err, "failed to add image repository finalizer", l.Action, l.ActionUpdate) - return ctrl.Result{}, err - } else { - log.Info("added image repository finalizer") - } + return ctrl.Result{}, nil + } + if imageRepository.Status.State != imagerepositoryv1beta1.ImageRepositoryStateReady { return ctrl.Result{}, nil } // Make sure, that image repository name is the same as on creation. - // Do it here to aviod webhook creation. - if !strings.HasSuffix(imageRepository.Status.Image.URL, imageRepository.Spec.Image.Name) { - imageRepositoryName := strings.TrimPrefix(imageRepository.Status.Image.URL, fmt.Sprintf("quay.io/%s/", r.QuayOrganization)) + // Do it here to avoid webhook creation. + imageRepositoryName := strings.TrimPrefix(imageRepository.Status.Image.URL, fmt.Sprintf("quay.io/%s/", r.QuayOrganization)) + if imageRepository.Spec.Image.Name != imageRepositoryName { + 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", 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 } return ctrl.Result{}, nil } // Change image visibility if requested - if imageRepository.Status.Image.Visibility != imageRepository.Spec.Image.Visibility { + if imageRepository.Spec.Image.Visibility != imageRepository.Status.Image.Visibility && imageRepository.Spec.Image.Visibility != "" { if err := r.ChangeImageRepositoryVisibility(ctx, imageRepository); err != nil { return ctrl.Result{}, err } return ctrl.Result{}, nil } - // Rotate credentials + // Rotate credentials if requested regenerateToken := imageRepository.Spec.Credentials.RegenerateToken if regenerateToken != nil && *regenerateToken { if err := r.RegenerateImageRepositoryCredentials(ctx, imageRepository); err != nil { @@ -182,10 +169,10 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context imageRepositoryName = imageRepository.Namespace + "-" + imageRepository.Spec.Image.Name } - visibility := "public" - if imageRepository.Spec.Image.Visibility != "" { - visibility = string(imageRepository.Spec.Image.Visibility) + if imageRepository.Spec.Image.Visibility == "" { + imageRepository.Spec.Image.Visibility = imagerepositoryv1beta1.ImageVisibilityPublic } + visibility := string(imageRepository.Spec.Image.Visibility) repository, err := r.QuayClient.CreateRepository(quay.RepositoryRequest{ Namespace: r.QuayOrganization, @@ -201,9 +188,16 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context } else { imageRepository.Status.Message = err.Error() } - _ = r.Client.Status().Update(ctx, imageRepository) + if err := r.Client.Status().Update(ctx, imageRepository); err != nil { + log.Error(err, "failed to update image repository status") + } return nil } + if repository == nil { + err := fmt.Errorf("unexpected response from Quay: created image repository data object is nil") + log.Error(err, "nil repository") + return err + } robotAccountName := generateQuayRobotAccountName(imageRepositoryName, false) robotAccount, err := r.QuayClient.CreateRobotAccount(r.QuayOrganization, robotAccountName) @@ -211,6 +205,11 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context log.Error(err, "failed to create robot account", "RobotAccountName", robotAccountName, l.Action, l.ActionAdd, l.Audit, "true") return err } + if robotAccount == nil { + err := fmt.Errorf("unexpected response from Quay: robot account data object is nil") + log.Error(err, "nil robot account") + return err + } err = r.QuayClient.AddPermissionsForRepositoryToRobotAccount(r.QuayOrganization, repository.Name, robotAccount.Name, true) if err != nil { @@ -224,6 +223,7 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context return err } + status := imagerepositoryv1beta1.ImageRepositoryStatus{} if isComponentLinked(imageRepository) { // Pull secret provision and propagation pullRobotAccountName := generateQuayRobotAccountName(imageRepositoryName, true) @@ -232,6 +232,11 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context log.Error(err, "failed to create pull robot account", "RobotAccountName", pullRobotAccountName, l.Action, l.ActionAdd, l.Audit, "true") return err } + if robotAccount == nil { + err := fmt.Errorf("unexpected response from Quay: pull robot account data object is nil") + log.Error(err, "nil pull robot account") + return err + } err = r.QuayClient.AddPermissionsForRepositoryToRobotAccount(r.QuayOrganization, repository.Name, pullRobotAccount.Name, false) if err != nil { @@ -248,17 +253,29 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context return err } - imageRepository.Status.Credentials.PullRobotAccountName = pullRobotAccountName - imageRepository.Status.Credentials.PullSecretName = remoteSecretName + status.Credentials.PullRobotAccountName = pullRobotAccountName + status.Credentials.PullSecretName = remoteSecretName } - imageRepository.Status.State = imagerepositoryv1beta1.ImageRepositoryStateReady - imageRepository.Status.Image.URL = quayImageURL - imageRepository.Status.Image.Visibility = imageRepository.Spec.Image.Visibility - imageRepository.Status.Credentials.PushRobotAccountName = robotAccountName - imageRepository.Status.Credentials.PushSecretName = secretName - imageRepository.Status.Credentials.GenerationTimestamp = &metav1.Time{Time: time.Now()} + status.State = imagerepositoryv1beta1.ImageRepositoryStateReady + status.Image.URL = quayImageURL + status.Image.Visibility = imageRepository.Spec.Image.Visibility + status.Credentials.PushRobotAccountName = robotAccountName + status.Credentials.PushSecretName = secretName + status.Credentials.GenerationTimestamp = &metav1.Time{Time: time.Now()} + + imageRepository.Spec.Image.Name = imageRepositoryName + controllerutil.AddFinalizer(imageRepository, ImageRepositoryFinalizer) + if err := r.Client.Update(ctx, imageRepository); err != nil { + log.Error(err, "failed to update CR after provision") + return err + } else { + log.Info("provisioned image repository and added finalizer") + } + + imageRepository.Status = status if err := r.Client.Status().Update(ctx, imageRepository); err != nil { + log.Error(err, "failed to update CR status after provision") return err } @@ -281,10 +298,10 @@ func (r *ImageRepositoryReconciler) RegenerateImageRepositoryCredentials(ctx con if err := r.EnsureDockerSecret(ctx, imageRepository, robotAccount, secretName, quayImageURL); err != nil { return err } - log.WithValues("RobotAccountName", robotAccountName).Info("Regenerated push token") + log.Info("Regenerated push token", "RobotAccountName", robotAccountName) if isComponentLinked(imageRepository) { - pullRobotAccountName := imageRepository.Status.Credentials.PushRobotAccountName + pullRobotAccountName := imageRepository.Status.Credentials.PullRobotAccountName pullRobotAccount, err := r.QuayClient.RegenerateRobotAccountToken(r.QuayOrganization, pullRobotAccountName) if err != nil { log.Error(err, "failed to refresh pull token") @@ -295,24 +312,24 @@ func (r *ImageRepositoryReconciler) RegenerateImageRepositoryCredentials(ctx con if err := r.CreateRemotePullSecretUploadSecret(ctx, pullRobotAccount, imageRepository.Namespace, remoteSecretName, quayImageURL); err != nil { return err } - log.WithValues("RobotAccountName", pullRobotAccountName).Info("Regenerated pull token") + log.Info("Regenerated pull token", "RobotAccountName", pullRobotAccountName) } - imageRepositoryKey := types.NamespacedName{Namespace: imageRepository.Namespace, Name: imageRepository.Name} - if err := r.Client.Get(ctx, imageRepositoryKey, imageRepository); err != nil { - log.Error(err, "failed to get image repository") - return err - } + // if err := r.Client.Get(ctx, imageRepositoryKey, imageRepository); err != nil { + // log.Error(err, "failed to get image repository") + // return err + // } 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) return err } - if err := r.Client.Get(ctx, imageRepositoryKey, imageRepository); err != nil { - log.Error(err, "failed to get image repository") - return err - } + // imageRepositoryKey := types.NamespacedName{Namespace: imageRepository.Namespace, Name: imageRepository.Name} + // if err := r.Client.Get(ctx, imageRepositoryKey, imageRepository); err != nil { + // log.Error(err, "failed to get image repository") + // 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) @@ -332,7 +349,7 @@ func (r *ImageRepositoryReconciler) CleanupImageRepository(ctx context.Context, log.Error(err, "failed to delete push robot account", l.Action, l.ActionDelete, l.Audit, "true") } if isRobotAccountDeleted { - log.WithValues("RobotAccountName", robotAccountName).Info("Deleted push robot account", l.Action, l.ActionDelete) + log.Info("Deleted push robot account", "RobotAccountName", robotAccountName, l.Action, l.ActionDelete) } if isComponentLinked(imageRepository) { @@ -342,7 +359,7 @@ func (r *ImageRepositoryReconciler) CleanupImageRepository(ctx context.Context, log.Error(err, "failed to delete pull robot account", l.Action, l.ActionDelete, l.Audit, "true") } if isPullRobotAccountDeleted { - log.WithValues("RobotAccountName", pullRobotAccountName).Info("Deleted pull robot account", l.Action, l.ActionDelete) + log.Info("Deleted pull robot account", "RobotAccountName", pullRobotAccountName, l.Action, l.ActionDelete) } } @@ -352,7 +369,7 @@ func (r *ImageRepositoryReconciler) CleanupImageRepository(ctx context.Context, log.Error(err, "failed to delete image repository", l.Action, l.ActionDelete, l.Audit, "true") } if isImageRepositoryDeleted { - log.WithValues("ImageRepository", imageRepositoryName).Info("Deleted image repository", l.Action, l.ActionDelete) + log.Info("Deleted image repository", "ImageRepository", imageRepositoryName, l.Action, l.ActionDelete) } } @@ -385,16 +402,19 @@ func (r *ImageRepositoryReconciler) ChangeImageRepositoryVisibility(ctx context. return err } - imageRepositoryKey := types.NamespacedName{Namespace: imageRepository.Namespace, Name: imageRepository.Name} - if err := r.Client.Get(ctx, imageRepositoryKey, imageRepository); err != nil { - log.Error(err, "failed to get image repository", l.Action, l.ActionView) - return err - } + // imageRepositoryKey := types.NamespacedName{Namespace: imageRepository.Namespace, Name: imageRepository.Name} + // if err := r.Client.Get(ctx, imageRepositoryKey, imageRepository); err != nil { + // log.Error(err, "failed to get image repository", l.Action, l.ActionView) + // 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) return err } + + // Do not trigger a new reconcile since the error handled + return nil } log.Error(err, "failed to change image repository visibility") diff --git a/controllers/imagerepository_controller_test.go b/controllers/imagerepository_controller_test.go index da9c8ca..4120b10 100644 --- a/controllers/imagerepository_controller_test.go +++ b/controllers/imagerepository_controller_test.go @@ -21,16 +21,17 @@ import ( "fmt" "regexp" "strings" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/redhat-appstudio/image-controller/pkg/quay" + remotesecretv1beta1 "github.com/redhat-appstudio/remote-secret/api/v1beta1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" imagerepositoryv1beta1 "github.com/redhat-appstudio/image-controller/api/v1beta1" - remotesecretv1beta1 "github.com/redhat-appstudio/remote-secret/api/v1beta1" ) var _ = Describe("Image repository controller", func() { @@ -101,8 +102,7 @@ var _ = Describe("Image repository controller", func() { imageRepository := getImageRepository(resourceKey) - // TODO fix it - // Expect(imageRepository.Spec.Image.Name).To(Equal(expectedImageName)) + Expect(imageRepository.Spec.Image.Name).To(Equal(expectedImageName)) Expect(imageRepository.Spec.Image.Visibility).To(Equal(imagerepositoryv1beta1.ImageVisibilityPublic)) Expect(imageRepository.Status.State).To(Equal(imagerepositoryv1beta1.ImageRepositoryStateReady)) Expect(imageRepository.Status.Message).To(BeEmpty()) @@ -132,6 +132,8 @@ var _ = Describe("Image repository controller", func() { newToken := "push-token5678" ResetTestQuayClientToFails() + // Wait just for case it takes less than a second to regenerate credentials + time.Sleep(time.Second) isRegenerateRobotAccountTokenInvoked := false RegenerateRobotAccountTokenFunc = func(organization, robotName string) (*quay.RobotAccount, error) { @@ -166,7 +168,8 @@ var _ = Describe("Image repository controller", func() { Expect(dockerconfigJson).To(ContainSubstring(expectedImage)) authString, err := base64.StdEncoding.DecodeString(authRegexp.FindStringSubmatch(dockerconfigJson)[1]) Expect(err).To(Succeed()) - Expect(string(authString)).To(Equal(fmt.Sprintf("%s:%s", expectedRobotAccountPrefix, newToken))) + Expect(string(authString)).To(HavePrefix(expectedRobotAccountPrefix)) + Expect(string(authString)).To(HaveSuffix(newToken)) }) It("should update image visibility", func() { @@ -178,7 +181,7 @@ var _ = Describe("Image repository controller", func() { isChangeRepositoryVisibilityInvoked = true Expect(organization).To(Equal(testQuayOrg)) Expect(imageRepository).To(Equal(expectedImageName)) - Expect(visibility).To(Equal(imagerepositoryv1beta1.ImageVisibilityPrivate)) + Expect(visibility).To(Equal(string(imagerepositoryv1beta1.ImageVisibilityPrivate))) return nil } @@ -238,9 +241,6 @@ var _ = Describe("Image repository controller", func() { Context("Image repository for component provision", func() { It("should prepare environment", func() { - deleteNamespace(defaultNamespace) - createNamespace(defaultNamespace) - pushToken = "push-token1234" pullToken = "pull-token1234" expectedImageName = fmt.Sprintf("%s-%s/%s", defaultNamespace, defaultComponentApplication, defaultComponentName) @@ -256,18 +256,18 @@ var _ = Describe("Image repository controller", func() { CreateRepositoryFunc = func(repository quay.RepositoryRequest) (*quay.Repository, error) { defer GinkgoRecover() isCreateRepositoryInvoked = true - // Expect(repository.Repository).To(Equal(expectedImageName)) - // Expect(repository.Namespace).To(Equal(testQuayOrg)) - // Expect(repository.Visibility).To(Equal("public")) - // Expect(repository.Description).ToNot(BeEmpty()) + Expect(repository.Repository).To(Equal(expectedImageName)) + Expect(repository.Namespace).To(Equal(testQuayOrg)) + Expect(repository.Visibility).To(Equal("public")) + Expect(repository.Description).ToNot(BeEmpty()) return &quay.Repository{Name: expectedImageName}, nil } isCreatePushRobotAccountInvoked := false isCreatePullRobotAccountInvoked := false CreateRobotAccountFunc = func(organization, robotName string) (*quay.RobotAccount, error) { defer GinkgoRecover() - // Expect(organization).To(Equal(testQuayOrg)) - // Expect(strings.HasPrefix(robotName, expectedRobotAccountPrefix)).To(BeTrue()) + Expect(organization).To(Equal(testQuayOrg)) + Expect(strings.HasPrefix(robotName, expectedRobotAccountPrefix)).To(BeTrue()) if strings.HasSuffix(robotName, "_pull") { isCreatePullRobotAccountInvoked = true return &quay.RobotAccount{Name: robotName, Token: pullToken}, nil @@ -309,15 +309,14 @@ var _ = Describe("Image repository controller", func() { waitImageRepositoryFinalizerOnImageRepository(resourceKey) imageRepository := getImageRepository(resourceKey) - // TODO fix - // Expect(imageRepository.Spec.Image.Name).To(Equal(expectedImageName)) + Expect(imageRepository.Spec.Image.Name).To(Equal(expectedImageName)) Expect(imageRepository.Spec.Image.Visibility).To(Equal(imagerepositoryv1beta1.ImageVisibilityPublic)) Expect(imageRepository.Status.State).To(Equal(imagerepositoryv1beta1.ImageRepositoryStateReady)) Expect(imageRepository.Status.Message).To(BeEmpty()) Expect(imageRepository.Status.Image.URL).To(Equal(expectedImage)) Expect(imageRepository.Status.Image.Visibility).To(Equal(imagerepositoryv1beta1.ImageVisibilityPublic)) Expect(imageRepository.Status.Credentials.PushRobotAccountName).To(HavePrefix(expectedRobotAccountPrefix)) - Expect(imageRepository.Status.Credentials.PushSecretName).To(HavePrefix(expectedImageName)) + Expect(imageRepository.Status.Credentials.PushSecretName).To(HavePrefix(strings.ReplaceAll(expectedImageName, "/", "-"))) Expect(imageRepository.Status.Credentials.PullRobotAccountName).To(HavePrefix(expectedRobotAccountPrefix)) Expect(imageRepository.Status.Credentials.PullRobotAccountName).To(HaveSuffix("_pull")) Expect(imageRepository.Status.Credentials.PullSecretName).To(Equal(expectedRemoteSecretName)) @@ -369,6 +368,8 @@ var _ = Describe("Image repository controller", func() { newPullToken := "pull-token5678" ResetTestQuayClientToFails() + // Wait just for case it takes less than a second to regenerate credentials + time.Sleep(time.Second) isRegenerateRobotAccountTokenForPushInvoked := false isRegenerateRobotAccountTokenForPullInvoked := false @@ -409,7 +410,8 @@ var _ = Describe("Image repository controller", func() { Expect(dockerconfigJson).To(ContainSubstring(expectedImage)) authString, err := base64.StdEncoding.DecodeString(authRegexp.FindStringSubmatch(dockerconfigJson)[1]) Expect(err).To(Succeed()) - Expect(string(authString)).To(Equal(fmt.Sprintf("%s:%s", expectedRobotAccountPrefix, newPushToken))) + pushRobotAccountName := imageRepository.Status.Credentials.PushRobotAccountName + Expect(string(authString)).To(Equal(fmt.Sprintf("%s:%s", pushRobotAccountName, newPushToken))) uploadSecretKey := types.NamespacedName{Name: "upload-secret-" + expectedRemoteSecretName, Namespace: defaultNamespace} uploadSecret := waitSecretExist(uploadSecretKey) @@ -462,8 +464,6 @@ var _ = Describe("Image repository controller", func() { Context("Image repository error scenarios", func() { It("should prepare environment", func() { - createNamespace(defaultNamespace) - pushToken = "push-token1234" expectedImageName = fmt.Sprintf("%s-%s", defaultNamespace, defaultImageRepositoryName) expectedImage = fmt.Sprintf("quay.io/%s/%s", testQuayOrg, expectedImageName) @@ -484,11 +484,15 @@ var _ = Describe("Image repository controller", func() { return nil, fmt.Errorf("payment required") } - createImageRepository(imageRepositoryConfig{}) + createImageRepository(imageRepositoryConfig{IsPrivate: true}) Eventually(func() bool { return isCreateRepositoryInvoked }, timeout, interval).Should(BeTrue()) - imageRepository := getImageRepository(resourceKey) + imageRepository := &imagerepositoryv1beta1.ImageRepository{} + Eventually(func() bool { + imageRepository = getImageRepository(resourceKey) + return string(imageRepository.Status.State) != "" + }, timeout, interval).Should(BeTrue()) Expect(imageRepository.Status.State).To(Equal(imagerepositoryv1beta1.ImageRepositoryStateFailed)) Expect(imageRepository.Status.Message).ToNot(BeEmpty()) Expect(imageRepository.Status.Message).To(ContainSubstring("exceeds current quay plan limit")) @@ -497,6 +501,7 @@ var _ = Describe("Image repository controller", func() { }) It("should add error message and revert visibility in spec if private visibility requested after provision but quota exceeded", func() { + deleteImageRepository(resourceKey) ResetTestQuayClient() CreateRepositoryFunc = func(repository quay.RepositoryRequest) (*quay.Repository, error) { @@ -516,7 +521,7 @@ var _ = Describe("Image repository controller", func() { isChangeRepositoryVisibilityInvoked = true Expect(organization).To(Equal(testQuayOrg)) Expect(imageRepository).To(Equal(expectedImageName)) - Expect(visibility).To(Equal(imagerepositoryv1beta1.ImageVisibilityPrivate)) + Expect(visibility).To(Equal(string(imagerepositoryv1beta1.ImageVisibilityPrivate))) return fmt.Errorf("payment required") } @@ -532,11 +537,13 @@ var _ = Describe("Image repository controller", func() { imageRepository.Status.Message != "" }, timeout, interval).Should(BeTrue()) + ResetTestQuayClient() deleteImageRepository(resourceKey) }) It("should fail if invalid image repository name given", func() { deleteImageRepository(resourceKey) + ResetTestQuayClient() imageRepository := getImageRepositoryConfig(imageRepositoryConfig{ ImageName: "wrong&name", diff --git a/controllers/suite_util_quay_client_test.go b/controllers/suite_util_quay_client_test.go index 2197eb3..3853bfe 100644 --- a/controllers/suite_util_quay_client_test.go +++ b/controllers/suite_util_quay_client_test.go @@ -57,34 +57,42 @@ func ResetTestQuayClient() { func ResetTestQuayClientToFails() { CreateRepositoryFunc = func(repository quay.RepositoryRequest) (*quay.Repository, error) { + defer GinkgoRecover() Fail("CreateRepositoryFunc invoked") return nil, nil } DeleteRepositoryFunc = func(organization, imageRepository string) (bool, error) { + defer GinkgoRecover() Fail("DeleteRepository invoked") return true, nil } ChangeRepositoryVisibilityFunc = func(organization, imageRepository string, visibility string) error { + defer GinkgoRecover() Fail("ChangeRepositoryVisibility invoked") return nil } GetRobotAccountFunc = func(organization, robotName string) (*quay.RobotAccount, error) { + defer GinkgoRecover() Fail("GetRobotAccount invoked") return nil, nil } CreateRobotAccountFunc = func(organization, robotName string) (*quay.RobotAccount, error) { + defer GinkgoRecover() Fail("CreateRobotAccount invoked") return nil, nil } DeleteRobotAccountFunc = func(organization, robotName string) (bool, error) { + defer GinkgoRecover() Fail("DeleteRobotAccount invoked") return true, nil } AddPermissionsForRepositoryToRobotAccountFunc = func(organization, imageRepository, robotAccountName string, isWrite bool) error { + defer GinkgoRecover() Fail("AddPermissionsForRepositoryToRobotAccount invoked") return nil } RegenerateRobotAccountTokenFunc = func(organization, robotName string) (*quay.RobotAccount, error) { + defer GinkgoRecover() Fail("RegenerateRobotAccountToken invoked") return nil, nil } diff --git a/controllers/suite_util_test.go b/controllers/suite_util_test.go index 87e5499..6b284dc 100644 --- a/controllers/suite_util_test.go +++ b/controllers/suite_util_test.go @@ -24,7 +24,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" k8sErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -107,14 +106,14 @@ func getImageRepository(imageRepositoryKey types.NamespacedName) *imagerepositor func deleteImageRepository(imageRepositoryKey types.NamespacedName) { imageRepository := &imagerepositoryv1beta1.ImageRepository{} if err := k8sClient.Get(ctx, imageRepositoryKey, imageRepository); err != nil { - if errors.IsNotFound(err) { + if k8sErrors.IsNotFound(err) { return } Fail("Failed to get image repository") } Expect(k8sClient.Delete(ctx, imageRepository)).To(Succeed()) Eventually(func() bool { - return errors.IsNotFound(k8sClient.Get(ctx, imageRepositoryKey, imageRepository)) + return k8sErrors.IsNotFound(k8sClient.Get(ctx, imageRepositoryKey, imageRepository)) }, timeout, interval).Should(BeTrue()) } @@ -301,22 +300,6 @@ func createNamespace(name string) { } } -func deleteNamespace(name string) { - namespace := corev1.Namespace{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "Namespace", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - } - - if err := k8sClient.Delete(ctx, &namespace); err != nil && !k8sErrors.IsNotFound(err) { - Fail(err.Error()) - } -} - func waitSecretExist(secretKey types.NamespacedName) *corev1.Secret { secret := &corev1.Secret{} Eventually(func() bool { diff --git a/main.go b/main.go index d90c256..eb042ff 100644 --- a/main.go +++ b/main.go @@ -29,11 +29,13 @@ import ( uberzap "go.uber.org/zap" uberzapcore "go.uber.org/zap/zapcore" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -93,6 +95,7 @@ func main() { MetricsBindAddress: metricsAddr, Port: 9443, HealthProbeBindAddress: probeAddr, + ClientDisableCacheFor: getCacheExcludedObjectsTypes(), LeaderElection: enableLeaderElection, LeaderElectionID: "ed4c18c3.appstudio.redhat.com", // LeaderElectionReleaseOnCancel defines if the leader should step down voluntarily @@ -163,3 +166,11 @@ func main() { os.Exit(1) } } + +func getCacheExcludedObjectsTypes() []client.Object { + return []client.Object{ + &imagerepositoryv1beta1.ImageRepository{}, + &corev1.Secret{}, + &corev1.ConfigMap{}, + } +}