[go: up one dir, main page]

Fix namespace root ancestor preloader when namespace is not present

Dmitry Gruzdrequested to merge
make-namespace-root-ancestor-preloader-safer into master

What does this MR do and why?

This change makes the namespace preloader more robust by adding safety checks when loading root ancestor data. Previously, the code would crash if it tried to process a namespace that didn't have a corresponding root ancestor in the database. Now it safely skips over these "orphaned" namespaces instead of failing, and includes tests to verify this protective behavior works correctly.

Click to see an exception from staging 
NoMethodError
undefined method `first' for nil:NilClass
[app/models/namespaces/preloaders/namespace_root_ancestor_preloader.rb:20:in `block in execute', activerecord (7.1.5.1) lib/active_record/relation/delegation.rb:100:in `each', activerecord (7.1.5.1) lib/active_record/relation/delegation.rb:100:in `each', app/models/namespaces/preloaders/namespace_root_ancestor_preloader.rb:19:in `execute', ee/app/models/preloaders/user_member_roles_in_groups_preloader.rb:33:in `abilities_for_user_grouped_by_group', ee/app/models/preloaders/user_member_roles_in_groups_preloader.rb:24:in `block in execute', lib/gitlab/safe_request_loader.rb:43:in `update_resource_data', lib/gitlab/safe_request_loader.rb:22:in `execute', lib/gitlab/safe_request_loader.rb:6:in `execute', ee/app/models/preloaders/user_member_roles_in_groups_preloader.rb:19:in `execute', ee/app/models/authz/group.rb:13:in `permitted', ee/lib/search/elastic/filters.rb:1365:in `process_features_for_groups', ee/lib/search/elastic/filters.rb:1355:in `add_group_membership_filters', ee/lib/search/elastic/filters.rb:1332:in `build_project_and_group_membership_filters', ee/lib/search/elastic/filters.rb:1247:in `block in build_membership_filter', ee/lib/elastic/latest/query_context.rb:25:in `name', ee/lib/search/elastic/filters.rb:1246:in `build_membership_filter', ee/lib/search/elastic/filters.rb:32:in `block in by_search_level_and_membership', ee/lib/search/elastic/concerns/filter_utils.rb:12:in `add_filter', ee/lib/search/elastic/filters.rb:31:in `by_search_level_and_membership', ee/lib/elastic/latest/git_class_proxy.rb:307:in `blob_query', ee/lib/elastic/latest/git_class_proxy.rb:182:in `search_blob', ee/lib/elastic/latest/git_class_proxy.rb:19:in `elastic_search', ee/lib/elastic/latest/git_class_proxy.rb:195:in `elastic_search_and_wrap', ee/lib/elastic/latest/git_class_proxy.rb:28:in `elastic_search_as_found_blob', ee/lib/elastic/multi_version_util.rb:76:in `elastic_search_as_found_blob', ee/lib/gitlab/elastic/search_results.rb:487:in `block in blobs', gems/gitlab-utils/lib/gitlab/utils/strong_memoize.rb:34:in `strong_memoize', ee/lib/gitlab/elastic/search_results.rb:486:in `blobs', ee/lib/gitlab/elastic/search_results.rb:233:in `blobs_count', ee/lib/gitlab/elastic/search_results.rb:187:in `formatted_count', app/controllers/search_controller.rb:95:in `block in count', benchmark (0.4.0) lib/benchmark.rb:323:in `realtime', app/controllers/search_controller.rb:89:in `count', actionpack (7.1.5.1) lib/action_controller/metal/basic_implicit_render.rb:6:in `send_action', actionpack (7.1.5.1) lib/abstract_controller/base.rb:224:in `process_action', actionpack (7.1.5.1) lib/action_controller/metal/rendering.rb:165:in `process_action', actionpack (7.1.5.1) lib/abstract_controller/callbacks.rb:259:in `block in process_action', activesupport (7.1.5.1) lib/active_support/callbacks.rb:121:in `block in run_callbacks', app/controllers/application_controller.rb:534:in `block in allow_gitaly_ref_name_caching', lib/gitlab/gitaly_client.rb:479:in `allow_ref_name_caching', app/controllers/application_controller.rb:533:in `allow_gitaly_ref_name_caching', activesupport (7.1.5.1) lib/active_support/callbacks.rb:130:in `block in run_callbacks', app/controllers/application_controller.rb:485:in `set_current_admin', activesupport (7.1.5.1) lib/active_support/callbacks.rb:130:in `block in run_callbacks', lib/gitlab/session.rb:11:in `with_session', app/controllers/application_controller.rb:476:in `set_session_storage', activesupport (7.1.5.1) lib/active_support/callbacks.rb:130:in `block in run_callbacks', lib/gitlab/i18n.rb:116:in `with_locale', lib/gitlab/i18n.rb:122:in `with_user_locale', app/controllers/application_controller.rb:467:in `set_locale', activesupport (7.1.5.1) lib/active_support/callbacks.rb:130:in `block in run_callbacks', app/controllers/application_controller.rb:456:in `set_current_context', activesupport (7.1.5.1) lib/active_support/callbacks.rb:130:in `block in run_callbacks', lib/gitlab/ip_address_state.rb:11:in `with', app/controllers/application_controller.rb:462:in `set_current_ip_address', activesupport (7.1.5.1) lib/active_support/callbacks.rb:130:in `block in run_callbacks', marginalia (1.11.1) lib/marginalia.rb:109:in `record_query_comment', activesupport (7.1.5.1) lib/active_support/callbacks.rb:130:in `block in run_callbacks', sentry-rails (5.23.0) lib/sentry/rails/controller_transaction.rb:34:in `block in sentry_around_action', sentry-ruby (5.23.0) lib/sentry/hub.rb:138:in `with_child_span', sentry-ruby (5.23.0) lib/sentry-ruby.rb:515:in `with_child_span', sentry-rails (5.23.0) lib/sentry/rails/controller_transaction.rb:18:in `sentry_around_action', activesupport (7.1.5.1) lib/active_support/callbacks.rb:130:in `block in run_callbacks', activesupport (7.1.5.1) lib/active_support/callbacks.rb:141:in `run_callbacks', actionpack (7.1.5.1) lib/abstract_controller/callbacks.rb:258:in `process_action', actionpack (7.1.5.1) lib/action_controller/metal/rescue.rb:25:in `process_action', actionpack (7.1.5.1) lib/action_controller/metal/instrumentation.rb:74:in `block in process_action', activesupport (7.1.5.1) lib/active_support/notifications.rb:206:in `block in instrument', activesupport (7.1.5.1) lib/active_support/notifications/instrumenter.rb:58:in `instrument', activesupport (7.1.5.1) lib/active_support/notifications.rb:206:in `instrument', actionpack (7.1.5.1) lib/action_controller/metal/instrumentation.rb:73:in `process_action', actionpack (7.1.5.1) lib/action_controller/metal/params_wrapper.rb:261:in `process_action', activerecord (7.1.5.1) lib/active_record/railties/controller_runtime.rb:32:in `process_action', actionpack (7.1.5.1) lib/abstract_controller/base.rb:160:in `process', actionview (7.1.5.1) lib/action_view/rendering.rb:40:in `process', actionpack (7.1.5.1) lib/action_controller/metal.rb:227:in `dispatch', actionpack (7.1.5.1) lib/action_controller/metal.rb:307:in `block in dispatch', lib/gitlab/middleware/action_controller_static_context.rb:23:in `call', actionpack (7.1.5.1) lib/action_controller/metal.rb:307:in `dispatch', actionpack (7.1.5.1) lib/action_dispatch/routing/route_set.rb:49:in `dispatch', actionpack (7.1.5.1) lib/action_dispatch/routing/route_set.rb:32:in `serve', actionpack (7.1.5.1) lib/action_dispatch/journey/router.rb:51:in `block in serve', config/initializers/action_dispatch_journey_router.rb:52:in `block in find_routes', config/initializers/action_dispatch_journey_router.rb:25:in `map!', config/initializers/action_dispatch_journey_router.rb:25:in `find_routes', actionpack (7.1.5.1) lib/action_dispatch/journey/router.rb:32:in `serve', actionpack (7.1.5.1) lib/action_dispatch/routing/route_set.rb:882:in `call', gitlab-experiment (0.9.1) lib/gitlab/experiment/middleware.rb:19:in `call', omniauth (2.1.3) lib/omniauth/strategy.rb:202:in `call!', omniauth (2.1.3) lib/omniauth/strategy.rb:169:in `call', omniauth (2.1.3) lib/omniauth/strategy.rb:472:in `call_app!', ee/lib/omni_auth/strategies/group_saml.rb:41:in `other_phase', omniauth (2.1.3) lib/omniauth/strategy.rb:195:in `call!', omniauth (2.1.3) lib/omniauth/strategy.rb:169:in `call', omniauth (2.1.3) lib/omniauth/strategy.rb:202:in `call!', omniauth (2.1.3) lib/omniauth/strategy.rb:169:in `call', omniauth (2.1.3) lib/omniauth/strategy.rb:202:in `call!', omniauth (2.1.3) lib/omniauth/strategy.rb:169:in `call', omniauth (2.1.3) lib/omniauth/strategy.rb:202:in `call!', omniauth (2.1.3) lib/omniauth/strategy.rb:169:in `call', flipper (0.28.3) lib/flipper/middleware/memoizer.rb:72:in `memoized_call', flipper (0.28.3) lib/flipper/middleware/memoizer.rb:37:in `call', lib/gitlab/metrics/elasticsearch_rack_middleware.rb:16:in `call', lib/gitlab/middleware/sidekiq_shard_awareness_validation.rb:20:in `block in call', lib/gitlab/sidekiq_sharding/validator.rb:42:in `enabled', lib/gitlab/middleware/sidekiq_shard_awareness_validation.rb:20:in `call', lib/gitlab/middleware/memory_report.rb:13:in `call', lib/gitlab/middleware/speedscope.rb:13:in `call', lib/gitlab/database/load_balancing/rack_middleware.rb:23:in `call', lib/gitlab/middleware/rails_queue_duration.rb:33:in `call', lib/gitlab/etag_caching/middleware.rb:21:in `call', lib/gitlab/metrics/rack_middleware.rb:16:in `block in call', lib/gitlab/metrics/web_transaction.rb:46:in `run', lib/gitlab/metrics/rack_middleware.rb:16:in `call', lib/gitlab/middleware/go.rb:21:in `call', lib/gitlab/middleware/query_analyzer.rb:11:in `block in call', lib/gitlab/database/query_analyzer.rb:83:in `within', lib/gitlab/middleware/query_analyzer.rb:11:in `call', lib/ci/job_token/middleware.rb:11:in `call', batch-loader (2.0.5) lib/batch_loader/middleware.rb:11:in `call', rack-attack (6.7.0) lib/rack/attack.rb:103:in `call', apollo_upload_server (2.1.6) lib/apollo_upload_server/middleware.rb:19:in `call', lib/gitlab/middleware/multipart.rb:173:in `call', rack-attack (6.7.0) lib/rack/attack.rb:127:in `call', warden (1.2.9) lib/warden/manager.rb:36:in `block in call', warden (1.2.9) lib/warden/manager.rb:34:in `catch', warden (1.2.9) lib/warden/manager.rb:34:in `call', rack-cors (2.0.2) lib/rack/cors.rb:102:in `call', rack (2.2.17) lib/rack/tempfile_reaper.rb:15:in `call', rack (2.2.17) lib/rack/etag.rb:27:in `call', rack (2.2.17) lib/rack/conditional_get.rb:27:in `call', rack (2.2.17) lib/rack/head.rb:12:in `call', actionpack (7.1.5.1) lib/action_dispatch/http/permissions_policy.rb:36:in `call', actionpack (7.1.5.1) lib/action_dispatch/http/content_security_policy.rb:36:in `call', lib/gitlab/middleware/read_only/controller.rb:40:in `call', lib/gitlab/middleware/read_only.rb:18:in `call', lib/gitlab/middleware/unauthenticated_session_expiry.rb:18:in `call', rack (2.2.17) lib/rack/session/abstract/id.rb:266:in `context', rack (2.2.17) lib/rack/session/abstract/id.rb:260:in `call', lib/gitlab/middleware/secure_headers.rb:11:in `call', actionpack (7.1.5.1) lib/action_dispatch/middleware/cookies.rb:689:in `call', lib/gitlab/middleware/same_site_cookies.rb:27:in `call', actionpack (7.1.5.1) lib/action_dispatch/middleware/callbacks.rb:29:in `block in call', activesupport (7.1.5.1) lib/active_support/callbacks.rb:101:in `run_callbacks', actionpack (7.1.5.1) lib/action_dispatch/middleware/callbacks.rb:28:in `call', sentry-rails (5.23.0) lib/sentry/rails/rescued_exception_interceptor.rb:14:in `call', actionpack (7.1.5.1) lib/action_dispatch/middleware/debug_exceptions.rb:29:in `call', lib/gitlab/middleware/path_traversal_check.rb:40:in `call', lib/gitlab/middleware/handle_malformed_strings.rb:21:in `call', sentry-ruby (5.23.0) lib/sentry/rack/capture_exceptions.rb:30:in `block (2 levels) in call', sentry-ruby (5.23.0) lib/sentry/hub.rb:299:in `with_session_tracking', sentry-ruby (5.23.0) lib/sentry-ruby.rb:428:in `with_session_tracking', sentry-ruby (5.23.0) lib/sentry/rack/capture_exceptions.rb:21:in `block in call', sentry-ruby (5.23.0) lib/sentry/hub.rb:89:in `with_scope', sentry-ruby (5.23.0) lib/sentry-ruby.rb:408:in `with_scope', sentry-ruby (5.23.0) lib/sentry/rack/capture_exceptions.rb:20:in `call', actionpack (7.1.5.1) lib/action_dispatch/middleware/show_exceptions.rb:31:in `call', lib/gitlab/middleware/basic_health_check.rb:25:in `call', lograge (0.11.2) lib/lograge/rails_ext/rack/logger.rb:15:in `call_app', railties (7.1.5.1) lib/rails/rack/logger.rb:24:in `block in call', activesupport (7.1.5.1) lib/active_support/tagged_logging.rb:139:in `block in tagged', activesupport (7.1.5.1) lib/active_support/tagged_logging.rb:39:in `tagged', activesupport (7.1.5.1) lib/active_support/tagged_logging.rb:139:in `tagged', activesupport (7.1.5.1) lib/active_support/broadcast_logger.rb:241:in `method_missing', railties (7.1.5.1) lib/rails/rack/logger.rb:24:in `call', actionpack (7.1.5.1) lib/action_dispatch/middleware/remote_ip.rb:92:in `call', lib/gitlab/middleware/handle_ip_spoof_attack_error.rb:25:in `call', lib/gitlab/middleware/request_context.rb:15:in `call', lib/gitlab/middleware/webhook_recursion_detection.rb:15:in `call', request_store (1.7.0) lib/request_store/middleware.rb:19:in `call', rack (2.2.17) lib/rack/method_override.rb:24:in `call', rack (2.2.17) lib/rack/runtime.rb:22:in `call', rack-timeout (0.7.0) lib/rack/timeout/core.rb:154:in `block in call', rack-timeout (0.7.0) lib/rack/timeout/support/timeout.rb:19:in `timeout', rack-timeout (0.7.0) lib/rack/timeout/core.rb:153:in `call', config/initializers/fix_local_cache_middleware.rb:11:in `call', lib/gitlab/middleware/compressed_json.rb:44:in `call', actionpack (7.1.5.1) lib/action_dispatch/middleware/executor.rb:14:in `call', lib/gitlab/middleware/rack_multipart_tempfile_factory.rb:19:in `call', lib/gitlab/metrics/requests_rack_middleware.rb:83:in `call', gitlab-labkit (0.39.0) lib/labkit/middleware/rack.rb:22:in `block in call', gitlab-labkit (0.39.0) lib/labkit/context.rb:35:in `with_context', gitlab-labkit (0.39.0) lib/labkit/middleware/rack.rb:21:in `call', rack (2.2.17) lib/rack/sendfile.rb:110:in `call', lib/gitlab/middleware/sidekiq_web_static.rb:20:in `call', actionpack (7.1.5.1) lib/action_dispatch/middleware/request_id.rb:28:in `call', rack (2.2.17) lib/rack/events.rb:112:in `call', railties (7.1.5.1) lib/rails/engine.rb:536:in `call', railties (7.1.5.1) lib/rails/railtie.rb:226:in `public_send', railties (7.1.5.1) lib/rails/railtie.rb:226:in `method_missing', lib/gitlab/middleware/release_env.rb:12:in `call', rack (2.2.17) lib/rack/urlmap.rb:74:in `block in call', rack (2.2.17) lib/rack/urlmap.rb:58:in `each', rack (2.2.17) lib/rack/urlmap.rb:58:in `call', puma (6.6.0) lib/puma/configuration.rb:279:in `call', puma (6.6.0) lib/puma/request.rb:99:in `block in handle_request', puma (6.6.0) lib/puma/thread_pool.rb:390:in `with_force_shutdown', puma (6.6.0) lib/puma/request.rb:98:in `handle_request', puma (6.6.0) lib/puma/server.rb:472:in `process_client', puma (6.6.0) lib/puma/server.rb:254:in `block in run', puma (6.6.0) lib/puma/thread_pool.rb:167:in `block in spawn_thread']

References

Screenshots or screen recordings

Before After

How to set up and validate locally

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 Dmitry Gruzd

Merge request reports