Implement allowlist for puppet module content

[bastelfreak]

This implements puppetlabs/puppet-specifications#157

• By default every file is ignored
• Only files from the official specification for puppet modules are added to the allowlist
• support for .pdkignore, .pmtignore and .gitignore is removed

[bastelfreak]

For public test: voxpupuli/puppet-example#55

[rwaffen]

LGTM, but minor question

[ekohl]

These values for IGNORED worked for me.

[bastelfreak]

I tested this in puppet-example and it seems to work now:

$ tar xfv puppet-example-0.3.1-rc0.tar.gz
puppet-example-0.3.1-rc0
puppet-example-0.3.1-rc0/CHANGELOG.md
puppet-example-0.3.1-rc0/README.md
puppet-example-0.3.1-rc0/manifests
puppet-example-0.3.1-rc0/manifests/init.pp
puppet-example-0.3.1-rc0/metadata.json

[ekohl]

That's also where I tested it, but perhaps we should enhance the acceptance test:

puppet-modulebuilder/spec/acceptance/puppet/modulebuilder/builder_spec.rb

Lines 68 to 101 in f4d83c7

	before do
	# Force the module to be built...
	built_tarball = tarball_name
	# Use puppet to "install" it...
	require 'open3'
	output, status = Open3.capture2e("bundle exec puppet module install --force --ignore-dependencies --target-dir #{extract_path} --verbose #{built_tarball}")
	raise "Failed to install the module using Puppet. Exit code #{status.exitstatus}: #{output}" unless status.exitstatus.zero?
	raise 'Failed to install the module using Puppet. Missing extract directory' if extracted_module_path.nil?
	end
	after do
	FileUtils.rm_rf(extract_path)
	end
	it 'expands the expected paths' do # This is expected
	# No development directories
	expect('/spec/*').to be_an_empty_glob
	expect('/.vscode/*').to be_an_empty_glob
	expect('/tmp/*').to be_an_empty_glob
	# No development files
	expect('/.fixtures').to be_an_empty_glob
	expect('/.gitignore').to be_an_empty_glob
	expect('/Rakefile').to be_an_empty_glob
	# No CI files
	expect('/.travis.yml').to be_an_empty_glob
	expect('/appveyor.yml').to be_an_empty_glob
	# Important Extracted files
	expect('/manifests/*').to be_identical_as_soource
	expect('/templates/*').to be_identical_as_soource
	expect('/lib/*').to be_identical_as_soource
	end

Introduce multiple layers in manifests to make sure it also works for module::sub::class.

Perhaps even introduce an explicit test that lists the files in the tarball and compare them to an explicit list.

[jordanbreen28]

@bastelfreak 👋 I think we are gonna need to keep .pdkignore support as we look to modularise the PDK and pull in modulebuilder for the build functionality.

I'll bring up the other portions of this PR for triage with the team, nice work

[bastelfreak]

Why is it required to keep .pdkignore? You cannot reliable produce valid module releases with that approach because you've a deny list and you can only deny files you're aware of. So you can end up with files in your module release that must not be there.

[jordanbreen28]

@bastelfreak we cannot commit to both this and puppetlabs/pdk#1360, and removal of the .pdkignore will require a major release of the PDK (and conversation around if we even want to do this).

As the work to implement modulebuilder has already been committed to, we aren't going to be able to remove support for .pdkignore here in modulebuilder, just yet anyway.

An alternative approach may be to keep .pdkignore support in modulebuilder, but update the .pdkignore to be an allow list. However this again would require a major release.

[bastelfreak]

removal of the .pdkignore will require a major release of the PDK

Yes. But why is that a problem? Major releases are required from time to time. And as long as there's a migration path that shouldn't be a problem.

puppet-modulebuilder is the open source library that's used to build module releases. Puppet and the community agreed on the new module layout in puppetlabs/puppet-specifications#157. And that request originated from a PE customer due to poor code-manager performance because of too many useless files in their code environments.

Why can't we get a new major release of puppet-modulebuilder that honours the new layout. And pdk, as a downstream consumer, later switches to that version? You could stick to the 1.x releases and with the next major PDK release switch to puppet-modulebuilder 2?

By blocking this PR, nobody in the ecosystem can benefit from it.

As the work to implement modulebuilder has already been committed to

I think this is the actual problem. Puppet heavily relies on open source and someone at Puppet prioritizes work internally, without speaking to open source users,contributors,customers,partners.

[jordanbreen28]

@bastelfreak I hear ya, I do. The main issue here is the removal of the .pdkignore. But you raise some valid points, I've brought this up internally and myself or one of the team will update this PR again

[bastelfreak]

Thanks a lot!

[davidsandilands]

@bastelfreak to reiterate on Jordans point I also agree on your planning point. This is something we will be looking to fix and define in Q3 with the community. Our products and their subset components should be clear on their lifecycles and OS support. This should create a reasonable schedule to understand where contributions will be published, if it will be instantly or within a certain schedule with necessary dependencies and customer needs tested/met. It also needs us to have the right community time and discussions to ensure contributions can be planned for in our engineering resourcing schedule and not end up as something we endlessly have to push back.

For us there are more complex issues at play with how PDK is adopted by customers currently (who also do not current receive guidance on how quickly to update),therefore most are not even on 3.x yet and other products such as CD4PE are reliant on PDK.

I appreciate this is all jam tomorrow talk but you will see progress on this shortly.

[bastelfreak]

For us there are more complex issues at play with how PDK is adopted by customers currently (who also do not current receive guidance on how quickly to update),therefore most are not even on 3.x yet and other products such as CD4PE are reliant on PDK.

I totally understand that. But I don't see how pdk is directly related here. There's no requirement for pdk to depend on the latest version of puppet-modulebuilder. We could easily to a major release with #79 + patchspec 2.

[pmcmaw]

Hey @bastelfreak we have chatted about this topic again, in order to progress we cannot remove the .pdkignore, I understand the frustration but this is the only way we can see this PR moving forward. We just have no bandwidth to maintain multiple versions if any bug fixes are required in the future.

The pdkignore piece can align in the future when we start considering a PDKv4 however for now, this is not something we can move forward with.

[ekohl]

Today puppet-modulebuilder isn't used in PDK, right? If so, why does it matter?

[davidsandilands]

@pmcmaw this was discussed in the vox monthly sync with myself and @LukasAud and as there are some customer needs here when working at scale it changes the ask and need we can cover in tomorrows triage