diff --git a/spec/tooling/lib/tooling/helm3_client_spec.rb b/spec/tooling/lib/tooling/helm3_client_spec.rb index 5a015ddfa7c..26644c6f2bf 100644 --- a/spec/tooling/lib/tooling/helm3_client_spec.rb +++ b/spec/tooling/lib/tooling/helm3_client_spec.rb @@ -34,7 +34,7 @@ RSpec.describe Tooling::Helm3Client do describe '#releases' do it 'raises an error if the Helm command fails' do expect(Gitlab::Popen).to receive(:popen_with_detail) - .with([%(helm list --max 256 --offset 0 --output json)]) + .with(["helm", "list", "--max 256", "--offset 0", "--output json"]) .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: false))) expect { subject.releases.to_a }.to raise_error(described_class::CommandFailedError) @@ -42,7 +42,7 @@ RSpec.describe Tooling::Helm3Client do it 'calls helm list with default arguments' do expect(Gitlab::Popen).to receive(:popen_with_detail) - .with([%(helm list --max 256 --offset 0 --output json)]) + .with(["helm", "list", "--max 256", "--offset 0", "--output json"]) .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true))) subject.releases.to_a @@ -50,7 +50,7 @@ RSpec.describe Tooling::Helm3Client do it 'calls helm list with extra arguments' do expect(Gitlab::Popen).to receive(:popen_with_detail) - .with([%(helm list --max 256 --offset 0 --output json --deployed)]) + .with(["helm", "list", "--max 256", "--offset 0", "--output json", "--deployed"]) .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true))) subject.releases(args: ['--deployed']).to_a @@ -58,7 +58,7 @@ RSpec.describe Tooling::Helm3Client do it 'returns a list of Release objects' do expect(Gitlab::Popen).to receive(:popen_with_detail) - .with([%(helm list --max 256 --offset 0 --output json --deployed)]) + .with(["helm", "list", "--max 256", "--offset 0", "--output json", "--deployed"]) .and_return(Gitlab::Popen::Result.new([], raw_helm_list_page2, '', double(success?: true))) expect(Gitlab::Popen).to receive(:popen_with_detail).ordered .and_return(Gitlab::Popen::Result.new([], raw_helm_list_empty, '', double(success?: true))) @@ -79,13 +79,13 @@ RSpec.describe Tooling::Helm3Client do it 'automatically paginates releases' do expect(Gitlab::Popen).to receive(:popen_with_detail).ordered - .with([%(helm list --max 256 --offset 0 --output json)]) + .with(["helm", "list", "--max 256", "--offset 0", "--output json"]) .and_return(Gitlab::Popen::Result.new([], raw_helm_list_page1, '', double(success?: true))) expect(Gitlab::Popen).to receive(:popen_with_detail).ordered - .with([%(helm list --max 256 --offset 256 --output json)]) + .with(["helm", "list", "--max 256", "--offset 256", "--output json"]) .and_return(Gitlab::Popen::Result.new([], raw_helm_list_page2, '', double(success?: true))) expect(Gitlab::Popen).to receive(:popen_with_detail).ordered - .with([%(helm list --max 256 --offset 512 --output json)]) + .with(["helm", "list", "--max 256", "--offset 512", "--output json"]) .and_return(Gitlab::Popen::Result.new([], raw_helm_list_empty, '', double(success?: true))) releases = subject.releases.to_a @@ -97,7 +97,7 @@ RSpec.describe Tooling::Helm3Client do describe '#delete' do it 'raises an error if the Helm command fails' do expect(Gitlab::Popen).to receive(:popen_with_detail) - .with([%(helm uninstall --namespace #{release_name} #{release_name})]) + .with(["helm", "uninstall", "--namespace", release_name, release_name]) .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: false))) expect { subject.delete(release_name: release_name, namespace: release_name) } @@ -106,7 +106,7 @@ RSpec.describe Tooling::Helm3Client do it 'calls helm uninstall with default arguments' do expect(Gitlab::Popen).to receive(:popen_with_detail) - .with([%(helm uninstall --namespace #{release_name} #{release_name})]) + .with(["helm", "uninstall", "--namespace", release_name, release_name]) .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true))) subject.delete(release_name: release_name, namespace: release_name) @@ -117,7 +117,7 @@ RSpec.describe Tooling::Helm3Client do it 'raises an error if the Helm command fails' do expect(Gitlab::Popen).to receive(:popen_with_detail) - .with([%(helm uninstall --namespace #{release_name[0]} #{release_name[0]})]) + .with(["helm", "uninstall", "--namespace", release_name[0], release_name[0]]) .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: false))) expect { subject.delete(release_name: release_name) }.to raise_error(described_class::CommandFailedError) @@ -126,7 +126,7 @@ RSpec.describe Tooling::Helm3Client do it 'calls helm uninstall with multiple release names and a namespace' do release_name.each do |release| expect(Gitlab::Popen).to receive(:popen_with_detail) - .with([%(helm uninstall --namespace namespace #{release})]) + .with(["helm", "uninstall", "--namespace", "namespace", release]) .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true))) end @@ -136,7 +136,7 @@ RSpec.describe Tooling::Helm3Client do it 'calls helm uninstall with multiple release names and no namespace' do release_name.each do |release| expect(Gitlab::Popen).to receive(:popen_with_detail) - .with([%(helm uninstall --namespace #{release} #{release})]) + .with(["helm", "uninstall", "--namespace", release, release]) .and_return(Gitlab::Popen::Result.new([], '', '', double(success?: true))) end diff --git a/spec/tooling/lib/tooling/kubernetes_client_spec.rb b/spec/tooling/lib/tooling/kubernetes_client_spec.rb index 8d127f1345b..ce5f940dd04 100644 --- a/spec/tooling/lib/tooling/kubernetes_client_spec.rb +++ b/spec/tooling/lib/tooling/kubernetes_client_spec.rb @@ -49,7 +49,7 @@ RSpec.describe Tooling::KubernetesClient do before do allow(instance).to receive(:run_command).with( - "kubectl get namespace --all-namespaces --sort-by='{.metadata.creationTimestamp}' -o json" + %W[kubectl get namespace --all-namespaces --sort-by='{.metadata.creationTimestamp}' -o json] ).and_return(kubectl_namespaces_json) end @@ -73,8 +73,9 @@ RSpec.describe Tooling::KubernetesClient do let(:namespace_2_name) { 'review-apps' } # This is not a review apps namespace, so we should not try to delete it it 'only deletes the review app namespaces' do - expect(instance).to receive(:run_command).with("kubectl delete namespace --now --ignore-not-found #{namespace_1_name}") - + expect(instance).to receive(:run_command).with( + %W[kubectl delete namespace --now --ignore-not-found #{namespace_1_name}] + ) subject end end @@ -84,8 +85,10 @@ RSpec.describe Tooling::KubernetesClient do let(:namespace_2_name) { 'review-another-review-app' } it 'deletes all of the stale namespaces' do - expect(instance).to receive(:run_command).with("kubectl delete namespace --now --ignore-not-found #{namespace_1_name} #{namespace_2_name}") - + namespaces = [namespace_1_name, namespace_2_name].join(' ') + expect(instance).to receive(:run_command).with( + %W[kubectl delete namespace --now --ignore-not-found #{namespaces}] + ) subject end end @@ -109,7 +112,9 @@ RSpec.describe Tooling::KubernetesClient do let(:namespaces) { %w[review-ns-1 review-ns-2] } it 'deletes the namespaces' do - expect(instance).to receive(:run_command).with("kubectl delete namespace --now --ignore-not-found #{namespaces.join(' ')}") + expect(instance).to receive(:run_command).with( + %W[kubectl delete namespace --now --ignore-not-found #{namespaces.join(' ')}] + ) subject end @@ -151,7 +156,7 @@ RSpec.describe Tooling::KubernetesClient do it 'returns an array of namespaces' do allow(instance).to receive(:run_command).with( - "kubectl get namespace --all-namespaces --sort-by='{.metadata.creationTimestamp}' -o json" + %W[kubectl get namespace --all-namespaces --sort-by='{.metadata.creationTimestamp}' -o json] ).and_return(kubectl_namespaces_json) expect(subject).to match_array(%w[review-first-review-app]) @@ -170,10 +175,10 @@ RSpec.describe Tooling::KubernetesClient do end context 'when executing a successful command' do - let(:command) { 'true' } # https://linux.die.net/man/1/true + let(:command) { ['true'] } # https://linux.die.net/man/1/true it 'displays the name of the command to stdout' do - expect(instance).to receive(:puts).with("Running command: `#{command}`") + expect(instance).to receive(:puts).with("Running command: `#{command.join(' ')}`") subject end @@ -184,10 +189,10 @@ RSpec.describe Tooling::KubernetesClient do end context 'when executing an unsuccessful command' do - let(:command) { 'false' } # https://linux.die.net/man/1/false + let(:command) { ['false'] } # https://linux.die.net/man/1/false it 'displays the name of the command to stdout' do - expect(instance).to receive(:puts).with("Running command: `#{command}`") + expect(instance).to receive(:puts).with("Running command: `#{command.join(' ')}`") expect { subject }.to raise_error(described_class::CommandFailedError) end diff --git a/tooling/lib/tooling/helm3_client.rb b/tooling/lib/tooling/helm3_client.rb index 9059387351a..ee73bf8cc9d 100644 --- a/tooling/lib/tooling/helm3_client.rb +++ b/tooling/lib/tooling/helm3_client.rb @@ -39,15 +39,16 @@ module Tooling private def run_command(command) - final_command = ['helm', *command].join(' ') - puts "Running command: `#{final_command}`" # rubocop:disable Rails/Output + final_command = ['helm', *command] + final_command_str = final_command.join(' ') + puts "Running command: `#{final_command_str}`" # rubocop:disable Rails/Output -- Only review apps calls this - result = Gitlab::Popen.popen_with_detail([final_command]) + result = Gitlab::Popen.popen_with_detail(final_command) if result.status.success? result.stdout.chomp.freeze else - raise CommandFailedError, "The `#{final_command}` command failed (status: #{result.status}) with the following error:\n#{result.stderr}" + raise CommandFailedError, "The `#{final_command_str}` command failed (status: #{result.status}) with the following error:\n#{result.stderr}" end end diff --git a/tooling/lib/tooling/kubernetes_client.rb b/tooling/lib/tooling/kubernetes_client.rb index 276a7e64473..ac8e2d4cfbf 100644 --- a/tooling/lib/tooling/kubernetes_client.rb +++ b/tooling/lib/tooling/kubernetes_client.rb @@ -22,7 +22,7 @@ module Tooling def delete_namespaces(namespaces) return if namespaces.any? { |ns| !K8S_ALLOWED_NAMESPACES_REGEX.match?(ns) } - run_command("kubectl delete namespace --now --ignore-not-found #{namespaces.join(' ')}") + run_command(%W[kubectl delete namespace --now --ignore-not-found #{namespaces.join(' ')}]) end def delete_namespaces_by_exact_names(resource_names:, wait:) @@ -39,7 +39,7 @@ module Tooling end def namespaces_created_before(created_before:) - response = run_command("kubectl get namespace --all-namespaces --sort-by='{.metadata.creationTimestamp}' -o json") + response = run_command(%W[kubectl get namespace --all-namespaces --sort-by='{.metadata.creationTimestamp}' -o json]) items = JSON.parse(response)['items'] # rubocop:disable Gitlab/Json items.filter_map do |item| @@ -53,14 +53,15 @@ module Tooling end def run_command(command) - puts "Running command: `#{command}`" + command_str = command.join(' ') + puts "Running command: `#{command_str}`" - result = Gitlab::Popen.popen_with_detail([command]) + result = Gitlab::Popen.popen_with_detail(command) if result.status.success? result.stdout.chomp.freeze else - raise CommandFailedError, "The `#{command}` command failed (status: #{result.status}) with the following error:\n#{result.stderr}" + raise CommandFailedError, "The `#{command_str}` command failed (status: #{result.status}) with the following error:\n#{result.stderr}" end end end