rook / rook

Storage Orchestration for Kubernetes
https://rook.io
Apache License 2.0
12.41k stars 2.69k forks source link

rgw: add support for admin and reader accepted roles #14838

Open kayrus opened 1 month ago

kayrus commented 1 month ago

Is this a bug report or feature request?

What should the feature do:

The recent keystone integration uses only accepted roles. However Ceph supports admin and reader (available since Ceph v19) accepted roles.

What is use case behind this feature:

Add a role for cloud admin access and account level read-only access.

Environment:

travisn commented 1 month ago

@Lykos153 could you take a look?

kayrus commented 1 month ago

If this makes sense I started locally with a draft:

diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go
index b9d1f4fed..3df6b6d24 100755
--- a/pkg/apis/ceph.rook.io/v1/types.go
+++ b/pkg/apis/ceph.rook.io/v1/types.go
@@ -1727,8 +1727,12 @@ type KeystoneSpec struct {
    Url string `json:"url"`
    // The name of the secret containing the credentials for the service user account used by RGW. It has to be in the same namespace as the object store resource.
    ServiceUserSecretName string `json:"serviceUserSecretName"`
-   // The roles requires to serve requests.
+   // The roles require to serve requests.
    AcceptedRoles []string `json:"acceptedRoles"`
+   // The cloud admin roles require to serve requests.
+   AcceptedAdminRoles []string `json:"acceptedAdminRoles,omitempty"`
+   // The reader roles require to serve the read-only requests.
+   AcceptedReaderRoles []string `json:"acceptedReaderRoles,omitempty"`
    // Create new users in their own tenants of the same name. Possible values are true, false, swift and s3. The latter have the effect of splitting the identity space such that only the indicated protocol will use implicit tenants.
    // +optional
    ImplicitTenants ImplicitTenantSetting `json:"implicitTenants,omitempty"`
diff --git a/pkg/operator/ceph/object/config.go b/pkg/operator/ceph/object/config.go
index 31082d7bf..6b8c85ae1 100644
--- a/pkg/operator/ceph/object/config.go
+++ b/pkg/operator/ceph/object/config.go
@@ -169,7 +169,7 @@ func (c *clusterConfig) setFlagsMonConfigStore(rgwConfig *rgwConfig) error {
    configOptions["rgw_zone"] = rgwConfig.Zone
    configOptions["rgw_zonegroup"] = rgwConfig.ZoneGroup

-   configOptions, err := configureKeystoneAuthentication(rgwConfig, configOptions)
+   configOptions, err := c.configureKeystoneAuthentication(rgwConfig, configOptions)
    if err != nil {
        return err
    }
@@ -233,7 +233,7 @@ func (c *clusterConfig) setFlagsMonConfigStore(rgwConfig *rgwConfig) error {
    return nil
 }

-func configureKeystoneAuthentication(rgwConfig *rgwConfig, configOptions map[string]string) (map[string]string, error) {
+func (c *clusterConfig) configureKeystoneAuthentication(rgwConfig *rgwConfig, configOptions map[string]string) (map[string]string, error) {

    keystone := rgwConfig.Auth.Keystone
    if keystone == nil {
@@ -245,6 +245,18 @@ func configureKeystoneAuthentication(rgwConfig *rgwConfig, configOptions map[str

    configOptions["rgw_keystone_url"] = keystone.Url
    configOptions["rgw_keystone_accepted_roles"] = strings.Join(keystone.AcceptedRoles, ",")
+
+   if len(keystone.AcceptedAdminRoles) > 0 {
+       configOptions["rgw_keystone_accepted_admin_roles"] = strings.Join(keystone.AcceptedAdminRoles, ",")
+   }
+
+   if len(keystone.AcceptedReaderRoles) > 0 {
+       if !c.clusterInfo.CephVersion.IsAtLeastSquid() {
+           return nil, errors.New("rgw keystone accepted_reader_roles are supported from ceph v19 onwards")
+       }
+       configOptions["rgw_keystone_accepted_reader_roles"] = strings.Join(keystone.AcceptedReaderRoles, ",")
+   }
+
    if keystone.ImplicitTenants != "" {
        lc := strings.ToLower(string(keystone.ImplicitTenants))
Lykos153 commented 8 hours ago

@kayrus That patch looks promising. Do you want to create a draft PR so we can work out the details together?