Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grant read access to additional users on image repository creation, #143

Merged
merged 1 commit into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions config/rbac/role.yaml
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

48 changes: 47 additions & 1 deletion controllers/imagerepository_controller.go
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,10 @@ func (r *ImageRepositoryReconciler) ProvisionImageRepository(ctx context.Context
}
}

if err = r.GrantAdditionalRepositoryAccess(ctx, imageRepository); 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 +418,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 +436,45 @@ 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")

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)
}
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
70 changes: 46 additions & 24 deletions controllers/imagerepository_controller_test.go
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,19 +500,25 @@ 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
}
Expand Down Expand Up @@ -565,8 +571,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 +654,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 +678,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
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, " ")
mmorhun marked this conversation as resolved.
Show resolved Hide resolved

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
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
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 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
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
Loading