Merge branch 'ab/fingerprint256-migration' into 'master'

Ignore errors from invalid keys

See merge request gitlab-org/gitlab!23006

lib/gitlab/background_migration/migrate_fingerprint_sha256_within_keys.rb

@@ -24,13 +24,15 @@ module Gitlab
 
           fingerprints = []
           Key.where(id: start_id..stop_id, fingerprint_sha256: nil).find_each do |regular_key|
-            fingerprint = Base64.decode64(generate_ssh_public_key(regular_key.key))
+            if fingerprint = generate_ssh_public_key(regular_key.key)
+              bytea = ActiveRecord::Base.connection.escape_bytea(Base64.decode64(fingerprint))
 
               fingerprints << {
                 id: regular_key.id,
-              fingerprint_sha256: ActiveRecord::Base.connection.escape_bytea(fingerprint)
+                fingerprint_sha256: bytea
               }
             end
+          end
 
           Gitlab::Database.bulk_insert(TEMP_TABLE, fingerprints)
 
@@ -48,7 +50,7 @@ module Gitlab
       private
 
       def generate_ssh_public_key(regular_key)
-        Gitlab::SSHPublicKey.new(regular_key).fingerprint("SHA256").gsub("SHA256:", "")
+        Gitlab::SSHPublicKey.new(regular_key).fingerprint("SHA256")&.gsub("SHA256:", "")
       end
 
       def execute(query)

spec/lib/gitlab/background_migration/migrate_fingerprint_sha256_within_keys_spec.rb

@@ -37,6 +37,25 @@ describe Gitlab::BackgroundMigration::MigrateFingerprintSha256WithinKeys, :migra
     expect(key_2.fingerprint_sha256).to eq('zMNbLekgdjtcgDv8VSC0z5lpdACMG3Q4PUoIz5+H2jM')
   end
 
+  context 'with invalid keys' do
+    before do
+      key = Key.find(1017)
+      # double space after "ssh-rsa" leads to a
+      # OpenSSL::PKey::PKeyError in Net::SSH::KeyFactory.load_data_public_key
+      key.update_column(:key, key.key.gsub('ssh-rsa ', 'ssh-rsa  '))
+    end
+
+    it 'ignores errors and does not set the fingerprint' do
+      fingerprint_migrator.perform(1, 10000)
+
+      key_1 = Key.find(1017)
+      key_2 = Key.find(1027)
+
+      expect(key_1.fingerprint_sha256).to be_nil
+      expect(key_2.fingerprint_sha256).not_to be_nil
+    end
+  end
+
   it 'migrates all keys' do
     expect(Key.where(fingerprint_sha256: nil).count).to eq(Key.all.count)