puppetlabs / puppetlabs-stdlib

Puppet Labs Standard Library module
http://forge.puppetlabs.com/puppetlabs/stdlib
Apache License 2.0
348 stars 580 forks source link

Parse deferred templates non deferred first #1425

Open traylenator opened 7 months ago

traylenator commented 7 months ago

Summary

Parse templates with deferred values twice so complex data types can be used.

Additional Context

Currently it is not possible to have a template file.epp

<%- |
Stdlib::Port $port,
String[1] $password,
| %>
port <%= $port %>
password <%= $password %>

and run

file{'/tmp/junk':
  content => stdlib::deferrable_epp(
      'module/file.epp',
      { 'port' => '1234', pass => Deferred('secrets::get',['mysecret'])}
   ),
}

since the deferred template substitution will fail:

Error: Failed to apply catalog: Evaluation Error: Resource type not found: Stdlib::Port (file: inlined-epp-text, line: 2, column: 3)

due to Stdlib::Port not being available on the agent node.

This change now parses the EPP twice. The first non deferred parse will reduce the template to:

port = 1234
password <%= $password %>

and this simpler template will be parsed in deferred mode.

Note the original template type for password must accept the intermediate generated value of <%= $password %> which is typically case for a secret password. If the deferred value is more complicated then the argument preparse can be set false to not attempt the epp substitution of non deferred values.

Provide a detailed description of all the changes present in this pull request.

Related Issues (if any)

Fixes https://github.com/voxpupuli/puppet-redis/issues/513

Checklist

traylenator commented 7 months ago

@alexjfisher I removed the commented out tests since https://github.com/puppetlabs/rspec-puppet/pull/24 is merged and it seems to pass now ... bit confused by though tests - think they need some more lines now I expect.

traylenator commented 7 months ago

Very open to idea that this new feature should be behind a parameter:

file{'/tmp/junk':
  content => stdlib::deferrable_epp(
      'module/file.epp',
      { 'port' => '1234', pass => Deferred('secrets::get',['mysecret'])},
      prerender => true,
   ),
}

or something?

alexjfisher commented 7 months ago

Very open to idea that this new feature should be behind a parameter:

file{'/tmp/junk':
  content => stdlib::deferrable_epp(
      'module/file.epp',
      { 'port' => '1234', pass => Deferred('secrets::get',['mysecret'])},
      prerender => true,
   ),
}

or something?

Because of

Note the original template type for password must accept the intermediate generated value of <%= $password %> which is typically case for a secret password.

this might be necessary, but maybe the default can be the new behaviour?

traylenator commented 7 months ago

this might be necessary, but maybe the default can be the new behaviour?

Yes someone using a secret array type, while rare I expect, will have to do something. Set new preparse=>false or update their template to accept a string also.

Have added preparse => true as a default.

alexjfisher commented 7 months ago

this might be necessary, but maybe the default can be the new behaviour?

Yes someone using a secret array type, while rare I expect, will have to do something. Set new preparse=>false or update their template to accept a string also.

Have added preparse => true as a default.

That works for me. (I think more likely than a non string type is someone using Pattern for cases where passwords eg. don't accept special characters.)

traylenator commented 7 months ago

As this is it's backwards incompatible. Changing to default false it would be compatible.

I think true is better.

Depends a bit how near we are to a major bump.