Remove ability to grant teams access to quay repos based on config map (#279)

STONEBLD-4522

Signed-off-by: Robert Cerven <rcerven@redhat.com>

Author: rcerven

cmd/main.go

@@ -240,16 +240,6 @@ func main() {
 		os.Exit(1)
 	}
 
-	if err = (&controllers.QuayUsersConfigMapReconciler{
-		Client:           mgr.GetClient(),
-		Scheme:           mgr.GetScheme(),
-		BuildQuayClient:  buildQuayClientFunc,
-		QuayOrganization: quayOrganization,
-	}).SetupWithManager(mgr); err != nil {
-		setupLog.Error(err, "unable to create controller", "controller", "ConfigMap")
-		os.Exit(1)
-	}
-
 	if err = (&controllers.ApplicationPullSecretCreator{
 		Client: mgr.GetClient(),
 		Scheme: mgr.GetScheme(),

config/rbac/role.yaml

@@ -8,14 +8,6 @@ rules:
   - ""
   resources:
   - configmaps
-  verbs:
-  - get
-  - list
-  - update
-  - watch
-- apiGroups:
-  - ""
-  resources:
   - namespaces
   verbs:
   - get

internal/controller/imagerepository_controller.go

@@ -49,8 +49,6 @@ const (
 	ImageRepositoryFinalizer = "appstudio.openshift.io/image-repository"
 
 	updateComponentAnnotationName        = "image-controller.appstudio.redhat.com/update-component-image"
-	additionalUsersConfigMapName         = "image-controller-additional-users"
-	additionalUsersConfigMapKey          = "quay.io"
 	skipRepositoryDeletionAnnotationName = "image-controller.appstudio.redhat.com/skip-repository-deletion"
 
 	waitForRelatedComponentInitialDelay           = 5
@@ -573,10 +571,6 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context
 		return err
 	}
 
-	if err = r.GrantRepositoryAccessToTeam(ctx, imageRepository); err != nil {
-		return err
-	}
-
 	var notificationStatus []imagerepositoryv1alpha1.NotificationStatus
 	if notificationStatus, err = r.SetNotifications(ctx, imageRepository); err != nil {
 		return err
@@ -666,46 +660,6 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepositoryAccess(ctx context.C
 	return data, nil
 }
 
-// GrantRepositoryAccessToTeam will add additional repository access to team, based on config map
-func (r *ImageRepositoryReconciler) GrantRepositoryAccessToTeam(ctx context.Context, imageRepository *imagerepositoryv1alpha1.ImageRepository) error {
-	log := ctrllog.FromContext(ctx).WithName("GrantAdditionalRepositoryAccessToTeam")
-
-	additionalUsersConfigMap := &corev1.ConfigMap{}
-	if err := r.Client.Get(ctx, types.NamespacedName{Name: additionalUsersConfigMapName, Namespace: imageRepository.Namespace}, additionalUsersConfigMap); err != nil {
-		if errors.IsNotFound(err) {
-			log.Info("Config map with additional users doesn't exist", "ConfigMapName", additionalUsersConfigMapName, l.Action, l.ActionView)
-			return nil
-		}
-		log.Error(err, "failed to read config map with additional users", "ConfigMapName", additionalUsersConfigMapName, l.Action, l.ActionView)
-		return err
-	}
-	_, usersExist := additionalUsersConfigMap.Data[additionalUsersConfigMapKey]
-	if !usersExist {
-		log.Info("Config map with additional users doesn't have the key", "ConfigMapName", additionalUsersConfigMapName, "ConfigMapKey", additionalUsersConfigMapKey, l.Action, l.ActionView)
-		return nil
-	}
-
-	imageRepositoryName := imageRepository.Spec.Image.Name
-	teamName := getQuayTeamName(imageRepository.Namespace)
-
-	// get team, if team doesn't exist it will be created, we don't care about users as that will be taken care of by config map controller
-	// so in this case if config map exists, team already exists as well with appropriate users
-	log.Info("Ensure team", "TeamName", teamName)
-	if _, err := r.QuayClient.EnsureTeam(r.QuayOrganization, teamName); err != nil {
-		log.Error(err, "failed to get or create team", "TeamName", teamName, l.Action, l.ActionView)
-		return err
-	}
-
-	// add repo permission to the team
-	log.Info("Adding repository permission to the team", "TeamName", teamName, "RepositoryName", imageRepositoryName)
-	if err := r.QuayClient.AddReadPermissionsForRepositoryToTeam(r.QuayOrganization, imageRepositoryName, teamName); err != nil {
-		log.Error(err, "failed to grant repo permission to the team", "TeamName", teamName, "RepositoryName", imageRepositoryName, l.Action, l.ActionAdd)
-		return err
-	}
-
-	return nil
-}
-
 // RegenerateImageRepositoryCredentials rotates robot account(s) token and updates corresponding secret(s)
 func (r *ImageRepositoryReconciler) RegenerateImageRepositoryCredentials(ctx context.Context, imageRepository *imagerepositoryv1alpha1.ImageRepository) error {
 	log := ctrllog.FromContext(ctx)

internal/controller/imagerepository_controller_test.go

@@ -463,7 +463,7 @@ var _ = Describe("Image repository controller", func() {
 			}, timeout, interval).WithTimeout(ensureTimeout).Should(BeTrue())
 		})
 
-		assertProvisionRepository := func(updateComponentAnnotation, grantRepoPermission, setNotification bool) {
+		assertProvisionRepository := func(updateComponentAnnotation, setNotification bool) {
 			quay.RepositoryExistsFunc = func(organization, imageRepository string) (bool, error) {
 				return true, nil
 			}
@@ -503,6 +503,7 @@ var _ = Describe("Image repository controller", func() {
 				defer GinkgoRecover()
 				Expect(organization).To(Equal(quay.TestQuayOrg))
 				Expect(imageRepository).To(Equal(expectedImageName))
+				Expect(isRobot).To(BeTrue())
 				if strings.HasPrefix(accountName, expectedRobotAccountPrefix) || accountName == expectedNamespaceRobotAccountName {
 					if strings.HasPrefix(accountName, expectedRobotAccountPrefix) {
 						// method is called for pull or push robot account
@@ -524,27 +525,6 @@ var _ = Describe("Image repository controller", func() {
 				}
 				return nil
 			}
-			isEnsureTeamInvoked := false
-			isAddReadPermissionsForRepositoryToTeamInvoked := false
-			if grantRepoPermission {
-				quay.EnsureTeamFunc = func(organization, teamName string) ([]quay.Member, error) {
-					defer GinkgoRecover()
-					Expect(organization).To(Equal(quay.TestQuayOrg))
-					expectedTeamName := getQuayTeamName(resourceKey.Namespace)
-					Expect(teamName).To(Equal(expectedTeamName))
-					isEnsureTeamInvoked = true
-					return nil, nil
-				}
-				quay.AddReadPermissionsForRepositoryToTeamFunc = func(organization, imageRepository, teamName string) error {
-					defer GinkgoRecover()
-					Expect(organization).To(Equal(quay.TestQuayOrg))
-					Expect(imageRepository).To(Equal(expectedImageName))
-					expectedTeamName := getQuayTeamName(resourceKey.Namespace)
-					Expect(teamName).To(Equal(expectedTeamName))
-					isAddReadPermissionsForRepositoryToTeamInvoked = true
-					return nil
-				}
-			}
 			isCreateNotificationInvoked := false
 			isGetNotificationsInvoked := false
 			if setNotification {
@@ -618,11 +598,6 @@ var _ = Describe("Image repository controller", func() {
 			}
 			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())
-			}
-
 			waitImageRepositoryFinalizerOnImageRepository(resourceKey)
 
 			component := getComponent(componentKey)
@@ -720,7 +695,7 @@ var _ = Describe("Image repository controller", func() {
 		}
 
 		It("should provision image repository for component, without update component annotation", func() {
-			assertProvisionRepository(false, false, true)
+			assertProvisionRepository(false, true)
 
 			quay.DeleteRobotAccountFunc = func(organization, robotAccountName string) (bool, error) {
 				return true, nil
@@ -733,67 +708,8 @@ var _ = Describe("Image repository controller", func() {
 			assertSecretsGoneFromServiceAccounts()
 		})
 
-		It("should provision image repository for component, with update component annotation and grant permission to team", func() {
-			usersConfigMapKey := types.NamespacedName{Name: additionalUsersConfigMapName, Namespace: resourceKey.Namespace}
-			expectedTeamName := getQuayTeamName(resourceKey.Namespace)
-			isEnsureTeamInvoked := false
-			isListRepositoryPermissionsForTeamInvoked := false
-			countAddUserToTeamInvoked := 0
-			isDeleteTeamInvoked := false
-
-			quay.EnsureTeamFunc = func(organization, teamName string) ([]quay.Member, error) {
-				defer GinkgoRecover()
-				Expect(organization).To(Equal(quay.TestQuayOrg))
-				Expect(teamName).To(Equal(expectedTeamName))
-				isEnsureTeamInvoked = true
-				return []quay.Member{}, nil
-			}
-			quay.ListRepositoryPermissionsForTeamFunc = func(organization, teamName string) ([]quay.TeamPermission, error) {
-				defer GinkgoRecover()
-				Expect(organization).To(Equal(quay.TestQuayOrg))
-				Expect(teamName).To(Equal(expectedTeamName))
-				isListRepositoryPermissionsForTeamInvoked = true
-				return []quay.TeamPermission{}, nil
-			}
-			quay.AddUserToTeamFunc = func(organization, teamName, userName string) (bool, error) {
-				defer GinkgoRecover()
-				Expect(organization).To(Equal(quay.TestQuayOrg))
-				Expect(teamName).To(Equal(expectedTeamName))
-				Expect(userName).To(BeElementOf([]string{"user1", "user2"}))
-				countAddUserToTeamInvoked++
-				return false, nil
-			}
-
-			createUsersConfigMap(usersConfigMapKey, []string{"user1", "user2"})
-			Eventually(func() bool { return isEnsureTeamInvoked }, timeout, interval).Should(BeTrue())
-			Eventually(func() bool { return isListRepositoryPermissionsForTeamInvoked }, timeout, interval).Should(BeTrue())
-			Eventually(func() int { return countAddUserToTeamInvoked }, timeout, interval).Should(Equal(2))
-			waitQuayTeamUsersFinalizerOnConfigMap(usersConfigMapKey)
-
-			assertProvisionRepository(true, true, true)
-
-			quay.DeleteTeamFunc = func(organization, teamName string) error {
-				defer GinkgoRecover()
-				Expect(organization).To(Equal(quay.TestQuayOrg))
-				Expect(teamName).To(Equal(expectedTeamName))
-				isDeleteTeamInvoked = true
-				return nil
-			}
-			deleteUsersConfigMap(usersConfigMapKey)
-			Eventually(func() bool { return isDeleteTeamInvoked }, timeout, interval).Should(BeTrue())
-
-			quay.DeleteRobotAccountFunc = func(organization, robotAccountName string) (bool, error) {
-				return true, nil
-			}
-			quay.DeleteRepositoryFunc = func(organization, imageRepository string) (bool, error) {
-				return true, nil
-			}
-			deleteImageRepository(resourceKey)
-			assertSecretsGoneFromServiceAccounts()
-		})
-
 		It("should provision image repository for component, with update component annotation", func() {
-			assertProvisionRepository(true, false, false)
+			assertProvisionRepository(true, false)
 		})
 
 		It("should regenerate pull & push tokens and create pull && push secrets when they don't exist", func() {

internal/controller/suite_test.go

@@ -117,14 +117,6 @@ var _ = BeforeSuite(func() {
 	}).SetupWithManager(k8sManager)
 	Expect(err).ToNot(HaveOccurred())
 
-	err = (&QuayUsersConfigMapReconciler{
-		Client:           k8sManager.GetClient(),
-		Scheme:           k8sManager.GetScheme(),
-		BuildQuayClient:  func() (quay.QuayService, error) { return quay.TestQuayClient{}, nil },
-		QuayOrganization: quay.TestQuayOrg,
-	}).SetupWithManager(k8sManager)
-	Expect(err).ToNot(HaveOccurred())
-
 	err = (&ApplicationPullSecretCreator{
 		Client: k8sManager.GetClient(),
 		Scheme: k8sManager.GetScheme(),

internal/controller/suite_util_test.go

@@ -20,7 +20,6 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
-	"strings"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -371,22 +370,6 @@ func getNamespace(name string) corev1.Namespace {
 	return ns
 }
 
-func deleteNamespace(name string) {
-	namespace := corev1.Namespace{
-		TypeMeta: metav1.TypeMeta{
-			APIVersion: "v1",
-			Kind:       "Namespace",
-		},
-		ObjectMeta: metav1.ObjectMeta{
-			Name: name,
-		},
-	}
-
-	if err := k8sClient.Delete(ctx, &namespace); err != nil && !k8sErrors.IsNotFound(err) {
-		Fail(err.Error())
-	}
-}
-
 func waitSecretExist(secretKey types.NamespacedName) *corev1.Secret {
 	secret := &corev1.Secret{}
 	Eventually(func() bool {
@@ -482,65 +465,6 @@ func deleteServiceAccount(serviceAccountKey types.NamespacedName) {
 	}, timeout, interval).Should(BeTrue())
 }
 
-func createUsersConfigMap(configMapKey types.NamespacedName, users []string) {
-	configMapData := map[string]string{}
-	configMapData[additionalUsersConfigMapKey] = strings.Join(users, " ")
-
-	usersConfigMap := corev1.ConfigMap{
-		ObjectMeta: metav1.ObjectMeta{Name: configMapKey.Name, Namespace: configMapKey.Namespace},
-		Data:       configMapData,
-	}
-
-	if err := k8sClient.Create(ctx, &usersConfigMap); err != nil && !k8sErrors.IsAlreadyExists(err) {
-		Fail(err.Error())
-	}
-}
-
-func deleteUsersConfigMap(configMapKey types.NamespacedName) {
-	usersConfigMap := corev1.ConfigMap{}
-	if err := k8sClient.Get(ctx, configMapKey, &usersConfigMap); err != nil {
-		if k8sErrors.IsNotFound(err) {
-			return
-		}
-		Fail(err.Error())
-	}
-	if err := k8sClient.Delete(ctx, &usersConfigMap); err != nil && !k8sErrors.IsNotFound(err) {
-		Fail(err.Error())
-	}
-	Eventually(func() bool {
-		return k8sErrors.IsNotFound(k8sClient.Get(ctx, configMapKey, &usersConfigMap))
-	}, timeout, interval).Should(BeTrue())
-}
-
-func addUsersToUsersConfigMap(configMapKey types.NamespacedName, addUsers []string) {
-	usersConfigMap := corev1.ConfigMap{}
-	Eventually(func() bool {
-		Expect(k8sClient.Get(ctx, configMapKey, &usersConfigMap)).Should(Succeed())
-		return usersConfigMap.ResourceVersion != ""
-	}, timeout, interval).Should(BeTrue())
-
-	currentUsers, usersExist := usersConfigMap.Data[additionalUsersConfigMapKey]
-	if !usersExist {
-		Fail("users config map is missing key")
-	}
-
-	newUsers := strings.Join(addUsers, " ")
-	allUsers := fmt.Sprintf("%s %s", currentUsers, newUsers)
-	usersConfigMap.Data[additionalUsersConfigMapKey] = allUsers
-
-	Expect(k8sClient.Update(ctx, &usersConfigMap)).Should(Succeed())
-}
-
-func waitQuayTeamUsersFinalizerOnConfigMap(usersConfigMapKey types.NamespacedName) {
-	usersConfigMap := &corev1.ConfigMap{}
-	Eventually(func() bool {
-		if err := k8sClient.Get(ctx, usersConfigMapKey, usersConfigMap); err != nil {
-			return false
-		}
-		return controllerutil.ContainsFinalizer(usersConfigMap, ConfigMapFinalizer)
-	}, timeout, interval).Should(BeTrue())
-}
-
 func verifySecretAuth(secretDockerconfigJson, expectedImage, robotAccountName, token string) {
 	var authDataJson dockerConfigJson
 	Expect(json.Unmarshal([]byte(secretDockerconfigJson), &authDataJson)).To(Succeed())