From 6f4e8acc078cb6d8909554ceda059acb3fc22c66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Depriester?= Date: Tue, 13 May 2025 22:17:59 +0200 Subject: [PATCH] feat: add support of granting membership on roles --- PROJECT | 8 ++ README.md | 2 + api/v1alpha1/postgresrole_types.go | 2 + api/v1alpha1/zz_generated.deepcopy.go | 7 +- ...-operator.hoppscale.com_postgresroles.yaml | 4 + config/crd/kustomization.yaml | 1 + config/rbac/kustomization.yaml | 1 - config/rbac/role.yaml | 32 ----- go.mod | 2 +- go.sum | 2 - .../controller/postgresrole_controller.go | 56 +++++++++ .../postgresrole_controller_test.go | 30 +++++ internal/postgresql/role_membership.go | 49 ++++++++ internal/postgresql/role_membership_test.go | 109 ++++++++++++++++++ 14 files changed, 268 insertions(+), 37 deletions(-) delete mode 100644 config/rbac/role.yaml create mode 100644 internal/postgresql/role_membership.go create mode 100644 internal/postgresql/role_membership_test.go diff --git a/PROJECT b/PROJECT index 87f1fe7..1c60f6d 100644 --- a/PROJECT +++ b/PROJECT @@ -16,4 +16,12 @@ resources: kind: PostgresDatabase path: github.com/hoppscale/managed-postgres-operator/api/v1alpha1 version: v1alpha1 +- api: + crdVersion: v1 + namespaced: true + controller: true + domain: managed-postgres-operator.hoppscale.com + kind: PostgresRole + path: github.com/hoppscale/managed-postgres-operator/api/v1alpha1 + version: v1alpha1 version: "3" diff --git a/README.md b/README.md index 0ec09fa..7001412 100644 --- a/README.md +++ b/README.md @@ -37,4 +37,6 @@ spec: replication: false # Is the role used for replication? bypassRLS: false # Should the role bypass the defined row-level security (RLS) policies? passwordSecretName: "my-secret" # Name of the secret from where the role's password should be retrieved under the key `password` + memberOfRoles: # List of roles the role should be member of + - anotherRole ``` diff --git a/api/v1alpha1/postgresrole_types.go b/api/v1alpha1/postgresrole_types.go index 6854890..2646311 100644 --- a/api/v1alpha1/postgresrole_types.go +++ b/api/v1alpha1/postgresrole_types.go @@ -36,6 +36,8 @@ type PostgresRoleSpec struct { BypassRLS bool `json:"bypassRLS,omitempty"` PasswordSecretName string `json:"passwordSecretName,omitempty"` + + MemberOfRoles []string `json:"memberOfRoles,omitempty"` } // PostgresRoleStatus defines the observed state of PostgresRole. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 599ca49..3092636 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -123,7 +123,7 @@ func (in *PostgresRole) DeepCopyInto(out *PostgresRole) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) out.Status = in.Status } @@ -180,6 +180,11 @@ func (in *PostgresRoleList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresRoleSpec) DeepCopyInto(out *PostgresRoleSpec) { *out = *in + if in.MemberOfRoles != nil { + in, out := &in.MemberOfRoles, &out.MemberOfRoles + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PostgresRoleSpec. diff --git a/config/crd/bases/managed-postgres-operator.hoppscale.com_postgresroles.yaml b/config/crd/bases/managed-postgres-operator.hoppscale.com_postgresroles.yaml index 431c395..23ca90b 100644 --- a/config/crd/bases/managed-postgres-operator.hoppscale.com_postgresroles.yaml +++ b/config/crd/bases/managed-postgres-operator.hoppscale.com_postgresroles.yaml @@ -49,6 +49,10 @@ spec: type: boolean login: type: boolean + memberOfRoles: + items: + type: string + type: array name: description: PostgreSQL role name type: string diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index b7b8ea0..5de0798 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -3,6 +3,7 @@ # It should be run by config/default resources: - bases/managed-postgres-operator.hoppscale.com_postgresdatabases.yaml +- bases/managed-postgres-operator.hoppscale.com_postgresroles.yaml # +kubebuilder:scaffold:crdkustomizeresource patches: diff --git a/config/rbac/kustomization.yaml b/config/rbac/kustomization.yaml index 3487beb..e623cb0 100644 --- a/config/rbac/kustomization.yaml +++ b/config/rbac/kustomization.yaml @@ -5,7 +5,6 @@ resources: # runtime. Be sure to update RoleBinding and ClusterRoleBinding # subjects if changing service account names. - service_account.yaml -- role.yaml - role_binding.yaml - leader_election_role.yaml - leader_election_role_binding.yaml diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml deleted file mode 100644 index 9389055..0000000 --- a/config/rbac/role.yaml +++ /dev/null @@ -1,32 +0,0 @@ ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: manager-role -rules: -- apiGroups: - - managed-postgres-operator.hoppscale.com - resources: - - postgresfunctions - verbs: - - create - - delete - - get - - list - - patch - - update - - watch -- apiGroups: - - managed-postgres-operator.hoppscale.com - resources: - - postgresfunctions/finalizers - verbs: - - update -- apiGroups: - - managed-postgres-operator.hoppscale.com - resources: - - postgresfunctions/status - verbs: - - get - - patch - - update diff --git a/go.mod b/go.mod index cbe13cc..23a6181 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/onsi/gomega v1.37.0 github.com/pashagolub/pgxmock/v4 v4.7.0 go.uber.org/zap v1.27.0 + k8s.io/api v0.32.4 k8s.io/apimachinery v0.32.4 k8s.io/client-go v0.32.4 sigs.k8s.io/controller-runtime v0.20.4 @@ -96,7 +97,6 @@ require ( gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - k8s.io/api v0.32.4 // indirect k8s.io/apiextensions-apiserver v0.32.4 // indirect k8s.io/apiserver v0.32.4 // indirect k8s.io/component-base v0.32.4 // indirect diff --git a/go.sum b/go.sum index 64e3968..fb02cb8 100644 --- a/go.sum +++ b/go.sum @@ -52,8 +52,6 @@ github.com/google/btree v1.1.3 h1:CVpQJjYgC4VbzxeGVHfvZrv1ctoYCAI8vbl07Fcxlyg= github.com/google/btree v1.1.3/go.mod h1:qOPhT0dTNdNzV6Z/lhRX0YXUafgPLFUh+gZMl761Gm4= github.com/google/cel-go v0.22.1 h1:AfVXx3chM2qwoSbM7Da8g8hX8OVSkBFwX+rz2+PcK40= github.com/google/cel-go v0.22.1/go.mod h1:BuznPXXfQDpXKWQ9sPW3TzlAJN5zzFe+i9tIs0yC4s8= -github.com/google/cel-go v0.25.0 h1:jsFw9Fhn+3y2kBbltZR4VEz5xKkcIFRPDnuEzAGv5GY= -github.com/google/cel-go v0.25.0/go.mod h1:hjEb6r5SuOSlhCHmFoLzu8HGCERvIsDAbxDAyNU/MmI= github.com/google/gnostic-models v0.6.9 h1:MU/8wDLif2qCXZmzncUQ/BOfxWfthHi63KqpoNbWqVw= github.com/google/gnostic-models v0.6.9/go.mod h1:CiWsm0s6BSQd1hRn8/QmxqB6BesYcbSZxsz9b0KuDBw= github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= diff --git a/internal/controller/postgresrole_controller.go b/internal/controller/postgresrole_controller.go index c30d476..01d07f8 100644 --- a/internal/controller/postgresrole_controller.go +++ b/internal/controller/postgresrole_controller.go @@ -144,6 +144,11 @@ func (r *PostgresRoleReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrlFailResult, err } + err = r.reconcileRoleMembership(desiredRole.Name, resource.Spec.MemberOfRoles) + if err != nil { + return ctrlFailResult, err + } + if !resource.Status.Succeeded { resource.Status.Succeeded = true if err = r.Client.Status().Update(context.Background(), resource); err != nil { @@ -225,3 +230,54 @@ func (r *PostgresRoleReconciler) reconcileOnCreation(existingRole, desiredRole * return err } + +func (r *PostgresRoleReconciler) reconcileRoleMembership(role string, desiredMembership []string) (err error) { + // Listing current membership + existingRoleMembership, err := postgresql.GetRoleMembership(r.PGPools.Default, role) + if err != nil { + r.logging.Error(err, "failed to retrieve role's membership") + return err + } + + // Revoking membership + for _, existingGroupRole := range existingRoleMembership { + found := false + for _, desiredGroupRole := range desiredMembership { + if desiredGroupRole == existingGroupRole { + found = true + break + } + } + + if !found { + err = postgresql.RevokeRoleMembership(r.PGPools.Default, existingGroupRole, role) + if err != nil { + r.logging.Error(err, "failed to revoke role membership") + return err + } + r.logging.Info(fmt.Sprintf("Role \"%s\" has been revoked from the group \"%s\"", role, existingGroupRole)) + } + } + + // Granting membership + for _, desiredGroupRole := range desiredMembership { + found := false + for _, existingGroupRole := range existingRoleMembership { + if desiredGroupRole == existingGroupRole { + found = true + break + } + } + + if !found { + err = postgresql.GrantRoleMembership(r.PGPools.Default, desiredGroupRole, role) + if err != nil { + r.logging.Error(err, "failed to grant role membership") + return err + } + r.logging.Info(fmt.Sprintf("Role \"%s\" has been granted to the group \"%s\"", role, desiredGroupRole)) + } + } + + return err +} diff --git a/internal/controller/postgresrole_controller_test.go b/internal/controller/postgresrole_controller_test.go index 171fd52..c1e8146 100644 --- a/internal/controller/postgresrole_controller_test.go +++ b/internal/controller/postgresrole_controller_test.go @@ -155,6 +155,14 @@ var _ = Describe("PostgresRole Controller", func() { ) pgpoolsMock["default"].ExpectExec(fmt.Sprintf("^%s$", regexp.QuoteMeta(`CREATE ROLE "foo" WITH NOSUPERUSER NOINHERIT CREATEROLE CREATEDB NOLOGIN NOREPLICATION NOBYPASSRLS`))). WillReturnResult(pgxmock.NewResult("foo", 1)) + pgpoolsMock["default"].ExpectQuery(fmt.Sprintf("^%s$", regexp.QuoteMeta(postgresql.GetRoleMembershipStatement))). + WithArgs("foo"). + WillReturnRows( + pgxmock.NewRows([]string{ + "group_role", + }), + ) + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ NamespacedName: typeNamespacedName, }) @@ -219,6 +227,14 @@ var _ = Describe("PostgresRole Controller", func() { ) pgpoolsMock["default"].ExpectExec(fmt.Sprintf("^%s$", regexp.QuoteMeta(`CREATE ROLE "foo" WITH NOSUPERUSER NOINHERIT CREATEROLE CREATEDB NOLOGIN NOREPLICATION NOBYPASSRLS`))). WillReturnResult(pgxmock.NewResult("foo", 1)) + pgpoolsMock["default"].ExpectQuery(fmt.Sprintf("^%s$", regexp.QuoteMeta(postgresql.GetRoleMembershipStatement))). + WithArgs("foo"). + WillReturnRows( + pgxmock.NewRows([]string{ + "group_role", + }), + ) + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ NamespacedName: typeNamespacedName, }) @@ -263,6 +279,13 @@ var _ = Describe("PostgresRole Controller", func() { false, ), ) + pgpoolsMock["default"].ExpectQuery(fmt.Sprintf("^%s$", regexp.QuoteMeta(postgresql.GetRoleMembershipStatement))). + WithArgs("foo"). + WillReturnRows( + pgxmock.NewRows([]string{ + "group_role", + }), + ) _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ NamespacedName: typeNamespacedName, @@ -305,6 +328,13 @@ var _ = Describe("PostgresRole Controller", func() { ) pgpoolsMock["default"].ExpectExec(fmt.Sprintf("^%s$", regexp.QuoteMeta(`CREATE ROLE "foo" WITH NOSUPERUSER NOINHERIT CREATEROLE CREATEDB NOLOGIN NOREPLICATION NOBYPASSRLS PASSWORD 'password'`))). WillReturnResult(pgxmock.NewResult("foo", 1)) + pgpoolsMock["default"].ExpectQuery(fmt.Sprintf("^%s$", regexp.QuoteMeta(postgresql.GetRoleMembershipStatement))). + WithArgs("foo"). + WillReturnRows( + pgxmock.NewRows([]string{ + "group_role", + }), + ) _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ NamespacedName: typeNamespacedName, diff --git a/internal/postgresql/role_membership.go b/internal/postgresql/role_membership.go new file mode 100644 index 0000000..67ccf0b --- /dev/null +++ b/internal/postgresql/role_membership.go @@ -0,0 +1,49 @@ +package postgresql + +import ( + "context" + "fmt" + + "github.com/jackc/pgx/v5" +) + +const GetRoleMembershipStatement = "SELECT roleid::regrole::text AS group_role FROM pg_auth_members WHERE member::regrole::text = $1" + +func GetRoleMembership(pgpool PGPoolInterface, role string) (membership []string, err error) { + rows, err := pgpool.Query(context.Background(), GetRoleMembershipStatement, role) + if err != nil { + err = fmt.Errorf("pg query failed: %s", err) + return + } + defer rows.Close() + + membership, err = pgx.CollectRows(rows, pgx.RowTo[string]) + if err != nil { + err = fmt.Errorf("failed to collect rows: %s", err) + return + } + + return +} + +func GrantRoleMembership(pgpool PGPoolInterface, groupRole, role string) (err error) { + sanitizedGroupRole := pgx.Identifier{groupRole}.Sanitize() + sanitizedRole := pgx.Identifier{role}.Sanitize() + _, err = pgpool.Exec(context.Background(), fmt.Sprintf("GRANT %s TO %s", sanitizedGroupRole, sanitizedRole)) + if err != nil { + err = fmt.Errorf("pg exec failed: %s", err) + return + } + return +} + +func RevokeRoleMembership(pgpool PGPoolInterface, groupRole, role string) (err error) { + sanitizedGroupRole := pgx.Identifier{groupRole}.Sanitize() + sanitizedRole := pgx.Identifier{role}.Sanitize() + _, err = pgpool.Exec(context.Background(), fmt.Sprintf("REVOKE %s FROM %s", sanitizedGroupRole, sanitizedRole)) + if err != nil { + err = fmt.Errorf("pg exec failed: %s", err) + return + } + return +} diff --git a/internal/postgresql/role_membership_test.go b/internal/postgresql/role_membership_test.go new file mode 100644 index 0000000..5211d6b --- /dev/null +++ b/internal/postgresql/role_membership_test.go @@ -0,0 +1,109 @@ +package postgresql + +import ( + "fmt" + "regexp" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + pgxmock "github.com/pashagolub/pgxmock/v4" +) + +var _ = Describe("PostgreSQL Role", func() { + var pgpoolMock pgxmock.PgxPoolIface + var pgpool PGPoolInterface + + BeforeEach(func() { + mock, err := pgxmock.NewPool() + if err != nil { + Fail(err.Error()) + } + pgpoolMock = mock + pgpool = mock + }) + AfterEach(func() { + pgpoolMock.Close() + }) + + Context("Calling GetRoleMemberships", func() { + When("the role has no membership", func() { + It("should an empty list of roles and no error", func() { + pgpoolMock.ExpectQuery(fmt.Sprintf("^%s$", regexp.QuoteMeta(GetRoleMembershipStatement))). + WithArgs("foo"). + WillReturnRows( + pgxmock.NewRows([]string{ + "group_role", + }), + ) + + result, err := GetRoleMembership(pgpool, "foo") + + Expect(err).NotTo(HaveOccurred()) + if err := pgpoolMock.ExpectationsWereMet(); err != nil { + Fail(err.Error()) + } + + Expect(result).To(BeEmpty()) + }) + }) + + When("the role is a member of multiple roles", func() { + It("should return the list of group roles and no error", func() { + pgpoolMock.ExpectQuery(fmt.Sprintf("^%s$", regexp.QuoteMeta(GetRoleMembershipStatement))). + WithArgs("foo"). + WillReturnRows( + pgxmock.NewRows([]string{ + "group_role", + }). + AddRow( + "alpha", + ). + AddRow( + "beta", + ), + ) + + result, err := GetRoleMembership(pgpool, "foo") + + Expect(err).NotTo(HaveOccurred()) + if err := pgpoolMock.ExpectationsWereMet(); err != nil { + Fail(err.Error()) + } + + Expect(result).To(Equal([]string{"alpha", "beta"})) + }) + }) + }) + + Context("Calling GrantRoleMembership", func() { + It("should successfully grant the role to the group role", func() { + + pgpoolMock.ExpectExec(fmt.Sprintf("^%s$", regexp.QuoteMeta(`GRANT "foo" TO "bar"`))). + WillReturnResult(pgxmock.NewResult("", 1)) + + err := GrantRoleMembership(pgpool, "foo", "bar") + + Expect(err).NotTo(HaveOccurred()) + if err := pgpoolMock.ExpectationsWereMet(); err != nil { + Fail(err.Error()) + } + }) + }) + + Context("Calling RevokeRoleMembership", func() { + It("should successfully revoke the role from the group role", func() { + + pgpoolMock.ExpectExec(fmt.Sprintf("^%s$", regexp.QuoteMeta(`REVOKE "foo" FROM "bar"`))). + WillReturnResult(pgxmock.NewResult("", 1)) + + err := RevokeRoleMembership(pgpool, "foo", "bar") + + Expect(err).NotTo(HaveOccurred()) + if err := pgpoolMock.ExpectationsWereMet(); err != nil { + Fail(err.Error()) + } + }) + }) + +})