reana-admin: fix setting default quota limits

This commit fixes a bug in the reana-admin CLI command
``quota-set-default-limits`` that prevented admins from retroactively
setting a default quota limit for all existing users who do not have a
custom quota limit set. Specifically, the bug was caused by the fact
that the code assumed that all users who do not have a custom quota
limit set have a quota limit of ``null``. This assumption is incorrect
because when there is no default global quota limit set, each user's
initial quota limit is set to zero, not ``null``.

Note that any previously set user limits, either via old defaults or via
custom settings, will be kept during the upgrade, and won't be
automatically updated to match the new default limit value.

Closes #625

AUTHORS.rst

@@ -7,6 +7,7 @@ The list of contributors in alphabetical order:
 - `Audrius Mecionis <https://orcid.org/0000-0002-3759-1663>`_
 - `Anton Khodak <https://orcid.org/0000-0003-3263-4553>`_
 - `Camila Diaz <https://orcid.org/0000-0001-5543-797X>`_
+- `Daan Rosendal <https://github.com/DaanRosendal>`_
 - `Diego Rodriguez <https://orcid.org/0000-0003-0649-2002>`_
 - `Dinos Kousidis <https://orcid.org/0000-0002-4914-4289>`_
 - `Domenic Gosein <https://orcid.org/0000-0002-1546-0435>`_

CHANGES.rst

@@ -4,6 +4,7 @@ Changes
 Version 0.9.1 (UNRELEASED)
 --------------------------
 
+- Fixes ``quota-set-default-limits`` command to propagate default quota limits to all users without custom quota limit values..
 - Changes ``launch`` endpoint also include the warnings of the validation of the workflow specification.
 - Changes OpenAPI specification with respect to return the maximum inactivity time before automatic closure of interactive sessions in ``info`` endpoint.
 - Adds the timestamp of when the workflow was stopped (``run_stopped_at``) to the workflow list and the workflow status endpoints.

reana_server/reana_admin/cli.py

@@ -16,20 +16,20 @@
 from typing import List, Optional
 
 import click
-import tablib
 import requests
+import tablib
 from flask.cli import with_appcontext
 from invenio_accounts.utils import register_user
 from kubernetes.client.rest import ApiException
 from reana_commons.config import (
-    REANAConfig,
     REANA_RESOURCE_HEALTH_COLORS,
     REANA_RUNTIME_KUBERNETES_NAMESPACE,
+    REANAConfig,
 )
-from reana_commons.email import send_email, REANA_EMAIL_SENDER
+from reana_commons.email import REANA_EMAIL_SENDER, send_email
 from reana_commons.errors import REANAEmailNotificationError
-from reana_commons.utils import click_table_printer
 from reana_commons.k8s.api_client import current_k8s_corev1_api_client
+from reana_commons.utils import click_table_printer
 from reana_db.config import DEFAULT_QUOTA_LIMITS
 from reana_db.database import Session
 from reana_db.models import (
@@ -46,6 +46,7 @@
 )
 from reana_db.utils import update_workspace_retention_rules
 
+from reana_server.api_client import current_rwc_api_client
 from reana_server.config import ADMIN_USER_ID, REANA_HOSTNAME
 from reana_server.reana_admin.check_workflows import check_workspaces
 from reana_server.reana_admin.options import (
@@ -55,8 +56,8 @@
 )
 from reana_server.reana_admin.retention_rule_deleter import RetentionRuleDeleter
 from reana_server.status import STATUS_OBJECT_TYPES
-from reana_server.api_client import current_rwc_api_client
 from reana_server.utils import (
+    JinjaEnv,
     _create_user,
     _export_users,
     _get_user_by_criteria,
@@ -65,7 +66,6 @@
     _validate_email,
     _validate_password,
     create_user_workspace,
-    JinjaEnv,
 )
 
 
