Loading
Add rate limit on epic and work item hierarchy reorders
What does this MR do and why?
Adds a per-user rate limit to reordering epics and work items in the hierarchy tree, behind the work_item_hierarchy_reorder_rate_limit feature flag.
Tree reorders reach the application through two GraphQL mutations, and both are covered by the same rule:
- The legacy
epicTreeReordermutation callsEpics::TreeReorderService. No frontend in this repository calls it any more, so it is only reached by API clients. - The work items hierarchy tree calls the
workItemsHierarchyReordermutation when a child is dragged and dropped, which callsWorkItems::ParentLinks::ReorderService.
File walkthrough:
lib/gitlab/application_rate_limiter/labkit_adapter/supported_rate_limits.rb: registers awork_item_hierarchy_reorderrule with labkit namelimit_work_item_hierarchy_reorders_by_user,characteristics: %i[user],period: 1.minute,action: :limit.app/graphql/mutations/work_items/hierarchy/reorder.rb: definesRATE_LIMIT_KEY = :work_item_hierarchy_reorderand adds a privatecheck_rate_limit!called as the first statement of#resolve. It returns unless the feature flag is enabled forcurrent_user, and raises::RateLimitedService::RateLimitedErrorwhen::Gitlab::ApplicationRateLimiter.throttled?reports the user as throttled.ee/app/graphql/mutations/epic_tree/reorder.rb: reuses the same key and adds the same flag-gated check as the first statement of#resolve.config/feature_flags/gitlab_com_derisk/work_item_hierarchy_reorder_rate_limit.yml: newgitlab_com_deriskflag, milestone 19.6, groupgroup::portfolio planning,default_enabled: false.spec/requests/api/graphql/mutations/work_items/hierarchy/reorder_spec.rbandee/spec/requests/api/graphql/mutations/epic_tree/reorder_spec.rb: use the existing shared example'rate limited endpoint'withgraphql: true, including the second-scope request that proves the limit is per user, and cover the flag-disabled case (the limiter is never consulted for this key).
Design notes:
- The check lives in the mutations rather than
WorkItems::ParentLinks::ReorderService. That service has other callers that are out of scope here:Epics::TreeReorderServiceitself (to sync the legacy epic tree with the work item hierarchy, so a service-level check would count everyepicTreeReordertwice), the hierarchy widget callback used byworkItemCreateandworkItemUpdate, the legacy epic link and epic issue update services, and the work item children REST API. Both mutations share one key, so a user's budget is shared across them. - In
workItemsHierarchyReorderthe check runs in#resolve, after#ready?has loaded and authorized the work item, adjacent item, and parent, so probing IDs the caller cannot read does not consume the budget.epicTreeReorderauthorizes insideEpics::TreeReorderService, so there the check runs before authorization. The budget is per user, so this only affects the caller's own budget. RateLimitedService::RateLimitedErroris already rescued byGraphqlControllerthrough itsERROR_STATUS_MAP, which maps it to429, and the request is logged throughGitlab::AuthLogger. On a failed drag and drop, the work items tree shows the error and rolls back its optimistic update.- The throttle bypass header does not apply, because
Gitlab::ApplicationRateLimiter.throttled_request?needs the request object. This matchesWorkItems::DeleteServiceand the bulk update limit in !263775 (merged). The feature flag is the kill switch during rollout. - Not covered: the work item children REST API (
lib/api/work_items/children.rb) can also reorder a child. It is outside the scope of this issue.
References
- Feature issue: https://gitlab.com/gitlab-org/gitlab/-/work_items/605437
- Rollout issue: #635031
- Audit parent: https://gitlab.com/gitlab-org/gitlab/-/work_items/605322
- Same pattern for bulk updates: !263775 (merged)
- Same pattern for work item deletion: !253621 (merged)
Screenshots or screen recordings
This is a backend-only change with no UI impact, so there is nothing to show.
How to set up and validate locally
- Enable the flag in a Rails console:
Feature.enable(:work_item_hierarchy_reorder_rate_limit). - To make the limit easy to trip, temporarily lower the
limit:value for thework_item_hierarchy_reorderentry inlib/gitlab/application_rate_limiter/labkit_adapter/supported_rate_limits.rb. - Work items: open a work item with at least two child items, then drag a child to a new position repeatedly. Confirm the request to
/api/graphqlreturns429once the limit trips, the tree showsThis endpoint has been requested too many times. Try again later., and the child returns to its original position. - Epics: in GraphiQL at
http://gdk.test:3000/-/graphql-explorer, repeatedly run the mutation against an epic tree you can update, replacing the IDs:Confirm the response carries the same error once the limit trips. Because both mutations share one budget, requests from the previous step count toward this one.mutation { epicTreeReorder(input: { baseEpicId: "gid://gitlab/Epic/1", moved: { id: "gid://gitlab/Epic/3", adjacentReferenceId: "gid://gitlab/Epic/2", relativePosition: after } }) { errors } } - The specs are a safer way to validate the behavior:
bundle exec rspec spec/requests/api/graphql/mutations/work_items/hierarchy/reorder_spec.rb ee/spec/requests/api/graphql/mutations/epic_tree/reorder_spec.rb
MR acceptance checklist
Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Edited by Matt D'Angelo