Code review fixups

app/services/google_cloud/service_accounts_service.rb

 # frozen_string_literal: true
 
 module GoogleCloud
+  ##
+  # GCP keys used to store Google Cloud Service Accounts
+  GCP_KEYS = %w[GCP_PROJECT_ID GCP_SERVICE_ACCOUNT GCP_SERVICE_ACCOUNT_KEY].freeze
+
   ##
   # This service deals with GCP Service Accounts in GitLab
 
   class ServiceAccountsService < ::BaseService
     ##
-    # Find GCP Service accounts in a GitLab project
+    # Find GCP Service Accounts in a GitLab project
     #
     # This method looks up GitLab project's CI vars
-    # and returns Google Cloud service accounts cominations
-    # lining GitLab project and environment to GCP projects
+    # and returns Google Cloud Service Accounts combinations
+    # aligning GitLab project and environment to GCP projects
 
     def find_for_project
-      list = []
-      group_vars_by_environment.each do |environment_scope, value|
-        list.append({ environment: environment_scope,
+      group_vars_by_environment.map do |environment_scope, value|
+        {
+          environment: environment_scope,
           gcp_project: value['GCP_PROJECT_ID'],
-                      service_account_exists: !value['GCP_SERVICE_ACCOUNT'].nil?,
-                      service_account_key_exists: !value['GCP_SERVICE_ACCOUNT_KEY'].nil? })
+          service_account_exists: value['GCP_SERVICE_ACCOUNT'].present?,
+          service_account_key_exists: value['GCP_SERVICE_ACCOUNT_KEY'].present?
+        }
       end
-      list
     end
 
     private
 
     def group_vars_by_environment
-      gcp_keys = %w[GCP_PROJECT_ID GCP_SERVICE_ACCOUNT GCP_SERVICE_ACCOUNT_KEY]
-      grouped = {}
-      filtered_vars = @project.variables.filter { |variable| gcp_keys.include? variable.key }
-      filtered_vars.each do |variable|
-        unless grouped[variable.environment_scope]
-          grouped[variable.environment_scope] = {}
-        end
-
+      filtered_vars = @project.variables.filter { |variable| GCP_KEYS.include? variable.key }
+      filtered_vars.each_with_object({}) do |variable, grouped|
+        grouped[variable.environment_scope] ||= {}
         grouped[variable.environment_scope][variable.key] = variable.value
       end
-      grouped
     end
   end
 end

spec/services/google_cloud/service_accounts_service_spec.rb

@@ -15,44 +15,43 @@ RSpec.describe GoogleCloud::ServiceAccountsService do
       end
 
       it 'returns an empty list' do
-        expect(service.find_for_project.length).to equal(0)
+        expect(service.find_for_project.length).to eq(0)
       end
     end
 
     context 'when a project has GCP service account ci vars' do
       before do
         project.variables.build(environment_scope: '*', key: 'GCP_PROJECT_ID', value: 'prj1')
-        project.variables.build(environment_scope: '*', key: 'GCP_SERVICE_ACCOUNT_KEY', value: '')
+        project.variables.build(environment_scope: '*', key: 'GCP_SERVICE_ACCOUNT_KEY', value: 'mock')
         project.variables.build(environment_scope: 'staging', key: 'GCP_PROJECT_ID', value: 'prj2')
-        project.variables.build(environment_scope: 'staging', key: 'GCP_SERVICE_ACCOUNT', value: '')
+        project.variables.build(environment_scope: 'staging', key: 'GCP_SERVICE_ACCOUNT', value: 'mock')
         project.variables.build(environment_scope: 'production', key: 'GCP_PROJECT_ID', value: 'prj3')
-        project.variables.build(environment_scope: 'production', key: 'GCP_SERVICE_ACCOUNT', value: '')
-        project.variables.build(environment_scope: 'production', key: 'GCP_SERVICE_ACCOUNT_KEY', value: '')
+        project.variables.build(environment_scope: 'production', key: 'GCP_SERVICE_ACCOUNT', value: 'mock')
+        project.variables.build(environment_scope: 'production', key: 'GCP_SERVICE_ACCOUNT_KEY', value: 'mock')
         project.save!
       end
 
       it 'returns a list of service accounts' do
         list = service.find_for_project
-        first = list[0]
-        second = list[1]
-        third = list[2]
-
-        expect(list.length).to equal(3)
-
-        expect(first[:environment]).to equal('*')
-        expect(first[:gcp_project]).to equal('prj1')
-        expect(first[:service_account_exists]).to equal(false)
-        expect(first[:service_account_key_exists]).to equal(true)
-
-        expect(second[:environment]).to equal('staging')
-        expect(second[:gcp_project]).to equal('prj2')
-        expect(second[:service_account_exists]).to equal(true)
-        expect(second[:service_account_key_exists]).to equal(false)
-
-        expect(third[:environment]).to equal('production')
-        expect(third[:gcp_project]).to equal('prj3')
-        expect(third[:service_account_exists]).to equal(true)
-        expect(third[:service_account_key_exists]).to equal(true)
+
+        aggregate_failures 'testing list of service accounts' do
+          expect(list.length).to eq(3)
+
+          expect(list.first[:environment]).to eq('*')
+          expect(list.first[:gcp_project]).to eq('prj1')
+          expect(list.first[:service_account_exists]).to eq(false)
+          expect(list.first[:service_account_key_exists]).to eq(true)
+
+          expect(list.second[:environment]).to eq('staging')
+          expect(list.second[:gcp_project]).to eq('prj2')
+          expect(list.second[:service_account_exists]).to eq(true)
+          expect(list.second[:service_account_key_exists]).to eq(false)
+
+          expect(list.third[:environment]).to eq('production')
+          expect(list.third[:gcp_project]).to eq('prj3')
+          expect(list.third[:service_account_exists]).to eq(true)
+          expect(list.third[:service_account_key_exists]).to eq(true)
+        end
       end
     end
   end