Add customizable warning not to directly edit settings.py

[wbclark]

No description provided.

[ehelms]

Following up from some IRC conversation, my take on things was:

I like if we could across our ecosystem have a consistent header for our modules. You can see for example, https://github.com/theforeman/puppet-foreman/blob/master/templates/_header.erb, is used across a few different modules already as a format. This PR raises a desire to sometimes customize the degree and intent of the message for the file. Thus, I would propose a hybrid of the two. A standard starting header followed by any custom message, e.g.

### File managed with puppet ###
## Module:           '<%= scope.to_hash['module_name'] %>'
#######################################################################
# WARNING: This file is externally managed and you should never make manual edits.  #
#                                                                                      #
# Not only are your edits likely to be overwritten, there is also a strong possibility        #
# of breaking your system. Refer to documentation for the software that installed        #
# Pulpcore to determine the safe and persistent way to modify the configuration.        #
#####################################################################

And I think it's fair to challenge if the current headers message, what would serve as our base template across modules, needs some updating or modification to be more relevant to more use cases. Could it be configurable by the installer? Possibly. One tricky bit could be that comments may be different amongst different files (that is # vs //).

[wbclark]

Updated this with a custom function, generate_header_content

This takes an Array of Strings (raw_content), an Integer line_width, and a String comment_glyph and generates lines for a nicely formatted header, containing the raw_content, line wrapped and enclosed in the comment_glyph.

It also introduces a new class pulpcore::header_strings which allows to override the default strings that are passed into raw_content when generating the header.

This way downstream projects can use Hiera to add custom content to headers, while pure Puppet users will see a sensible default for Puppet.

In the future, most of this logic should be removed from this module and placed in a common module that can be used throughout the installer ecosystem.

TODO: Add tests

[wbclark]

In the current implementation, see the complete header as well as the beginning of the rest of the settings.py content below:

################################################################################
# !!! WARNING: DO NOT EDIT THIS FILE !!!                                       #
################################################################################
# File managed by Puppet.                                                      #
# Module: pulpcore                                                             #
################################################################################
# Not only are your edits likely to be overwritten, there is also a strong     #
# possibility of breaking your system if you change configuration here without #
# making required changes elsewhere. Refer to the documentation you used to    #
# install Pulpcore to determine the safe and persistent way to modify the      #
# configuration.                                                               #
################################################################################

CONTENT_HOST = "centos7-64-1"
CONTENT_ORIGIN = "https://centos7-64-1"

In general with this method, the number of text blocks in the header is optional. Any Array of Strings can be passed, and each string will be line wrapped and enclosed in the comment_glyph.

As currently implemented in this module, the # of blocks and content of the first text block is static, while the 2nd and 3rd are configurable by setting pulpcore::header_strings::custom_installer_name and pulpcore::header_strins::custom_settings_explanation

With appropriate hiera in foreman-installer, this might look instead like, e.g.:

################################################################################
# !!! WARNING: DO NOT EDIT THIS FILE !!!                                       #
################################################################################
# File managed by foreman-installer.                                           #
# Module: pulpcore                                                             #
################################################################################
# Configure Pulpcore using foreman-installer. See foreman-installer --help     #
################################################################################

Or any other custom string providing appropriate guidance to the user.

[wbclark]

Suggested change

	$settings_header_content = pulpcore::generate_header_content(
	[
	$warning_header,
	$installer_header,
	$explanation,
	],
	80,
	'#',
	)
	$settings_header_content = pulpcore::generate_header_content(
	{
	raw_content => [
	$warning_header,
	$installer_header,
	$explanation,
	],
	line_width => 80,
	comment_glyph => '#',
	}
	)

I'm not actually sure if Puppet function calls support named arguments in this way, but if so I'd like to give it a try for better readability

[wbclark]

I suppose a name like wrap_and_comment_header_content would be more descriptive here. Open to suggestions

Also trying to determine what would be the correct place to open a PR with this function upstream. Perhaps it makes the most sense in the Concat module?

[ehelms]

I'll spend more time digging into it. My initial two thoughts are:

1. This appears to be some complex logic, I do get it's aiming for allowing for high customizability.
2. Given how "generic" this is, I wonder if it would make sense in a light weight module that provides this functionality for use by other modules.

[ekohl]

Given how "generic" this is, I wonder if it would make sense in a light weight module that provides this functionality for use by other modules.

voxpupuli/puppet-extlib#171 does take that idea. I think that or stdlib would be good places.

[ehelms]

This has been open for a year now, I am therefore tempted to close it as it can be re-visited in the future if desired. @wbclark thoughts?