Skip to content

Commit

Permalink
Add UserReference to permissions.rabbitmq.com spec
Browse files Browse the repository at this point in the history
  • Loading branch information
ChunyiLyu committed Jun 30, 2021
1 parent 794b1a4 commit 3f52a78
Show file tree
Hide file tree
Showing 7 changed files with 131 additions and 20 deletions.
6 changes: 4 additions & 2 deletions api/v1beta1/permission_types.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
package v1beta1

import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
)

// PermissionSpec defines the desired state of Permission
type PermissionSpec struct {
// Name of an existing user; required property; cannot be updated
// +kubebuilder:validation:Required
// Name of an existing user; must provide user or userReference, else create/update will fail; cannot be updated
User string `json:"user"`
// Reference to an existing user.rabbitmq.com object; must provide user or userReference, else create/update will fail; cannot be updated
UserReference *corev1.LocalObjectReference `json:"userReference,omitempty"`
// Name of an existing vhost; required property; cannot be updated
// +kubebuilder:validation:Required
Vhost string `json:"vhost"`
Expand Down
30 changes: 30 additions & 0 deletions api/v1beta1/permission_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
)
Expand Down Expand Up @@ -77,4 +78,33 @@ var _ = Describe("Permission spec", func() {
Expect(fetchedPermission.Spec.Permissions.Write).To(Equal(".*"))
Expect(fetchedPermission.Spec.Permissions.Read).To(Equal("^?"))
})

It("creates a permission object with user reference provided", func() {
permission := Permission{
ObjectMeta: metav1.ObjectMeta{
Name: "user-ref-permission",
Namespace: namespace,
},
Spec: PermissionSpec{
UserReference: &corev1.LocalObjectReference{
Name: "a-created-user",
},
Vhost: "/test",
Permissions: VhostPermissions{},
RabbitmqClusterReference: RabbitmqClusterReference{
Name: "some-cluster",
},
},
}
Expect(k8sClient.Create(ctx, &permission)).To(Succeed())
fetchedPermission := &Permission{}
Expect(k8sClient.Get(ctx, types.NamespacedName{
Name: permission.Name,
Namespace: permission.Namespace,
}, fetchedPermission)).To(Succeed())
Expect(fetchedPermission.Spec.UserReference.Name).To(Equal("a-created-user"))
Expect(fetchedPermission.Spec.User).To(Equal(""))
Expect(fetchedPermission.Spec.Vhost).To(Equal("/test"))
Expect(fetchedPermission.Spec.RabbitmqClusterReference.Name).To(Equal("some-cluster"))
})
})
7 changes: 6 additions & 1 deletion api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 12 additions & 2 deletions config/crd/bases/rabbitmq.com_permissions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,19 @@ spec:
- name
type: object
user:
description: Name of an existing user; required property; cannot be
updated
description: Name of an existing user; must provide user or userReference,
else create/update will fail; cannot be updated
type: string
userReference:
description: Reference to an existing user.rabbitmq.com object; must
provide user or userReference, else create/update will fail; cannot
be updated
properties:
name:
description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
TODO: Add other useful fields. apiVersion, kind, uid?'
type: string
type: object
vhost:
description: Name of an existing vhost; required property; cannot
be updated
Expand Down
68 changes: 58 additions & 10 deletions controllers/permission_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"k8s.io/apimachinery/pkg/types"
"time"

