puppetlabs-stdlib icon indicating copy to clipboard operation
puppetlabs-stdlib copied to clipboard

Parse deferred templates non deferred first

Open traylenator opened this issue 1 year ago • 6 comments

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

  • [X] 🟢 Spec tests.
  • [x] 🟢 Acceptance tests.
  • [X] Manually verified. Verified on a separate master and agent with puppet-redis module as per bug.

traylenator avatar Apr 15 '24 09:04 traylenator

@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 avatar Apr 15 '24 10:04 traylenator

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?

traylenator avatar Apr 15 '24 10:04 traylenator

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?

alexjfisher avatar Apr 15 '24 10:04 alexjfisher

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.

traylenator avatar Apr 15 '24 11:04 traylenator

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.)

alexjfisher avatar Apr 16 '24 08:04 alexjfisher

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.

traylenator avatar Apr 16 '24 08:04 traylenator