New controller watching config maps named: image-controller-additiona…

config/rbac/role.yaml

@@ -70,5 +70,5 @@ rules:
   verbs:
   - get
   - list
+  - update
   - watch
-

controllers/imagerepository_controller.go

@@ -346,7 +346,7 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context
 		}
 	}
 
-	if err = r.GrantAdditionalRepositoryAccess(ctx, imageRepository); err != nil {
+	if err = r.GrantRepositoryAccessToTeam(ctx, imageRepository); err != nil {
 		return err
 	}
 
@@ -436,8 +436,9 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepositoryAccess(ctx context.C
 	return data, nil
 }
 
-func (r *ImageRepositoryReconciler) GrantAdditionalRepositoryAccess(ctx context.Context, imageRepository *imagerepositoryv1alpha1.ImageRepository) error {
-	log := ctrllog.FromContext(ctx).WithName("GrantAdditionalRepositoryAccess")
+// 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 {
@@ -448,30 +449,30 @@ func (r *ImageRepositoryReconciler) GrantAdditionalRepositoryAccess(ctx context.
 		log.Error(err, "failed to read config map with additional users", "ConfigMapName", additionalUsersConfigMapName, l.Action, l.ActionView)
 		return err
 	}
-	additionalUsersStr, usersExist := additionalUsersConfigMap.Data[additionalUsersConfigMapKey]
+	_, 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
 	}
 
-	additionalUsers := strings.Fields(strings.TrimSpace(additionalUsersStr))
-	log.Info("Additional users configured in config map", "AdditionalUsers", additionalUsers)
-
 	imageRepositoryName := imageRepository.Spec.Image.Name
+	teamName := getQuayTeamName(imageRepository.Namespace)
 
-	for _, user := range additionalUsers {
-		err := r.QuayClient.AddPermissionsForRepositoryToAccount(r.QuayOrganization, imageRepositoryName, user, false, false)
-		if err != nil {
-			if strings.Contains(err.Error(), "Invalid username:") {
-				log.Info("failed to add permissions for account, because it doesn't exist", "AccountName", user)
-				continue
-			}
+	// 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
+	}
 
-			log.Error(err, "failed to add permissions for account", "AccountName", user, l.Action, l.ActionUpdate, l.Audit, "true")
-			return err
-		}
-		log.Info("Additional user access was granted for", "UserName", user)
+	// 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
 }
 

controllers/imagerepository_controller_test.go

@@ -476,7 +476,7 @@ var _ = Describe("Image repository controller", func() {
 			createServiceAccount(defaultNamespace, buildPipelineServiceAccountName)
 		})
 
-		assertProvisionRepository := func(updateComponentAnnotation bool, additionalUser string) {
+		assertProvisionRepository := func(updateComponentAnnotation, grantRepoPermission bool) {
 			isCreateRepositoryInvoked := false
 			quay.CreateRepositoryFunc = func(repository quay.RepositoryRequest) (*quay.Repository, error) {
 				defer GinkgoRecover()
@@ -506,22 +506,37 @@ var _ = Describe("Image repository controller", func() {
 				defer GinkgoRecover()
 				Expect(organization).To(Equal(quay.TestQuayOrg))
 				Expect(imageRepository).To(Equal(expectedImageName))
-
-				if isRobot {
-					Expect(strings.HasPrefix(accountName, expectedRobotAccountPrefix)).To(BeTrue())
-					if strings.HasSuffix(accountName, "_pull") {
-						Expect(isWrite).To(BeFalse())
-						isAddPullPermissionsToAccountInvoked = true
-					} else {
-						Expect(isWrite).To(BeTrue())
-						isAddPushPermissionsToAccountInvoked = true
-					}
-				} else {
-					Expect(accountName).To(Equal(additionalUser))
+				Expect(strings.HasPrefix(accountName, expectedRobotAccountPrefix)).To(BeTrue())
+				if strings.HasSuffix(accountName, "_pull") {
 					Expect(isWrite).To(BeFalse())
+					isAddPullPermissionsToAccountInvoked = true
+				} else {
+					Expect(isWrite).To(BeTrue())
+					isAddPushPermissionsToAccountInvoked = true
 				}
 				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
 			quay.CreateNotificationFunc = func(organization, repository string, notification quay.Notification) (*quay.Notification, error) {
 				isCreateNotificationInvoked = true
@@ -574,6 +589,10 @@ var _ = Describe("Image repository controller", func() {
 			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())
+			if grantRepoPermission {
+				Eventually(func() bool { return isEnsureTeamInvoked }, timeout, interval).Should(BeTrue())
+				Eventually(func() bool { return isAddReadPermissionsForRepositoryToTeamInvoked }, timeout, interval).Should(BeTrue())
+			}
 
 			waitImageRepositoryFinalizerOnImageRepository(resourceKey)
 
@@ -654,7 +673,7 @@ var _ = Describe("Image repository controller", func() {
 		}
 
 		It("should provision image repository for component, without update component annotation", func() {
-			assertProvisionRepository(false, "")
+			assertProvisionRepository(false, false)
 
 			quay.DeleteRobotAccountFunc = func(organization, robotAccountName string) (bool, error) {
 				return true, nil
@@ -666,24 +685,66 @@ var _ = Describe("Image repository controller", func() {
 			deleteImageRepository(resourceKey)
 		})
 
-		It("should provision image repository for component, with update component annotation and add additional user from config map", func() {
+		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}
-			createUsersConfigMap(usersConfigMapKey, []string{"user1"})
-			assertProvisionRepository(true, "user1")
+			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)
+
+			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
 			}
-
-			deleteUsersConfigMap(usersConfigMapKey)
 			deleteImageRepository(resourceKey)
 		})
 
 		It("should provision image repository for component, with update component annotation", func() {
-			assertProvisionRepository(true, "")
+			assertProvisionRepository(true, false)
 		})
 
 		It("should regenerate tokens and update secrets", func() {

controllers/suite_test.go

@@ -119,6 +119,14 @@ var _ = BeforeSuite(func() {
 	}).SetupWithManager(k8sManager)
 	Expect(err).ToNot(HaveOccurred())
 
+	err = (&QuayUsersConfigMapReconciler{
+		Client:           k8sManager.GetClient(),
+		Scheme:           k8sManager.GetScheme(),
+		BuildQuayClient:  func(l logr.Logger) quay.QuayService { return quay.TestQuayClient{} },
+		QuayOrganization: quay.TestQuayOrg,
+	}).SetupWithManager(k8sManager)
+	Expect(err).ToNot(HaveOccurred())
+
 	go func() {
 		defer GinkgoRecover()
 		err = k8sManager.Start(ctx)

controllers/suite_util_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package controllers
 
 import (
+	"fmt"
 	"strings"
 	"time"
 
@@ -284,6 +285,22 @@ func createNamespace(name string) {
 	}
 }
 
+func deleteNamespace(name string) {
+	namespace := corev1.Namespace{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "v1",
+			Kind:       "Namespace",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name: name,
+		},
+	}
+
+	if err := k8sClient.Delete(ctx, &namespace); err != nil && !k8sErrors.IsNotFound(err) {
+		Fail(err.Error())
+	}
+}
+
 func waitSecretExist(secretKey types.NamespacedName) *corev1.Secret {
 	secret := &corev1.Secret{}
 	Eventually(func() bool {
@@ -375,4 +392,36 @@ func deleteUsersConfigMap(configMapKey types.NamespacedName) {
 	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())
 }