diff --git a/CHANGELOG.md b/CHANGELOG.md index 056aec9a3..986f2e03a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,8 @@ _None_ ### Internal Changes -_None_ +- `buildkite_pipeline_upload`: makes sure all values passed in the environment parameter are strings [#608] +- `buildkite_pipeline_upload`: prepend `.buildkite/` to the `pipeline_file` parameter to enforce our conventions [#608] ## 12.2.1 diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_pipeline_upload_action.rb b/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_pipeline_upload_action.rb index 6a92733f9..d1a68b2fc 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_pipeline_upload_action.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_pipeline_upload_action.rb @@ -1,17 +1,20 @@ module Fastlane module Actions class BuildkitePipelineUploadAction < Action - DEFAULT_ENV_FILE = File.join('.buildkite', 'shared-pipeline-vars').freeze + DEFAULT_BUILDKITE_PIPELINE_FOLDER = '.buildkite'.freeze + DEFAULT_ENV_FILE = File.join(DEFAULT_BUILDKITE_PIPELINE_FOLDER, 'shared-pipeline-vars').freeze def self.run(params) pipeline_file = params[:pipeline_file] + pipeline_file = File.join(DEFAULT_BUILDKITE_PIPELINE_FOLDER, pipeline_file) unless File.absolute_path?(pipeline_file) env_file = params[:env_file] - environment = params[:environment] + # Both keys and values need to be passed as strings otherwise Fastlane.sh will fail to parse the command. + environment = params[:environment].to_h { |k, v| [k.to_s, v.to_s] } UI.user_error!("Pipeline file not found: #{pipeline_file}") unless File.exist?(pipeline_file) UI.user_error!('This action can only be called from a Buildkite CI build') unless ENV['BUILDKITE'] == 'true' - UI.message "Adding steps from `#{pipeline_file}` to the current build" + UI.message("Adding steps from `#{pipeline_file}` to the current build") if env_file && File.exist?(env_file) UI.message(" - Sourcing environment file beforehand: #{env_file}") @@ -31,7 +34,7 @@ def self.available_options [ FastlaneCore::ConfigItem.new( key: :pipeline_file, - description: 'The path to the YAML pipeline file to upload', + description: 'The path to the YAML pipeline file to upload. If a relative path is provided, it will be prefixed with the `.buildkite/` folder path. Absolute paths are used as-is', optional: false, type: String ), diff --git a/spec/buildkite_pipeline_upload_action_spec.rb b/spec/buildkite_pipeline_upload_action_spec.rb index b6cfc6db7..face60b25 100644 --- a/spec/buildkite_pipeline_upload_action_spec.rb +++ b/spec/buildkite_pipeline_upload_action_spec.rb @@ -2,6 +2,7 @@ describe Fastlane::Actions::BuildkitePipelineUploadAction do let(:pipeline_file) { 'path/to/pipeline.yml' } + let(:loaded_pipeline_file) { File.join('.buildkite', pipeline_file) } let(:env_file) { 'path/to/env_file' } let(:env_file_default) { Fastlane::Actions::BuildkitePipelineUploadAction::DEFAULT_ENV_FILE } let(:environment) { { 'AKEY' => 'AVALUE' } } @@ -23,14 +24,14 @@ end it 'raises an error when pipeline_file does not exist' do - allow(File).to receive(:exist?).with(pipeline_file).and_return(false) + allow(File).to receive(:exist?).with(loaded_pipeline_file).and_return(false) expect do run_described_fastlane_action(pipeline_file: pipeline_file) end.to raise_error(FastlaneCore::Interface::FastlaneError, /Pipeline file not found/) end it 'raises an error when not running on Buildkite' do - allow(File).to receive(:exist?).with(pipeline_file).and_return(true) + allow(File).to receive(:exist?).with(loaded_pipeline_file).and_return(true) allow(ENV).to receive(:[]).with('BUILDKITE').and_return(nil) expect do @@ -39,10 +40,10 @@ end it 'passes the environment hash to the shell command' do - allow(File).to receive(:exist?).with(pipeline_file).and_return(true) + allow(File).to receive(:exist?).with(loaded_pipeline_file).and_return(true) expect(Fastlane::Action).to receive(:sh).with( environment, - 'buildkite-agent', 'pipeline', 'upload', pipeline_file + 'buildkite-agent', 'pipeline', 'upload', loaded_pipeline_file ) expect_upload_pipeline_message @@ -53,11 +54,11 @@ end it 'passes the environment hash to the shell command also with an env_file' do - allow(File).to receive(:exist?).with(pipeline_file).and_return(true) + allow(File).to receive(:exist?).with(loaded_pipeline_file).and_return(true) allow(File).to receive(:exist?).with(env_file).and_return(true) expect(Fastlane::Action).to receive(:sh).with( environment, - "source #{env_file.shellescape} && buildkite-agent pipeline upload #{pipeline_file.shellescape}" + "source #{env_file.shellescape} && buildkite-agent pipeline upload #{loaded_pipeline_file.shellescape}" ) expect_upload_pipeline_message expect_sourcing_env_file_message(env_file) @@ -72,13 +73,13 @@ describe 'pipeline upload' do before do - allow(File).to receive(:exist?).with(pipeline_file).and_return(true) + allow(File).to receive(:exist?).with(loaded_pipeline_file).and_return(true) end it 'calls the right command to upload the pipeline without env_file' do expect(Fastlane::Action).to receive(:sh).with( environment_default, - 'buildkite-agent', 'pipeline', 'upload', pipeline_file + 'buildkite-agent', 'pipeline', 'upload', loaded_pipeline_file ) expect_upload_pipeline_message @@ -89,7 +90,7 @@ allow(File).to receive(:exist?).with(env_file).and_return(true) expect(Fastlane::Action).to receive(:sh).with( environment_default, - "source #{env_file.shellescape} && buildkite-agent pipeline upload #{pipeline_file.shellescape}" + "source #{env_file.shellescape} && buildkite-agent pipeline upload #{loaded_pipeline_file.shellescape}" ) expect_upload_pipeline_message expect_sourcing_env_file_message(env_file) @@ -105,7 +106,7 @@ allow(File).to receive(:exist?).with(non_existent_env_file).and_return(false) expect(Fastlane::Action).to receive(:sh).with( environment_default, - 'buildkite-agent', 'pipeline', 'upload', pipeline_file + 'buildkite-agent', 'pipeline', 'upload', loaded_pipeline_file ) expect(Fastlane::UI).not_to receive(:message).with(/Sourcing environment file/) @@ -119,7 +120,7 @@ allow(File).to receive(:exist?).with(env_file_default).and_return(true) expect(Fastlane::Action).to receive(:sh).with( environment_default, - "source #{env_file_default} && buildkite-agent pipeline upload #{pipeline_file.shellescape}" + "source #{env_file_default} && buildkite-agent pipeline upload #{loaded_pipeline_file.shellescape}" ) expect_upload_pipeline_message expect_sourcing_env_file_message(env_file_default) @@ -128,11 +129,48 @@ pipeline_file: pipeline_file ) end + + it 'prefixes relative pipeline paths with the `.buildkite/` folder' do + relative_path = 'relative/pipeline.yml' + expected_path = File.join('.buildkite', relative_path) + allow(File).to receive(:exist?).with(expected_path).and_return(true) + allow(File).to receive(:absolute_path?).with(relative_path).and_return(false) + + expect(Fastlane::Action).to receive(:sh).with( + environment_default, + 'buildkite-agent', 'pipeline', 'upload', expected_path + ) + expect(Fastlane::UI).to receive(:message).with( + "Adding steps from `#{expected_path}` to the current build" + ) + + run_described_fastlane_action( + pipeline_file: relative_path + ) + end + + it 'uses absolute pipeline paths as-is' do + absolute_path = '/absolute/path/to/pipeline.yml' + allow(File).to receive(:exist?).with(absolute_path).and_return(true) + allow(File).to receive(:absolute_path?).with(absolute_path).and_return(true) + + expect(Fastlane::Action).to receive(:sh).with( + environment_default, + 'buildkite-agent', 'pipeline', 'upload', absolute_path + ) + expect(Fastlane::UI).to receive(:message).with( + "Adding steps from `#{absolute_path}` to the current build" + ) + + run_described_fastlane_action( + pipeline_file: absolute_path + ) + end end describe 'error handling' do it 'raises an error when the pipeline upload fails' do - allow(File).to receive(:exist?).with(pipeline_file).and_return(true) + allow(File).to receive(:exist?).with(loaded_pipeline_file).and_return(true) allow(Fastlane::Action).to receive(:sh).and_raise(StandardError.new('Upload failed')) expect do @@ -145,7 +183,7 @@ def expect_upload_pipeline_message expect(Fastlane::UI).to receive(:message).with( - "Adding steps from `#{pipeline_file}` to the current build" + "Adding steps from `#{loaded_pipeline_file}` to the current build" ) end