Merge branch 'master' into docs-patch-71docs-patch-71

7 files changed, 130 insertions, 81 deletions

diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb
index b89348b7a7e..4eb0545eb6c 100644
--- a/spec/controllers/admin/application_settings_controller_spec.rb
+++ b/spec/controllers/admin/application_settings_controller_spec.rb
@@ -41,7 +41,7 @@ describe Admin::ApplicationSettingsController do
     it 'returns JSON data' do
       get :usage_data, format: :json
 
-      body = JSON.parse(response.body)
+      body = json_response
       expect(body["version"]).to eq(Gitlab::VERSION)
       expect(body).to include('counts')
       expect(response.status).to eq(200)
@@ -116,6 +116,39 @@ describe Admin::ApplicationSettingsController do
         end
       end
     end
+
+    describe 'verify panel actions' do
+      shared_examples 'renders correct panels' do
+        it 'renders correct action on error' do
+          expect_next_instance_of(ApplicationSettings::UpdateService) do |service|
+            allow(service).to receive(:execute).and_return(false)
+          end
+
+          patch action, params: { application_setting: { unused_param: true } }
+
+          expect(subject).to render_template(action)
+        end
+
+        it 'redirects to same panel on success' do
+          expect_next_instance_of(ApplicationSettings::UpdateService) do |service|
+            allow(service).to receive(:execute).and_return(true)
+          end
+
+          referer_path = public_send("#{action}_admin_application_settings_path")
+          request.env["HTTP_REFERER"] = referer_path
+
+          patch action, params: { application_setting: { unused_param: true } }
+
+          expect(subject).to redirect_to(referer_path)
+        end
+      end
+
+      (Admin::ApplicationSettingsController::VALID_SETTING_PANELS - %w(show templates geo)).each do |valid_action|
+        it_behaves_like 'renders correct panels' do
+          let(:action) { valid_action }
+        end
+      end
+    end
   end
 
   describe 'PUT #reset_registration_token' do
diff --git a/spec/controllers/admin/clusters/applications_controller_spec.rb b/spec/controllers/admin/clusters/applications_controller_spec.rb
index 76f261e7d3f..9d6edcd80c0 100644
--- a/spec/controllers/admin/clusters/applications_controller_spec.rb
+++ b/spec/controllers/admin/clusters/applications_controller_spec.rb
@@ -13,16 +13,6 @@ describe Admin::Clusters::ApplicationsController do
     it { expect { subject }.to be_allowed_for(:admin) }
     it { expect { subject }.to be_denied_for(:user) }
     it { expect { subject }.to be_denied_for(:external) }
-
-    context 'when instance clusters are disabled' do
-      before do
-        stub_feature_flags(instance_clusters: false)
-      end
-
-      it 'returns 404' do
-        is_expected.to have_http_status(:not_found)
-      end
-    end
   end
 
   let(:cluster) { create(:cluster, :instance, :provided_by_gcp) }
@@ -94,7 +84,7 @@ describe Admin::Clusters::ApplicationsController do
       patch :update, params: params
     end
 
-    let!(:application) { create(:clusters_applications_cert_managers, :installed, cluster: cluster) }
+    let!(:application) { create(:clusters_applications_cert_manager, :installed, cluster: cluster) }
     let(:application_name) { application.name }
     let(:params) { { application: application_name, id: cluster.id, email: "new-email@example.com" } }
 
diff --git a/spec/controllers/admin/clusters_controller_spec.rb b/spec/controllers/admin/clusters_controller_spec.rb
index 7b77cb186a4..e5501535875 100644
--- a/spec/controllers/admin/clusters_controller_spec.rb
+++ b/spec/controllers/admin/clusters_controller_spec.rb
@@ -17,66 +17,48 @@ describe Admin::ClustersController do
       get :index, params: params
     end
 
-    context 'when feature flag is not enabled' do
-      before do
-        stub_feature_flags(instance_clusters: false)
-      end
+    describe 'functionality' do
+      context 'when instance has one or more clusters' do
+        let!(:enabled_cluster) do
+          create(:cluster, :provided_by_gcp, :instance)
+        end
 
-      it 'responds with not found' do
-        get_index
+        let!(:disabled_cluster) do
+          create(:cluster, :disabled, :provided_by_gcp, :production_environment, :instance)
+        end
 
-        expect(response).to have_gitlab_http_status(404)
-      end
-    end
+        it 'lists available clusters' do
+          get_index
 
