Commit 2dfecc4f authored by Manoj M J's avatar Manoj M J

Prevent timeouts when updating `share_with_group_lock` of a group

This change makes sure that authorizations
are refreshed in async manner when the
`share_with_group_lock ` attribute of a group
is updated, thus preventing 500 errors due to
`Rack::Timeout::RequestTimeoutException`.

Changelog: fixed
parent d850bb85
...@@ -528,21 +528,23 @@ class Namespace < ApplicationRecord ...@@ -528,21 +528,23 @@ class Namespace < ApplicationRecord
# Until we compare the inconsistency rates of the new specialized worker and # Until we compare the inconsistency rates of the new specialized worker and
# the old approach, we still run AuthorizedProjectsWorker # the old approach, we still run AuthorizedProjectsWorker
# but with some delay and lower urgency as a safety net. # but with some delay and lower urgency as a safety net.
Group enqueue_jobs_for_groups_requiring_authorizations_refresh(priority: UserProjectAccessChangedService::LOW_PRIORITY)
.joins(project_group_links: :project)
.where(projects: { namespace_id: id })
.distinct
.find_each do |group|
group.refresh_members_authorized_projects(
blocking: false,
priority: UserProjectAccessChangedService::LOW_PRIORITY
)
end
else else
Group enqueue_jobs_for_groups_requiring_authorizations_refresh(priority: UserProjectAccessChangedService::HIGH_PRIORITY)
.joins(project_group_links: :project) end
.where(projects: { namespace_id: id }) end
.find_each(&:refresh_members_authorized_projects)
def enqueue_jobs_for_groups_requiring_authorizations_refresh(priority:)
groups_requiring_authorizations_refresh = Group
.joins(project_group_links: :project)
.where(projects: { namespace_id: id })
.distinct
groups_requiring_authorizations_refresh.find_each do |group|
group.refresh_members_authorized_projects(
blocking: false,
priority: priority
)
end end
end end
......
...@@ -1303,6 +1303,7 @@ RSpec.describe Namespace do ...@@ -1303,6 +1303,7 @@ RSpec.describe Namespace do
context 'refreshing project access on updating share_with_group_lock' do context 'refreshing project access on updating share_with_group_lock' do
let(:group) { create(:group, share_with_group_lock: false) } let(:group) { create(:group, share_with_group_lock: false) }
let(:project) { create(:project, :private, group: group) } let(:project) { create(:project, :private, group: group) }
let(:another_project) { create(:project, :private, group: group) }
let_it_be(:shared_with_group_one) { create(:group) } let_it_be(:shared_with_group_one) { create(:group) }
let_it_be(:shared_with_group_two) { create(:group) } let_it_be(:shared_with_group_two) { create(:group) }
...@@ -1315,6 +1316,7 @@ RSpec.describe Namespace do ...@@ -1315,6 +1316,7 @@ RSpec.describe Namespace do
shared_with_group_one.add_developer(group_one_user) shared_with_group_one.add_developer(group_one_user)
shared_with_group_two.add_developer(group_two_user) shared_with_group_two.add_developer(group_two_user)
create(:project_group_link, group: shared_with_group_one, project: project) create(:project_group_link, group: shared_with_group_one, project: project)
create(:project_group_link, group: shared_with_group_one, project: another_project)
create(:project_group_link, group: shared_with_group_two, project: project) create(:project_group_link, group: shared_with_group_two, project: project)
end end
...@@ -1322,6 +1324,9 @@ RSpec.describe Namespace do ...@@ -1322,6 +1324,9 @@ RSpec.describe Namespace do
expect(AuthorizedProjectUpdate::ProjectRecalculateWorker) expect(AuthorizedProjectUpdate::ProjectRecalculateWorker)
.to receive(:perform_async).with(project.id).once .to receive(:perform_async).with(project.id).once
expect(AuthorizedProjectUpdate::ProjectRecalculateWorker)
.to receive(:perform_async).with(another_project.id).once
execute_update execute_update
end end
...@@ -1354,11 +1359,23 @@ RSpec.describe Namespace do ...@@ -1354,11 +1359,23 @@ RSpec.describe Namespace do
stub_feature_flags(specialized_worker_for_group_lock_update_auth_recalculation: false) stub_feature_flags(specialized_worker_for_group_lock_update_auth_recalculation: false)
end end
it 'refreshes the permissions of the members of the old and new namespace' do it 'updates authorizations leading to users from shared groups losing access', :sidekiq_inline do
expect { execute_update } expect { execute_update }
.to change { group_one_user.authorized_projects.include?(project) }.from(true).to(false) .to change { group_one_user.authorized_projects.include?(project) }.from(true).to(false)
.and change { group_two_user.authorized_projects.include?(project) }.from(true).to(false) .and change { group_two_user.authorized_projects.include?(project) }.from(true).to(false)
end end
it 'updates the authorizations in a non-blocking manner' do
expect(AuthorizedProjectsWorker).to(
receive(:bulk_perform_async)
.with([[group_one_user.id]])).once
expect(AuthorizedProjectsWorker).to(
receive(:bulk_perform_async)
.with([[group_two_user.id]])).once
execute_update
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment