Merge branch '11-5-stable-prepare-rc5' into '11-5-stable'

Prepare 11.5 RC5 release

See merge request gitlab-org/gitlab-ce!22981

12 files changed, 81 insertions, 25 deletions

diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue
index a153edd0476..c1dfa036678 100644
--- a/app/assets/javascripts/notes/components/noteable_discussion.vue
+++ b/app/assets/javascripts/notes/components/noteable_discussion.vue
@@ -433,7 +433,7 @@ Please check your network connection and try again.`;
                     <div class="discussion-with-resolve-btn">
                       <button
                         type="button"
-                        class="js-vue-discussion-reply btn btn-text-field mr-2 qa-discussion-reply"
+                        class="js-vue-discussion-reply btn btn-text-field mr-sm-2 qa-discussion-reply"
                         title="Add a reply"
                         @click="showReplyForm"
                       >
@@ -442,7 +442,7 @@ Please check your network connection and try again.`;
                       <div v-if="discussion.resolvable">
                         <button
                           type="button"
-                          class="btn btn-default mx-sm-2"
+                          class="btn btn-default mr-sm-2"
                           @click="resolveHandler()"
                         >
                           <i
@@ -455,7 +455,7 @@ Please check your network connection and try again.`;
                       </div>
                       <div
                         v-if="discussion.resolvable"
-                        class="btn-group discussion-actions"
+                        class="btn-group discussion-actions ml-sm-2"
                         role="group"
                       >
                         <div
diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue
index ed5ac112dc0..e555279a6ac 100644
--- a/app/assets/javascripts/notes/components/notes_app.vue
+++ b/app/assets/javascripts/notes/components/notes_app.vue
@@ -50,6 +50,7 @@ export default {
   },
   data() {
     return {
+      isFetching: false,
       currentFilter: null,
     };
   },
@@ -141,6 +142,10 @@ export default {
       return discussion.individual_note ? { note: discussion.notes[0] } : { discussion };
     },
     fetchNotes() {
+      if (this.isFetching) return null;
+
+      this.isFetching = true;
+
       return this.fetchDiscussions({ path: this.getNotesDataByProp('discussionsPath') })
         .then(() => {
           this.initPolling();
@@ -149,6 +154,7 @@ export default {
           this.setLoadingState(false);
           this.setNotesFetchedState(true);
           eventHub.$emit('fetchedNotesData');
+          this.isFetching = false;
         })
         .then(() => this.$nextTick())
         .then(() => this.checkLocationHash())
diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss
index dcb1275d182..c57c1eee350 100644
--- a/app/assets/stylesheets/pages/notes.scss
+++ b/app/assets/stylesheets/pages/notes.scss
@@ -595,7 +595,6 @@ $note-form-margin-left: 70px;
 
 .discussion-actions {
   float: right;
-  margin-left: 10px;
   color: $gray-darkest;
 
   @include media-breakpoint-down(xs) {
diff --git a/app/controllers/concerns/send_file_upload.rb b/app/controllers/concerns/send_file_upload.rb
index 0bb7b7efed0..515a9eede8e 100644
--- a/app/controllers/concerns/send_file_upload.rb
+++ b/app/controllers/concerns/send_file_upload.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module SendFileUpload
-  def send_upload(file_upload, send_params: {}, redirect_params: {}, attachment: nil, disposition: 'attachment')
+  def send_upload(file_upload, send_params: {}, redirect_params: {}, attachment: nil, proxy: false, disposition: 'attachment')
     if attachment
       # Response-Content-Type will not override an existing Content-Type in
       # Google Cloud Storage, so the metadata needs to be cleared on GCS for
@@ -17,7 +17,7 @@ module SendFileUpload
 
     if file_upload.file_storage?
       send_file file_upload.path, send_params
-    elsif file_upload.class.proxy_download_enabled?
+    elsif file_upload.class.proxy_download_enabled? || proxy
       headers.store(*Gitlab::Workhorse.send_url(file_upload.url(**redirect_params)))
       head :ok
     else
diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb
index 312e256ea6c..ae9c17802b9 100644
--- a/app/controllers/projects/artifacts_controller.rb
+++ b/app/controllers/projects/artifacts_controller.rb
@@ -16,7 +16,7 @@ class Projects::ArtifactsController < Projects::ApplicationController
   def download
     return render_404 unless artifacts_file
 
-    send_upload(artifacts_file, attachment: artifacts_file.filename)
+    send_upload(artifacts_file, attachment: artifacts_file.filename, proxy: params[:proxy])
   end
 
   def browse
diff --git a/app/services/clusters/gcp/kubernetes/create_or_update_namespace_service.rb b/app/services/clusters/gcp/kubernetes/create_or_update_namespace_service.rb
index a888fab2789..2b607681082 100644
--- a/app/services/clusters/gcp/kubernetes/create_or_update_namespace_service.rb
+++ b/app/services/clusters/gcp/kubernetes/create_or_update_namespace_service.rb
@@ -16,8 +16,6 @@ module Clusters
           configure_kubernetes_token
 
           kubernetes_namespace.save!
-        rescue ::Kubeclient::HttpError => err
-          raise err unless err.error_code = 404
         end
 
         private
diff --git a/app/workers/cluster_platform_configure_worker.rb b/app/workers/cluster_platform_configure_worker.rb
index 68e8335a09d..8f3689f0166 100644
--- a/app/workers/cluster_platform_configure_worker.rb
+++ b/app/workers/cluster_platform_configure_worker.rb
@@ -17,6 +17,6 @@ class ClusterPlatformConfigureWorker
     end
 
   rescue ::Kubeclient::HttpError => err
-    Rails.logger.error "Failed to create/update Kubernetes Namespace. id: #{kubernetes_namespace.id} message: #{err.message}"
+    Rails.logger.error "Failed to create/update Kubernetes namespace for cluster_id: #{cluster_id} with error: #{err.message}"
   end
 end
diff --git a/changelogs/unreleased/fix-error-handling-bugs-in-kubernetes-integration.yml b/changelogs/unreleased/fix-error-handling-bugs-in-kubernetes-integration.yml
new file mode 100644
index 00000000000..f2a117fe63f
--- /dev/null
+++ b/changelogs/unreleased/fix-error-handling-bugs-in-kubernetes-integration.yml
@@ -0,0 +1,5 @@
+---
+title: Fix error handling bugs in kubernetes integration
+merge_request: 22922
+author:
+type: fixed
diff --git a/doc/administration/custom_hooks.md b/doc/administration/custom_hooks.md
index c58ced7d520..60ad4bf4e8f 100644
--- a/doc/administration/custom_hooks.md
+++ b/doc/administration/custom_hooks.md
@@ -60,7 +60,7 @@ installations, this can be set in `gitlab-shell/config.yml`.
 
 The hooks are searched and executed in this order:
 
-1. `<project>.git/hooks/` - symlink to `gitlab-shell/hooks` global dir
+1. `gitlab-shell/hooks` directory as known to Gitaly
 1. `<project>.git/hooks/<hook_name>` -  executed by `git` itself, this is `gitlab-shell/hooks/<hook_name>`
 1. `<project>.git/custom_hooks/<hook_name>` - per project hook (this is already existing behavior)
 1. `<project>.git/custom_hooks/<hook_name>.d/*` - per project hooks
diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb
index 767fba7fd58..4f1f6bb31f3 100644
--- a/spec/controllers/concerns/send_file_upload_spec.rb
+++ b/spec/controllers/concerns/send_file_upload_spec.rb
@@ -28,8 +28,9 @@ describe SendFileUpload do
   describe '#send_upload' do
     let(:controller) { controller_class.new }
     let(:temp_file) { Tempfile.new('test') }
+    let(:params) { {} }
 
-    subject { controller.send_upload(uploader) }
+    subject { controller.send_upload(uploader, **params) }
 
     before do
       FileUtils.touch(temp_file)
@@ -52,7 +53,7 @@ describe SendFileUpload do
     end
 
     context 'with attachment' do
-      let(:send_attachment) { controller.send_upload(uploader, attachment: 'test.js') }
+      let(:params) { { attachment: 'test.js' } }
 
       it 'sends a file with content-type of text/plain' do
         expected_params = {
@@ -62,7 +63,7 @@ describe SendFileUpload do
         }
         expect(controller).to receive(:send_file).with(uploader.path, expected_params)
 
-        send_attachment
+        subject
       end
 
       context 'with a proxied file in object storage' do
@@ -83,7 +84,7 @@ describe SendFileUpload do
           expect(controller).to receive(:headers) { headers }
           expect(controller).to receive(:head).with(:ok)
 
-          send_attachment
+          subject
         end
       end
     end
@@ -95,11 +96,7 @@ describe SendFileUpload do
         uploader.store!(temp_file)
       end
 
-      context 'and proxying is enabled' do
-        before do
-          allow(Gitlab.config.uploads.object_store).to receive(:proxy_download) { true }
-        end
-
+      shared_examples 'proxied file' do
         it 'sends a file' do
           headers = double
           expect(Gitlab::Workhorse).not_to receive(:send_url).with(/response-content-disposition/)
@@ -115,6 +112,14 @@ describe SendFileUpload do
         end
       end
 
+      context 'and proxying is enabled' do
+        before do
+          allow(Gitlab.config.uploads.object_store).to receive(:proxy_download) { true }
+        end
+
+        it_behaves_like 'proxied file'
+      end
+
       context 'and proxying is disabled' do
         before do
           allow(Gitlab.config.uploads.object_store).to receive(:proxy_download) { false }
@@ -125,6 +130,12 @@ describe SendFileUpload do
 
           subject
         end
+
+        context 'with proxy requested' do
+          let(:params) { { proxy: true } }
+
+          it_behaves_like 'proxied file'
+        end
       end
     end
   end
diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb
index 6091185e252..b3c8d6a954e 100644
--- a/spec/controllers/projects/artifacts_controller_spec.rb
+++ b/spec/controllers/projects/artifacts_controller_spec.rb
@@ -47,14 +47,37 @@ describe Projects::ArtifactsController do
       context 'when codequality file type is supplied' do
         let(:file_type) { 'codequality' }
 
-        before do
-          create(:ci_job_artifact, :codequality, job: job)
+        context 'when file is stored locally' do
+          before do
+            create(:ci_job_artifact, :codequality, job: job)
+          end
+
+          it 'sends the codequality report' do
+            expect(controller).to receive(:send_file).with(job.job_artifacts_codequality.file.path, hash_including(disposition: 'attachment')).and_call_original
+
+            download_artifact(file_type: file_type)
+          end
         end
 
-        it 'sends the codequality report' do
-          expect(controller).to receive(:send_file).with(job.job_artifacts_codequality.file.path, hash_including(disposition: 'attachment')).and_call_original
+        context 'when file is stored remotely' do
+          before do
+            stub_artifacts_object_storage
+            create(:ci_job_artifact, :remote_store, :codequality, job: job)
+          end
+
+          it 'sends the codequality report' do
+            expect(controller).to receive(:redirect_to).and_call_original
 
-          download_artifact(file_type: file_type)
+            download_artifact(file_type: file_type)
+          end
+
+          context 'when proxied' do
+            it 'sends the codequality report' do
+              expect(Gitlab::Workhorse).to receive(:send_url).and_call_original
+
+              download_artifact(file_type: file_type, proxy: true)
+            end
+          end
         end
       end
     end
diff --git a/spec/workers/cluster_platform_configure_worker_spec.rb b/spec/workers/cluster_platform_configure_worker_spec.rb
index 1a7ad8923f6..b51f6e07c6a 100644
--- a/spec/workers/cluster_platform_configure_worker_spec.rb
+++ b/spec/workers/cluster_platform_configure_worker_spec.rb
@@ -30,4 +30,18 @@ describe ClusterPlatformConfigureWorker, '#execute' do
       described_class.new.perform(123)
     end
   end
+
+  context 'when kubeclient raises error' do
+    let(:cluster) { create(:cluster, :project) }
+
+    it 'rescues and logs the error' do
+      allow_any_instance_of(Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService).to receive(:execute).and_raise(::Kubeclient::HttpError.new(500, 'something baaaad happened', ''))
+
+      expect(Rails.logger)
+        .to receive(:error)
+        .with("Failed to create/update Kubernetes namespace for cluster_id: #{cluster.id} with error: something baaaad happened")
+
+      described_class.new.perform(cluster.id)
+    end
+  end
 end