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