Skip to content

Commit

Permalink
Grant read access to additional users on image repository creation,
Browse files Browse the repository at this point in the history 
based on config map in the user namespace named "image-controller-additional-users"

STONEBLD-2666

Signed-off-by: Robert Cerven <[email protected]>
  • Loading branch information
@rcerven
rcerven committed Aug 7, 2024
1 parent 2db388b commit 2464d43
Show file tree
Hide file tree
Showing 9 changed files with 280 additions and 54 deletions.
8 changes: 8 additions & 0 deletions config/rbac/role.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,12 @@ rules:
- get
- patch
- update
- apiGroups:
- ""
resources:
- configmaps
verbs:
- get
- list
- watch

55 changes: 54 additions & 1 deletion controllers/imagerepository_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ const (

buildPipelineServiceAccountName = "appstudio-pipeline"
updateComponentAnnotationName = "image-controller.appstudio.redhat.com/update-component-image"
additionalUsersConfigMapName = "image-controller-additional-users"
additionalUsersConfigMapKey = "quay.io"
)

// ImageRepositoryReconciler reconciles a ImageRepository object
Expand Down Expand Up @@ -79,6 +81,7 @@ func setMetricsTime(idForMetrics string, reconcileStartTime time.Time) {
//+kubebuilder:rbac:groups=appstudio.redhat.com,resources=imagerepositories/status,verbs=get;update;patch
//+kubebuilder:rbac:groups=appstudio.redhat.com,resources=imagerepositories/finalizers,verbs=update
//+kubebuilder:rbac:groups=appstudio.redhat.com,resources=components,verbs=get;list;watch
//+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

Expand Down Expand Up @@ -343,6 +346,11 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context
}
}

err = r.GrantAdditionalRepositoryAccess(ctx, imageRepository)
if err != nil {
return err
}