-    context 'when feature flag is enabled' do
-      before do
-        stub_feature_flags(instance_clusters: true)
-      end
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to render_template(:index)
+          expect(assigns(:clusters)).to match_array([enabled_cluster, disabled_cluster])
+        end
 
-      describe 'functionality' do
-        context 'when instance has one or more clusters' do
-          let!(:enabled_cluster) do
-            create(:cluster, :provided_by_gcp, :instance)
-          end
+        context 'when page is specified' do
+          let(:last_page) { Clusters::Cluster.instance_type.page.total_pages }
 
-          let!(:disabled_cluster) do
-            create(:cluster, :disabled, :provided_by_gcp, :production_environment, :instance)
+          before do
+            allow(Clusters::Cluster).to receive(:paginates_per).and_return(1)
+            create_list(:cluster, 2, :provided_by_gcp, :production_environment, :instance)
           end
 
-          it 'lists available clusters' do
-            get_index
+          it 'redirects to the page' do
+            get_index(page: last_page)
 
             expect(response).to have_gitlab_http_status(:ok)
-            expect(response).to render_template(:index)
-            expect(assigns(:clusters)).to match_array([enabled_cluster, disabled_cluster])
-          end
-
-          context 'when page is specified' do
-            let(:last_page) { Clusters::Cluster.instance_type.page.total_pages }
-
-            before do
-              allow(Clusters::Cluster).to receive(:paginates_per).and_return(1)
-              create_list(:cluster, 2, :provided_by_gcp, :production_environment, :instance)
-            end
-
-            it 'redirects to the page' do
-              get_index(page: last_page)
-
-              expect(response).to have_gitlab_http_status(:ok)
-              expect(assigns(:clusters).current_page).to eq(last_page)
-            end
+            expect(assigns(:clusters).current_page).to eq(last_page)
           end
         end
+      end
 
-        context 'when instance does not have a cluster' do
-          it 'returns an empty state page' do
-            get_index
+      context 'when instance does not have a cluster' do
+        it 'returns an empty state page' do
+          get_index
 
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(response).to render_template(:index, partial: :empty_state)
-            expect(assigns(:clusters)).to eq([])
-          end
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to render_template(:index, partial: :empty_state)
+          expect(assigns(:clusters)).to eq([])
         end
       end
     end
@@ -396,6 +378,7 @@ describe Admin::ClustersController do
         cluster: {
           enabled: false,
           name: 'my-new-cluster-name',
+          managed: false,
           base_domain: domain
         }
       }
@@ -409,6 +392,7 @@ describe Admin::ClustersController do
       expect(flash[:notice]).to eq('Kubernetes cluster was successfully updated.')
       expect(cluster.enabled).to be_falsey
       expect(cluster.name).to eq('my-new-cluster-name')
+      expect(cluster).not_to be_managed
       expect(cluster.domain).to eq('test-domain.com')
     end
 
@@ -433,6 +417,7 @@ describe Admin::ClustersController do
               cluster: {
                 enabled: false,
                 name: 'my-new-cluster-name',
+                managed: false,
                 domain: domain
               }
             }
@@ -445,6 +430,7 @@ describe Admin::ClustersController do
             expect(response).to have_http_status(:no_content)
             expect(cluster.enabled).to be_falsey
             expect(cluster.name).to eq('my-new-cluster-name')
+            expect(cluster).not_to be_managed
           end
         end
 
diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb
index 509d8944e3a..1123563c1e3 100644
--- a/spec/controllers/admin/groups_controller_spec.rb
+++ b/spec/controllers/admin/groups_controller_spec.rb
@@ -68,5 +68,13 @@ describe Admin::GroupsController do
         post :update, params: { id: group.to_param, group: { project_creation_level: ::Gitlab::Access::NO_ONE_PROJECT_ACCESS } }
       end.to change { group.reload.project_creation_level }.to(::Gitlab::Access::NO_ONE_PROJECT_ACCESS)
     end
+
+    it 'updates the subgroup_creation_level successfully' do
+      expect do
+        post :update,
+             params: { id: group.to_param,
+                       group: { subgroup_creation_level: ::Gitlab::Access::OWNER_SUBGROUP_ACCESS } }
+      end.to change { group.reload.subgroup_creation_level }.to(::Gitlab::Access::OWNER_SUBGROUP_ACCESS)
+    end
   end
 end
