Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(PDK-1086) Change pdk build --force to warn if missing module metadata and continue #643

Merged
merged 1 commit into from
Apr 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions lib/pdk/cli/build.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,12 @@ module PDK::CLI
#
unless module_metadata.forge_ready?
if opts[:force]
PDK.logger.error _('This module is missing required fields in the metadata.json. Re-run the build command without --force to add this information.')
exit 1
PDK.logger.warn _(
'This module is missing the following fields in the metadata.json: %{fields}. ' \
'These missing fields may affect the visibility of the module on the Forge.',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are any of the fields things that would cause the Forge to reject it? If so maybe this should say "may affect your ability to publish this module on the Forge" instead?

) % {
fields: module_metadata.missing_fields.join(', '),
}
else
module_metadata.interview_for_forge!
module_metadata.write!('metadata.json')
Expand Down
4 changes: 2 additions & 2 deletions lib/pdk/module/metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,13 @@ def puppet_requirement
end
end

private

def missing_fields
fields = DEFAULTS.keys - %w[data_provider requirements dependencies]
fields.select { |key| @data[key].nil? || @data[key].empty? }
end

private

# Do basic validation and parsing of the name parameter.
def process_name(data)
validate_name(data['name'])
Expand Down
69 changes: 69 additions & 0 deletions spec/acceptance/build_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
require 'spec_helper_acceptance'

describe 'pdk build', module_command: true do
context 'when run inside of a module' do
include_context 'in a new module', 'build'

metadata = {
'name' => 'testuser-build',
'version' => '0.1.0',
'author' => 'testuser',
'summary' => 'a test module',
'source' => 'https://github.com/testuser/puppet-build',
'project_page' => 'https://testuser.github.io/puppet-build',
'issues_url' => 'https://github.com/testuser/puppet-build/issues',
'dependencies' => [],
'operatingsystem_support' => [{ 'operatingsystem' => 'windows', 'operatingsystemrelease' => ['10'] }],
'requirements' => [{ 'name' => 'puppet', 'version_requirement' => '> 4.10.0 < 7.0.0' }],
'pdk-version' => '1.2.3',
'template-url' => 'https://github.com/puppetlabs/pdk-templates',
'template-ref' => 'heads/master-0-g1234abc',
}

context 'when the module has complete metadata' do
before(:all) do
File.open('metadata.json', 'w') do |f|
f.puts metadata.to_json
end
end

after(:all) do
FileUtils.remove_entry_secure('pkg')
end

describe command('pdk build --force') do
its(:exit_status) { is_expected.to eq(0) }
its(:stdout) { is_expected.to have_no_output }
its(:stderr) { is_expected.not_to match(%r{WARN|ERR}) }
its(:stderr) { is_expected.to match(%r{Build of #{metadata['name']} has completed successfully}) }

describe file(File.join('pkg', "#{metadata['name']}-#{metadata['version']}.tar.gz")) do
it { is_expected.to be_file }
end
end
end

context 'when the module has incomplete metadata' do
before(:all) do
File.open('metadata.json', 'w') do |f|
f.puts metadata.reject { |k, _| k == 'source' }.to_json
end
end

after(:all) do
FileUtils.remove_entry_secure('pkg')
end

describe command('pdk build --force') do
its(:exit_status) { is_expected.to eq(0) }
its(:stdout) { is_expected.to have_no_output }
its(:stderr) { is_expected.to match(%r{WARN.+missing the following fields.+source}) }
its(:stderr) { is_expected.to match(%r{Build of #{metadata['name']} has completed successfully}) }

describe file(File.join('pkg', "#{metadata['name']}-#{metadata['version']}.tar.gz")) do
it { is_expected.to be_file }
end
end
end
end
end
7 changes: 4 additions & 3 deletions spec/unit/pdk/cli/build_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
context 'and the module contains incomplete metadata' do
before(:each) do
allow(mock_metadata_obj).to receive(:forge_ready?).and_return(false)
allow(mock_metadata_obj).to receive(:missing_fields).and_return(%w[operatingsystem_support source])
allow(PDK::Module::Build).to receive(:new).with(any_args).and_return(mock_builder)
end

Expand All @@ -71,9 +72,9 @@
context 'with --force option' do
let(:command_opts) { ['--force'] }

it 'outputs an error message' do
expect(logger).to receive(:error).with(a_string_matching(%r{This module is missing required fields in the metadata.json}i))
expect { PDK::CLI.run(['build'] + command_opts) }.to exit_nonzero
it 'outputs an warning and continues' do
expect(logger).to receive(:warn).with(a_string_matching(%r{fields in the metadata\.json: operatingsystem_support, source}im))
expect { PDK::CLI.run(['build'] + command_opts) }.not_to raise_error
end
end
end
Expand Down