@@ -638,24 +638,53 @@ def set_quota_limit(
 
 @reana_admin.command(
     "quota-set-default-limits",
-    help="Set default quota limits to users that do not have any.",
+    help="""Set default quota limits for users who do not have any custom limits
+         defined.
+
+    Note that any previously set user limits, either via old defaults or via
+    custom settings, will be kept during the upgrade, and won't be automatically
+    updated to match the new default limit value.""",
 )
 @admin_access_token_option
 @click.pass_context
 def set_default_quota_limit(ctx, admin_access_token: str):
-    """Set default quota limits to users that do not have any."""
-    users_without_quota_limits = User.query.filter(~User.resources.any()).all()
+    """Set default quota limits for users who do not have any custom limits defined."""
+    users_without_quota_limits = (
+        Session.query(User)
+        .filter(
+            User.id_.in_(
+                Session.query(UserResource.user_id).filter(
+                    UserResource.quota_limit == 0
+                )
+            )
+        )
+        .all()
+    )
+
     if not users_without_quota_limits:
         click.secho("There are no users without quota limits.", fg="green")
         sys.exit(0)
-    for resource in Resource.query:
-        ctx.invoke(
-            set_quota_limit,
-            emails=[user.email for user in users_without_quota_limits],
-            resource_name=resource.name,
-            resource_type=resource.type_.name,
-            limit=DEFAULT_QUOTA_LIMITS.get(resource.type_.name),
-        )
+
+    resources = Resource.query.all()
+
+    for user in users_without_quota_limits:
+        for resource in resources:
+            user_resource = UserResource.query.filter_by(
+                user_id=user.id_, resource_id=resource.id_
+            ).first()
+
+            if user_resource and user_resource.quota_limit == 0:
+                # If no limit exists, set the default limit
+                default_limit = DEFAULT_QUOTA_LIMITS.get(resource.type_.name)
+                if default_limit is not None and default_limit != 0:
+                    ctx.invoke(
+                        set_quota_limit,
+                        emails=[user.email],
+                        resource_name=resource.name,
+                        resource_type=resource.type_.name,
+                        limit=default_limit,
+                        admin_access_token=admin_access_token,
+                    )
 
 
 @reana_admin.command("queue-consume")
@@ -901,10 +930,10 @@ def check_workflows(
 ) -> None:
     """Check consistency of selected workflow run statuses between database, message queue and Kubernetes."""
     from .check_workflows import (
-        check_workflows,
+        InfoCollectionError,
         check_interactive_sessions,
+        check_workflows,
         display_results,
-        InfoCollectionError,
     )
 
     click.secho("Checking if workflows are in-sync...", fg="yellow")

tests/test_cli.py

@@ -8,26 +8,28 @@
 
 """Test command line application."""
 
-import datetime
 import csv
+import datetime
 import io
 import pathlib
 import secrets
-from unittest.mock import Mock, MagicMock, patch
 import uuid
+from unittest.mock import MagicMock, Mock, patch
 
-from click.testing import CliRunner
 import pytest
+from click.testing import CliRunner
 from pytest_reana.test_utils import make_mock_api_client
 from reana_db.models import (
-    generate_uuid,
     AuditLogAction,
     InteractiveSession,
+    Resource,
+    RunStatus,
     User,
+    UserResource,
     UserTokenStatus,
-    RunStatus,
     Workflow,
     WorkspaceRetentionRuleStatus,
+    generate_uuid,
 )
 
 from reana_server.api_client import WorkflowSubmissionPublisher
@@ -968,3 +970,33 @@ def test_check_workspaces(
         if workflow:
             assert len(result.errors) == 2
             assert any(workflow.workspace_path in str(error) for error in result.errors)
+
+
+def test_quota_set_default_limits_for_user_with_custom_limits(default_user, session):
+    """Test setting default quota when there are is one user with custom quota limits."""
+    runner = CliRunner()
+
+    resources = session.query(Resource).all()
+
+    for resource in resources:
+        user_resource = (
+            session.query(UserResource)
+            .filter_by(user_id=default_user.id_, resource_id=resource.id_)
+            .first()
+        )
+
+        if user_resource:
+            user_resource.quota_limit = 12345
+
+    session.commit()
+
+    result = runner.invoke(
+        reana_admin,
+        [
+            "quota-set-default-limits",
+            "--admin-access-token",
+            default_user.access_token,
+        ],
+    )
+
+    assert "There are no users without quota limits." in result.output