var notificationStatus []imagerepositoryv1alpha1.NotificationStatus
if notificationStatus, err = r.SetNotifications(ctx, imageRepository); err != nil {
return err
Expand Down Expand Up @@ -411,7 +419,7 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepositoryAccess(ctx context.C
return nil, err
}

err = r.QuayClient.AddPermissionsForRepositoryToRobotAccount(r.QuayOrganization, imageRepositoryName, robotAccount.Name, !isPullOnly)
err = r.QuayClient.AddPermissionsForRepositoryToAccount(r.QuayOrganization, imageRepositoryName, robotAccount.Name, true, !isPullOnly)
if err != nil {
log.Error(err, "failed to add permissions to robot account", "RobotAccountName", robotAccountName, l.Action, l.ActionUpdate, l.Audit, "true")
return nil, err
Expand All @@ -429,6 +437,51 @@ 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")
ctx = ctrllog.IntoContext(ctx, log)

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
}
additionalUsersStr, 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

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
}

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)
}
listPerms, _ := r.QuayClient.ListPermissionsForRepository(r.QuayOrganization, imageRepositoryName)
log.Info("Repository has now following permissions", "RepositoryName", imageRepositoryName)
for _, user := range listPerms {
log.Info(fmt.Sprintf("name: %s; role: %s; is robot: %t; is org member: %t", user.Name, user.Role, user.IsRobot, user.IsOrgMember))
}
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)
Expand Down
73 changes: 49 additions & 24 deletions controllers/imagerepository_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,14 @@ var _ = Describe("Image repository controller", func() {
Expect(strings.HasPrefix(robotName, expectedRobotAccountPrefix)).To(BeTrue())
return &quay.RobotAccount{Name: robotName, Token: pushToken}, nil
}
isAddPushPermissionsToRobotAccountInvoked := false
quay.AddPermissionsForRepositoryToRobotAccountFunc = func(organization, imageRepository, robotAccountName string, isWrite bool) error {
isAddPushPermissionsToAccountInvoked := false
quay.AddPermissionsForRepositoryToAccountFunc = func(organization, imageRepository, accountName string, isRobot, isWrite bool) error {
defer GinkgoRecover()
isAddPushPermissionsToRobotAccountInvoked = true
isAddPushPermissionsToAccountInvoked = true
Expect(organization).To(Equal(quay.TestQuayOrg))
Expect(imageRepository).To(Equal(expectedImageName))
Expect(isWrite).To(BeTrue())
Expect(strings.HasPrefix(robotAccountName, expectedRobotAccountPrefix)).To(BeTrue())
Expect(strings.HasPrefix(accountName, expectedRobotAccountPrefix)).To(BeTrue())
return nil
}

Expand All @@ -105,7 +105,7 @@ var _ = Describe("Image repository controller", func() {

Eventually(func() bool { return isCreateRepositoryInvoked }, timeout, interval).Should(BeTrue())
Eventually(func() bool { return isCreateRobotAccountInvoked }, timeout, interval).Should(BeTrue())
Eventually(func() bool { return isAddPushPermissionsToRobotAccountInvoked }, timeout, interval).Should(BeTrue())
Eventually(func() bool { return isAddPushPermissionsToAccountInvoked }, timeout, interval).Should(BeTrue())
Eventually(func() bool { return isCreateNotificationInvoked }, timeout, interval).Should(BeFalse())

waitImageRepositoryFinalizerOnImageRepository(resourceKey)
Expand Down Expand Up @@ -356,14 +356,14 @@ var _ = Describe("Image repository controller", func() {
Expect(strings.HasPrefix(robotName, expectedRobotAccountPrefix)).To(BeTrue())
return &quay.RobotAccount{Name: robotName, Token: pushToken}, nil
}
isAddPushPermissionsToRobotAccountInvoked := false
quay.AddPermissionsForRepositoryToRobotAccountFunc = func(organization, imageRepository, robotAccountName string, isWrite bool) error {
isAddPushPermissionsToAccountInvoked := false
quay.AddPermissionsForRepositoryToAccountFunc = func(organization, imageRepository, accountName string, isRobot, isWrite bool) error {
defer GinkgoRecover()
isAddPushPermissionsToRobotAccountInvoked = true
isAddPushPermissionsToAccountInvoked = true
Expect(organization).To(Equal(quay.TestQuayOrg))
Expect(imageRepository).To(Equal(expectedImageName))
Expect(isWrite).To(BeTrue())
Expect(strings.HasPrefix(robotAccountName, expectedRobotAccountPrefix)).To(BeTrue())
Expect(strings.HasPrefix(accountName, expectedRobotAccountPrefix)).To(BeTrue())
return nil
}

Expand All @@ -377,7 +377,7 @@ var _ = Describe("Image repository controller", func() {

Eventually(func() bool { return isCreateRepositoryInvoked }, timeout, interval).Should(BeTrue())
Eventually(func() bool { return isCreateRobotAccountInvoked }, timeout, interval).Should(BeTrue())
Eventually(func() bool { return isAddPushPermissionsToRobotAccountInvoked }, timeout, interval).Should(BeTrue())
Eventually(func() bool { return isAddPushPermissionsToAccountInvoked }, timeout, interval).Should(BeTrue())
Eventually(func() bool { return isCreateNotificationInvoked }, timeout, interval).Should(BeFalse())

waitImageRepositoryFinalizerOnImageRepository(resourceKey)
Expand Down Expand Up @@ -476,7 +476,7 @@ var _ = Describe("Image repository controller", func() {
createServiceAccount(defaultNamespace, buildPipelineServiceAccountName)
})

assertProvisionRepository := func(updateComponentAnnotation bool) {
assertProvisionRepository := func(updateComponentAnnotation bool, additionalUser string) {
isCreateRepositoryInvoked := false
quay.CreateRepositoryFunc = func(repository quay.RepositoryRequest) (*quay.Repository, error) {
defer GinkgoRecover()
Expand All @@ -500,22 +500,31 @@ var _ = Describe("Image repository controller", func() {
isCreatePushRobotAccountInvoked = true
return &quay.RobotAccount{Name: robotName, Token: pushToken}, nil
}
isAddPushPermissionsToRobotAccountInvoked := false
isAddPullPermissionsToRobotAccountInvoked := false
quay.AddPermissionsForRepositoryToRobotAccountFunc = func(organization, imageRepository, robotAccountName string, isWrite bool) error {
isAddPushPermissionsToAccountInvoked := false
isAddPullPermissionsToAccountInvoked := 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(robotAccountName, expectedRobotAccountPrefix)).To(BeTrue())
if strings.HasSuffix(robotAccountName, "_pull") {
Expect(isWrite).To(BeFalse())
isAddPullPermissionsToRobotAccountInvoked = true

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(isWrite).To(BeTrue())
isAddPushPermissionsToRobotAccountInvoked = true
Expect(accountName).To(Equal(additionalUser))
Expect(isWrite).To(BeFalse())
}
return nil
}
quay.ListPermissionsForRepositoryFunc = func(organization, imageRepository string) (map[string]quay.UserAccount, error) {
return nil, nil
}
isCreateNotificationInvoked := false
quay.CreateNotificationFunc = func(organization, repository string, notification quay.Notification) (*quay.Notification, error) {
isCreateNotificationInvoked = true
Expand Down Expand Up @@ -565,8 +574,8 @@ 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 isAddPushPermissionsToRobotAccountInvoked }, timeout, interval).Should(BeTrue())
Eventually(func() bool { return isAddPullPermissionsToRobotAccountInvoked }, 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())

waitImageRepositoryFinalizerOnImageRepository(resourceKey)
Expand Down Expand Up @@ -648,7 +657,22 @@ var _ = Describe("Image repository controller", func() {
}

It("should provision image repository for component, without update component annotation", func() {
assertProvisionRepository(false)
assertProvisionRepository(false, "")

quay.DeleteRobotAccountFunc = func(organization, robotAccountName string) (bool, error) {
return true, nil
}
quay.DeleteRepositoryFunc = func(organization, imageRepository string) (bool, error) {
return true, nil
}

deleteImageRepository(resourceKey)
})

It("should provision image repository for component, with update component annotation and add additional user from config map", func() {
usersConfigMapKey := types.NamespacedName{Name: additionalUsersConfigMapName, Namespace: resourceKey.Namespace}
createUsersConfigMap(usersConfigMapKey, []string{"user1"})
assertProvisionRepository(true, "user1")

quay.DeleteRobotAccountFunc = func(organization, robotAccountName string) (bool, error) {
return true, nil
Expand All @@ -657,11 +681,12 @@ var _ = Describe("Image repository controller", func() {
return true, nil
}

deleteUsersConfigMap(usersConfigMapKey)
deleteImageRepository(resourceKey)
})

It("should provision image repository for component, with update component annotation", func() {
assertProvisionRepository(true)
assertProvisionRepository(true, "")
})

It("should regenerate tokens and update secrets", func() {
Expand Down
28 changes: 28 additions & 0 deletions controllers/suite_util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package controllers

import (
"strings"
"time"

. "github.com/onsi/ginkgo/v2"
Expand Down Expand Up @@ -348,3 +349,30 @@ func deleteServiceAccount(serviceAccountKey types.NamespacedName) {
return k8sErrors.IsNotFound(k8sClient.Get(ctx, serviceAccountKey, serviceAccount))
}, 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())
}
}
7 changes: 7 additions & 0 deletions pkg/quay/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ type RobotAccount struct {
Message string `json:"message"`
}

type UserAccount struct {
Name string `json:"name"`
Role string `json:"role"`
IsRobot bool `json:"is_robot"`
IsOrgMember bool `json:"is_org_member"`
}

// Quay API can sometimes return {"error": "..."} and sometimes {"error_message": "..."} without the field error
// In some cases the error is send alongside the response in the {"message": "..."} field.
type QuayError struct {
Expand Down
47 changes: 38 additions & 9 deletions pkg/quay/quay.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ type QuayService interface {
GetRobotAccount(organization string, robotName string) (*RobotAccount, error)
CreateRobotAccount(organization string, robotName string) (*RobotAccount, error)
DeleteRobotAccount(organization string, robotName string) (bool, error)
AddPermissionsForRepositoryToRobotAccount(organization, imageRepository, robotAccountName string, isWrite bool) error
AddPermissionsForRepositoryToAccount(organization, imageRepository, accountName string, isRobot, isWrite bool) error
ListPermissionsForRepository(organization, imageRepository string) (map[string]UserAccount, error)
RegenerateRobotAccountToken(organization string, robotName string) (*RobotAccount, error)
GetAllRepositories(organization string) ([]Repository, error)
GetAllRobotAccounts(organization string) ([]RobotAccount, error)
Expand Down Expand Up @@ -359,18 +360,46 @@ func (c *QuayClient) DeleteRobotAccount(organization string, robotName string) (
return false, errors.New(data.ErrorMessage)
}

// AddPermissionsForRepositoryToRobotAccount allows given robot account to access to the given repository.
// ListPermissionsForRepository list permissions for the given repository.
func (c *QuayClient) ListPermissionsForRepository(organization, imageRepository string) (map[string]UserAccount, error) {
url := fmt.Sprintf("%s/repository/%s/%s/permissions/user/", c.url, organization, imageRepository)

resp, err := c.doRequest(url, http.MethodGet, nil)

if err != nil {
return nil, fmt.Errorf("failed to Do request, error: %s", err)
}
if resp.GetStatusCode() != 200 {
return nil, fmt.Errorf("error getting permissions, got status code %d", resp.GetStatusCode())
}

type Response struct {
Permissions map[string]UserAccount `json:"permissions"`
}
var response Response
if err := resp.GetJson(&response); err != nil {
return nil, err
}

return response.Permissions, nil
}

// AddPermissionsForRepositoryToAccount allows given robot account to access to the given repository.
// If isWrite is true, then pull and push permissions are added, otherwise - pull access only.
func (c *QuayClient) AddPermissionsForRepositoryToRobotAccount(organization, imageRepository, robotAccountName string, isWrite bool) error {
var robotAccountFullName string
if robotName, err := handleRobotName(robotAccountName); err == nil {
robotAccountFullName = organization + "+" + robotName
func (c *QuayClient) AddPermissionsForRepositoryToAccount(organization, imageRepository, accountName string, isRobot, isWrite bool) error {
var accountFullName string
if isRobot {
if robotName, err := handleRobotName(accountName); err == nil {
accountFullName = organization + "+" + robotName
} else {
return err
}
} else {
return err
accountFullName = accountName
}

// url := "https://quay.io/api/v1/repository/redhat-appstudio/test-repo-using-api/permissions/user/redhat-appstudio+createdbysbose"
url := fmt.Sprintf("%s/repository/%s/%s/permissions/user/%s", c.url, organization, imageRepository, robotAccountFullName)
url := fmt.Sprintf("%s/repository/%s/%s/permissions/user/%s", c.url, organization, imageRepository, accountFullName)

role := "read"
if isWrite {
Expand All @@ -392,7 +421,7 @@ func (c *QuayClient) AddPermissionsForRepositoryToRobotAccount(organization, ima
message = data.Error
}
}
return fmt.Errorf("failed to add permissions to the robot account. Status code: %d, message: %s", resp.GetStatusCode(), message)
return fmt.Errorf("failed to add permissions to the account: %s. Status code: %d, message: %s", accountFullName, resp.GetStatusCode(), message)
}
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/quay/quay_debug_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func TestAddPermissionsToRobotAccount(t *testing.T) {
}
quayClient := NewQuayClient(&http.Client{Transport: &http.Transport{}}, quayToken, quayApiUrl)

err := quayClient.AddPermissionsForRepositoryToRobotAccount(quayOrgName, quayImageRepoName, quayRobotAccountName, true)
err := quayClient.AddPermissionsForRepositoryToAccount(quayOrgName, quayImageRepoName, quayRobotAccountName, true, true)
if err != nil {
t.Fatal(err)
}
Expand Down
Loading

0 comments on commit 2464d43

Please sign in to comment.