diff --git a/spec/controllers/admin/requests_profiles_controller_spec.rb b/spec/controllers/admin/requests_profiles_controller_spec.rb
index 10850cb4603..345f7720c25 100644
--- a/spec/controllers/admin/requests_profiles_controller_spec.rb
+++ b/spec/controllers/admin/requests_profiles_controller_spec.rb
@@ -10,38 +10,63 @@ describe Admin::RequestsProfilesController do
   end
 
   describe '#show' do
-    let(:basename) { "profile_#{Time.now.to_i}.html" }
     let(:tmpdir) { Dir.mktmpdir('profiler-test') }
     let(:test_file) { File.join(tmpdir, basename) }
-    let(:profile) { Gitlab::RequestProfiler::Profile.new(basename) }
-    let(:sample_data) do
-      <<~HTML
-        <!DOCTYPE html>
-        <html>
-        <body>
-        <h1>My First Heading</h1>
-        <p>My first paragraph.</p>
-        </body>
-        </html>
-      HTML
+
+    subject do
+      get :show, params: { name: basename }
     end
 
     before do
       stub_const('Gitlab::RequestProfiler::PROFILES_DIR', tmpdir)
-      output = File.open(test_file, 'w')
-      output.write(sample_data)
-      output.close
+      File.write(test_file, sample_data)
     end
 
     after do
-      File.unlink(test_file)
+      FileUtils.rm_rf(tmpdir)
     end
 
-    it 'loads an HTML profile' do
-      get :show, params: { name: basename }
+    context 'when loading HTML profile' do
+      let(:basename) { "profile_#{Time.now.to_i}_execution.html" }
+
+      let(:sample_data) do
+        '<html> <body> <h1>Heading</h1> <p>paragraph.</p> </body> </html>'
+      end
+
+      it 'renders the data' do
+        subject
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response.body).to eq(sample_data)
+      end
+    end
+
+    context 'when loading TXT profile' do
+      let(:basename) { "profile_#{Time.now.to_i}_memory.txt" }
+
+      let(:sample_data) do
+        <<~TXT
+          Total allocated: 112096396 bytes (1080431 objects)
+          Total retained:  10312598 bytes (53567 objects)
+        TXT
+      end
+
+      it 'renders the data' do
+        subject
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response.body).to eq(sample_data)
+      end
+    end
+
+    context 'when loading PDF profile' do
+      let(:basename) { "profile_#{Time.now.to_i}_anything.pdf" }
+
+      let(:sample_data) { 'mocked pdf content' }
 
-      expect(response).to have_gitlab_http_status(200)
-      expect(response.body).to eq(sample_data)
+      it 'fails to render the data' do
+        expect { subject }.to raise_error(ActionController::UrlGenerationError, /No route matches.*unmatched constraints:/)
+      end
     end
   end
 end
diff --git a/spec/controllers/admin/runners_controller_spec.rb b/spec/controllers/admin/runners_controller_spec.rb
index 78c5e2a2656..bbeda7dae0f 100644
--- a/spec/controllers/admin/runners_controller_spec.rb
+++ b/spec/controllers/admin/runners_controller_spec.rb
@@ -23,10 +23,11 @@ describe Admin::RunnersController do
 
       control_count = ActiveRecord::QueryRecorder.new { get :index }.count
 
-      create(:ci_runner, :tagged_only)
+      create_list(:ci_runner, 5, :tagged_only)
 
       # There is still an N+1 query for `runner.builds.count`
-      expect { get :index }.not_to exceed_query_limit(control_count + 1)
+      # We also need to add 1 because it takes 2 queries to preload tags
+      expect { get :index }.not_to exceed_query_limit(control_count + 6)
 
       expect(response).to have_gitlab_http_status(200)
       expect(response.body).to have_content('tag1')
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index 89a0eba66f7..d7428f8b52c 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/spec/controllers/admin/users_controller_spec.rb
@@ -279,6 +279,12 @@ describe Admin::UsersController do
         expect(warden.user).to eq(user)
       end
 
+      it 'logs the beginning of the impersonation event' do
+        expect(Gitlab::AppLogger).to receive(:info).with("User #{admin.username} has started impersonating #{user.username}").and_call_original
+
+        post :impersonate, params: { id: user.username }
+      end
+
       it "redirects to root" do
         post :impersonate, params: { id: user.username }