diff --git a/api/v1alpha1/imagerepository_types.go b/api/v1alpha1/imagerepository_types.go index 55d34721..e90bb253 100644 --- a/api/v1alpha1/imagerepository_types.go +++ b/api/v1alpha1/imagerepository_types.go @@ -64,6 +64,9 @@ type ImageCredentials struct { // Refreshes both, push and pull tokens. // The field gets cleared after the refresh. RegenerateToken *bool `json:"regenerate-token,omitempty"` + // RegenerateNamespacePullToken defines a request to refresh namespace pull robot credentials. + // The field gets cleared after the refresh. + RegenerateNamespacePullToken *bool `json:"regenerate-namespace-pull-token,omitempty"` // VerifyLinking defines a request to verify and fix // secret linking in pipeline service account. // The field gets cleared after fixing. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 56b9150e..b089757d 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -51,6 +51,11 @@ func (in *ImageCredentials) DeepCopyInto(out *ImageCredentials) { *out = new(bool) **out = **in } + if in.RegenerateNamespacePullToken != nil { + in, out := &in.RegenerateNamespacePullToken, &out.RegenerateNamespacePullToken + *out = new(bool) + **out = **in + } if in.VerifyLinking != nil { in, out := &in.VerifyLinking, &out.VerifyLinking *out = new(bool) diff --git a/cmd/coverage_init.go b/cmd/coverage_init.go index f6c996c7..946f1975 100644 --- a/cmd/coverage_init.go +++ b/cmd/coverage_init.go @@ -7,4 +7,3 @@ package main // from the running binary during E2E tests. import _ "github.com/konflux-ci/coverport/instrumentation/go" // starts coverage server via init() - diff --git a/cmd/main.go b/cmd/main.go index f8ba719b..e1ff35f4 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -47,11 +47,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" "github.com/go-logr/logr" + compapiv1alpha1 "github.com/konflux-ci/application-api/api/v1alpha1" imagerepositoryv1alpha1 "github.com/konflux-ci/image-controller/api/v1alpha1" controllers "github.com/konflux-ci/image-controller/internal/controller" controllermetrics "github.com/konflux-ci/image-controller/pkg/metrics" "github.com/konflux-ci/image-controller/pkg/quay" - appstudioredhatcomv1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1" // +kubebuilder:scaffold:imports ) @@ -69,7 +69,7 @@ var ( func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) - utilruntime.Must(appstudioredhatcomv1alpha1.AddToScheme(scheme)) + utilruntime.Must(compapiv1alpha1.AddToScheme(scheme)) utilruntime.Must(imagerepositoryv1alpha1.AddToScheme(scheme)) // +kubebuilder:scaffold:scheme } diff --git a/config/crd/bases/appstudio.redhat.com_imagerepositories.yaml b/config/crd/bases/appstudio.redhat.com_imagerepositories.yaml index ffe2eb7a..7ccfd893 100644 --- a/config/crd/bases/appstudio.redhat.com_imagerepositories.yaml +++ b/config/crd/bases/appstudio.redhat.com_imagerepositories.yaml @@ -49,6 +49,11 @@ spec: credentials: description: Credentials management. properties: + regenerate-namespace-pull-token: + description: |- + RegenerateNamespacePullToken defines a request to refresh namespace pull robot credentials. + The field gets cleared after the refresh. + type: boolean regenerate-token: description: |- RegenerateToken defines a request to refresh image accessing credentials. diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index d9dd888a..941a989b 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -13,6 +13,14 @@ rules: - list - update - watch +- apiGroups: + - "" + resources: + - namespaces + verbs: + - get + - list + - watch - apiGroups: - "" resources: diff --git a/go.mod b/go.mod index 54072d6d..95a648ee 100644 --- a/go.mod +++ b/go.mod @@ -7,10 +7,11 @@ toolchain go1.24.6 require ( github.com/go-logr/logr v1.4.3 github.com/h2non/gock v1.2.0 + github.com/konflux-ci/application-api v0.0.0-20260205151641-c691ffebedf8 + github.com/konflux-ci/coverport/instrumentation/go v0.0.0-20251127103713-95b5b5e04a62 github.com/onsi/ginkgo/v2 v2.26.0 github.com/onsi/gomega v1.38.2 github.com/prometheus/client_golang v1.19.1 - github.com/redhat-appstudio/application-api v0.0.0-20231026192857-89515ad2504f go.uber.org/zap v1.27.0 gotest.tools/v3 v3.5.2 k8s.io/api v0.31.0 @@ -30,7 +31,6 @@ require ( github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/emicklei/go-restful/v3 v3.11.0 // indirect - github.com/evanphx/json-patch v4.5.0+incompatible // indirect github.com/evanphx/json-patch/v5 v5.9.11 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fsnotify/fsnotify v1.7.0 // indirect @@ -56,7 +56,6 @@ require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect - github.com/konflux-ci/coverport/instrumentation/go v0.0.0-20251127103713-95b5b5e04a62 // indirect github.com/mailru/easyjson v0.7.7 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect diff --git a/go.sum b/go.sum index 6184bbaa..e2abfce4 100644 --- a/go.sum +++ b/go.sum @@ -20,8 +20,8 @@ github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g= github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= -github.com/evanphx/json-patch v4.5.0+incompatible h1:ouOWdg56aJriqS0huScTkVXPC5IcNrDCXZ6OoTAWu7M= -github.com/evanphx/json-patch v4.5.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= +github.com/evanphx/json-patch v0.5.2 h1:xVCHIVMUu1wtM/VkR9jVZ45N3FhZfYMMYGorLCR8P3k= +github.com/evanphx/json-patch v0.5.2/go.mod h1:ZWS5hhDbVDyob71nXKNL0+PWn6ToqBHMikGIFbs31qQ= github.com/evanphx/json-patch/v5 v5.9.11 h1:/8HVnzMq13/3x9TPvjG08wUGqBTmZBsCWzjTM0wiaDU= github.com/evanphx/json-patch/v5 v5.9.11/go.mod h1:3j+LviiESTElxA4p3EMKAB9HXj3/XEtnUf6OZxqIQTM= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= @@ -92,6 +92,8 @@ github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnr github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= +github.com/konflux-ci/application-api v0.0.0-20260205151641-c691ffebedf8 h1:HjVmLXbIzsqOAU1gN+qhJdUIIaDiUtd2nc9YvLg8iHo= +github.com/konflux-ci/application-api v0.0.0-20260205151641-c691ffebedf8/go.mod h1:948Z+a1IbfRT0RtoHzWWSN9YEucSbMJTHaMhz7dVICc= github.com/konflux-ci/coverport/instrumentation/go v0.0.0-20251127103713-95b5b5e04a62 h1:lMTed+H0EesSqsH3iQXtLoy/+SpbBT0BS1J0izeEtFM= github.com/konflux-ci/coverport/instrumentation/go v0.0.0-20251127103713-95b5b5e04a62/go.mod h1:WVMHU9A2464s/vjH1xOTm4LJDD4xP+VlEiU+KM0gkSU= github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= @@ -135,8 +137,6 @@ github.com/prometheus/common v0.55.0 h1:KEi6DK7lXW/m7Ig5i47x0vRzuBsHuvJdi5ee6Y3G github.com/prometheus/common v0.55.0/go.mod h1:2SECS4xJG1kd8XF9IcM1gMX6510RAEL65zxzNImwdc8= github.com/prometheus/procfs v0.15.1 h1:YagwOFzUgYfKKHX6Dr+sHT7km/hxC76UB0learggepc= github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoGhij/e3PBqk= -github.com/redhat-appstudio/application-api v0.0.0-20231026192857-89515ad2504f h1:PoKf7gCV/g5blkzVlODkqeynmfIACcR7NqWF8eqnuec= -github.com/redhat-appstudio/application-api v0.0.0-20231026192857-89515ad2504f/go.mod h1:YvckuKHe82eWloGk0/BpSw4YYG2owrGZAanztbOj3pQ= github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII= github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= diff --git a/internal/controller/application_controller.go b/internal/controller/application_controller.go index 239f3ca7..5acd26ee 100644 --- a/internal/controller/application_controller.go +++ b/internal/controller/application_controller.go @@ -31,13 +31,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ctrllog "sigs.k8s.io/controller-runtime/pkg/log" + compapiv1alpha1 "github.com/konflux-ci/application-api/api/v1alpha1" imagerepositoryv1alpha1 "github.com/konflux-ci/image-controller/api/v1alpha1" l "github.com/konflux-ci/image-controller/pkg/logs" - appstudioredhatcomv1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1" ) const ( - IntegrationTestsServiceAccountName = "konflux-integration-runner" + IntegrationServiceAccountName = "konflux-integration-runner" ApplicationSecretLinkToSaFinalizer = "application-secret-link-to-integration-tests-sa.appstudio.openshift.io/finalizer" ) @@ -58,7 +58,7 @@ type ApplicationPullSecretCreator struct { // SetupWithManager sets up the controller with the Manager. func (r *ApplicationPullSecretCreator) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&appstudioredhatcomv1alpha1.Application{}). + For(&compapiv1alpha1.Application{}). Complete(r) } @@ -73,7 +73,7 @@ func (r *ApplicationPullSecretCreator) Reconcile(ctx context.Context, req ctrl.R ctx = ctrllog.IntoContext(ctx, log) // fetch the application instance - application := &appstudioredhatcomv1alpha1.Application{} + application := &compapiv1alpha1.Application{} err := r.Client.Get(ctx, req.NamespacedName, application) if err != nil { if errors.IsNotFound(err) { @@ -132,7 +132,7 @@ func (r *ApplicationPullSecretCreator) Reconcile(ctx context.Context, req ctrl.R } } - if err := r.updateServiceAccountWithApplicationPullSecret(ctx, applicationPullSecretName, application.Namespace); err != nil { + if err := r.updateIntegrationServiceAccountWithApplicationPullSecret(ctx, applicationPullSecretName, application.Namespace); err != nil { return ctrl.Result{}, err } @@ -147,7 +147,7 @@ func getApplicationPullSecretName(applicationName string) string { // getComponentIdsForApplication returns components id for all components owned by the application func (r *ApplicationPullSecretCreator) getComponentIdsForApplication(ctx context.Context, applicationId types.UID, namespace string) ([]types.UID, error) { log := ctrllog.FromContext(ctx) - componentsList := &appstudioredhatcomv1alpha1.ComponentList{} + componentsList := &compapiv1alpha1.ComponentList{} if err := r.Client.List(ctx, componentsList, &client.ListOptions{Namespace: namespace}); err != nil { log.Error(err, "failed to list components") return nil, err @@ -198,7 +198,7 @@ func (r *ApplicationPullSecretCreator) getImageRepositoryPullSecretNamesForCompo // createApplicationPullSecret creates or updates a single kubernetes.io/dockerconfigjson secret // by combining data from individual pull secrets. -func (r *ApplicationPullSecretCreator) createApplicationPullSecret(ctx context.Context, applicationPullSecretName string, application *appstudioredhatcomv1alpha1.Application, individualSecretNames []string) error { +func (r *ApplicationPullSecretCreator) createApplicationPullSecret(ctx context.Context, applicationPullSecretName string, application *compapiv1alpha1.Application, individualSecretNames []string) error { log := ctrllog.FromContext(ctx) log.Info("Creating application pull secret", "secretName", applicationPullSecretName) @@ -282,19 +282,19 @@ func (r *ApplicationPullSecretCreator) createApplicationPullSecret(ctx context.C return nil } -// udateServiceAccountWithApplicationPullSecret updates the ServiceAccount to include +// updateIntegrationServiceAccountWithApplicationPullSecret updates the ServiceAccount to include // the application pull secret as an imagePullSecret and as a Secret -func (r *ApplicationPullSecretCreator) updateServiceAccountWithApplicationPullSecret(ctx context.Context, applicationPullSecretName string, namespace string) error { +func (r *ApplicationPullSecretCreator) updateIntegrationServiceAccountWithApplicationPullSecret(ctx context.Context, applicationPullSecretName string, namespace string) error { log := ctrllog.FromContext(ctx) // fetch namespace SA namespaceServiceAccount := &corev1.ServiceAccount{} - if err := r.Client.Get(ctx, types.NamespacedName{Name: IntegrationTestsServiceAccountName, Namespace: namespace}, namespaceServiceAccount); err != nil { + if err := r.Client.Get(ctx, types.NamespacedName{Name: IntegrationServiceAccountName, Namespace: namespace}, namespaceServiceAccount); err != nil { if errors.IsNotFound(err) { - log.Info("Namespace ServiceAccount not found", "serviceAccountName", IntegrationTestsServiceAccountName, "namespace", namespace) + log.Info("Integration ServiceAccount not found", "serviceAccountName", IntegrationServiceAccountName, "namespace", namespace) return nil } - log.Error(err, "failed to read namespace ServiceAccount", "serviceAccountName", IntegrationTestsServiceAccountName, "namespace", namespace, l.Action, l.ActionView) + log.Error(err, "failed to read integration ServiceAccount", "serviceAccountName", IntegrationServiceAccountName, "namespace", namespace, l.Action, l.ActionView) return err } @@ -337,7 +337,7 @@ func (r *ApplicationPullSecretCreator) updateServiceAccountWithApplicationPullSe return nil } -func (r *ApplicationPullSecretCreator) doesApplicationPullSecretExist(ctx context.Context, applicationPullSecretName string, application *appstudioredhatcomv1alpha1.Application) (bool, error) { +func (r *ApplicationPullSecretCreator) doesApplicationPullSecretExist(ctx context.Context, applicationPullSecretName string, application *compapiv1alpha1.Application) (bool, error) { log := ctrllog.FromContext(ctx) applicationPullSecret := &corev1.Secret{} @@ -355,15 +355,15 @@ func (r *ApplicationPullSecretCreator) doesApplicationPullSecretExist(ctx contex // unlinkApplicationSecretFromIntegrationTestsSa ensures that the given secret is not linked with the integration tests service account. func (r *ApplicationPullSecretCreator) unlinkApplicationSecretFromIntegrationTestsSa(ctx context.Context, secretNameToRemove, namespace string) error { - log := ctrllog.FromContext(ctx).WithValues("ServiceAccountName", IntegrationTestsServiceAccountName, "SecretName", secretNameToRemove) + log := ctrllog.FromContext(ctx).WithValues("ServiceAccountName", IntegrationServiceAccountName, "SecretName", secretNameToRemove) serviceAccount := &corev1.ServiceAccount{} - err := r.Client.Get(ctx, types.NamespacedName{Name: IntegrationTestsServiceAccountName, Namespace: namespace}, serviceAccount) + err := r.Client.Get(ctx, types.NamespacedName{Name: IntegrationServiceAccountName, Namespace: namespace}, serviceAccount) if err != nil { if errors.IsNotFound(err) { return nil } - log.Error(err, "failed to read namespace service account", l.Action, l.ActionView) + log.Error(err, "failed to read integration service account", l.Action, l.ActionView) return err } diff --git a/internal/controller/application_controller_test.go b/internal/controller/application_controller_test.go index 067d995d..aef2e4a2 100644 --- a/internal/controller/application_controller_test.go +++ b/internal/controller/application_controller_test.go @@ -46,13 +46,13 @@ var _ = Describe("Application controller", func() { var registrySecret1 = "registry1.example.com" var registrySecret2 = "registry2.example.com" - Context("Create application secret and link it to Namespace ServiceAccount when it exists", func() { + Context("Create application secret and link it to Integration ServiceAccount when it exists", func() { BeforeEach(func() { quay.ResetTestQuayClient() }) AfterEach(func() { - deleteServiceAccount(types.NamespacedName{Name: IntegrationTestsServiceAccountName, Namespace: appSecretTestNamespace}) + deleteServiceAccount(types.NamespacedName{Name: IntegrationServiceAccountName, Namespace: appSecretTestNamespace}) deleteSecret(namespacePullSecretName) deleteImageRepository(imageRepository2Key) deleteImageRepository(imageRepository1Key) @@ -67,14 +67,14 @@ var _ = Describe("Application controller", func() { createNamespace(appSecretTestNamespace) }) - It("should create empty application pull secret and without link it to not existing namespace SA, because no components are owned by application", func() { + It("should create empty application pull secret and without link it to not existing integration SA, because no components are owned by application", func() { createComponent(componentConfig{ComponentKey: component1Key, ComponentApplication: applicationKey.Name}) createApplication(applicationConfig{ApplicationKey: applicationKey}) // wait until application secret is created applicationSecret := waitSecretExist(namespacePullSecretName) - // verify that namespace SA doesn't exist + // verify that integration SA doesn't exist saList := getServiceAccountList(appSecretTestNamespace) Expect(len(saList)).Should(Equal(0)) @@ -85,12 +85,12 @@ var _ = Describe("Application controller", func() { Expect(len(decodedSecret.Auths)).Should(Equal(0)) }) - It("should create empty application pull secret and link it to namespace SA, because no components are owned by application", func() { - createServiceAccount(appSecretTestNamespace, IntegrationTestsServiceAccountName) + It("should create empty application pull secret and link it to integration SA, because no components are owned by application", func() { + createServiceAccount(appSecretTestNamespace, IntegrationServiceAccountName) createComponent(componentConfig{ComponentKey: component1Key, ComponentApplication: applicationKey.Name}) createApplication(applicationConfig{ApplicationKey: applicationKey}) - // wait until empty secret is linked to namespace SA + // wait until empty secret is linked to integration SA Eventually(func() bool { saList := getServiceAccountList(appSecretTestNamespace) saCount := len(saList) @@ -105,7 +105,7 @@ var _ = Describe("Application controller", func() { saList := getServiceAccountList(appSecretTestNamespace) Expect(len(saList)).Should(Equal(1)) - Expect(saList[0].Name).Should(Equal(IntegrationTestsServiceAccountName)) + Expect(saList[0].Name).Should(Equal(IntegrationServiceAccountName)) Expect(len(saList[0].Secrets)).Should(Equal(1)) Expect(len(saList[0].ImagePullSecrets)).Should(Equal(1)) @@ -117,8 +117,8 @@ var _ = Describe("Application controller", func() { Expect(len(decodedSecret.Auths)).Should(Equal(0)) }) - It("should create an application pull secret with 2 secrets and link it to namespace SA", func() { - createServiceAccount(appSecretTestNamespace, IntegrationTestsServiceAccountName) + It("should create an application pull secret with 2 secrets and link it to integration SA", func() { + createServiceAccount(appSecretTestNamespace, IntegrationServiceAccountName) pullSecret1Data := generateDockerConfigJson(registrySecret1, "user1", "pass1") pullSecret1Key := types.NamespacedName{Name: pullSecret1, Namespace: appSecretTestNamespace} createDockerConfigSecret(pullSecret1Key, pullSecret1Data, true) @@ -132,7 +132,7 @@ var _ = Describe("Application controller", func() { component1 := createComponent(componentConfig{ComponentKey: component1Key, ComponentApplication: applicationKey.Name}) component2 := createComponent(componentConfig{ComponentKey: component2Key, ComponentApplication: applicationKey.Name}) - // wait until empty secret is linked to namespace SA + // wait until empty secret is linked to integration SA Eventually(func() bool { saList := getServiceAccountList(appSecretTestNamespace) saCount := len(saList) @@ -146,10 +146,10 @@ var _ = Describe("Application controller", func() { }, timeout, interval).WithTimeout(ensureTimeout).Should(BeTrue()) // delete application SA and empty secret as it will be created on next reconcile - deleteServiceAccount(types.NamespacedName{Name: IntegrationTestsServiceAccountName, Namespace: appSecretTestNamespace}) + deleteServiceAccount(types.NamespacedName{Name: IntegrationServiceAccountName, Namespace: appSecretTestNamespace}) deleteSecret(namespacePullSecretName) // recreate empty SA - createServiceAccount(appSecretTestNamespace, IntegrationTestsServiceAccountName) + createServiceAccount(appSecretTestNamespace, IntegrationServiceAccountName) // set component's owner to application component1.OwnerReferences = []metav1.OwnerReference{{ @@ -178,6 +178,8 @@ var _ = Describe("Application controller", func() { Name: component1.Name, UID: component1.UID, }}, + // set annotation so that imageRepository isn't updated with the annotation + Annotations: map[string]string{namespacePullSecretEnsuredAnnotation: "true"}, } imageConfig2 := imageRepositoryConfig{ ResourceKey: &imageRepository2Key, @@ -188,6 +190,8 @@ var _ = Describe("Application controller", func() { Name: component2.Name, UID: component2.UID, }}, + // set annotation so that imageRepository isn't updated with the annotation + Annotations: map[string]string{namespacePullSecretEnsuredAnnotation: "true"}, } ir1 := createImageRepository(imageConfig1) ir2 := createImageRepository(imageConfig2) @@ -221,7 +225,7 @@ var _ = Describe("Application controller", func() { application.Spec.DisplayName = "updated-name" Expect(k8sClient.Update(ctx, application)).To(Succeed()) - // wait until empty secret is linked to namespace SA + // wait until empty secret is linked to integration SA Eventually(func() bool { saList := getServiceAccountList(appSecretTestNamespace) saCount := len(saList) @@ -251,21 +255,21 @@ var _ = Describe("Application controller", func() { Expect(decodedSecret.Auths).To(HaveKey(registrySecret2)) Expect(decodedSecret.Auths[registrySecret2].Auth).To(Equal(base64.StdEncoding.EncodeToString([]byte(authSecret2)))) - konfluxSA := getServiceAccount(appSecretTestNamespace, IntegrationTestsServiceAccountName) + konfluxSA := getServiceAccount(appSecretTestNamespace, IntegrationServiceAccountName) Expect(konfluxSA.ImagePullSecrets).To(HaveLen(1)) Expect(konfluxSA.ImagePullSecrets[0].Name).To(Equal(namespacePullSecretName.Name)) Expect(konfluxSA.Secrets).To(HaveLen(1)) Expect(konfluxSA.Secrets[0].Name).To(Equal(namespacePullSecretName.Name)) - // verify that after application removal application secret is no longer linked in namespace SA + // verify that after application removal application secret is no longer linked in integration SA deleteApplication(applicationKey) - konfluxSA = getServiceAccount(appSecretTestNamespace, IntegrationTestsServiceAccountName) + konfluxSA = getServiceAccount(appSecretTestNamespace, IntegrationServiceAccountName) Expect(konfluxSA.ImagePullSecrets).To(HaveLen(0)) Expect(konfluxSA.Secrets).To(HaveLen(0)) }) - It("should create an application pull secret with 1 secret because other secret isn't SecretTypeDockerConfigJson and link it to namespace SA", func() { - createServiceAccount(appSecretTestNamespace, IntegrationTestsServiceAccountName) + It("should create an application pull secret with 1 secret because other secret isn't SecretTypeDockerConfigJson and link it to integration SA", func() { + createServiceAccount(appSecretTestNamespace, IntegrationServiceAccountName) pullSecret1Data := generateDockerConfigJson(registrySecret1, "user1", "pass1") pullSecret1Key := types.NamespacedName{Name: pullSecret1, Namespace: appSecretTestNamespace} createDockerConfigSecret(pullSecret1Key, pullSecret1Data, true) @@ -279,7 +283,7 @@ var _ = Describe("Application controller", func() { component1 := createComponent(componentConfig{ComponentKey: component1Key, ComponentApplication: applicationKey.Name}) component2 := createComponent(componentConfig{ComponentKey: component2Key, ComponentApplication: applicationKey.Name}) - // wait until empty secret is linked to namespace SA + // wait until empty secret is linked to integration SA Eventually(func() bool { saList := getServiceAccountList(appSecretTestNamespace) saCount := len(saList) @@ -293,10 +297,10 @@ var _ = Describe("Application controller", func() { }, timeout, interval).WithTimeout(ensureTimeout).Should(BeTrue()) // delete application SA and empty secret as it will be created on next reconcile - deleteServiceAccount(types.NamespacedName{Name: IntegrationTestsServiceAccountName, Namespace: appSecretTestNamespace}) + deleteServiceAccount(types.NamespacedName{Name: IntegrationServiceAccountName, Namespace: appSecretTestNamespace}) deleteSecret(namespacePullSecretName) // recreate empty SA - createServiceAccount(appSecretTestNamespace, IntegrationTestsServiceAccountName) + createServiceAccount(appSecretTestNamespace, IntegrationServiceAccountName) // set component's owner to application component1.OwnerReferences = []metav1.OwnerReference{{ @@ -325,6 +329,8 @@ var _ = Describe("Application controller", func() { Name: component1.Name, UID: component1.UID, }}, + // set annotation so that imageRepository isn't updated with the annotation + Annotations: map[string]string{namespacePullSecretEnsuredAnnotation: "true"}, } imageConfig2 := imageRepositoryConfig{ ResourceKey: &imageRepository2Key, @@ -335,6 +341,8 @@ var _ = Describe("Application controller", func() { Name: component2.Name, UID: component2.UID, }}, + // set annotation so that imageRepository isn't updated with the annotation + Annotations: map[string]string{namespacePullSecretEnsuredAnnotation: "true"}, } ir1 := createImageRepository(imageConfig1) ir2 := createImageRepository(imageConfig2) @@ -368,7 +376,7 @@ var _ = Describe("Application controller", func() { application.Spec.DisplayName = "updated-name" Expect(k8sClient.Update(ctx, application)).To(Succeed()) - // wait until empty secret is linked to namespace SA + // wait until empty secret is linked to integration SA Eventually(func() bool { saList := getServiceAccountList(appSecretTestNamespace) saCount := len(saList) @@ -396,7 +404,7 @@ var _ = Describe("Application controller", func() { Expect(decodedSecret.Auths).To(HaveKey(registrySecret1)) Expect(decodedSecret.Auths[registrySecret1].Auth).To(Equal(base64.StdEncoding.EncodeToString([]byte(authSecret1)))) - konfluxSA := getServiceAccount(appSecretTestNamespace, IntegrationTestsServiceAccountName) + konfluxSA := getServiceAccount(appSecretTestNamespace, IntegrationServiceAccountName) Expect(konfluxSA.ImagePullSecrets).To(HaveLen(1)) Expect(konfluxSA.ImagePullSecrets[0].Name).To(Equal(namespacePullSecretName.Name)) Expect(konfluxSA.Secrets).To(HaveLen(1)) diff --git a/internal/controller/component_image_controller.go b/internal/controller/component_image_controller.go index 99b42d92..a4f687af 100644 --- a/internal/controller/component_image_controller.go +++ b/internal/controller/component_image_controller.go @@ -32,10 +32,10 @@ import ( ctrllog "sigs.k8s.io/controller-runtime/pkg/log" "github.com/go-logr/logr" + compapiv1alpha1 "github.com/konflux-ci/application-api/api/v1alpha1" imagerepositoryv1alpha1 "github.com/konflux-ci/image-controller/api/v1alpha1" l "github.com/konflux-ci/image-controller/pkg/logs" "github.com/konflux-ci/image-controller/pkg/quay" - appstudioredhatcomv1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1" ) const ( @@ -71,7 +71,7 @@ type ComponentReconciler struct { // SetupWithManager sets up the controller with the Manager. func (r *ComponentReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&appstudioredhatcomv1alpha1.Component{}). + For(&compapiv1alpha1.Component{}). Complete(r) } @@ -85,7 +85,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( ctx = ctrllog.IntoContext(ctx, log) // Fetch the Component instance - component := &appstudioredhatcomv1alpha1.Component{} + component := &compapiv1alpha1.Component{} err := r.Client.Get(ctx, req.NamespacedName, component) if err != nil { if errors.IsNotFound(err) { @@ -107,7 +107,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } log.Info("Image repository finalizer removed from the Component", l.Action, l.ActionDelete, "componentName", component.Name) - r.waitComponentUpdateInCache(ctx, req.NamespacedName, func(component *appstudioredhatcomv1alpha1.Component) bool { + r.waitComponentUpdateInCache(ctx, req.NamespacedName, func(component *compapiv1alpha1.Component) bool { return !controllerutil.ContainsFinalizer(component, ImageRepositoryComponentFinalizer) }) } @@ -156,7 +156,12 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } if imageRepositoryFound == "" { - imageRepositoryName := fmt.Sprintf("imagerepository-for-%s-%s", component.Spec.Application, component.Name) + imageRepositoryName := "" + if component.Spec.Application != "" { + imageRepositoryName = fmt.Sprintf("imagerepository-for-%s-%s", component.Spec.Application, component.Name) + } else { + imageRepositoryName = fmt.Sprintf("imagerepository-for-%s", component.Name) + } log.Info("Will create image repository", "ImageRepositoryName", imageRepositoryName, "ComponentName", component.Name) imageRepository := &imagerepositoryv1alpha1.ImageRepository{ @@ -168,8 +173,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( Name: imageRepositoryName, Namespace: component.Namespace, Labels: map[string]string{ - ApplicationNameLabelName: component.Spec.Application, - ComponentNameLabelName: component.Name, + ComponentNameLabelName: component.Name, }, Annotations: map[string]string{ updateComponentAnnotationName: "true", @@ -182,6 +186,10 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( }, } + if component.Spec.Application != "" { + imageRepository.ObjectMeta.Labels[ApplicationNameLabelName] = component.Spec.Application + } + if err := r.Client.Create(ctx, imageRepository); err != nil { log.Error(err, "failed to create image repository", "ImageRepositoryName", imageRepositoryName, "ComponentName", component.Name) return ctrl.Result{}, err @@ -203,7 +211,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } log.Info("Component updated successfully, 'generate' annotation removed", "ComponentName", component.Name) - r.waitComponentUpdateInCache(ctx, req.NamespacedName, func(component *appstudioredhatcomv1alpha1.Component) bool { + r.waitComponentUpdateInCache(ctx, req.NamespacedName, func(component *compapiv1alpha1.Component) bool { _, exists := component.Annotations[GenerateImageAnnotationName] return !exists }) @@ -211,7 +219,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, nil } -func (r *ComponentReconciler) reportError(ctx context.Context, component *appstudioredhatcomv1alpha1.Component, messsage string) error { +func (r *ComponentReconciler) reportError(ctx context.Context, component *compapiv1alpha1.Component, messsage string) error { lookUpKey := types.NamespacedName{Name: component.Name, Namespace: component.Namespace} if err := r.Client.Get(ctx, lookUpKey, component); err != nil { return err @@ -231,10 +239,10 @@ func (r *ComponentReconciler) reportError(ctx context.Context, component *appstu // For example, instead of one initial pipeline run we could get two. // To resolve the problem above, instead of just ending the reconcile loop here, // we are waiting for the cache update. This approach prevents next reconciles with outdated cache. -func (r *ComponentReconciler) waitComponentUpdateInCache(ctx context.Context, componentKey types.NamespacedName, componentUpdated func(component *appstudioredhatcomv1alpha1.Component) bool) { +func (r *ComponentReconciler) waitComponentUpdateInCache(ctx context.Context, componentKey types.NamespacedName, componentUpdated func(component *compapiv1alpha1.Component) bool) { log := ctrllog.FromContext(ctx).WithName("waitComponentUpdateInCache") - component := &appstudioredhatcomv1alpha1.Component{} + component := &compapiv1alpha1.Component{} isComponentInCacheUpToDate := false for i := 0; i < 10; i++ { if err := r.Client.Get(ctx, componentKey, component); err == nil { diff --git a/internal/controller/component_image_controller_test.go b/internal/controller/component_image_controller_test.go index 8206319e..8dcca684 100644 --- a/internal/controller/component_image_controller_test.go +++ b/internal/controller/component_image_controller_test.go @@ -32,10 +32,13 @@ import ( ) var _ = Describe("Component image controller", func() { - var imageTestNamespace = "component-image-controller-test" + var ( + imageTestNamespace = "component-image-controller-test" + ) BeforeEach(func() { createNamespace(imageTestNamespace) + createNamespace(kubeSystemNamespace) }) Context("Image repository provision flow", func() { @@ -44,6 +47,10 @@ var _ = Describe("Component image controller", func() { Name: fmt.Sprintf("imagerepository-for-%s-%s", defaultComponentApplication, resourceImageProvisionKey.Name), Namespace: resourceImageProvisionKey.Namespace, } + var imageRepositoryWithoutApplicationName = types.NamespacedName{ + Name: fmt.Sprintf("imagerepository-for-%s", resourceImageProvisionKey.Name), + Namespace: resourceImageProvisionKey.Namespace, + } var applicationKey = types.NamespacedName{Name: defaultComponentApplication, Namespace: imageTestNamespace} var componentSaName = getComponentSaName(resourceImageProvisionKey.Name) @@ -59,7 +66,7 @@ var _ = Describe("Component image controller", func() { It("should prepare environment", func() { createServiceAccount(imageTestNamespace, componentSaName) - createServiceAccount(imageTestNamespace, IntegrationTestsServiceAccountName) + createServiceAccount(imageTestNamespace, IntegrationServiceAccountName) // wait for application SA to be created Eventually(func() bool { @@ -74,7 +81,8 @@ var _ = Describe("Component image controller", func() { It("should do image repository provision", func() { expectedVisibility := imagerepositoryv1alpha1.ImageVisibility("private") createComponent(componentConfig{ - ComponentKey: resourceImageProvisionKey, + ComponentKey: resourceImageProvisionKey, + ComponentApplication: defaultComponentApplication, Annotations: map[string]string{ GenerateImageAnnotationName: "{\"visibility\": \"private\"}", }, @@ -104,10 +112,45 @@ var _ = Describe("Component image controller", func() { deleteImageRepository(imageRepositoryName) }) + It("should do image repository provision when component doesn't have application", func() { + expectedVisibility := imagerepositoryv1alpha1.ImageVisibility("private") + createComponent(componentConfig{ + ComponentKey: resourceImageProvisionKey, + Annotations: map[string]string{ + GenerateImageAnnotationName: "{\"visibility\": \"private\"}", + }, + }) + // wait for component_image_controller to finish + waitComponentAnnotationGone(resourceImageProvisionKey, GenerateImageAnnotationName) + + imageRepositoriesList := &imagerepositoryv1alpha1.ImageRepositoryList{} + Expect(k8sClient.List(ctx, imageRepositoriesList, &client.ListOptions{Namespace: resourceImageProvisionKey.Namespace})).To(Succeed()) + Expect(imageRepositoriesList.Items).To(HaveLen(1)) + + component := getComponent(resourceImageProvisionKey) + // wait for imagerepository_controller to finish + waitImageRepositoryFinalizerOnImageRepository(imageRepositoryWithoutApplicationName) + imageRepository := getImageRepository(imageRepositoryWithoutApplicationName) + + _, applicationLabelExists := imageRepository.ObjectMeta.Labels[ApplicationNameLabelName] + Expect(applicationLabelExists).To(BeFalse()) + Expect(imageRepository.ObjectMeta.Labels[ComponentNameLabelName]).To(Equal(component.Name)) + Expect(imageRepository.Spec.Image.Visibility).To(Equal(expectedVisibility)) + Expect(imageRepository.ObjectMeta.OwnerReferences[0].UID).To(Equal(component.UID)) + Expect(imageRepository.ObjectMeta.Annotations[updateComponentAnnotationName]).To(BeEmpty()) + + component = getComponent(resourceImageProvisionKey) + Expect(component.Annotations[ImageAnnotationName]).To(BeEmpty()) + Expect(component.Spec.ContainerImage).ToNot(BeEmpty()) + + deleteImageRepository(imageRepositoryWithoutApplicationName) + }) + It("should accept deprecated true value for repository options", func() { expectedVisibility := imagerepositoryv1alpha1.ImageVisibility("public") createComponent(componentConfig{ - ComponentKey: resourceImageProvisionKey, + ComponentKey: resourceImageProvisionKey, + ComponentApplication: defaultComponentApplication, Annotations: map[string]string{ GenerateImageAnnotationName: "true", }, @@ -137,7 +180,7 @@ var _ = Describe("Component image controller", func() { deleteImageRepository(imageRepositoryName) deleteServiceAccount(types.NamespacedName{Name: componentSaName, Namespace: imageTestNamespace}) - deleteServiceAccount(types.NamespacedName{Name: IntegrationTestsServiceAccountName, Namespace: imageTestNamespace}) + deleteServiceAccount(types.NamespacedName{Name: IntegrationServiceAccountName, Namespace: imageTestNamespace}) }) }) @@ -152,7 +195,7 @@ var _ = Describe("Component image controller", func() { createApplication(applicationConfig{ApplicationKey: applicationKey}) createServiceAccount(imageTestNamespace, componentSaName) - createServiceAccount(imageTestNamespace, IntegrationTestsServiceAccountName) + createServiceAccount(imageTestNamespace, IntegrationServiceAccountName) // wait for application SA to be created Eventually(func() bool { @@ -163,7 +206,7 @@ var _ = Describe("Component image controller", func() { }) It("should do nothing if generate annotation is not set", func() { - createComponent(componentConfig{ComponentKey: resourceImageErrorKey}) + createComponent(componentConfig{ComponentKey: resourceImageErrorKey, ComponentApplication: defaultComponentApplication}) time.Sleep(ensureTimeout) waitComponentAnnotationGone(resourceImageErrorKey, GenerateImageAnnotationName) @@ -270,7 +313,7 @@ var _ = Describe("Component image controller", func() { deleteComponent(resourceImageErrorKey) deleteApplication(applicationKey) deleteServiceAccount(types.NamespacedName{Name: componentSaName, Namespace: imageTestNamespace}) - deleteServiceAccount(types.NamespacedName{Name: IntegrationTestsServiceAccountName, Namespace: imageTestNamespace}) + deleteServiceAccount(types.NamespacedName{Name: IntegrationServiceAccountName, Namespace: imageTestNamespace}) }) }) }) diff --git a/internal/controller/imagerepository_controller.go b/internal/controller/imagerepository_controller.go index 47932edd..a7dd5399 100644 --- a/internal/controller/imagerepository_controller.go +++ b/internal/controller/imagerepository_controller.go @@ -28,11 +28,11 @@ import ( "time" "github.com/go-logr/logr" + compapiv1alpha1 "github.com/konflux-ci/application-api/api/v1alpha1" imagerepositoryv1alpha1 "github.com/konflux-ci/image-controller/api/v1alpha1" l "github.com/konflux-ci/image-controller/pkg/logs" "github.com/konflux-ci/image-controller/pkg/metrics" "github.com/konflux-ci/image-controller/pkg/quay" - appstudioredhatcomv1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -62,6 +62,12 @@ const ( componentSaNamePrefix = "build-pipeline-" imageRepositoryNameChangedMessagePrefix = "Image repository name changed after creation" imageRepositoryNameChangedMessageSuffix = "That doesn't change image name in the registry. To do that, delete ImageRepository object and re-create it with new image name." + + kubeSystemNamespace = "kube-system" + namespacePullSecretName = "components-namespace-pull" + // when missing, will ensure namespace pull secret and namespace pull robot account and add the annotation + // when true, namespace pull secret and namespace pull robot account created + namespacePullSecretEnsuredAnnotation = "image-controller.appstudio.redhat.com/namespace-pull-secret-ensured" ) // ImageRepositoryReconciler reconciles a ImageRepository object @@ -95,6 +101,7 @@ func setMetricsTime(idForMetrics string, reconcileStartTime time.Time) { //+kubebuilder:rbac:groups=core,resources=configmaps,verbs=get;list;watch //+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;update;patch //+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=get;list;watch;update;patch +//+kubebuilder:rbac:groups=core,resources=namespaces,verbs=get;list;watch func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := ctrllog.FromContext(ctx).WithName("ImageRepository") @@ -131,10 +138,13 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ } // remove pull secret entry from application pull secret - err := r.removePullSecretFromApplicationPullSecret(ctx, imageRepository) - if err != nil { - log.Error(err, "failed to remove entry from application pull secret", "application", imageRepository.Labels[ApplicationNameLabelName], "secret", imageRepository.Status.Credentials.PullSecretName) - return ctrl.Result{}, err + applicationName := imageRepository.Labels[ApplicationNameLabelName] + if applicationName != "" { + err := r.removePullSecretFromApplicationPullSecret(ctx, imageRepository) + if err != nil { + log.Error(err, "failed to remove entry from application pull secret", "application", imageRepository.Labels[ApplicationNameLabelName], "secret", imageRepository.Status.Credentials.PullSecretName) + return ctrl.Result{}, err + } } // unlink pull secret for nudging component from nudged components SA @@ -159,6 +169,16 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ if skipDeletion { log.Info("Skip deletion was configured for image repository", "ImageRepository", imageRepository.Name) } + + // Remove permissions for repository from namespace robot account + if namespaceRobotName, err := r.getNamespaceRobotName(ctx, imageRepository.Namespace); err == nil { + namespaceRobot, err := r.QuayClient.GetRobotAccount(r.QuayOrganization, namespaceRobotName) + if err == nil && namespaceRobot != nil { + log.Info("Removing permissions from namespace robot account", "repoName", imageRepository.Spec.Image.Name, "robotName", namespaceRobot.Name) + _ = r.QuayClient.RemovePermissionsForRepositoryFromAccount(r.QuayOrganization, imageRepository.Spec.Image.Name, namespaceRobot.Name, true) + } + } + // Do not block deletion on failures r.CleanupImageRepository(ctx, imageRepository, !(imageRepositoryFound || skipDeletion)) @@ -215,22 +235,63 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, nil } - // Update component - if isComponentLinked(imageRepository) { - // update application pull secret - applicationName := imageRepository.Labels[ApplicationNameLabelName] - pullSecretName := getSecretName(imageRepository, true) - err := r.addPullSecretAuthToApplicationPullSecret(ctx, applicationName, imageRepository.Namespace, pullSecretName, imageRepository.Status.Image.URL, false) + // ensure that namespace pull secret and namespace pull robot account exist, and set annotation afterwards + _, namespacePullSecretEnsuredExists := imageRepository.Annotations[namespacePullSecretEnsuredAnnotation] + if !namespacePullSecretEnsuredExists { + namespaceRobot, err := r.getOrCreateNamespaceRobot(ctx, imageRepository.Namespace) if err != nil { - log.Error(err, "failed to update application pull secret with individual pull secret", "application", applicationName, "secret", pullSecretName) + return ctrl.Result{}, err + } + if err := r.ensureNamespacePullSecret(ctx, imageRepository, namespaceRobot); err != nil { return ctrl.Result{}, err } - // link secret to component SA + if imageRepository.Annotations == nil { + imageRepository.Annotations = make(map[string]string) + } + imageRepository.Annotations[namespacePullSecretEnsuredAnnotation] = "true" + + if err := r.Client.Update(ctx, imageRepository); err != nil { + log.Error(err, "failed to update imageRepository after setting namespace pull secret annotation", l.Action, l.ActionUpdate) + return ctrl.Result{}, err + } + log.Info("updated imageRepository after setting namespace pull secret annotation", l.Action, l.ActionUpdate) + + return ctrl.Result{}, nil + } + + // Update component's containerImage and link push secret to component SA, add pull secret to application secret + // link namespace pull secret to integration and component SA + if isComponentLinked(imageRepository) { + pullSecretName := getSecretName(imageRepository, true) + applicationName := imageRepository.Labels[ApplicationNameLabelName] + + // update application pull secret + if applicationName != "" { + err := r.addPullSecretAuthToApplicationPullSecret(ctx, applicationName, imageRepository.Namespace, pullSecretName, imageRepository.Status.Image.URL, false) + if err != nil { + log.Error(err, "failed to update application pull secret with individual pull secret", "applicationPullSecret", getApplicationPullSecretName(applicationName), "secret", pullSecretName) + return ctrl.Result{}, err + } + } + + // link push secret to component SA pushSecretName := getSecretName(imageRepository, false) componentSaName := getComponentSaName(imageRepository.Labels[ComponentNameLabelName]) - if err := r.linkSecretToServiceAccount(ctx, componentSaName, pushSecretName, imageRepository.Namespace, false); err != nil { - log.Error(err, "failed to link secret to service account", "SaName", componentSaName, "SecretName", pushSecretName, l.Action, l.ActionUpdate) + if err := r.linkSecretToServiceAccount(ctx, componentSaName, pushSecretName, imageRepository.Namespace, false, false); err != nil { + log.Error(err, "failed to link push secret to component service account", "SaName", componentSaName, "SecretName", pushSecretName, l.Action, l.ActionUpdate) + return ctrl.Result{}, err + } + + // link namespace pull secret to component SA + if err := r.linkSecretToServiceAccount(ctx, componentSaName, namespacePullSecretName, imageRepository.Namespace, false, false); err != nil { + log.Error(err, "failed to link namespace pull secret to component service account", "SaName", componentSaName, "SecretName", namespacePullSecretName, l.Action, l.ActionUpdate) + return ctrl.Result{}, err + } + + // link namespace pull secret to integration SA + if err := r.linkSecretToServiceAccount(ctx, IntegrationServiceAccountName, namespacePullSecretName, imageRepository.Namespace, true, true); err != nil { + log.Error(err, "failed to link namespace pull secret to integration service account", "SaName", IntegrationServiceAccountName, "SecretName", namespacePullSecretName, l.Action, l.ActionUpdate) return ctrl.Result{}, err } @@ -238,7 +299,7 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ if updateComponentAnnotationExists && updateComponentAnnotation == "true" { componentName := imageRepository.Labels[ComponentNameLabelName] - component := &appstudioredhatcomv1alpha1.Component{} + component := &compapiv1alpha1.Component{} componentKey := types.NamespacedName{Namespace: imageRepository.Namespace, Name: componentName} if err := r.Client.Get(ctx, componentKey, component); err != nil { if errors.IsNotFound(err) { @@ -317,6 +378,15 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, nil } + // Rotate credentials for namespace robot + regenerateNamespacePullToken := imageRepository.Spec.Credentials.RegenerateNamespacePullToken + if regenerateNamespacePullToken != nil && *regenerateNamespacePullToken { + if err := r.RegenerateNamespaceRobotCredentials(ctx, imageRepository); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil + } + // Check and fix linking is requested verifyLinking := imageRepository.Spec.Credentials.VerifyLinking if verifyLinking != nil && *verifyLinking { @@ -355,7 +425,7 @@ func (r *ImageRepositoryReconciler) CheckComponentExistence(ctx context.Context, log := ctrllog.FromContext(ctx).WithName("CheckComponentExistence") componentName := imageRepository.Labels[ComponentNameLabelName] - component := &appstudioredhatcomv1alpha1.Component{} + component := &compapiv1alpha1.Component{} componentKey := types.NamespacedName{Namespace: imageRepository.Namespace, Name: componentName} if err := r.Client.Get(ctx, componentKey, component); err != nil { if errors.IsNotFound(err) { @@ -403,10 +473,10 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context log := ctrllog.FromContext(ctx).WithName("ImageRepositoryProvision") ctx = ctrllog.IntoContext(ctx, log) - var component *appstudioredhatcomv1alpha1.Component + var component *compapiv1alpha1.Component if isComponentLinked(imageRepository) { componentName := imageRepository.Labels[ComponentNameLabelName] - component = &appstudioredhatcomv1alpha1.Component{} + component = &compapiv1alpha1.Component{} componentKey := types.NamespacedName{Namespace: imageRepository.Namespace, Name: componentName} if err := r.Client.Get(ctx, componentKey, component); err != nil { if errors.IsNotFound(err) { @@ -422,6 +492,18 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context } } + namespaceRobot, err := r.getOrCreateNamespaceRobot(ctx, imageRepository.Namespace) + if err != nil { + return err + } + + _, namespacePullSecretEnsuredExists := imageRepository.Annotations[namespacePullSecretEnsuredAnnotation] + if !namespacePullSecretEnsuredExists { + if err := r.ensureNamespacePullSecret(ctx, imageRepository, namespaceRobot); err != nil { + return err + } + } + imageRepositoryName := "" if imageRepository.Spec.Image.Name == "" { if isComponentLinked(imageRepository) { @@ -471,6 +553,11 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context return err } + // add permission to the repository for namespace robot account + if err := r.QuayClient.AddPermissionsForRepositoryToAccount(r.QuayOrganization, imageRepositoryName, namespaceRobot.Name, true, false); err != nil { + return err + } + pushCredentialsInfo, err := r.ProvisionImageRepositoryAccess(ctx, imageRepository, false) if err != nil { return err @@ -501,6 +588,11 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context status.Credentials.PullSecretName = pullCredentialsInfo.SecretName status.Notifications = notificationStatus + if imageRepository.Annotations == nil { + imageRepository.Annotations = make(map[string]string) + } + imageRepository.Annotations[namespacePullSecretEnsuredAnnotation] = "true" + imageRepository.Spec.Image.Name = imageRepositoryName controllerutil.AddFinalizer(imageRepository, ImageRepositoryFinalizer) if isComponentLinked(imageRepository) { @@ -558,7 +650,7 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepositoryAccess(ctx context.C } secretName := getSecretName(imageRepository, isPullOnly) - if err := r.EnsureSecret(ctx, imageRepository, secretName, robotAccount, quayImageURL); err != nil { + if err := r.EnsureSecret(ctx, imageRepository, secretName, robotAccount, quayImageURL, true); err != nil { return nil, err } @@ -635,6 +727,28 @@ func (r *ImageRepositoryReconciler) RegenerateImageRepositoryCredentials(ctx con return nil } +// RegenerateNamespaceRobotCredentials rotates namespace robot token and updates corresponding secret +func (r *ImageRepositoryReconciler) RegenerateNamespaceRobotCredentials(ctx context.Context, imageRepository *imagerepositoryv1alpha1.ImageRepository) error { + log := ctrllog.FromContext(ctx) + + namespaceRobotName, err := r.getNamespaceRobotName(ctx, imageRepository.Namespace) + if err != nil { + return err + } + + if err := r.RegenerateNamespaceRobotAccessToken(ctx, imageRepository, namespaceRobotName); err != nil { + return err + } + + imageRepository.Spec.Credentials.RegenerateNamespacePullToken = nil + if err := r.Client.Update(ctx, imageRepository); err != nil { + log.Error(err, "failed to update imageRepository", l.Action, l.ActionUpdate) + return err + } + + return nil +} + // 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) @@ -658,23 +772,47 @@ func (r *ImageRepositoryReconciler) RegenerateImageRepositoryAccessToken(ctx con if isPullOnly { secretName = imageRepository.Status.Credentials.PullSecretName } - if err := r.EnsureSecret(ctx, imageRepository, secretName, robotAccount, quayImageURL); err != nil { + if err := r.EnsureSecret(ctx, imageRepository, secretName, robotAccount, quayImageURL, true); err != nil { return err } // update also secret in application secret if isComponentLinked(imageRepository) && isPullOnly { applicationName := imageRepository.Labels[ApplicationNameLabelName] - err := r.addPullSecretAuthToApplicationPullSecret(ctx, applicationName, imageRepository.Namespace, secretName, imageRepository.Status.Image.URL, true) - if err != nil { - log.Error(err, "failed to update application pull secret after individual pull secret change", "application", applicationName, "secret", secretName) - return err + if applicationName != "" { + err := r.addPullSecretAuthToApplicationPullSecret(ctx, applicationName, imageRepository.Namespace, secretName, imageRepository.Status.Image.URL, true) + if err != nil { + log.Error(err, "failed to update application pull secret after individual pull secret change", "applicationPullSecret", getApplicationPullSecretName(applicationName), "secret", secretName) + return err + } } } return nil } +// RegenerateNamespaceRobotAccessToken rotates namespace robot account token and updates new one to the corresponding Secret. +func (r *ImageRepositoryReconciler) RegenerateNamespaceRobotAccessToken(ctx context.Context, imageRepository *imagerepositoryv1alpha1.ImageRepository, namespaceRobotName string) error { + log := ctrllog.FromContext(ctx).WithName("RegenerateNamespaceRobotAccessToken") + ctx = ctrllog.IntoContext(ctx, log) + + namespaceRobotAccount, err := r.QuayClient.RegenerateRobotAccountToken(r.QuayOrganization, namespaceRobotName) + if err != nil { + log.Error(err, "failed to refresh namespace robot account token") + return err + } else { + log.Info("Refreshed quay namespace robot account token") + } + + quayImageURL := fmt.Sprintf("quay.io/%s/%s", r.QuayOrganization, imageRepository.Namespace) + + if err := r.EnsureSecret(ctx, imageRepository, namespacePullSecretName, namespaceRobotAccount, quayImageURL, false); err != nil { + return err + } + + return nil +} + // CleanupImageRepository deletes image repository and corresponding robot account(s). func (r *ImageRepositoryReconciler) CleanupImageRepository(ctx context.Context, imageRepository *imagerepositoryv1alpha1.ImageRepository, removeRepository bool) { log := ctrllog.FromContext(ctx).WithName("RepositoryCleanup") @@ -756,7 +894,7 @@ func (r *ImageRepositoryReconciler) ChangeImageRepositoryVisibility(ctx context. return err } -func (r *ImageRepositoryReconciler) EnsureSecret(ctx context.Context, imageRepository *imagerepositoryv1alpha1.ImageRepository, secretName string, robotAccount *quay.RobotAccount, imageURL string) error { +func (r *ImageRepositoryReconciler) EnsureSecret(ctx context.Context, imageRepository *imagerepositoryv1alpha1.ImageRepository, secretName string, robotAccount *quay.RobotAccount, imageURL string, setOwnership bool) error { log := ctrllog.FromContext(ctx).WithValues("SecretName", secretName) secret := &corev1.Secret{} @@ -779,9 +917,11 @@ func (r *ImageRepositoryReconciler) EnsureSecret(ctx context.Context, imageRepos StringData: generateDockerconfigSecretData(imageURL, robotAccount), } - if err := controllerutil.SetOwnerReference(imageRepository, secret, r.Scheme); err != nil { - log.Error(err, "failed to set owner for image repository secret") - return err + if setOwnership { + if err := controllerutil.SetOwnerReference(imageRepository, secret, r.Scheme); err != nil { + log.Error(err, "failed to set owner for image repository secret") + return err + } } if err := r.Client.Create(ctx, secret); err != nil { @@ -810,9 +950,8 @@ func generateQuayRobotAccountName(imageRepositoryName string, isPullOnly bool) s if len(imageNamePrefix) > 220 { imageNamePrefix = imageNamePrefix[:220] } - imageNamePrefix = strings.ReplaceAll(imageNamePrefix, "/", "_") - imageNamePrefix = strings.ReplaceAll(imageNamePrefix, ".", "_") - imageNamePrefix = strings.ReplaceAll(imageNamePrefix, "-", "_") + + imageNamePrefix = sanitizeNameForQuay(imageNamePrefix) randomSuffix := getRandomString(10) @@ -824,6 +963,14 @@ func generateQuayRobotAccountName(imageRepositoryName string, isPullOnly bool) s return robotAccountName } +func sanitizeNameForQuay(name string) string { + name = strings.ReplaceAll(name, "/", "_") + name = strings.ReplaceAll(name, ".", "_") + name = strings.ReplaceAll(name, "-", "_") + + return name +} + // removeDuplicateUnderscores replaces sequence of underscores with only one. // Example: ab__cd___e => ab_cd_e func removeDuplicateUnderscores(s string) string { @@ -844,7 +991,7 @@ func getSecretName(imageRepository *imagerepositoryv1alpha1.ImageRepository, isP } func isComponentLinked(imageRepository *imagerepositoryv1alpha1.ImageRepository) bool { - return imageRepository.Labels[ApplicationNameLabelName] != "" && imageRepository.Labels[ComponentNameLabelName] != "" + return imageRepository.Labels[ComponentNameLabelName] != "" } func getRandomString(length int) string { @@ -907,7 +1054,7 @@ func getComponentSaName(componentName string) string { func (r *ImageRepositoryReconciler) addPullSecretAuthToApplicationPullSecret(ctx context.Context, applicationName, namespace, pullSecretName, imageURL string, overwrite bool) error { log := ctrllog.FromContext(ctx) - application := &appstudioredhatcomv1alpha1.Application{} + application := &compapiv1alpha1.Application{} if err := r.Client.Get(ctx, types.NamespacedName{Name: applicationName, Namespace: namespace}, application); err != nil { log.Error(err, "failed to get Application", "application", applicationName) return err @@ -1137,3 +1284,78 @@ func (r *ImageRepositoryReconciler) removePullSecretFromApplicationPullSecret(ct log.Info("Application pull secret updated after removing registry auth", "application", imageRepository.Labels[ApplicationNameLabelName]) return nil } + +// ensureNamespacePullSecret ensures that namespace pull secret exists and add permissions for all image repositories in the namespace +func (r *ImageRepositoryReconciler) ensureNamespacePullSecret(ctx context.Context, imageRepository *imagerepositoryv1alpha1.ImageRepository, namespaceRobot *quay.RobotAccount) error { + log := ctrllog.FromContext(ctx) + + quayImageURL := fmt.Sprintf("quay.io/%s/%s", r.QuayOrganization, imageRepository.Namespace) + if err := r.EnsureSecret(ctx, imageRepository, namespacePullSecretName, namespaceRobot, quayImageURL, false); err != nil { + return err + } + + imageRepositoriesList := &imagerepositoryv1alpha1.ImageRepositoryList{} + if err := r.Client.List(ctx, imageRepositoriesList, &client.ListOptions{Namespace: imageRepository.Namespace}); err != nil { + log.Error(err, "failed to list image repositories") + return err + } + + for _, namespaceImageRepository := range imageRepositoriesList.Items { + if namespaceImageRepository.Status.State != imagerepositoryv1alpha1.ImageRepositoryStateReady { + continue + } + imageRepositoryName := imageRepository.Spec.Image.Name + if err := r.QuayClient.AddPermissionsForRepositoryToAccount(r.QuayOrganization, imageRepositoryName, namespaceRobot.Name, true, false); err != nil { + return err + } + } + + return nil +} + +// getOrCreateNamespaceRobot gets the namespace robot account, creating it if it doesn't exist +func (r *ImageRepositoryReconciler) getOrCreateNamespaceRobot(ctx context.Context, namespace string) (*quay.RobotAccount, error) { + log := ctrllog.FromContext(ctx) + + namespaceRobotName, err := r.getNamespaceRobotName(ctx, namespace) + if err != nil { + return nil, err + } + + namespaceRobot, err := r.QuayClient.GetRobotAccount(r.QuayOrganization, namespaceRobotName) + if err != nil { + log.Error(err, "failed to get namespace robot account", "RobotAccountName", namespaceRobotName) + return nil, err + } + + if namespaceRobot == nil { + namespaceRobot, err = r.QuayClient.CreateRobotAccount(r.QuayOrganization, namespaceRobotName) + if err != nil { + log.Error(err, "failed to create robot account", "RobotAccountName", namespaceRobotName, l.Action, l.ActionAdd, l.Audit, "true") + return nil, err + } + } + + return namespaceRobot, nil +} + +func (r *ImageRepositoryReconciler) getNamespaceRobotName(ctx context.Context, namespace string) (string, error) { + log := ctrllog.FromContext(ctx) + + ns, err := r.getKubeSystemNamespace(ctx) + if err != nil { + log.Error(err, "failed to get the kube system namespace.", "KubeSystemNamespaceName", kubeSystemNamespace) + return "", err + } + + namespaceRobotName := fmt.Sprintf("%s_%s", namespace, ns.UID) + namespaceRobotName = sanitizeNameForQuay(namespaceRobotName) + + return namespaceRobotName, nil +} + +func (r *ImageRepositoryReconciler) getKubeSystemNamespace(ctx context.Context) (*corev1.Namespace, error) { + ns := &corev1.Namespace{} + err := r.Client.Get(ctx, types.NamespacedName{Name: kubeSystemNamespace}, ns) + return ns, err +} diff --git a/internal/controller/imagerepository_controller_service_account.go b/internal/controller/imagerepository_controller_service_account.go index 16480fdd..c58bc1c6 100644 --- a/internal/controller/imagerepository_controller_service_account.go +++ b/internal/controller/imagerepository_controller_service_account.go @@ -32,13 +32,18 @@ import ( // linkSecretToServiceAccount ensures that the given secret is linked with the provided service account, // add also to ImagePullSecrets if requested -func (r *ImageRepositoryReconciler) linkSecretToServiceAccount(ctx context.Context, saName, secretNameToAdd, namespace string, addImagePullSecret bool) error { +func (r *ImageRepositoryReconciler) linkSecretToServiceAccount(ctx context.Context, saName, secretNameToAdd, namespace string, addImagePullSecret, notFoundOk bool) error { log := ctrllog.FromContext(ctx).WithValues("ServiceAccountName", saName, "SecretName", secretNameToAdd) serviceAccount := &corev1.ServiceAccount{} err := r.Client.Get(ctx, types.NamespacedName{Name: saName, Namespace: namespace}, serviceAccount) if err != nil { if errors.IsNotFound(err) { + if notFoundOk { + log.Info("ServiceAccount not found", "ServiceAccountName", saName) + return nil + } + log.Error(err, "service account doesn't exist yet", l.Action, l.ActionView) return err } @@ -242,9 +247,9 @@ func (r *ImageRepositoryReconciler) VerifyAndFixSecretsLinking(ctx context.Conte applicationPullSecretName := getApplicationPullSecretName(applicationName) if isComponentLinked(imageRepository) { - // link secret to service account if isn't linked already - if err := r.linkSecretToServiceAccount(ctx, componentSaName, pushSecretName, imageRepository.Namespace, false); err != nil { - log.Error(err, "failed to link secret to service account", componentSaName, "SecretName", pushSecretName, l.Action, l.ActionUpdate) + // link secret to component service account if isn't linked already + if err := r.linkSecretToServiceAccount(ctx, componentSaName, pushSecretName, imageRepository.Namespace, false, false); err != nil { + log.Error(err, "failed to link secret to component service account", componentSaName, "SecretName", pushSecretName, l.Action, l.ActionUpdate) return err } @@ -254,16 +259,18 @@ func (r *ImageRepositoryReconciler) VerifyAndFixSecretsLinking(ctx context.Conte return err } - // link secret to service account if isn't linked already - if err := r.linkSecretToServiceAccount(ctx, IntegrationTestsServiceAccountName, applicationPullSecretName, imageRepository.Namespace, true); err != nil { - log.Error(err, "failed to link secret to service account", "saName", IntegrationTestsServiceAccountName, "SecretName", applicationPullSecretName, l.Action, l.ActionUpdate) - return err - } + if applicationName != "" { + // link secret to integration service account if isn't linked already + if err := r.linkSecretToServiceAccount(ctx, IntegrationServiceAccountName, applicationPullSecretName, imageRepository.Namespace, true, true); err != nil { + log.Error(err, "failed to link secret to integration service account", "saName", IntegrationServiceAccountName, "SecretName", applicationPullSecretName, l.Action, l.ActionUpdate) + return err + } - // clean duplicate secret links - if err := r.cleanUpSecretInServiceAccount(ctx, IntegrationTestsServiceAccountName, applicationPullSecretName, imageRepository.Namespace, true); err != nil { - log.Error(err, "failed to clean up secret in service account", "saName", IntegrationTestsServiceAccountName, "SecretName", applicationPullSecretName, l.Action, l.ActionUpdate) - return err + // clean duplicate secret links + if err := r.cleanUpSecretInServiceAccount(ctx, IntegrationServiceAccountName, applicationPullSecretName, imageRepository.Namespace, true); err != nil { + log.Error(err, "failed to clean up secret in service account", "saName", IntegrationServiceAccountName, "SecretName", applicationPullSecretName, l.Action, l.ActionUpdate) + return err + } } } diff --git a/internal/controller/imagerepository_controller_test.go b/internal/controller/imagerepository_controller_test.go index 41515043..810ca549 100644 --- a/internal/controller/imagerepository_controller_test.go +++ b/internal/controller/imagerepository_controller_test.go @@ -31,17 +31,25 @@ import ( ) var _ = Describe("Image repository controller", func() { + const ( + namespaceRobotToken = "namespace_token" + namespaceRobotTokenRefreshed1 = "namespace_token_new1" + namespaceRobotTokenRefreshed2 = "namespace_token_new2" + ) var ( - pushToken string - pullToken string - expectedRobotAccountPrefix string - expectedImageName string - expectedImage string + pushToken string + pullToken string + expectedRobotAccountPrefix string + expectedNamespaceRobotAccountName string + expectedNamespaceImage string + expectedImageName string + expectedImage string ) BeforeEach(func() { createNamespace(defaultNamespace) + createNamespace(kubeSystemNamespace) }) Context("Image repository provision without component", func() { @@ -56,7 +64,11 @@ var _ = Describe("Image repository controller", func() { pullToken = "pull-token1234" expectedImageName = fmt.Sprintf("%s/%s", defaultNamespace, resourceKey.Name) expectedImage = fmt.Sprintf("quay.io/%s/%s", quay.TestQuayOrg, expectedImageName) - expectedRobotAccountPrefix = strings.ReplaceAll(strings.ReplaceAll(expectedImageName, "-", "_"), "/", "_") + expectedRobotAccountPrefix = sanitizeNameForQuay(expectedImageName) + kubeSystemNamespace := getNamespace(kubeSystemNamespace) + expectedNamespaceRobotAccountName = fmt.Sprintf("%s_%s", resourceKey.Namespace, kubeSystemNamespace.UID) + expectedNamespaceRobotAccountName = sanitizeNameForQuay(expectedNamespaceRobotAccountName) + expectedNamespaceImage = fmt.Sprintf("quay.io/%s/%s", quay.TestQuayOrg, resourceKey.Namespace) }) It("should provision image repository", func() { @@ -72,9 +84,15 @@ var _ = Describe("Image repository controller", func() { } isCreatePushRobotAccountInvoked := false isCreatePullRobotAccountInvoked := false + isCreateNamespaceRobotAccountInvoked := false quay.CreateRobotAccountFunc = func(organization, robotName string) (*quay.RobotAccount, error) { defer GinkgoRecover() Expect(organization).To(Equal(quay.TestQuayOrg)) + if robotName == expectedNamespaceRobotAccountName { + isCreateNamespaceRobotAccountInvoked = true + return &quay.RobotAccount{Name: robotName, Token: namespaceRobotToken}, nil + } + Expect(strings.HasPrefix(robotName, expectedRobotAccountPrefix)).To(BeTrue()) if strings.HasSuffix(robotName, "_pull") { isCreatePullRobotAccountInvoked = true @@ -85,26 +103,39 @@ var _ = Describe("Image repository controller", func() { } isAddPushPermissionsToAccountInvoked := false isAddPullPermissionsToAccountInvoked := false + isAddPullPermissionsToNamespaceAccountInvoked := false quay.AddPermissionsForRepositoryToAccountFunc = func(organization, imageRepository, accountName string, isRobot, isWrite bool) error { defer GinkgoRecover() Expect(organization).To(Equal(quay.TestQuayOrg)) Expect(imageRepository).To(Equal(expectedImageName)) - Expect(strings.HasPrefix(accountName, expectedRobotAccountPrefix)).To(BeTrue()) - if strings.HasSuffix(accountName, "_pull") { - Expect(isWrite).To(BeFalse()) - isAddPullPermissionsToAccountInvoked = true + + if strings.HasPrefix(accountName, expectedRobotAccountPrefix) || accountName == expectedNamespaceRobotAccountName { + if strings.HasPrefix(accountName, expectedRobotAccountPrefix) { + // method is called for pull or push robot account + if strings.HasSuffix(accountName, "_pull") { + Expect(isWrite).To(BeFalse()) + isAddPullPermissionsToAccountInvoked = true + } else { + Expect(isWrite).To(BeTrue()) + isAddPushPermissionsToAccountInvoked = true + } + } else { + // method is called for namespace pull robot account + Expect(isWrite).To(BeFalse()) + isAddPullPermissionsToNamespaceAccountInvoked = true + } + } else { - Expect(isWrite).To(BeTrue()) - isAddPushPermissionsToAccountInvoked = true + Fail("AddPermissionsForRepositoryToAccountFunc was invoked for unknown robot account") } + return nil } - isCreateNotificationInvoked := false - quay.CreateNotificationFunc = func(organization, repository string, notification quay.Notification) (*quay.Notification, error) { - isCreateNotificationInvoked = true - Expect(organization).To(Equal(quay.TestQuayOrg)) - return &quay.Notification{UUID: "uuid"}, nil + isGetRobotAccountInvoked := false + quay.GetRobotAccountFunc = func(organization, robotName string) (*quay.RobotAccount, error) { + isGetRobotAccountInvoked = true + return nil, nil } createImageRepository(imageRepositoryConfig{ResourceKey: &resourceKey}) @@ -112,13 +143,17 @@ var _ = Describe("Image repository controller", func() { Eventually(func() bool { return isCreateRepositoryInvoked }, timeout, interval).Should(BeTrue()) Eventually(func() bool { return isCreatePullRobotAccountInvoked }, timeout, interval).Should(BeTrue()) Eventually(func() bool { return isCreatePushRobotAccountInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return isGetRobotAccountInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return isCreateNamespaceRobotAccountInvoked }, timeout, interval).Should(BeTrue()) Eventually(func() bool { return isAddPushPermissionsToAccountInvoked }, timeout, interval).Should(BeTrue()) Eventually(func() bool { return isAddPullPermissionsToAccountInvoked }, timeout, interval).Should(BeTrue()) - Eventually(func() bool { return isCreateNotificationInvoked }, timeout, interval).Should(BeFalse()) + Eventually(func() bool { return isAddPullPermissionsToNamespaceAccountInvoked }, timeout, interval).Should(BeTrue()) waitImageRepositoryFinalizerOnImageRepository(resourceKey) imageRepository := getImageRepository(resourceKey) + Expect(imageRepository.Annotations).To(HaveLen(1)) + Expect(imageRepository.Annotations[namespacePullSecretEnsuredAnnotation]).To(Equal("true")) Expect(imageRepository.Spec.Image.Name).To(Equal(expectedImageName)) Expect(imageRepository.Spec.Image.Visibility).To(Equal(imagerepositoryv1alpha1.ImageVisibilityPublic)) Expect(imageRepository.OwnerReferences).To(HaveLen(0)) @@ -144,13 +179,20 @@ var _ = Describe("Image repository controller", func() { defer deleteSecret(pullSecretKey) verifySecretSpec(pullSecret, "ImageRepository", imageRepository.GetName(), pullSecret.Name) + namespaceSecretKey := types.NamespacedName{Name: namespacePullSecretName, Namespace: imageRepository.Namespace} + namespaceSecret := waitSecretExist(namespaceSecretKey) + defer deleteSecret(namespaceSecretKey) + verifySecretSpec(namespaceSecret, "", "", namespacePullSecretName) + pushSecretDockerconfigJson := string(pushSecret.Data[corev1.DockerConfigJsonKey]) verifySecretAuth(pushSecretDockerconfigJson, expectedImage, imageRepository.Status.Credentials.PushRobotAccountName, pushToken) pullSecretDockerconfigJson := string(pullSecret.Data[corev1.DockerConfigJsonKey]) verifySecretAuth(pullSecretDockerconfigJson, expectedImage, imageRepository.Status.Credentials.PullRobotAccountName, pullToken) + namespaceSecretDockerconfigJson := string(namespaceSecret.Data[corev1.DockerConfigJsonKey]) + verifySecretAuth(namespaceSecretDockerconfigJson, expectedNamespaceImage, expectedNamespaceRobotAccountName, namespaceRobotToken) }) - It("should regenerate token", func() { + It("should regenerate pull & push tokens and create pull && push secrets when they don't exist", func() { newPushToken := "push-token5678" newPullToken := "pull-token5678" @@ -188,17 +230,47 @@ var _ = Describe("Image repository controller", func() { pushSecretKey := types.NamespacedName{Name: imageRepository.Status.Credentials.PushSecretName, Namespace: imageRepository.Namespace} pushSecret := waitSecretExist(pushSecretKey) - Expect(pushSecret.Type).To(Equal(corev1.SecretTypeDockerConfigJson)) + verifySecretSpec(pushSecret, "ImageRepository", imageRepository.GetName(), pushSecret.Name) pushSecretDockerconfigJson := string(pushSecret.Data[corev1.DockerConfigJsonKey]) verifySecretAuth(pushSecretDockerconfigJson, expectedImage, imageRepository.Status.Credentials.PushRobotAccountName, newPushToken) pullSecretKey := types.NamespacedName{Name: imageRepository.Status.Credentials.PullSecretName, Namespace: imageRepository.Namespace} pullSecret := waitSecretExist(pullSecretKey) - Expect(pullSecret.Type).To(Equal(corev1.SecretTypeDockerConfigJson)) + verifySecretSpec(pullSecret, "ImageRepository", imageRepository.GetName(), pullSecret.Name) pullSecretDockerconfigJson := string(pullSecret.Data[corev1.DockerConfigJsonKey]) verifySecretAuth(pullSecretDockerconfigJson, expectedImage, imageRepository.Status.Credentials.PullRobotAccountName, newPullToken) }) + It("should regenerate namespace pull token and create namespace pull secret when it doesn't exists", func() { + newNamespaceToken := namespaceRobotTokenRefreshed1 + + isRegenerateNamespaceRobotAccountTokenInvoked := false + quay.RegenerateRobotAccountTokenFunc = func(organization, robotName string) (*quay.RobotAccount, error) { + defer GinkgoRecover() + Expect(organization).To(Equal(quay.TestQuayOrg)) + Expect(robotName).To(Equal(expectedNamespaceRobotAccountName)) + isRegenerateNamespaceRobotAccountTokenInvoked = true + return &quay.RobotAccount{Name: robotName, Token: newNamespaceToken}, nil + } + + imageRepository := getImageRepository(resourceKey) + regenerateToken := true + imageRepository.Spec.Credentials = &imagerepositoryv1alpha1.ImageCredentials{RegenerateNamespacePullToken: ®enerateToken} + Expect(k8sClient.Update(ctx, imageRepository)).To(Succeed()) + + Eventually(func() bool { return isRegenerateNamespaceRobotAccountTokenInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { + imageRepository := getImageRepository(resourceKey) + return imageRepository.Spec.Credentials.RegenerateNamespacePullToken == nil + }, timeout, interval).Should(BeTrue()) + + namespaceSecretKey := types.NamespacedName{Name: namespacePullSecretName, Namespace: imageRepository.Namespace} + namespaceSecret := waitSecretExist(namespaceSecretKey) + verifySecretSpec(namespaceSecret, "", "", namespacePullSecretName) + namespaceSecretDockerconfigJson := string(namespaceSecret.Data[corev1.DockerConfigJsonKey]) + verifySecretAuth(namespaceSecretDockerconfigJson, expectedNamespaceImage, expectedNamespaceRobotAccountName, newNamespaceToken) + }) + It("should update image visibility", func() { isChangeRepositoryVisibilityInvoked := false quay.ChangeRepositoryVisibilityFunc = func(organization, imageRepository, visibility string) error { @@ -245,6 +317,60 @@ var _ = Describe("Image repository controller", func() { }, timeout, interval).Should(BeTrue()) }) + // TODO: remove after all IRs are processed and all have new namespace pull ensured annotation + It("old already provisioned image repository without annotation will on next reconcile create namespace pull secret and robot account", func() { + // Delete the namespace secret to verify it gets recreated + namespaceSecretKey := types.NamespacedName{Name: namespacePullSecretName, Namespace: resourceKey.Namespace} + deleteSecret(namespaceSecretKey) + + isGetRobotAccountInvoked := false + quay.GetRobotAccountFunc = func(organization, robotName string) (*quay.RobotAccount, error) { + isGetRobotAccountInvoked = true + return nil, nil + } + isCreateNamespaceRobotAccountInvoked := false + quay.CreateRobotAccountFunc = func(organization, robotName string) (*quay.RobotAccount, error) { + defer GinkgoRecover() + Expect(organization).To(Equal(quay.TestQuayOrg)) + if robotName == expectedNamespaceRobotAccountName { + isCreateNamespaceRobotAccountInvoked = true + return &quay.RobotAccount{Name: robotName, Token: namespaceRobotTokenRefreshed1}, nil + } + return nil, nil + } + isAddPullPermissionsToNamespaceAccountInvoked := false + quay.AddPermissionsForRepositoryToAccountFunc = func(organization, imageRepository, accountName string, isRobot, isWrite bool) error { + defer GinkgoRecover() + Expect(organization).To(Equal(quay.TestQuayOrg)) + Expect(imageRepository).To(Equal(expectedImageName)) + if accountName == expectedNamespaceRobotAccountName { + Expect(isWrite).To(BeFalse()) + isAddPullPermissionsToNamespaceAccountInvoked = true + } + return nil + } + + // remove namespace pull secret ensured annotation to force new reconcile and simulate old IR + imageRepository := getImageRepository(resourceKey) + delete(imageRepository.Annotations, namespacePullSecretEnsuredAnnotation) + Expect(k8sClient.Update(ctx, imageRepository)).To(Succeed()) + + // Wait for the annotation to be set to true + Eventually(func() bool { + imageRepository := getImageRepository(resourceKey) + return imageRepository.Annotations[namespacePullSecretEnsuredAnnotation] == "true" + }, timeout, interval).Should(BeTrue()) + + Eventually(func() bool { return isGetRobotAccountInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return isCreateNamespaceRobotAccountInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return isAddPullPermissionsToNamespaceAccountInvoked }, timeout, interval).Should(BeTrue()) + + namespaceSecret := waitSecretExist(namespaceSecretKey) + verifySecretSpec(namespaceSecret, "", "", namespacePullSecretName) + namespaceSecretDockerconfigJson := string(namespaceSecret.Data[corev1.DockerConfigJsonKey]) + verifySecretAuth(namespaceSecretDockerconfigJson, expectedNamespaceImage, expectedNamespaceRobotAccountName, namespaceRobotTokenRefreshed1) + }) + It("should cleanup repository", func() { isDeleteRobotAccountForPushInvoked := false isDeleteRobotAccountForPullInvoked := false @@ -267,12 +393,33 @@ var _ = Describe("Image repository controller", func() { Expect(imageRepository).To(Equal(expectedImageName)) return true, nil } + isGetRobotAccountInvoked := false + quay.GetRobotAccountFunc = func(organization, robotName string) (*quay.RobotAccount, error) { + isGetRobotAccountInvoked = true + return &quay.RobotAccount{Name: expectedNamespaceRobotAccountName, Token: namespaceRobotToken}, nil + } + isRemovePermissionsToRepositoryForAccountInvoked := false + quay.RemovePermissionsForRepositoryFromAccountFunc = func(organization, imageRepository, accountName string, isRobot bool) error { + isRemovePermissionsToRepositoryForAccountInvoked = true + Expect(organization).To(Equal(quay.TestQuayOrg)) + Expect(imageRepository).To(Equal(expectedImageName)) + Expect(accountName).To(Equal(expectedNamespaceRobotAccountName)) + return nil + } deleteImageRepository(resourceKey) Eventually(func() bool { return isDeleteRobotAccountForPullInvoked }, timeout, interval).Should(BeTrue()) Eventually(func() bool { return isDeleteRobotAccountForPushInvoked }, timeout, interval).Should(BeTrue()) Eventually(func() bool { return isDeleteRepositoryInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return isGetRobotAccountInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return isRemovePermissionsToRepositoryForAccountInvoked }, timeout, interval).Should(BeTrue()) + + namespaceSecretKey := types.NamespacedName{Name: namespacePullSecretName, Namespace: resourceKey.Namespace} + namespaceSecret := waitSecretExist(namespaceSecretKey) + verifySecretSpec(namespaceSecret, "", "", namespacePullSecretName) + namespaceSecretDockerconfigJson := string(namespaceSecret.Data[corev1.DockerConfigJsonKey]) + verifySecretAuth(namespaceSecretDockerconfigJson, expectedNamespaceImage, expectedNamespaceRobotAccountName, namespaceRobotTokenRefreshed1) }) }) @@ -286,7 +433,7 @@ var _ = Describe("Image repository controller", func() { BeforeEach(func() { quay.ResetTestQuayClientToFails() createApplication(applicationConfig{}) - createComponent(componentConfig{}) + createComponent(componentConfig{ComponentApplication: defaultComponentApplication}) }) AfterEach(func() { @@ -299,10 +446,13 @@ var _ = Describe("Image repository controller", func() { pullToken = "pull-token1234" expectedImageName = fmt.Sprintf("%s/%s", defaultNamespace, defaultComponentName) expectedImage = fmt.Sprintf("quay.io/%s/%s", quay.TestQuayOrg, expectedImageName) - expectedRobotAccountPrefix = strings.ReplaceAll(strings.ReplaceAll(expectedImageName, "-", "_"), "/", "_") + expectedRobotAccountPrefix = sanitizeNameForQuay(expectedImageName) + kubeSystemNamespace := getNamespace(kubeSystemNamespace) + expectedNamespaceRobotAccountName = fmt.Sprintf("%s_%s", resourceKey.Namespace, kubeSystemNamespace.UID) + expectedNamespaceRobotAccountName = sanitizeNameForQuay(expectedNamespaceRobotAccountName) createServiceAccount(defaultNamespace, componentSaName) - createServiceAccount(defaultNamespace, IntegrationTestsServiceAccountName) + createServiceAccount(defaultNamespace, IntegrationServiceAccountName) // wait for application SA to be created Eventually(func() bool { @@ -313,7 +463,7 @@ var _ = Describe("Image repository controller", func() { }, timeout, interval).WithTimeout(ensureTimeout).Should(BeTrue()) }) - assertProvisionRepository := func(updateComponentAnnotation, grantRepoPermission bool) { + assertProvisionRepository := func(updateComponentAnnotation, grantRepoPermission, setNotification bool) { quay.RepositoryExistsFunc = func(organization, imageRepository string) (bool, error) { return true, nil } @@ -329,9 +479,15 @@ var _ = Describe("Image repository controller", func() { } isCreatePushRobotAccountInvoked := false isCreatePullRobotAccountInvoked := false + isCreateNamespaceRobotAccountInvoked := false quay.CreateRobotAccountFunc = func(organization, robotName string) (*quay.RobotAccount, error) { defer GinkgoRecover() Expect(organization).To(Equal(quay.TestQuayOrg)) + if robotName == expectedNamespaceRobotAccountName { + isCreateNamespaceRobotAccountInvoked = true + return &quay.RobotAccount{Name: robotName, Token: namespaceRobotToken}, nil + } + Expect(strings.HasPrefix(robotName, expectedRobotAccountPrefix)).To(BeTrue()) if strings.HasSuffix(robotName, "_pull") { isCreatePullRobotAccountInvoked = true @@ -342,17 +498,29 @@ var _ = Describe("Image repository controller", func() { } isAddPushPermissionsToAccountInvoked := false isAddPullPermissionsToAccountInvoked := false + isAddPullPermissionsToNamespaceAccountInvoked := false quay.AddPermissionsForRepositoryToAccountFunc = func(organization, imageRepository, accountName string, isRobot, isWrite bool) error { defer GinkgoRecover() Expect(organization).To(Equal(quay.TestQuayOrg)) Expect(imageRepository).To(Equal(expectedImageName)) - Expect(strings.HasPrefix(accountName, expectedRobotAccountPrefix)).To(BeTrue()) - if strings.HasSuffix(accountName, "_pull") { - Expect(isWrite).To(BeFalse()) - isAddPullPermissionsToAccountInvoked = true + if strings.HasPrefix(accountName, expectedRobotAccountPrefix) || accountName == expectedNamespaceRobotAccountName { + if strings.HasPrefix(accountName, expectedRobotAccountPrefix) { + // method is called for pull or push robot account + if strings.HasSuffix(accountName, "_pull") { + Expect(isWrite).To(BeFalse()) + isAddPullPermissionsToAccountInvoked = true + } else { + Expect(isWrite).To(BeTrue()) + isAddPushPermissionsToAccountInvoked = true + } + } else { + // method is called for namespace pull robot account + Expect(isWrite).To(BeFalse()) + isAddPullPermissionsToNamespaceAccountInvoked = true + } + } else { - Expect(isWrite).To(BeTrue()) - isAddPushPermissionsToAccountInvoked = true + Fail("AddPermissionsForRepositoryToAccountFunc was invoked for unknown robot account") } return nil } @@ -378,25 +546,38 @@ var _ = Describe("Image repository controller", func() { } } isCreateNotificationInvoked := false - quay.CreateNotificationFunc = func(organization, repository string, notification quay.Notification) (*quay.Notification, error) { - isCreateNotificationInvoked = true - Expect(organization).To(Equal(quay.TestQuayOrg)) - return &quay.Notification{UUID: "uuid"}, nil - } isGetNotificationsInvoked := false - quay.GetNotificationsFunc = func(organization, repository string) ([]quay.Notification, error) { - isGetNotificationsInvoked = true - Expect(organization).To(Equal(quay.TestQuayOrg)) - return []quay.Notification{ - { - Title: "test-notification", - Event: string(imagerepositoryv1alpha1.NotificationEventRepoPush), - Method: string(imagerepositoryv1alpha1.NotificationMethodWebhook), - Config: quay.NotificationConfig{ - Url: "http://test-url", + if setNotification { + quay.CreateNotificationFunc = func(organization, repository string, notification quay.Notification) (*quay.Notification, error) { + isCreateNotificationInvoked = true + Expect(organization).To(Equal(quay.TestQuayOrg)) + return &quay.Notification{UUID: "uuid"}, nil + } + quay.GetNotificationsFunc = func(organization, repository string) ([]quay.Notification, error) { + isGetNotificationsInvoked = true + Expect(organization).To(Equal(quay.TestQuayOrg)) + return []quay.Notification{ + { + Title: "test-notification", + Event: string(imagerepositoryv1alpha1.NotificationEventRepoPush), + Method: string(imagerepositoryv1alpha1.NotificationMethodWebhook), + Config: quay.NotificationConfig{ + Url: "http://test-url", + }, }, - }, - }, nil + }, nil + } + } + isGetRobotAccountInvoked := false + quay.GetRobotAccountFunc = func(organization, robotName string) (*quay.RobotAccount, error) { + isGetRobotAccountInvoked = true + return nil, nil + } + quay.RemovePermissionsForRepositoryFromAccountFunc = func(organization, imageRepository, accountName string, isRobot bool) error { + Expect(organization).To(Equal(quay.TestQuayOrg)) + Expect(imageRepository).To(Equal(expectedImageName)) + Expect(accountName).To(Equal(expectedNamespaceRobotAccountName)) + return nil } imageRepositoryConfigObject := imageRepositoryConfig{ @@ -405,7 +586,9 @@ var _ = Describe("Image repository controller", func() { ApplicationNameLabelName: defaultComponentApplication, ComponentNameLabelName: defaultComponentName, }, - Notifications: []imagerepositoryv1alpha1.Notifications{ + } + if setNotification { + imageRepositoryConfigObject.Notifications = []imagerepositoryv1alpha1.Notifications{ { Title: "test-notification", Event: imagerepositoryv1alpha1.NotificationEventRepoPush, @@ -414,7 +597,7 @@ var _ = Describe("Image repository controller", func() { Url: "http://test-url", }, }, - }, + } } if updateComponentAnnotation { @@ -426,9 +609,15 @@ var _ = Describe("Image repository controller", func() { Eventually(func() bool { return isCreateRepositoryInvoked }, timeout, interval).Should(BeTrue()) Eventually(func() bool { return isCreatePushRobotAccountInvoked }, timeout, interval).Should(BeTrue()) Eventually(func() bool { return isCreatePullRobotAccountInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return isCreateNamespaceRobotAccountInvoked }, timeout, interval).Should(BeTrue()) Eventually(func() bool { return isAddPushPermissionsToAccountInvoked }, timeout, interval).Should(BeTrue()) Eventually(func() bool { return isAddPullPermissionsToAccountInvoked }, timeout, interval).Should(BeTrue()) - Eventually(func() bool { return isCreateNotificationInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return isAddPullPermissionsToNamespaceAccountInvoked }, timeout, interval).Should(BeTrue()) + if setNotification { + Eventually(func() bool { return isCreateNotificationInvoked }, timeout, interval).Should(BeTrue()) + } + Eventually(func() bool { return isGetRobotAccountInvoked }, timeout, interval).Should(BeTrue()) + if grantRepoPermission { Eventually(func() bool { return isEnsureTeamInvoked }, timeout, interval).Should(BeTrue()) Eventually(func() bool { return isAddReadPermissionsForRepositoryToTeamInvoked }, timeout, interval).Should(BeTrue()) @@ -439,14 +628,16 @@ var _ = Describe("Image repository controller", func() { component := getComponent(componentKey) imageRepository := getImageRepository(resourceKey) - Eventually(func() bool { return isGetNotificationsInvoked }, timeout, interval).Should(BeTrue()) + if setNotification { + Eventually(func() bool { return isGetNotificationsInvoked }, timeout, interval).Should(BeTrue()) + } if updateComponentAnnotation { Expect(component.Spec.ContainerImage).To(Equal(imageRepository.Status.Image.URL)) - Expect(imageRepository.Annotations).To(HaveLen(0)) } else { Expect(component.Spec.ContainerImage).To(BeEmpty()) } - + Expect(imageRepository.Annotations).To(HaveLen(1)) + Expect(imageRepository.Annotations[namespacePullSecretEnsuredAnnotation]).To(Equal("true")) Expect(imageRepository.Spec.Image.Name).To(Equal(expectedImageName)) Expect(imageRepository.Spec.Image.Visibility).To(Equal(imagerepositoryv1alpha1.ImageVisibilityPublic)) Expect(imageRepository.OwnerReferences).To(HaveLen(1)) @@ -463,9 +654,13 @@ var _ = Describe("Image repository controller", func() { Expect(imageRepository.Status.Credentials.PullRobotAccountName).To(HaveSuffix("_pull")) Expect(imageRepository.Status.Credentials.PullSecretName).To(Equal(imageRepository.Name + "-image-pull")) Expect(imageRepository.Status.Credentials.GenerationTimestamp).ToNot(BeNil()) - Expect(imageRepository.Status.Notifications).To(HaveLen(1)) - Expect(imageRepository.Status.Notifications[0].UUID).To(Equal("uuid")) - Expect(imageRepository.Status.Notifications[0].Title).To(Equal("test-notification")) + if setNotification { + Expect(imageRepository.Status.Notifications).To(HaveLen(1)) + Expect(imageRepository.Status.Notifications[0].UUID).To(Equal("uuid")) + Expect(imageRepository.Status.Notifications[0].Title).To(Equal("test-notification")) + } else { + Expect(imageRepository.Status.Notifications).To(HaveLen(0)) + } pushSecretKey := types.NamespacedName{Name: imageRepository.Status.Credentials.PushSecretName, Namespace: imageRepository.Namespace} pushSecret := waitSecretExist(pushSecretKey) @@ -482,35 +677,50 @@ var _ = Describe("Image repository controller", func() { defer deleteSecret(applicationSecretKey) verifySecretSpec(applicationSecret, "Application", applicationKey.Name, applicationSecretName) + namespaceSecretKey := types.NamespacedName{Name: namespacePullSecretName, Namespace: imageRepository.Namespace} + namespaceSecret := waitSecretExist(namespaceSecretKey) + defer deleteSecret(namespaceSecretKey) + verifySecretSpec(namespaceSecret, "", "", namespacePullSecretName) + pushSecretDockerconfigJson := string(pushSecret.Data[corev1.DockerConfigJsonKey]) verifySecretAuth(pushSecretDockerconfigJson, expectedImage, imageRepository.Status.Credentials.PushRobotAccountName, pushToken) pullSecretDockerconfigJson := string(pullSecret.Data[corev1.DockerConfigJsonKey]) verifySecretAuth(pullSecretDockerconfigJson, expectedImage, imageRepository.Status.Credentials.PullRobotAccountName, pullToken) applicationSecretDockerconfigJson := string(applicationSecret.Data[corev1.DockerConfigJsonKey]) verifySecretAuth(applicationSecretDockerconfigJson, expectedImage, imageRepository.Status.Credentials.PullRobotAccountName, pullToken) + namespaceSecretDockerconfigJson := string(namespaceSecret.Data[corev1.DockerConfigJsonKey]) + verifySecretAuth(namespaceSecretDockerconfigJson, expectedNamespaceImage, expectedNamespaceRobotAccountName, namespaceRobotToken) componentSa := getServiceAccount(defaultNamespace, componentSaName) - Expect(componentSa.Secrets).To(HaveLen(1)) + Expect(componentSa.Secrets).To(HaveLen(2)) Expect(componentSa.ImagePullSecrets).To(HaveLen(0)) Expect(componentSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: pushSecret.Name})) - applicationSa := getServiceAccount(defaultNamespace, IntegrationTestsServiceAccountName) - Expect(applicationSa.Secrets).To(HaveLen(1)) - Expect(applicationSa.ImagePullSecrets).To(HaveLen(1)) - Expect(applicationSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: applicationSecretName})) - Expect(applicationSa.ImagePullSecrets).To(ContainElement(corev1.LocalObjectReference{Name: applicationSecretName})) + Expect(componentSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: namespacePullSecretName})) + integrationSa := getServiceAccount(defaultNamespace, IntegrationServiceAccountName) + Expect(integrationSa.Secrets).To(HaveLen(2)) + Expect(integrationSa.ImagePullSecrets).To(HaveLen(2)) + Expect(integrationSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: applicationSecretName})) + Expect(integrationSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: namespacePullSecretName})) + Expect(integrationSa.ImagePullSecrets).To(ContainElement(corev1.LocalObjectReference{Name: applicationSecretName})) + Expect(integrationSa.ImagePullSecrets).To(ContainElement(corev1.LocalObjectReference{Name: namespacePullSecretName})) } assertSecretsGoneFromServiceAccounts := func() { componentSa := getServiceAccount(defaultNamespace, componentSaName) - Expect(componentSa.Secrets).To(HaveLen(0)) + Expect(componentSa.Secrets).To(HaveLen(1)) + Expect(componentSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: namespacePullSecretName})) Expect(componentSa.ImagePullSecrets).To(HaveLen(0)) - applicationSa := getServiceAccount(defaultNamespace, IntegrationTestsServiceAccountName) - Expect(applicationSa.Secrets).To(HaveLen(1)) - Expect(applicationSa.ImagePullSecrets).To(HaveLen(1)) + integrationSa := getServiceAccount(defaultNamespace, IntegrationServiceAccountName) + Expect(integrationSa.Secrets).To(HaveLen(2)) + Expect(integrationSa.ImagePullSecrets).To(HaveLen(2)) + Expect(integrationSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: applicationSecretName})) + Expect(integrationSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: namespacePullSecretName})) + Expect(integrationSa.ImagePullSecrets).To(ContainElement(corev1.LocalObjectReference{Name: applicationSecretName})) + Expect(integrationSa.ImagePullSecrets).To(ContainElement(corev1.LocalObjectReference{Name: namespacePullSecretName})) } It("should provision image repository for component, without update component annotation", func() { - assertProvisionRepository(false, false) + assertProvisionRepository(false, false, true) quay.DeleteRobotAccountFunc = func(organization, robotAccountName string) (bool, error) { return true, nil @@ -560,7 +770,7 @@ var _ = Describe("Image repository controller", func() { Eventually(func() int { return countAddUserToTeamInvoked }, timeout, interval).Should(Equal(2)) waitQuayTeamUsersFinalizerOnConfigMap(usersConfigMapKey) - assertProvisionRepository(true, true) + assertProvisionRepository(true, true, true) quay.DeleteTeamFunc = func(organization, teamName string) error { defer GinkgoRecover() @@ -583,10 +793,10 @@ var _ = Describe("Image repository controller", func() { }) It("should provision image repository for component, with update component annotation", func() { - assertProvisionRepository(true, false) + assertProvisionRepository(true, false, false) }) - It("should regenerate tokens and update secrets (secrets don't exist)", func() { + It("should regenerate pull & push tokens and create pull && push secrets when they don't exist", func() { newPushToken := "push-token5678" newPullToken := "pull-token5678" @@ -609,21 +819,6 @@ var _ = Describe("Image repository controller", func() { isRegenerateRobotAccountTokenForPushInvoked = true return &quay.RobotAccount{Name: robotName, Token: newPushToken}, nil } - isGetNotificationsInvoked := false - quay.GetNotificationsFunc = func(organization, repository string) ([]quay.Notification, error) { - isGetNotificationsInvoked = true - Expect(organization).To(Equal(quay.TestQuayOrg)) - return []quay.Notification{ - { - Title: "test-notification", - Event: string(imagerepositoryv1alpha1.NotificationEventRepoPush), - Method: string(imagerepositoryv1alpha1.NotificationMethodWebhook), - Config: quay.NotificationConfig{ - Url: "http://test-url", - }, - }, - }, nil - } imageRepository := getImageRepository(resourceKey) oldTokenGenerationTimestamp := *imageRepository.Status.Credentials.GenerationTimestamp @@ -639,31 +834,30 @@ var _ = Describe("Image repository controller", func() { imageRepository.Status.Credentials.GenerationTimestamp != nil && *imageRepository.Status.Credentials.GenerationTimestamp != oldTokenGenerationTimestamp }, timeout, interval).Should(BeTrue()) - Eventually(func() bool { return isGetNotificationsInvoked }, timeout, interval).Should(BeTrue()) pushSecretKey := types.NamespacedName{Name: imageRepository.Status.Credentials.PushSecretName, Namespace: imageRepository.Namespace} pushSecret := waitSecretExist(pushSecretKey) + verifySecretSpec(pushSecret, "ImageRepository", imageRepository.GetName(), pushSecret.Name) pullSecretKey := types.NamespacedName{Name: imageRepository.Status.Credentials.PullSecretName, Namespace: imageRepository.Namespace} pullSecret := waitSecretExist(pullSecretKey) + verifySecretSpec(pullSecret, "ImageRepository", imageRepository.GetName(), pullSecret.Name) applicationSecretKey := types.NamespacedName{Name: applicationSecretName, Namespace: imageRepository.Namespace} applicationSecret := waitSecretExist(applicationSecretKey) + verifySecretSpec(applicationSecret, "Application", imageRepository.Labels[ApplicationNameLabelName], applicationSecretName) - Expect(pushSecret.Type).To(Equal(corev1.SecretTypeDockerConfigJson)) pushSecretDockerconfigJson := string(pushSecret.Data[corev1.DockerConfigJsonKey]) verifySecretAuth(pushSecretDockerconfigJson, expectedImage, imageRepository.Status.Credentials.PushRobotAccountName, newPushToken) - Expect(pullSecret.Type).To(Equal(corev1.SecretTypeDockerConfigJson)) pullSecretDockerconfigJson := string(pullSecret.Data[corev1.DockerConfigJsonKey]) verifySecretAuth(pullSecretDockerconfigJson, expectedImage, imageRepository.Status.Credentials.PullRobotAccountName, newPullToken) - Expect(applicationSecret.Type).To(Equal(corev1.SecretTypeDockerConfigJson)) applicationSecretDockerconfigJson := string(applicationSecret.Data[corev1.DockerConfigJsonKey]) verifySecretAuth(applicationSecretDockerconfigJson, expectedImage, imageRepository.Status.Credentials.PullRobotAccountName, newPullToken) }) - It("should regenerate tokens and update secrets (secrets exist)", func() { + It("should regenerate pull & push tokens and update pull && push secrets when they exist", func() { newPushToken := "push-token98765" newPullToken := "pull-token98765" @@ -686,21 +880,6 @@ var _ = Describe("Image repository controller", func() { isRegenerateRobotAccountTokenForPushInvoked = true return &quay.RobotAccount{Name: robotName, Token: newPushToken}, nil } - isGetNotificationsInvoked := false - quay.GetNotificationsFunc = func(organization, repository string) ([]quay.Notification, error) { - isGetNotificationsInvoked = true - Expect(organization).To(Equal(quay.TestQuayOrg)) - return []quay.Notification{ - { - Title: "test-notification", - Event: string(imagerepositoryv1alpha1.NotificationEventRepoPush), - Method: string(imagerepositoryv1alpha1.NotificationMethodWebhook), - Config: quay.NotificationConfig{ - Url: "http://test-url", - }, - }, - }, nil - } imageRepository := getImageRepository(resourceKey) oldTokenGenerationTimestamp := *imageRepository.Status.Credentials.GenerationTimestamp @@ -716,34 +895,93 @@ var _ = Describe("Image repository controller", func() { imageRepository.Status.Credentials.GenerationTimestamp != nil && *imageRepository.Status.Credentials.GenerationTimestamp != oldTokenGenerationTimestamp }, timeout, interval).Should(BeTrue()) - Eventually(func() bool { return isGetNotificationsInvoked }, timeout, interval).Should(BeTrue()) pushSecretKey := types.NamespacedName{Name: imageRepository.Status.Credentials.PushSecretName, Namespace: imageRepository.Namespace} pushSecret := waitSecretExist(pushSecretKey) + verifySecretSpec(pushSecret, "ImageRepository", imageRepository.GetName(), pushSecret.Name) pullSecretKey := types.NamespacedName{Name: imageRepository.Status.Credentials.PullSecretName, Namespace: imageRepository.Namespace} pullSecret := waitSecretExist(pullSecretKey) + verifySecretSpec(pullSecret, "ImageRepository", imageRepository.GetName(), pullSecret.Name) applicationSecretKey := types.NamespacedName{Name: applicationSecretName, Namespace: imageRepository.Namespace} applicationSecret := waitSecretExist(applicationSecretKey) + verifySecretSpec(applicationSecret, "Application", imageRepository.Labels[ApplicationNameLabelName], applicationSecretName) - Expect(pushSecret.Type).To(Equal(corev1.SecretTypeDockerConfigJson)) pushSecretDockerconfigJson := string(pushSecret.Data[corev1.DockerConfigJsonKey]) verifySecretAuth(pushSecretDockerconfigJson, expectedImage, imageRepository.Status.Credentials.PushRobotAccountName, newPushToken) - Expect(pullSecret.Type).To(Equal(corev1.SecretTypeDockerConfigJson)) pullSecretDockerconfigJson := string(pullSecret.Data[corev1.DockerConfigJsonKey]) verifySecretAuth(pullSecretDockerconfigJson, expectedImage, imageRepository.Status.Credentials.PullRobotAccountName, newPullToken) - Expect(applicationSecret.Type).To(Equal(corev1.SecretTypeDockerConfigJson)) applicationSecretDockerconfigJson := string(applicationSecret.Data[corev1.DockerConfigJsonKey]) verifySecretAuth(applicationSecretDockerconfigJson, expectedImage, imageRepository.Status.Credentials.PullRobotAccountName, newPullToken) }) + It("should regenerate namespace pull token and create namespace pull secret when it doesn't exists", func() { + newNamespaceToken := namespaceRobotTokenRefreshed1 + + isRegenerateNamespaceRobotAccountTokenInvoked := false + quay.RegenerateRobotAccountTokenFunc = func(organization, robotName string) (*quay.RobotAccount, error) { + defer GinkgoRecover() + Expect(organization).To(Equal(quay.TestQuayOrg)) + Expect(robotName).To(Equal(expectedNamespaceRobotAccountName)) + isRegenerateNamespaceRobotAccountTokenInvoked = true + return &quay.RobotAccount{Name: robotName, Token: newNamespaceToken}, nil + } + + imageRepository := getImageRepository(resourceKey) + regenerateToken := true + imageRepository.Spec.Credentials = &imagerepositoryv1alpha1.ImageCredentials{RegenerateNamespacePullToken: ®enerateToken} + Expect(k8sClient.Update(ctx, imageRepository)).To(Succeed()) + + Eventually(func() bool { return isRegenerateNamespaceRobotAccountTokenInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { + imageRepository := getImageRepository(resourceKey) + return imageRepository.Spec.Credentials.RegenerateNamespacePullToken == nil + }, timeout, interval).Should(BeTrue()) + + namespaceSecretKey := types.NamespacedName{Name: namespacePullSecretName, Namespace: imageRepository.Namespace} + namespaceSecret := waitSecretExist(namespaceSecretKey) + verifySecretSpec(namespaceSecret, "", "", namespacePullSecretName) + namespaceSecretDockerconfigJson := string(namespaceSecret.Data[corev1.DockerConfigJsonKey]) + verifySecretAuth(namespaceSecretDockerconfigJson, expectedNamespaceImage, expectedNamespaceRobotAccountName, newNamespaceToken) + }) + + It("should regenerate namespace pull token and update namespace pull secret when it exists", func() { + newNamespaceToken := namespaceRobotTokenRefreshed2 + + isRegenerateNamespaceRobotAccountTokenInvoked := false + quay.RegenerateRobotAccountTokenFunc = func(organization, robotName string) (*quay.RobotAccount, error) { + defer GinkgoRecover() + Expect(organization).To(Equal(quay.TestQuayOrg)) + Expect(robotName).To(Equal(expectedNamespaceRobotAccountName)) + isRegenerateNamespaceRobotAccountTokenInvoked = true + return &quay.RobotAccount{Name: robotName, Token: newNamespaceToken}, nil + } + + imageRepository := getImageRepository(resourceKey) + regenerateToken := true + imageRepository.Spec.Credentials = &imagerepositoryv1alpha1.ImageCredentials{RegenerateNamespacePullToken: ®enerateToken} + Expect(k8sClient.Update(ctx, imageRepository)).To(Succeed()) + + Eventually(func() bool { return isRegenerateNamespaceRobotAccountTokenInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { + imageRepository := getImageRepository(resourceKey) + return imageRepository.Spec.Credentials.RegenerateNamespacePullToken == nil + }, timeout, interval).Should(BeTrue()) + + namespaceSecretKey := types.NamespacedName{Name: namespacePullSecretName, Namespace: imageRepository.Namespace} + namespaceSecret := waitSecretExist(namespaceSecretKey) + verifySecretSpec(namespaceSecret, "", "", namespacePullSecretName) + namespaceSecretDockerconfigJson := string(namespaceSecret.Data[corev1.DockerConfigJsonKey]) + verifySecretAuth(namespaceSecretDockerconfigJson, expectedNamespaceImage, expectedNamespaceRobotAccountName, newNamespaceToken) + }) + It("verify and fix, secret is missing from SAs", func() { quay.ResetTestQuayClient() - applicationSa := getServiceAccount(defaultNamespace, IntegrationTestsServiceAccountName) + applicationSa := getServiceAccount(defaultNamespace, IntegrationServiceAccountName) applicationSa.Secrets = []corev1.ObjectReference{} applicationSa.ImagePullSecrets = []corev1.LocalObjectReference{} Expect(k8sClient.Update(ctx, &applicationSa)).To(Succeed()) @@ -761,22 +999,25 @@ var _ = Describe("Image repository controller", func() { waitImageRepositoryCredentialSectionRequestGone(resourceKey, "verify") pushSecretName := fmt.Sprintf("%s-image-push", resourceKey.Name) - applicationSa = getServiceAccount(defaultNamespace, IntegrationTestsServiceAccountName) + applicationSa = getServiceAccount(defaultNamespace, IntegrationServiceAccountName) componentSa = getServiceAccount(defaultNamespace, componentSaName) - Expect(componentSa.Secrets).To(HaveLen(1)) + Expect(componentSa.Secrets).To(HaveLen(2)) Expect(componentSa.ImagePullSecrets).To(HaveLen(0)) Expect(componentSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: pushSecretName})) - Expect(applicationSa.Secrets).To(HaveLen(1)) - Expect(applicationSa.ImagePullSecrets).To(HaveLen(1)) + Expect(componentSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: namespacePullSecretName})) + Expect(applicationSa.Secrets).To(HaveLen(2)) + Expect(applicationSa.ImagePullSecrets).To(HaveLen(2)) Expect(applicationSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: applicationSecretName})) + Expect(applicationSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: namespacePullSecretName})) Expect(applicationSa.ImagePullSecrets).To(ContainElement(corev1.LocalObjectReference{Name: applicationSecretName})) + Expect(applicationSa.ImagePullSecrets).To(ContainElement(corev1.LocalObjectReference{Name: namespacePullSecretName})) }) It("verify and fix, secret is duplicated in SA, also is in ImagePullSecrets", func() { quay.ResetTestQuayClient() pushSecretName := fmt.Sprintf("%s-image-push", resourceKey.Name) - applicationSa := getServiceAccount(defaultNamespace, IntegrationTestsServiceAccountName) + applicationSa := getServiceAccount(defaultNamespace, IntegrationServiceAccountName) applicationSa.Secrets = []corev1.ObjectReference{{Name: applicationSecretName}, {Name: applicationSecretName}} applicationSa.ImagePullSecrets = []corev1.LocalObjectReference{{Name: applicationSecretName}, {Name: applicationSecretName}} Expect(k8sClient.Update(ctx, &applicationSa)).To(Succeed()) @@ -794,15 +1035,80 @@ var _ = Describe("Image repository controller", func() { waitImageRepositoryCredentialSectionRequestGone(resourceKey, "verify") - applicationSa = getServiceAccount(defaultNamespace, IntegrationTestsServiceAccountName) + applicationSa = getServiceAccount(defaultNamespace, IntegrationServiceAccountName) componentSa = getServiceAccount(defaultNamespace, componentSaName) - Expect(componentSa.Secrets).To(HaveLen(1)) + Expect(componentSa.Secrets).To(HaveLen(2)) Expect(componentSa.ImagePullSecrets).To(HaveLen(0)) Expect(componentSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: pushSecretName})) - Expect(applicationSa.Secrets).To(HaveLen(1)) - Expect(applicationSa.ImagePullSecrets).To(HaveLen(1)) + Expect(componentSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: namespacePullSecretName})) + Expect(applicationSa.Secrets).To(HaveLen(2)) + Expect(applicationSa.ImagePullSecrets).To(HaveLen(2)) Expect(applicationSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: applicationSecretName})) + Expect(applicationSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: namespacePullSecretName})) Expect(applicationSa.ImagePullSecrets).To(ContainElement(corev1.LocalObjectReference{Name: applicationSecretName})) + Expect(applicationSa.ImagePullSecrets).To(ContainElement(corev1.LocalObjectReference{Name: namespacePullSecretName})) + }) + + // TODO: remove after all IRs are processed and all have new namespace pull ensured annotation + It("old already provisioned image repository without annotation will on next reconcile create namespace pull secret and robot account", func() { + // Delete the namespace secret to verify it gets recreated + namespaceSecretKey := types.NamespacedName{Name: namespacePullSecretName, Namespace: resourceKey.Namespace} + deleteSecret(namespaceSecretKey) + + isGetRobotAccountInvoked := false + quay.GetRobotAccountFunc = func(organization, robotName string) (*quay.RobotAccount, error) { + isGetRobotAccountInvoked = true + return nil, nil + } + isCreateNamespaceRobotAccountInvoked := false + quay.CreateRobotAccountFunc = func(organization, robotName string) (*quay.RobotAccount, error) { + defer GinkgoRecover() + Expect(organization).To(Equal(quay.TestQuayOrg)) + if robotName == expectedNamespaceRobotAccountName { + isCreateNamespaceRobotAccountInvoked = true + return &quay.RobotAccount{Name: robotName, Token: namespaceRobotTokenRefreshed2}, nil + } + return nil, nil + } + isAddPullPermissionsToNamespaceAccountInvoked := false + quay.AddPermissionsForRepositoryToAccountFunc = func(organization, imageRepository, accountName string, isRobot, isWrite bool) error { + defer GinkgoRecover() + Expect(organization).To(Equal(quay.TestQuayOrg)) + Expect(imageRepository).To(Equal(expectedImageName)) + if accountName == expectedNamespaceRobotAccountName { + Expect(isWrite).To(BeFalse()) + isAddPullPermissionsToNamespaceAccountInvoked = true + } + return nil + } + + componentSaName := getComponentSaName(defaultComponentName) + + // remove namespace pull secret ensured annotation to force new reconcile and simulate old IR + imageRepository := getImageRepository(resourceKey) + delete(imageRepository.Annotations, namespacePullSecretEnsuredAnnotation) + Expect(k8sClient.Update(ctx, imageRepository)).To(Succeed()) + + // Wait for the annotation to be set to true + Eventually(func() bool { + imageRepository := getImageRepository(resourceKey) + return imageRepository.Annotations[namespacePullSecretEnsuredAnnotation] == "true" + }, timeout, interval).Should(BeTrue()) + + Eventually(func() bool { return isGetRobotAccountInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return isCreateNamespaceRobotAccountInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return isAddPullPermissionsToNamespaceAccountInvoked }, timeout, interval).Should(BeTrue()) + + namespaceSecret := waitSecretExist(namespaceSecretKey) + verifySecretSpec(namespaceSecret, "", "", namespacePullSecretName) + namespaceSecretDockerconfigJson := string(namespaceSecret.Data[corev1.DockerConfigJsonKey]) + verifySecretAuth(namespaceSecretDockerconfigJson, expectedNamespaceImage, expectedNamespaceRobotAccountName, namespaceRobotTokenRefreshed2) + + componentSa := getServiceAccount(defaultNamespace, componentSaName) + Expect(componentSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: namespacePullSecretName})) + integrationSa := getServiceAccount(defaultNamespace, IntegrationServiceAccountName) + Expect(integrationSa.Secrets).To(ContainElement(corev1.ObjectReference{Name: namespacePullSecretName})) + Expect(integrationSa.ImagePullSecrets).To(ContainElement(corev1.LocalObjectReference{Name: namespacePullSecretName})) }) It("should cleanup component repository", func() { @@ -827,29 +1133,43 @@ var _ = Describe("Image repository controller", func() { Expect(imageRepository).To(Equal(expectedImageName)) return true, nil } + isGetRobotAccountInvoked := false + quay.GetRobotAccountFunc = func(organization, robotName string) (*quay.RobotAccount, error) { + isGetRobotAccountInvoked = true + return &quay.RobotAccount{Name: expectedNamespaceRobotAccountName, Token: namespaceRobotToken}, nil + } + isRemovePermissionsToRepositoryForAccountInvoked := false + quay.RemovePermissionsForRepositoryFromAccountFunc = func(organization, imageRepository, accountName string, isRobot bool) error { + isRemovePermissionsToRepositoryForAccountInvoked = true + Expect(organization).To(Equal(quay.TestQuayOrg)) + Expect(imageRepository).To(Equal(expectedImageName)) + Expect(accountName).To(Equal(expectedNamespaceRobotAccountName)) + return nil + } deleteImageRepository(resourceKey) Eventually(func() bool { return isDeleteRobotAccountForPushInvoked }, timeout, interval).Should(BeTrue()) Eventually(func() bool { return isDeleteRobotAccountForPullInvoked }, timeout, interval).Should(BeTrue()) Eventually(func() bool { return isDeleteRepositoryInvoked }, timeout, interval).Should(BeTrue()) - - applicationSa := getServiceAccount(defaultNamespace, IntegrationTestsServiceAccountName) - Expect(applicationSa.Secrets).To(HaveLen(1)) - Expect(applicationSa.ImagePullSecrets).To(HaveLen(1)) + Eventually(func() bool { return isGetRobotAccountInvoked }, timeout, interval).Should(BeTrue()) + Eventually(func() bool { return isRemovePermissionsToRepositoryForAccountInvoked }, timeout, interval).Should(BeTrue()) applicationSecretKey := types.NamespacedName{Name: applicationSecretName, Namespace: defaultNamespace} applicationSecret := waitSecretExist(applicationSecretKey) applicationSecretDockerconfigJson := string(applicationSecret.Data[corev1.DockerConfigJsonKey]) verifySecretAuthEmpty(applicationSecretDockerconfigJson) - // verify that secret is unlinked from SAs - componentSa := getServiceAccount(defaultNamespace, componentSaName) - Expect(componentSa.Secrets).To(HaveLen(0)) - Expect(componentSa.ImagePullSecrets).To(HaveLen(0)) + namespaceSecretKey := types.NamespacedName{Name: namespacePullSecretName, Namespace: resourceKey.Namespace} + namespaceSecret := waitSecretExist(namespaceSecretKey) + verifySecretSpec(namespaceSecret, "", "", namespacePullSecretName) + namespaceSecretDockerconfigJson := string(namespaceSecret.Data[corev1.DockerConfigJsonKey]) + verifySecretAuth(namespaceSecretDockerconfigJson, expectedNamespaceImage, expectedNamespaceRobotAccountName, namespaceRobotTokenRefreshed2) + + assertSecretsGoneFromServiceAccounts() deleteServiceAccount(types.NamespacedName{Name: componentSaName, Namespace: defaultNamespace}) - deleteServiceAccount(types.NamespacedName{Name: IntegrationTestsServiceAccountName, Namespace: defaultNamespace}) + deleteServiceAccount(types.NamespacedName{Name: IntegrationServiceAccountName, Namespace: defaultNamespace}) }) }) @@ -864,7 +1184,7 @@ var _ = Describe("Image repository controller", func() { pushToken = "push-token1234" expectedImageName = fmt.Sprintf("%s/%s", defaultNamespace, resourceKey.Name) expectedImage = fmt.Sprintf("quay.io/%s/%s", quay.TestQuayOrg, expectedImageName) - expectedRobotAccountPrefix = strings.ReplaceAll(strings.ReplaceAll(expectedImageName, "-", "_"), "/", "_") + expectedRobotAccountPrefix = sanitizeNameForQuay(expectedImageName) }) It("should provision image repository", func() { @@ -1138,7 +1458,7 @@ var _ = Describe("Image repository controller", func() { It("don't remove repository if 2 imageRepositories use the same repo and one is removed", func() { customImageName := defaultNamespace + "/" + "my-image-used-by-multiple-imagerepositories" expectedImageName = customImageName - expectedRobotAccountPrefix = strings.ReplaceAll(strings.ReplaceAll(expectedImageName, "-", "_"), "/", "_") + expectedRobotAccountPrefix = sanitizeNameForQuay(expectedImageName) isCreateRepositoryInvoked := false quay.CreateRepositoryFunc = func(repository quay.RepositoryRequest) (*quay.Repository, error) { @@ -1209,7 +1529,7 @@ var _ = Describe("Image repository controller", func() { customImageName := defaultNamespace + "/" + "my-image-for-nudging-component" expectedImageName = customImageName - expectedRobotAccountPrefix = strings.ReplaceAll(strings.ReplaceAll(expectedImageName, "-", "_"), "/", "_") + expectedRobotAccountPrefix = sanitizeNameForQuay(expectedImageName) isCreateRepositoryInvoked := false quay.CreateRepositoryFunc = func(repository quay.RepositoryRequest) (*quay.Repository, error) { @@ -1300,7 +1620,7 @@ var _ = Describe("Image repository controller", func() { componentKey := types.NamespacedName{Name: "nudging-component", Namespace: defaultNamespace} componentSaName := getComponentSaName(componentKey.Name) createApplication(applicationConfig{ApplicationKey: applicationKey}) - createComponent(componentConfig{ComponentKey: componentKey}) + createComponent(componentConfig{ComponentKey: componentKey, ComponentApplication: defaultComponentApplication}) createServiceAccount(defaultNamespace, componentSaName) defer deleteComponent(componentKey) defer deleteApplication(applicationKey) @@ -1308,7 +1628,7 @@ var _ = Describe("Image repository controller", func() { customImageName := defaultNamespace + "/" + "my-image-for-nudging-component" expectedImageName = customImageName - expectedRobotAccountPrefix = strings.ReplaceAll(strings.ReplaceAll(expectedImageName, "-", "_"), "/", "_") + expectedRobotAccountPrefix = sanitizeNameForQuay(expectedImageName) isCreateRepositoryInvoked := false quay.CreateRepositoryFunc = func(repository quay.RepositoryRequest) (*quay.Repository, error) { @@ -1411,7 +1731,7 @@ var _ = Describe("Image repository controller", func() { It("don't remove repository if explicitly requested using annotation", func() { customImageName := defaultNamespace + "/" + "my-image-should-not-be-removed" expectedImageName = customImageName - expectedRobotAccountPrefix = strings.ReplaceAll(strings.ReplaceAll(expectedImageName, "-", "_"), "/", "_") + expectedRobotAccountPrefix = sanitizeNameForQuay(expectedImageName) isCreateRepositoryInvoked := false quay.CreateRepositoryFunc = func(repository quay.RepositoryRequest) (*quay.Repository, error) { @@ -1467,7 +1787,7 @@ var _ = Describe("Image repository controller", func() { pushToken = "push-token1234" expectedImageName = fmt.Sprintf("%s/%s", defaultNamespace, resourceKey.Name) expectedImage = fmt.Sprintf("quay.io/%s/%s", quay.TestQuayOrg, expectedImageName) - expectedRobotAccountPrefix = strings.ReplaceAll(strings.ReplaceAll(expectedImageName, "-", "_"), "/", "_") + expectedRobotAccountPrefix = sanitizeNameForQuay(expectedImageName) }) It("should permanently fail if private image repository requested on creation but quota exceeded", func() { diff --git a/internal/controller/imagerepository_controller_unit_test.go b/internal/controller/imagerepository_controller_unit_test.go index 185e8dd4..a99db006 100644 --- a/internal/controller/imagerepository_controller_unit_test.go +++ b/internal/controller/imagerepository_controller_unit_test.go @@ -220,7 +220,7 @@ func TestIsComponentLinked(t *testing.T) { expect: false, }, { - name: "Should not be linked to component if application label missing", + name: "Should be linked to component even if application label missing", imageRepository: &imagerepositoryv1alpha1.ImageRepository{ ObjectMeta: v1.ObjectMeta{ Labels: map[string]string{ @@ -228,7 +228,7 @@ func TestIsComponentLinked(t *testing.T) { }, }, }, - expect: false, + expect: true, }, { name: "Should not be linked to component if component label missing", diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index e08c860b..eaa84308 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -36,9 +36,9 @@ import ( ctrl "sigs.k8s.io/controller-runtime" + compapiv1alpha1 "github.com/konflux-ci/application-api/api/v1alpha1" imagerepositoryv1alpha1 "github.com/konflux-ci/image-controller/api/v1alpha1" "github.com/konflux-ci/image-controller/pkg/quay" - appstudioredhatcomv1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1" //+kubebuilder:scaffold:imports ) @@ -68,12 +68,12 @@ var _ = BeforeSuite(func() { By("bootstrapping test environment") - applicationApiDepVersion := "v0.0.0-20231026192857-89515ad2504f" + applicationApiDepVersion := "v0.0.0-20260205151641-c691ffebedf8" testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{ filepath.Join("..", "..", "config", "crd", "bases"), - filepath.Join(build.Default.GOPATH, "pkg", "mod", "github.com", "redhat-appstudio", "application-api@"+applicationApiDepVersion, "config", "crd", "bases"), + filepath.Join(build.Default.GOPATH, "pkg", "mod", "github.com", "konflux-ci", "application-api@"+applicationApiDepVersion, "config", "crd", "bases"), }, ErrorIfCRDPathMissing: true, } @@ -86,7 +86,7 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) Expect(cfg).NotTo(BeNil()) - err = appstudioredhatcomv1alpha1.AddToScheme(scheme.Scheme) + err = compapiv1alpha1.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) err = imagerepositoryv1alpha1.AddToScheme(scheme.Scheme) diff --git a/internal/controller/suite_util_test.go b/internal/controller/suite_util_test.go index bda24943..f7bb7e5c 100644 --- a/internal/controller/suite_util_test.go +++ b/internal/controller/suite_util_test.go @@ -33,9 +33,8 @@ import ( "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + compapiv1alpha1 "github.com/konflux-ci/application-api/api/v1alpha1" imagerepositoryv1alpha1 "github.com/konflux-ci/image-controller/api/v1alpha1" - appstudioapiv1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1" - appstudioredhatcomv1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1" ) const ( @@ -149,7 +148,7 @@ type applicationConfig struct { ApplicationKey types.NamespacedName } -func getSampleApplicationData(config applicationConfig) *appstudioredhatcomv1alpha1.Application { +func getSampleApplicationData(config applicationConfig) *compapiv1alpha1.Application { name := config.ApplicationKey.Name if name == "" { name = defaultComponentApplication @@ -159,7 +158,7 @@ func getSampleApplicationData(config applicationConfig) *appstudioredhatcomv1alp namespace = defaultNamespace } - return &appstudioredhatcomv1alpha1.Application{ + return &compapiv1alpha1.Application{ TypeMeta: metav1.TypeMeta{ APIVersion: "appstudio.redhat.com/v1alpha1", Kind: "Application", @@ -172,7 +171,7 @@ func getSampleApplicationData(config applicationConfig) *appstudioredhatcomv1alp } // createApplication creates sample application resource and verifies it was properly created. -func createApplication(config applicationConfig) *appstudioredhatcomv1alpha1.Application { +func createApplication(config applicationConfig) *compapiv1alpha1.Application { application := getSampleApplicationData(config) Expect(k8sClient.Create(ctx, application)).Should(Succeed()) @@ -181,8 +180,8 @@ func createApplication(config applicationConfig) *appstudioredhatcomv1alpha1.App return getApplication(applicationKey) } -func getApplication(applicationKey types.NamespacedName) *appstudioredhatcomv1alpha1.Application { - application := &appstudioredhatcomv1alpha1.Application{} +func getApplication(applicationKey types.NamespacedName) *compapiv1alpha1.Application { + application := &compapiv1alpha1.Application{} Eventually(func() bool { Expect(k8sClient.Get(ctx, applicationKey, application)).Should(Succeed()) return application.ResourceVersion != "" @@ -192,7 +191,7 @@ func getApplication(applicationKey types.NamespacedName) *appstudioredhatcomv1al // deleteApplication deletes the specified application resource and verifies it was properly deleted func deleteApplication(applicationKey types.NamespacedName) { - application := &appstudioredhatcomv1alpha1.Application{} + application := &compapiv1alpha1.Application{} // Check if the applicaiton exists if err := k8sClient.Get(ctx, applicationKey, application); k8sErrors.IsNotFound(err) { @@ -214,7 +213,7 @@ type componentConfig struct { Annotations map[string]string } -func getSampleComponentData(config componentConfig) *appstudioapiv1alpha1.Component { +func getSampleComponentData(config componentConfig) *compapiv1alpha1.Component { name := config.ComponentKey.Name if name == "" { name = defaultComponentName @@ -224,15 +223,12 @@ func getSampleComponentData(config componentConfig) *appstudioapiv1alpha1.Compon namespace = defaultNamespace } application := config.ComponentApplication - if application == "" { - application = defaultComponentApplication - } annotations := make(map[string]string) if config.Annotations != nil { annotations = config.Annotations } - return &appstudioapiv1alpha1.Component{ + return &compapiv1alpha1.Component{ TypeMeta: metav1.TypeMeta{ APIVersion: "appstudio.redhat.com/v1alpha1", Kind: "Component", @@ -242,7 +238,7 @@ func getSampleComponentData(config componentConfig) *appstudioapiv1alpha1.Compon Namespace: namespace, Annotations: annotations, }, - Spec: appstudioapiv1alpha1.ComponentSpec{ + Spec: compapiv1alpha1.ComponentSpec{ ComponentName: name, Application: application, }, @@ -250,7 +246,7 @@ func getSampleComponentData(config componentConfig) *appstudioapiv1alpha1.Compon } // createComponent creates sample component resource and verifies it was properly created. -func createComponent(config componentConfig) *appstudioapiv1alpha1.Component { +func createComponent(config componentConfig) *compapiv1alpha1.Component { component := getSampleComponentData(config) Expect(k8sClient.Create(ctx, component)).Should(Succeed()) @@ -259,8 +255,8 @@ func createComponent(config componentConfig) *appstudioapiv1alpha1.Component { return getComponent(componentKey) } -func getComponent(componentKey types.NamespacedName) *appstudioapiv1alpha1.Component { - component := &appstudioapiv1alpha1.Component{} +func getComponent(componentKey types.NamespacedName) *compapiv1alpha1.Component { + component := &compapiv1alpha1.Component{} Eventually(func() bool { Expect(k8sClient.Get(ctx, componentKey, component)).Should(Succeed()) return component.ResourceVersion != "" @@ -270,7 +266,7 @@ func getComponent(componentKey types.NamespacedName) *appstudioapiv1alpha1.Compo // deleteComponent deletes the specified component resource and verifies it was properly deleted func deleteComponent(componentKey types.NamespacedName) { - component := &appstudioapiv1alpha1.Component{} + component := &compapiv1alpha1.Component{} // Check if the component exists if err := k8sClient.Get(ctx, componentKey, component); k8sErrors.IsNotFound(err) { @@ -366,6 +362,15 @@ func createNamespace(name string) { } } +func getNamespace(name string) corev1.Namespace { + ns := corev1.Namespace{} + Eventually(func() bool { + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: kubeSystemNamespace}, &ns)).To(Succeed()) + return ns.ResourceVersion != "" + }, timeout, interval).Should(BeTrue()) + return ns +} + func deleteNamespace(name string) { namespace := corev1.Namespace{ TypeMeta: metav1.TypeMeta{ @@ -552,9 +557,11 @@ func verifySecretAuthEmpty(secretDockerconfigJson string) { } func verifySecretSpec(secret *corev1.Secret, ownerKind, ownerName, secretName string) { - Expect(secret.OwnerReferences).To(HaveLen(1)) - Expect(secret.OwnerReferences[0].Kind).To(Equal(ownerKind)) - Expect(secret.OwnerReferences[0].Name).To(Equal(ownerName)) + if ownerKind != "" && ownerName != "" { + Expect(secret.OwnerReferences).To(HaveLen(1)) + Expect(secret.OwnerReferences[0].Kind).To(Equal(ownerKind)) + Expect(secret.OwnerReferences[0].Name).To(Equal(ownerName)) + } Expect(secret.Labels[InternalSecretLabelName]).To(Equal("true")) Expect(secret.Name).To(Equal(secretName)) Expect(secret.Type).To(Equal(corev1.SecretTypeDockerConfigJson)) diff --git a/internal/controller/users_config_map_controller_test.go b/internal/controller/users_config_map_controller_test.go index 83bcc2ca..3a85c5a0 100644 --- a/internal/controller/users_config_map_controller_test.go +++ b/internal/controller/users_config_map_controller_test.go @@ -26,8 +26,10 @@ import ( var _ = Describe("Users config map controller", func() { Context("Users config map creation, update, removal, namespace doesn't start with number", func() { - var configTestNamespace = "config-map-namespace-test" - var usersConfigMapKey = types.NamespacedName{Name: additionalUsersConfigMapName, Namespace: configTestNamespace} + var ( + configTestNamespace = "config-map-namespace-test" + usersConfigMapKey = types.NamespacedName{Name: additionalUsersConfigMapName, Namespace: configTestNamespace} + ) expectedTeamName := "configxmapxnamespacextestxteam" imageRepositoryName1 := fmt.Sprintf("%s/some1/image1", configTestNamespace) imageRepositoryName2 := fmt.Sprintf("%s/other2/image2", configTestNamespace) @@ -38,6 +40,7 @@ var _ = Describe("Users config map controller", func() { It("should prepare environment", func() { createNamespace(configTestNamespace) + createNamespace(kubeSystemNamespace) }) It("team doesn't exist, requested 2 users, imageRepositories don't exist, create team with and add 2 users", func() { diff --git a/pkg/quay/quay.go b/pkg/quay/quay.go index 0e61c2ad..af3e68ce 100644 --- a/pkg/quay/quay.go +++ b/pkg/quay/quay.go @@ -36,6 +36,7 @@ type QuayService interface { CreateRobotAccount(organization string, robotName string) (*RobotAccount, error) DeleteRobotAccount(organization string, robotName string) (bool, error) AddPermissionsForRepositoryToAccount(organization, imageRepository, accountName string, isRobot, isWrite bool) error + RemovePermissionsForRepositoryFromAccount(organization, imageRepository, accountName string, isRobot bool) error ListPermissionsForRepository(organization, imageRepository string) (map[string]UserAccount, error) AddReadPermissionsForRepositoryToTeam(organization, imageRepository, teamName string) error ListRepositoryPermissionsForTeam(organization, teamName string) ([]TeamPermission, error) @@ -285,9 +286,19 @@ func (c *QuayClient) GetRobotAccount(organization string, robotName string) (*Ro if err := resp.GetJson(data); err != nil { return nil, err } + statusCode := resp.GetStatusCode() - if resp.GetStatusCode() != http.StatusOK { - return nil, errors.New(data.Message) + if statusCode != http.StatusOK { + message := "Failed to get robot account" + if data.Message != "" { + message = data.Message + } + + if statusCode == 400 && strings.Contains(message, "Could not find robot with specified username") { + return nil, nil + } + + return nil, errors.New(message) } return data, nil @@ -325,14 +336,24 @@ func (c *QuayClient) CreateRobotAccount(organization string, robotName string) ( message = data.Message } else if data.ErrorMessage != "" { message = data.ErrorMessage - } else { + } else if data.Error != "" { message = data.Error } } // Handle robot account already exists case if statusCode == 400 && strings.Contains(message, "Existing robot with name") { - return c.GetRobotAccount(organization, robotName) + robotAccount, err := c.GetRobotAccount(organization, robotName) + + if err != nil { + return nil, err + } + + if robotAccount == nil { + return nil, fmt.Errorf("robot account doesn't exist anymore") + } + + return robotAccount, nil } return nil, fmt.Errorf("failed to create robot account. Status code: %d, message: %s", statusCode, message) @@ -379,12 +400,12 @@ func (c *QuayClient) ListPermissionsForRepository(organization, imageRepository } if resp.GetStatusCode() != 200 { - var message string + message := "Failed to list permissions for repository" data := &QuayError{} if err := resp.GetJson(data); err == nil { if data.ErrorMessage != "" { message = data.ErrorMessage - } else { + } else if data.Error != "" { message = data.Error } } @@ -412,12 +433,12 @@ func (c *QuayClient) ListRepositoryPermissionsForTeam(organization, teamName str } if resp.GetStatusCode() != 200 { - var message string + message := "Failed to list repository permissions for team" data := &QuayError{} if err := resp.GetJson(data); err == nil { if data.ErrorMessage != "" { message = data.ErrorMessage - } else { + } else if data.Error != "" { message = data.Error } } else { @@ -454,12 +475,12 @@ func (c *QuayClient) AddUserToTeam(organization, teamName, userName string) (boo return true, fmt.Errorf("failed to add user: %s, to the team team: %s, user doesn't exist", userName, teamName) } - var message string + message := "Failed to add user to team" data := &QuayError{} if err := resp.GetJson(data); err == nil { if data.ErrorMessage != "" { message = data.ErrorMessage - } else { + } else if data.Error != "" { message = data.Error } } else { @@ -485,12 +506,12 @@ func (c *QuayClient) RemoveUserFromTeam(organization, teamName, userName string) return nil } - var message string + message := "Failed to remove user from team" data := &QuayError{} if err := resp.GetJson(data); err == nil { if data.ErrorMessage != "" { message = data.ErrorMessage - } else { + } else if data.Error != "" { message = data.Error } } else { @@ -513,12 +534,12 @@ func (c *QuayClient) DeleteTeam(organization, teamName string) error { return nil } - var message string + message := "Failed to delete team" data := &QuayError{} if err := resp.GetJson(data); err == nil { if data.ErrorMessage != "" { message = data.ErrorMessage - } else { + } else if data.Error != "" { message = data.Error } } else { @@ -549,12 +570,12 @@ func (c *QuayClient) EnsureTeam(organization, teamName string) ([]Member, error) } if resp.GetStatusCode() != 200 { - var message string + message := "Failed to create team" data := &QuayError{} if err := resp.GetJson(data); err == nil { if data.ErrorMessage != "" { message = data.ErrorMessage - } else { + } else if data.Error != "" { message = data.Error } } else { @@ -585,12 +606,12 @@ func (c *QuayClient) GetTeamMembers(organization, teamName string) ([]Member, er return nil, nil } - var message string + message := "Failed to get team members" data := &QuayError{} if err := resp.GetJson(data); err == nil { if data.ErrorMessage != "" { message = data.ErrorMessage - } else { + } else if data.Error != "" { message = data.Error } } else { @@ -637,22 +658,69 @@ func (c *QuayClient) AddPermissionsForRepositoryToAccount(organization, imageRep if err != nil { return err } + statusCode := resp.GetStatusCode() - if resp.GetStatusCode() != 200 { - var message string + if statusCode != 200 { + message := "Failed to add permissions for repository to account" data := &QuayError{} if err := resp.GetJson(data); err == nil { if data.ErrorMessage != "" { message = data.ErrorMessage - } else { + } else if data.Error != "" { message = data.Error } } - return fmt.Errorf("failed to add permissions to the account: %s. Status code: %d, message: %s", accountFullName, resp.GetStatusCode(), message) + return fmt.Errorf("failed to add permissions to the account: %s. Status code: %d, message: %s", accountFullName, statusCode, message) } return nil } +// RemovePermissionsForRepositoryFromAccount removes for given account access to the given repository +func (c *QuayClient) RemovePermissionsForRepositoryFromAccount(organization, imageRepository, accountName string, isRobot bool) error { + var accountFullName string + if isRobot { + if robotName, err := handleRobotName(accountName); err == nil { + accountFullName = organization + "+" + robotName + } else { + return err + } + } else { + accountFullName = accountName + } + + url := fmt.Sprintf("%s/repository/%s/%s/permissions/user/%s", c.url, organization, imageRepository, accountFullName) + + resp, err := c.doRequest(url, http.MethodDelete, nil) + if err != nil { + return err + } + statusCode := resp.GetStatusCode() + + if statusCode == 204 || statusCode == 404 { + return nil + } + + data := &QuayError{} + if err := resp.GetJson(data); err != nil { + return err + } + + message := "Failed to remove permissions for user from repository" + if data.Message != "" { + message = data.Message + } else if data.ErrorMessage != "" { + message = data.ErrorMessage + } else if data.Error != "" { + message = data.Error + } + + if statusCode == 400 && strings.Contains(message, "User does not have permission for repo") { + return nil + } + + return errors.New(message) +} + // AddReadPermissionsForRepositoryToTeam allows given team read access to the given repository. func (c *QuayClient) AddReadPermissionsForRepositoryToTeam(organization, imageRepository, teamName string) error { url := fmt.Sprintf("%s/repository/%s/%s/permissions/team/%s", c.url, organization, imageRepository, teamName) @@ -664,12 +732,12 @@ func (c *QuayClient) AddReadPermissionsForRepositoryToTeam(organization, imageRe } if resp.GetStatusCode() != 200 { - var message string + message := "Failed to add permissions for repository to team" data := &QuayError{} if err := resp.GetJson(data); err == nil { if data.ErrorMessage != "" { message = data.ErrorMessage - } else { + } else if data.Error != "" { message = data.Error } } else { diff --git a/pkg/quay/quay_debug_test.go b/pkg/quay/quay_debug_test.go index df76c19e..1170dec1 100644 --- a/pkg/quay/quay_debug_test.go +++ b/pkg/quay/quay_debug_test.go @@ -142,15 +142,15 @@ func TestGetRobotAccount(t *testing.T) { robotAccount, err := quayClient.GetRobotAccount(quayOrgName, quayRobotAccountName) if err != nil { - if err.Error() == "Could not find robot with specified username" { + t.Fatalf("Unknown error: %s\n", err.Error()) + } else { + if robotAccount == nil { t.Logf("Robot account %s does not exists", quayRobotAccountName) + } else if robotAccount.Name == quayOrgName+"+"+quayRobotAccountName { + t.Logf("Robot account %s exists", quayRobotAccountName) } else { - t.Fatalf("Unknown error: %s\n", err.Error()) + t.Fatalf("Unexpected response: %v\n", robotAccount) } - } else if robotAccount.Name == quayOrgName+"+"+quayRobotAccountName { - t.Logf("Robot account %s exists", quayRobotAccountName) - } else { - t.Fatalf("Unexpected response: %v\n", robotAccount) } } @@ -178,6 +178,18 @@ func TestAddPermissionsToRobotAccount(t *testing.T) { } } +func TestRemovePermissionsFromRobotAccount(t *testing.T) { + if quayToken == "" { + return + } + quayClient := NewQuayClient(&http.Client{Transport: &http.Transport{}}, quayToken, quayApiUrl) + + err := quayClient.RemovePermissionsForRepositoryFromAccount(quayOrgName, quayImageRepoName, quayRobotAccountName, true) + if err != nil { + t.Fatal(err) + } +} + func TestRegenerateRobotAccountToken(t *testing.T) { if quayToken == "" { return diff --git a/pkg/quay/quay_test.go b/pkg/quay/quay_test.go index 0e407609..21348bd5 100644 --- a/pkg/quay/quay_test.go +++ b/pkg/quay/quay_test.go @@ -1589,36 +1589,41 @@ func TestQuayClient_GetRobotAccount(t *testing.T) { } testCases := []struct { - name string - robot *RobotAccount - expectedErr string - statusCode int - response interface{} + name string + robot *RobotAccount + expectedErr string + expectedFound bool + statusCode int + response interface{} }{ { - name: "Get existing robot account", - robot: sampleRobot, - expectedErr: "", - statusCode: 200, - response: fmt.Sprintf(`{"name": "%s+%s", "created": "Wed, 12 Jul 2023 10:25:41 -0000", "last_accessed": null, "description": "", "token": "abc123", "unstructured_metadata": {}}`, org, robotName), + name: "Get existing robot account", + robot: sampleRobot, + expectedErr: "", + expectedFound: true, + statusCode: 200, + response: fmt.Sprintf(`{"name": "%s+%s", "created": "Wed, 12 Jul 2023 10:25:41 -0000", "last_accessed": null, "description": "", "token": "abc123", "unstructured_metadata": {}}`, org, robotName), }, { - name: "return error when server responds non-200", - robot: nil, - expectedErr: "Could not find robot with specified username", - statusCode: 400, - response: map[string]string{"message": "Could not find robot with specified username"}, + name: "return error when server responds 400 not found", + robot: sampleRobot, + expectedErr: "", + expectedFound: false, + statusCode: 400, + response: map[string]string{"message": "Could not find robot with specified username"}, }, { - name: "server responds an invalid JSON string", - robot: nil, - expectedErr: "failed to unmarshal response body", - statusCode: 400, // this field can be ignored for this case - response: "{\"error\": \"something is wrong}", + name: "server responds an invalid JSON string", + robot: nil, + expectedErr: "failed to unmarshal response body", + expectedFound: false, + statusCode: 400, // this field can be ignored for this case + response: "{\"error\": \"something is wrong}", }, { - name: "stop if http request fails", - expectedErr: "failed to Do request:", + name: "stop if http request fails", + expectedErr: "failed to Do request:", + expectedFound: false, }, } @@ -1638,8 +1643,12 @@ func TestQuayClient_GetRobotAccount(t *testing.T) { quayClient := NewQuayClient(client, "authtoken", testQuayApiUrl) robot, err := quayClient.GetRobotAccount(org, robotName) - if !reflect.DeepEqual(robot, tc.robot) { - t.Error("robots are not the same") + if tc.expectedFound { + if !reflect.DeepEqual(robot, tc.robot) { + t.Error("robots are not the same") + } + } else { + assert.Assert(t, robot == nil) } if tc.expectedErr == "" { assert.NilError(t, err) diff --git a/pkg/quay/test_quay_client.go b/pkg/quay/test_quay_client.go index 779c411b..cdf8bcda 100644 --- a/pkg/quay/test_quay_client.go +++ b/pkg/quay/test_quay_client.go @@ -30,27 +30,28 @@ type TestQuayClient struct{} var _ QuayService = (*TestQuayClient)(nil) var ( - CreateRepositoryFunc func(repository RepositoryRequest) (*Repository, error) - DeleteRepositoryFunc func(organization, imageRepository string) (bool, error) - RepositoryExistsFunc func(organization, imageRepository string) (bool, error) - ChangeRepositoryVisibilityFunc func(organization, imageRepository string, visibility string) error - GetRobotAccountFunc func(organization string, robotName string) (*RobotAccount, error) - CreateRobotAccountFunc func(organization string, robotName string) (*RobotAccount, error) - DeleteRobotAccountFunc func(organization string, robotName string) (bool, error) - AddPermissionsForRepositoryToAccountFunc func(organization, imageRepository, accountName string, isRobot, isWrite bool) error - ListPermissionsForRepositoryFunc func(organization, imageRepository string) (map[string]UserAccount, error) - AddReadPermissionsForRepositoryToTeamFunc func(organization, imageRepository, teamName string) error - ListRepositoryPermissionsForTeamFunc func(organization, teamName string) ([]TeamPermission, error) - AddUserToTeamFunc func(organization, teamName, userName string) (bool, error) - RemoveUserFromTeamFunc func(organization, teamName, userName string) error - DeleteTeamFunc func(organization, teamName string) error - EnsureTeamFunc func(organization, teamName string) ([]Member, error) - GetTeamMembersFunc func(organization, teamName string) ([]Member, error) - RegenerateRobotAccountTokenFunc func(organization string, robotName string) (*RobotAccount, error) - GetNotificationsFunc func(organization, repository string) ([]Notification, error) - CreateNotificationFunc func(organization, repository string, notification Notification) (*Notification, error) - UpdateNotificationFunc func(organization, repository string, notificationUuid string, notification Notification) (*Notification, error) - DeleteNotificationFunc func(organization, repository string, notificationUuid string) (bool, error) + CreateRepositoryFunc func(repository RepositoryRequest) (*Repository, error) + DeleteRepositoryFunc func(organization, imageRepository string) (bool, error) + RepositoryExistsFunc func(organization, imageRepository string) (bool, error) + ChangeRepositoryVisibilityFunc func(organization, imageRepository string, visibility string) error + GetRobotAccountFunc func(organization string, robotName string) (*RobotAccount, error) + CreateRobotAccountFunc func(organization string, robotName string) (*RobotAccount, error) + DeleteRobotAccountFunc func(organization string, robotName string) (bool, error) + AddPermissionsForRepositoryToAccountFunc func(organization, imageRepository, accountName string, isRobot, isWrite bool) error + RemovePermissionsForRepositoryFromAccountFunc func(organization, imageRepository, accountName string, isRobot bool) error + ListPermissionsForRepositoryFunc func(organization, imageRepository string) (map[string]UserAccount, error) + AddReadPermissionsForRepositoryToTeamFunc func(organization, imageRepository, teamName string) error + ListRepositoryPermissionsForTeamFunc func(organization, teamName string) ([]TeamPermission, error) + AddUserToTeamFunc func(organization, teamName, userName string) (bool, error) + RemoveUserFromTeamFunc func(organization, teamName, userName string) error + DeleteTeamFunc func(organization, teamName string) error + EnsureTeamFunc func(organization, teamName string) ([]Member, error) + GetTeamMembersFunc func(organization, teamName string) ([]Member, error) + RegenerateRobotAccountTokenFunc func(organization string, robotName string) (*RobotAccount, error) + GetNotificationsFunc func(organization, repository string) ([]Notification, error) + CreateNotificationFunc func(organization, repository string, notification Notification) (*Notification, error) + UpdateNotificationFunc func(organization, repository string, notificationUuid string, notification Notification) (*Notification, error) + DeleteNotificationFunc func(organization, repository string, notificationUuid string) (bool, error) ) func ResetTestQuayClient() { @@ -58,10 +59,13 @@ func ResetTestQuayClient() { DeleteRepositoryFunc = func(organization, imageRepository string) (bool, error) { return true, nil } RepositoryExistsFunc = func(organization, imageRepository string) (bool, error) { return true, nil } ChangeRepositoryVisibilityFunc = func(organization, imageRepository string, visibility string) error { return nil } - GetRobotAccountFunc = func(organization, robotName string) (*RobotAccount, error) { return &RobotAccount{}, nil } + GetRobotAccountFunc = func(organization, robotName string) (*RobotAccount, error) { + return &RobotAccount{Name: "robot", Token: "token"}, nil + } CreateRobotAccountFunc = func(organization, robotName string) (*RobotAccount, error) { return &RobotAccount{}, nil } DeleteRobotAccountFunc = func(organization, robotName string) (bool, error) { return true, nil } AddPermissionsForRepositoryToAccountFunc = func(organization, imageRepository, accountName string, isRobot, isWrite bool) error { return nil } + RemovePermissionsForRepositoryFromAccountFunc = func(organization, imageRepository, accountName string, isRobot bool) error { return nil } ListPermissionsForRepositoryFunc = func(organization, imageRepository string) (map[string]UserAccount, error) { return nil, nil } AddReadPermissionsForRepositoryToTeamFunc = func(organization, imageRepository, teamName string) error { return nil } ListRepositoryPermissionsForTeamFunc = func(organization, teamName string) ([]TeamPermission, error) { return []TeamPermission{}, nil } @@ -124,6 +128,11 @@ func ResetTestQuayClientToFails() { Fail("AddPermissionsForRepositoryToAccount invoked") return nil } + RemovePermissionsForRepositoryFromAccountFunc = func(organization, imageRepository, accountName string, isRobot bool) error { + defer GinkgoRecover() + Fail("RemovePermissionsToRepositoryForAccount invoked") + return nil + } ListPermissionsForRepositoryFunc = func(organization, imageRepository string) (map[string]UserAccount, error) { defer GinkgoRecover() Fail("ListPermissionsForRepository invoked") @@ -215,6 +224,9 @@ func (c TestQuayClient) DeleteRobotAccount(organization string, robotName string func (c TestQuayClient) AddPermissionsForRepositoryToAccount(organization, imageRepository, accountName string, isRobot, isWrite bool) error { return AddPermissionsForRepositoryToAccountFunc(organization, imageRepository, accountName, isRobot, isWrite) } +func (c TestQuayClient) RemovePermissionsForRepositoryFromAccount(organization, imageRepository, accountName string, isRobot bool) error { + return RemovePermissionsForRepositoryFromAccountFunc(organization, imageRepository, accountName, isRobot) +} func (c TestQuayClient) ListPermissionsForRepository(organization, imageRepository string) (map[string]UserAccount, error) { return ListPermissionsForRepositoryFunc(organization, imageRepository) }