Automatic merge of gitlab-org/gitlab-ce master

app/models/clusters/applications/cert_manager.rb

@@ -64,11 +64,15 @@ module Clusters
       end
 
       def delete_private_key
-        "kubectl delete secret -n #{Gitlab::Kubernetes::Helm::NAMESPACE} #{private_key_name} --ignore-not-found" if private_key_name.present?
+        return unless private_key_name.present?
+
+        args = %W(secret -n #{Gitlab::Kubernetes::Helm::NAMESPACE} #{private_key_name} --ignore-not-found)
+
+        Gitlab::Kubernetes::KubectlCmd.delete(*args)
       end
 
       def delete_crd(definition)
-        "kubectl delete crd #{definition} --ignore-not-found"
+        Gitlab::Kubernetes::KubectlCmd.delete("crd", definition, "--ignore-not-found")
       end
 
       def cluster_issuer_file

app/models/clusters/applications/knative.rb

@@ -89,7 +89,7 @@ module Clusters
 
       def delete_knative_services
         cluster.kubernetes_namespaces.map do |kubernetes_namespace|
-          "kubectl delete ksvc --all -n #{kubernetes_namespace.namespace}"
+          Gitlab::Kubernetes::KubectlCmd.delete("ksvc", "--all", "-n", kubernetes_namespace.namespace)
         end
       end
 
@@ -99,14 +99,14 @@ module Clusters
 
       def delete_knative_namespaces
         [
-          "kubectl delete --ignore-not-found ns knative-serving",
-          "kubectl delete --ignore-not-found ns knative-build"
+          Gitlab::Kubernetes::KubectlCmd.delete("--ignore-not-found", "ns", "knative-serving"),
+          Gitlab::Kubernetes::KubectlCmd.delete("--ignore-not-found", "ns", "knative-build")
         ]
       end
 
       def delete_knative_and_istio_crds
         api_resources.map do |crd|
-          "kubectl delete --ignore-not-found crd #{crd}"
+          Gitlab::Kubernetes::KubectlCmd.delete("--ignore-not-found", "crd", "#{crd}")
         end
       end
 
@@ -119,13 +119,13 @@ module Clusters
       def install_knative_metrics
         return [] unless cluster.application_prometheus_available?
 
-        ["kubectl apply -f #{METRICS_CONFIG}"]
+        [Gitlab::Kubernetes::KubectlCmd.apply_file(METRICS_CONFIG)]
       end
 
       def delete_knative_istio_metrics
         return [] unless cluster.application_prometheus_available?
 
-        ["kubectl delete --ignore-not-found -f #{METRICS_CONFIG}"]
+        [Gitlab::Kubernetes::KubectlCmd.delete("--ignore-not-found", "-f", METRICS_CONFIG)]
       end
 
       def verify_cluster?

app/models/clusters/applications/prometheus.rb

@@ -106,13 +106,13 @@ module Clusters
       def install_knative_metrics
         return [] unless cluster.application_knative_available?
 
-        ["kubectl apply -f #{Clusters::Applications::Knative::METRICS_CONFIG}"]
+        [Gitlab::Kubernetes::KubectlCmd.apply_file(Clusters::Applications::Knative::METRICS_CONFIG)]
       end
 
       def delete_knative_istio_metrics
         return [] unless cluster.application_knative_available?
 
-        ["kubectl delete -f #{Clusters::Applications::Knative::METRICS_CONFIG}"]
+        [Gitlab::Kubernetes::KubectlCmd.delete("-f", Clusters::Applications::Knative::METRICS_CONFIG)]
       end
     end
   end

lib/gitlab/kubernetes/helm/reset_command.rb

@@ -38,9 +38,9 @@ module Gitlab
         # Tracking this method to be removed here:
         # https://gitlab.com/gitlab-org/gitlab-ce/issues/52791#note_199374155
         def delete_tiller_replicaset
-          command = %w[kubectl delete replicaset -n gitlab-managed-apps -l name=tiller]
+          delete_args = %w[replicaset -n gitlab-managed-apps -l name=tiller]
 
-          command.shelljoin
+          Gitlab::Kubernetes::KubectlCmd.delete(*delete_args)
         end
 
         def reset_helm_command

lib/gitlab/kubernetes/kubectl_cmd.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Kubernetes
+    module KubectlCmd
+      class << self
+        def delete(*args)
+          %w(kubectl delete).concat(args).shelljoin
+        end
+
+        def apply_file(filename, *args)
+          raise ArgumentError, "filename is not present" unless filename.present?
+
+          %w(kubectl apply -f).concat([filename], args).shelljoin
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/kubernetes/kubectl_cmd_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+describe Gitlab::Kubernetes::KubectlCmd do
+  describe '.delete' do
+    it 'constructs string properly' do
+      args = %w(resource_type type --flag-1 --flag-2)
+
+      expected_command = 'kubectl delete resource_type type --flag-1 --flag-2'
+
+      expect(described_class.delete(*args)).to eq expected_command
+    end
+  end
+
+  describe '.apply_file' do
+    context 'without optional args' do
+      it 'requires filename to be present' do
+        expect { described_class.apply_file(nil) }.to raise_error(ArgumentError, "filename is not present")
+        expect { described_class.apply_file("  ") }.to raise_error(ArgumentError, "filename is not present")
+      end
+
+      it 'constructs string properly' do
+        expected_command = 'kubectl apply -f filename'
+
+        expect(described_class.apply_file('filename')).to eq expected_command
+      end
+    end
+
+    context 'with optional args' do
+      it 'constructs command properly with many args' do
+        args = %w(arg-1 --flag-0-1 arg-2 --flag-0-2)
+
+        expected_command = 'kubectl apply -f filename arg-1 --flag-0-1 arg-2 --flag-0-2'
+
+        expect(described_class.apply_file('filename', *args)).to eq expected_command
+      end
+
+      it 'constructs command properly with single arg' do
+        args = "arg-1"
+
+        expected_command = 'kubectl apply -f filename arg-1'
+
+        expect(described_class.apply_file('filename', args)).to eq(expected_command)
+      end
+    end
+  end
+end