Skip to content

Fix no method error from older_than_last_successful_deployment? calls

What does this MR do and why?

This MR solves the issue:

While observing logs for a FF rollout #382108 (comment 1182465983) noticed a few minor NoMethodError being observed during Projects::EnvironmentsController#folder.

Exception message: undefined method 'last_deployment' for nil:NilClass

Error trace:

app/models/deployment.rb:306:in `older_than_last_successful_deployment?', app/models/ci/build.rb:430:in `block in outdated_deployment?', lib/gitlab/utils/strong_memoize.rb:34:in `strong_memoize', app/models/ci/build.rb:426:in `outdated_deployment?', app/policies/ci/build_policy.rb:31:in `block in <class:BuildPolicy>', app/models/ability.rb:86:in `allowed?', lib/gitlab/allowable.rb:6:in `can?', app/serializers/ci/job_entity.rb:64:in `playable?', app/serializers/ci/job_entity.rb:32:in `block in <class:JobEntity>', app/serializers/base_serializer.rb:16:in `represent', app/serializers/concerns/with_pagination.rb:21:in `represent', app/serializers/environment_serializer.rb:28:in `represent', app/controllers/projects/environments_controller.rb:288:in `serialize_environments', app/controllers/projects/environments_controller.rb:81:in `block (2 levels) in folder', app/controllers/projects/environments_controller.rb:72:in `folder', ee/lib/gitlab/ip_address_state.rb:10:in `with', ee/app/controllers/ee/application_controller.rb:45:in `set_current_ip_address', app/controllers/application_controller.rb:537:in `set_current_admin', lib/gitlab/session.rb:11:in `with_session', app/controllers/application_controller.rb:528:in `set_session_storage', lib/gitlab/i18n.rb:107:in `with_locale', lib/gitlab/i18n.rb:113:in `with_user_locale', app/controllers/application_controller.rb:516:in `set_locale', app/controllers/application_controller.rb:510:in `set_current_context', ee/lib/omni_auth/strategies/group_saml.rb:41:in `other_phase', lib/gitlab/metrics/elasticsearch_rack_middleware.rb:16: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/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/jira/middleware.rb:19:in `call', lib/gitlab/middleware/go.rb:20:in `call', lib/gitlab/etag_caching/middleware.rb:21:in `call', lib/gitlab/middleware/query_analyzer.rb:11:in `block in call', lib/gitlab/database/query_analyzer.rb:37:in `within', lib/gitlab/middleware/query_analyzer.rb:11:in `call', lib/gitlab/middleware/multipart.rb:173:in `call', lib/gitlab/middleware/read_only/controller.rb:50:in `call', lib/gitlab/middleware/read_only.rb:18:in `call', lib/gitlab/middleware/same_site_cookies.rb:27:in `call', lib/gitlab/middleware/handle_malformed_strings.rb:21:in `call', lib/gitlab/middleware/basic_health_check.rb:25:in `call', lib/gitlab/middleware/handle_ip_spoof_attack_error.rb:25:in `call', lib/gitlab/middleware/request_context.rb:21:in `call', lib/gitlab/middleware/webhook_recursion_detection.rb:15:in `call', config/initializers/fix_local_cache_middleware.rb:11:in `call', lib/gitlab/middleware/compressed_json.rb:37:in `call', lib/gitlab/middleware/rack_multipart_tempfile_factory.rb:19:in `call', lib/gitlab/middleware/sidekiq_web_static.rb:20:in `call', lib/gitlab/metrics/requests_rack_middleware.rb:77:in `call', lib/gitlab/middleware/release_env.rb:13:in `call'

Bug since: Probably !97171 (merged)

Proposal:

  • Validate environment on the callers of this method.
  • Perhaps the below diff
diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index ea92b978d3af..d8cdfa733030 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -303,6 +303,8 @@ def includes_commit?(ancestor_sha)
   end

   def older_than_last_successful_deployment?
+    return false unless environment
+
     last_deployment_id = environment.last_deployment&.id

     return false unless last_deployment_id.present?

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Vitali Tatarintev

Merge request reports

Loading