run task/plan: Allow noop and environment option

[bastelfreak]

No description provided.

[smortex]

Looks good, i am only thinking about non default settings. Is it worth handling it?

[smortex]

I am wondering if some site hardcode noop=true in puppet configuration, does it make sense to make this parameter:

Suggested change

	Boolean $noop = false,
	Optional[Boolean] $noop = undef,

That way the default behavior would be the same as before, and passing noop=false would allow to actually apply changes.

[bastelfreak]

I considered it. When you currently set $noop to false (or keep the default), the parameter won't be passed at all to puppet agent. So it's not a breaking change. I think that's a sane default.

[smortex]

Sorry for the late response. My point was that when noop=true in puppet.conf, this module plan are currently unusable to actually do changes because puppet run in noop mode all the time because of the configuration setting.

With this PR, we have a chance to "fix" it by adding an explicit --no-noop parameter to the puppet command when we set noop=false.

I heard some people are setting noop=true in puppet.conf and manually run the agent in no-noop mode when they detect drift. These users are currently not able to do this using the provided tasks and plans.

[smortex]

Of course, in this case the 'sane default' is not to change noop unless a value was passed.

In any case, this PR as it is today is very fine for me if you think the value is not worth it.

[smortex]

Suggested change

	$arg_noop = $noop ? {
	true => { 'noop' => true, },
	default => {},
	}
	$arg_noop = {
	'noop' => $noop,
	}

[smortex]

Suggested change

	"noop": {
	"description": "run the puppet agent with --noop",
	"type": "Boolean",
	"default": false
	},
	"environment": {
	"noop": {
	"description": "run the puppet agent with --noop",
	"type": "Optional[Boolean]",
	},
	"environment": {

[smortex]

Suggested change

	def noop(params)
	params['noop'] == true ? '--noop' : ''
	end
	def noop(params)
	case params['noop']
	when true then '--noop'
	when false then '--no-noop'
	else ''
	end
	end

[mhashizume]

@bastelfreak LGTM, it looks like there are a few puppet-lint whitespace errors but once those are fixed I'm happy to approve and merge.

[bastelfreak]

@mhashizume I think all of the puppet-lint violations were already present in main. I fixed the ones in the run plan.

[mhashizume]

Thanks @bastelfreak !

[bastelfreak]

thanks for merging! Is it possible to make a new release soonish?

[mhashizume]

Yeah, we have some more work on this module ticketed out in preparation for Puppet 8 (you can see them in this epic with the prefix "[puppet_agent]") but will cut a release when we've finished.