Migrated gitlab-keys functionality is broken when the authorized_keys file does not exist

The following discussion from !10287 (merged) should be addressed:

@nick.thomas started a discussion: (+1 comment)

Without this, the spec fails with ENOENT

This gets master green again, but I wonder if this is a regression we might have introduced in the move from gitlab-shell to Gitlab::AuthorizedKeys

Backtrace:

     Errno::ENOENT:
       No such file or directory @ rb_sysopen - tmp/tests/authorized_keys
     # ./lib/gitlab/authorized_keys.rb:113:in `initialize'
     # ./lib/gitlab/authorized_keys.rb:113:in `open'
     # ./lib/gitlab/authorized_keys.rb:113:in `open_authorized_keys_file'
     # ./lib/gitlab/authorized_keys.rb:89:in `block in list_key_ids'
     # ./lib/gitlab/authorized_keys.rb:88:in `tap'
     # ./lib/gitlab/authorized_keys.rb:88:in `list_key_ids'
     # ./lib/gitlab/shell.rb:417:in `batch_read_key_ids'
     # ./lib/gitlab/shell.rb:243:in `remove_keys_not_found_in_db'
     # ./ee/lib/gitlab/background_migration/update_authorized_keys_file_since.rb:16:in `remove_keys_not_found_in_db'
     # ./ee/lib/gitlab/background_migration/update_authorized_keys_file_since.rb:21:in `perform'
     # ./lib/gitlab/background_migration.rb:58:in `perform'
     # ./app/workers/background_migration_worker.rb:26:in `perform'
     # /home/lupine/.gem/ruby/2.5.3/gems/sidekiq-5.2.5/lib/sidekiq/testing.rb:301:in `execute_job'
     # /home/lupine/.gem/ruby/2.5.3/gems/sidekiq-5.2.5/lib/sidekiq/testing.rb:296:in `block in process_job'
     # /home/lupine/.gem/ruby/2.5.3/gems/sidekiq-5.2.5/lib/sidekiq/middleware/chain.rb:128:in `block in invoke'
     # ./spec/support/sidekiq.rb:13:in `call'
     # /home/lupine/.gem/ruby/2.5.3/gems/sidekiq-5.2.5/lib/sidekiq/middleware/chain.rb:130:in `block in invoke'
     # ./lib/gitlab/sidekiq_status/server_middleware.rb:7:in `call'
     # /home/lupine/.gem/ruby/2.5.3/gems/sidekiq-5.2.5/lib/sidekiq/middleware/chain.rb:130:in `block in invoke'
     # /home/lupine/.gem/ruby/2.5.3/gems/sidekiq-5.2.5/lib/sidekiq/middleware/chain.rb:133:in `invoke'
     # /home/lupine/.gem/ruby/2.5.3/gems/sidekiq-5.2.5/lib/sidekiq/testing.rb:295:in `process_job'
     # /home/lupine/.gem/ruby/2.5.3/gems/sidekiq-5.2.5/lib/sidekiq/testing.rb:89:in `block in raw_push'
     # /home/lupine/.gem/ruby/2.5.3/gems/sidekiq-5.2.5/lib/sidekiq/testing.rb:85:in `each'
     # /home/lupine/.gem/ruby/2.5.3/gems/sidekiq-5.2.5/lib/sidekiq/testing.rb:85:in `raw_push'
     # /home/lupine/.gem/ruby/2.5.3/gems/sidekiq-5.2.5/lib/sidekiq/client.rb:104:in `push_bulk'
     # /home/lupine/.gem/ruby/2.5.3/gems/sidekiq-5.2.5/lib/sidekiq/client.rb:136:in `push_bulk'
     # ./app/workers/concerns/application_worker.rb:48:in `bulk_perform_async'
     # ./ee/db/migrate/20170626202753_update_authorized_keys_file.rb:105:in `update_authorized_keys_file_since'
     # ./ee/db/migrate/20170626202753_update_authorized_keys_file.rb:28:in `up'
     # ./ee/spec/migrations/update_authorized_keys_file_spec.rb:19:in `block (4 levels) in <top (required)>'
     # /home/lupine/.gem/ruby/2.5.3/gems/rspec-retry-0.4.5/lib/rspec/retry.rb:98:in `block in run'
     # /home/lupine/.gem/ruby/2.5.3/gems/rspec-retry-0.4.5/lib/rspec/retry.rb:88:in `loop'
     # /home/lupine/.gem/ruby/2.5.3/gems/rspec-retry-0.4.5/lib/rspec/retry.rb:88:in `run'
     # /home/lupine/.gem/ruby/2.5.3/gems/rspec-retry-0.4.5/lib/rspec_ext/rspec_ext.rb:12:in `run_with_retry'
     # /home/lupine/.gem/ruby/2.5.3/gems/rspec-retry-0.4.5/lib/rspec/retry.rb:22:in `block (2 levels) in setup'

@patrickbajao could I get you to investigate this as a follow-up? I'm interested in what happens if we run on a system where the authorized_keys file just doesn't exist.

Is the behaviour identical with and without the feature flag enabled?

Either way, I think it makes sense for every bit of functionality in Gitlab::AuthorizedKeys to cleanly handle the case where the authorized keys file doesn't exist - in this case, we could catch the ENOENT and return an empty array, for instance. WDYT?

Edited Mar 19, 2019 by Nick Thomas