"github.com/rabbitmq/messaging-topology-operator/internal"
Expand Down Expand Up @@ -34,6 +36,7 @@ type PermissionReconciler struct {

// +kubebuilder:rbac:groups=rabbitmq.com,resources=permissions,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=rabbitmq.com,resources=permissions/status,verbs=get;update;patch
// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list

func (r *PermissionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
logger := ctrl.LoggerFrom(ctx)
Expand Down Expand Up @@ -71,9 +74,16 @@ func (r *PermissionReconciler) Reconcile(ctx context.Context, req ctrl.Request)
return reconcile.Result{}, err
}

user := permission.Spec.User
if permission.Spec.UserReference != nil {
if user, err = r.getUserFromReference(ctx, permission); err != nil {
return reconcile.Result{}, err
}
}

if !permission.ObjectMeta.DeletionTimestamp.IsZero() {
logger.Info("Deleting")
return ctrl.Result{}, r.revokePermissions(ctx, rabbitClient, permission)
return ctrl.Result{}, r.revokePermissions(ctx, rabbitClient, permission, user)
}

if err := r.addFinalizerIfNeeded(ctx, permission); err != nil {
Expand All @@ -88,7 +98,7 @@ func (r *PermissionReconciler) Reconcile(ctx context.Context, req ctrl.Request)
logger.Info("Start reconciling",
"spec", string(spec))

if err := r.updatePermissions(ctx, rabbitClient, permission); err != nil {
if err := r.updatePermissions(ctx, rabbitClient, permission, user); err != nil {
// Set Condition 'Ready' to false with message
permission.Status.Conditions = []topology.Condition{topology.NotReady(err.Error())}
if writerErr := clientretry.RetryOnConflict(clientretry.DefaultRetry, func() error {
Expand All @@ -111,17 +121,55 @@ func (r *PermissionReconciler) Reconcile(ctx context.Context, req ctrl.Request)
return ctrl.Result{}, nil
}

func (r *PermissionReconciler) updatePermissions(ctx context.Context, client internal.RabbitMQClient, permission *topology.Permission) error {
func (r *PermissionReconciler) getUserFromReference(ctx context.Context, permission *topology.Permission) (string, error) {
logger := ctrl.LoggerFrom(ctx)

// get User from provided user reference
failureMsg := "failed to get User"
user := &topology.User{}
if err := r.Get(ctx, types.NamespacedName{Name: permission.Spec.UserReference.Name, Namespace: permission.Namespace}, user); err != nil {
r.Recorder.Event(permission, corev1.EventTypeWarning, "FailedUpdate", failureMsg)
logger.Error(err, failureMsg, "userReference", permission.Spec.UserReference.Name)
return "", err
}

// get username from the credential secret from User status
if user.Status.Credentials == nil {
err := fmt.Errorf("this User does not have a credential secret set in its status")
r.Recorder.Event(permission, corev1.EventTypeWarning, "FailedUpdate", failureMsg)
logger.Error(err, failureMsg, "userReference", permission.Spec.UserReference.Name)
return "", err
}

secret := &corev1.Secret{}
if err := r.Get(ctx, types.NamespacedName{Name: user.Status.Credentials.Name, Namespace: user.Namespace}, secret); err != nil {
r.Recorder.Event(permission, corev1.EventTypeWarning, "FailedUpdate", failureMsg)
logger.Error(err, failureMsg, "userReference", permission.Spec.UserReference.Name)
return "", err
}

username, ok := secret.Data["username"]
if !ok {
err := fmt.Errorf("could not find username key")
r.Recorder.Event(permission, corev1.EventTypeWarning, "FailedUpdate", failureMsg)
logger.Error(err, failureMsg, "userReference", permission.Spec.UserReference.Name)
return "", nil
}

return string(username), nil
}

func (r *PermissionReconciler) updatePermissions(ctx context.Context, client internal.RabbitMQClient, permission *topology.Permission, user string) error {
logger := ctrl.LoggerFrom(ctx)

if err := validateResponse(client.UpdatePermissionsIn(permission.Spec.Vhost, permission.Spec.User, internal.GeneratePermissions(permission))); err != nil {
if err := validateResponse(client.UpdatePermissionsIn(permission.Spec.Vhost, user, internal.GeneratePermissions(permission))); err != nil {
msg := "failed to set permission"
r.Recorder.Event(permission, corev1.EventTypeWarning, "FailedUpdate", msg)
logger.Error(err, msg, "user", permission.Spec.User, "vhost", permission.Spec.Vhost)
logger.Error(err, msg, "user", user, "vhost", permission.Spec.Vhost)
return err
}

logger.Info("Successfully set permission", "user", permission.Spec.User, "vhost", permission.Spec.Vhost)
logger.Info("Successfully set permission", "user", user, "vhost", permission.Spec.Vhost)
r.Recorder.Event(permission, corev1.EventTypeNormal, "SuccessfulUpdate", "successfully set permission")
return nil
}
Expand All @@ -136,16 +184,16 @@ func (r *PermissionReconciler) addFinalizerIfNeeded(ctx context.Context, permiss
return nil
}

func (r *PermissionReconciler) revokePermissions(ctx context.Context, client internal.RabbitMQClient, permission *topology.Permission) error {
func (r *PermissionReconciler) revokePermissions(ctx context.Context, client internal.RabbitMQClient, permission *topology.Permission, user string) error {
logger := ctrl.LoggerFrom(ctx)

err := validateResponseForDeletion(client.ClearPermissionsIn(permission.Spec.Vhost, permission.Spec.User))
err := validateResponseForDeletion(client.ClearPermissionsIn(permission.Spec.Vhost, user))
if errors.Is(err, NotFound) {
logger.Info("cannot find user or vhost in rabbitmq server; no need to delete permission", "user", permission.Spec.User, "vhost", permission.Spec.Vhost)
logger.Info("cannot find user or vhost in rabbitmq server; no need to delete permission", "user", user, "vhost", permission.Spec.Vhost)
} else if err != nil {
msg := "failed to delete permission"
r.Recorder.Event(permission, corev1.EventTypeWarning, "FailedDelete", msg)
logger.Error(err, msg, "user", permission.Spec.User, "vhost", permission.Spec.Vhost)
logger.Error(err, msg, "user", user, "vhost", permission.Spec.Vhost)
return err
}
r.Recorder.Event(permission, corev1.EventTypeNormal, "SuccessfulDelete", "successfully deleted permission")
Expand Down
3 changes: 2 additions & 1 deletion docs/api/rabbitmq.com.ref.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,8 @@ PermissionSpec defines the desired state of Permission
[cols="25a,75a", options="header"]
|===
| Field | Description
| *`user`* __string__ | Name of an existing user; required property; cannot be updated
| *`user`* __string__ | Name of an existing user; must provide user or userReference, else create/update will fail; cannot be updated
| *`userReference`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.20/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | Reference to an existing user.rabbitmq.com object; must provide user or userReference, else create/update will fail; cannot be updated
| *`vhost`* __string__ | Name of an existing vhost; required property; cannot be updated
| *`permissions`* __xref:{anchor_prefix}-github-com-rabbitmq-messaging-topology-operator-api-v1beta1-vhostpermissions[$$VhostPermissions$$]__ | Permissions to grant to the user in the specific vhost; required property. See RabbitMQ doc for more information: https://www.rabbitmq.com/access-control.html#user-management
| *`rabbitmqClusterReference`* __xref:{anchor_prefix}-github-com-rabbitmq-messaging-topology-operator-api-v1beta1-rabbitmqclusterreference[$$RabbitmqClusterReference$$]__ | Reference to the RabbitmqCluster that both the provided user and vhost are. Required property.
Expand Down
23 changes: 19 additions & 4 deletions system_tests/permissions_system_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"
. "github.com/onsi/gomega/gstruct"

Expand Down Expand Up @@ -71,9 +72,19 @@ var _ = Describe("Permission", func() {

AfterEach(func() {
Expect(k8sClient.Delete(ctx, user)).To(Succeed())
Eventually(func() string {
if err := k8sClient.Get(ctx, types.NamespacedName{Name: user.Name, Namespace: user.Namespace}, &topology.User{}); err != nil {
return err.Error()
}
return ""
}, 10).Should(ContainSubstring("not found"))
})

It("grants and revokes permissions successfully", func() {
DescribeTable("Server configurations updates", func(testcase string) {
if testcase == "UserReference" {
permission.Spec.User = ""
permission.Spec.UserReference = &corev1.LocalObjectReference{Name: user.Name}
}
Expect(k8sClient.Create(ctx, permission, &client.CreateOptions{})).To(Succeed())
var fetchedPermissionInfo rabbithole.PermissionInfo
Eventually(func() error {
Expand All @@ -83,7 +94,7 @@ var _ = Describe("Permission", func() {
}, 20, 2).Should(Not(HaveOccurred()))
Expect(fetchedPermissionInfo).To(MatchFields(IgnoreExtras, Fields{
"Vhost": Equal(permission.Spec.Vhost),
"User": Equal(permission.Spec.User),
"User": Equal(username),
"Configure": Equal(permission.Spec.Permissions.Configure),
"Read": Equal(permission.Spec.Permissions.Read),
"Write": Equal(permission.Spec.Permissions.Write),
Expand Down Expand Up @@ -123,7 +134,7 @@ var _ = Describe("Permission", func() {
}, 20, 2).Should(Equal(".*"))
Expect(fetchedPermissionInfo).To(MatchFields(IgnoreExtras, Fields{
"Vhost": Equal(permission.Spec.Vhost),
"User": Equal(permission.Spec.User),
"User": Equal(username),
"Configure": Equal(permission.Spec.Permissions.Configure),
"Read": Equal("^$"),
"Write": Equal(".*"),
Expand All @@ -136,5 +147,9 @@ var _ = Describe("Permission", func() {
Expect(err).NotTo(HaveOccurred())
return len(permissionInfos)
}, 10, 2).Should(Equal(0))
})
},

Entry("grants and revokes permissions successfully when spec.user is set", "User"),
Entry("grants and revokes permissions successfully when spec.userReference is set", "UserReference"),
)
})

0 comments on commit 3f52a78

Please sign in to comment.