puppetlabs / puppet

Server automation framework and application
https://puppet.com/open-source/#osp
Apache License 2.0
7.33k stars 2.18k forks source link

Allow `exec` `creates` to check for all files instead of at least one #9168

Open yakatz opened 7 months ago

yakatz commented 7 months ago

Describe the Bug

I tried to convert an exec resource from multiple invocations of test -f to use the built-in creates option, but it doesn't work as expected, it will not execute unless none of the files are present.

Original:

  exec {'generate snakeoil certificate':
    command => "/usr/bin/sscg -q \
     --cert-file     /etc/pki/tls/certs/localhost.crt   \
     --cert-key-file /etc/pki/tls/private/localhost.key \
     --ca-file       /etc/pki/tls/certs/localhost.crt   \
     --dhparams-file /tmp/dhparams.pem                  \
     --lifetime      3650                               \
     --hostname      ${facts['networking']['fqdn']}     \
     --email         root@${facts['networking']['fqdn']}",
    unless  => [
      '/usr/bin/test -f /etc/pki/tls/certs/localhost.crt',
      '/usr/bin/test -f /etc/pki/tls/private/localhost.key',
    ],
    notify  => Service['httpd'],
  }

Expected equivalent:

  exec {'generate snakeoil certificate':
    command => "/usr/bin/sscg -q \
     --cert-file     /etc/pki/tls/certs/localhost.crt   \
     --cert-key-file /etc/pki/tls/private/localhost.key \
     --ca-file       /etc/pki/tls/certs/localhost.crt   \
     --dhparams-file /tmp/dhparams.pem                  \
     --lifetime      3650                               \
     --hostname      ${facts['networking']['fqdn']}     \
     --email         root@${facts['networking']['fqdn']}",
    creates => [
      '/etc/pki/tls/certs/localhost.crt',
      '/etc/pki/tls/private/localhost.key',
    ],
    notify  => Service['httpd'],
  }

Debug output if both exist:

Info: Applying configuration version '1700663411'
Debug: /Stage[main]/Main/Exec[generate snakeoil certificate]/creates: Checking that 'creates' path '/etc/pki/tls/certs/localhost.crt' exists
Debug: /Stage[main]/Main/Exec[generate snakeoil certificate]: '/usr/libexec/httpd-ssl-gencerts' won't be executed because of failed check 'creates'
Debug: Finishing transaction 12600

Debug output if the first file is missing and the second one exists:

Info: Applying configuration version '1700663438'
Debug: /Stage[main]/Main/Exec[generate snakeoil certificate]/creates: Checking that 'creates' path '/etc/pki/tls/certs/localhost.crt' exists
Debug: /Stage[main]/Main/Exec[generate snakeoil certificate]/creates: Checking that 'creates' path '/etc/pki/tls/private/localhost.key' exists
Debug: /Stage[main]/Main/Exec[generate snakeoil certificate]: '/usr/libexec/httpd-ssl-gencerts' won't be executed because of failed check 'creates'
Debug: Finishing transaction 12600

Debug output if neither file exists (debug run without notify httpd):

Info: Applying configuration version '1700663608'
Debug: /Stage[main]/Main/Exec[generate snakeoil certificate]/creates: Checking that 'creates' path '/etc/pki/tls/certs/localhost.crt' exists
Debug: /Stage[main]/Main/Exec[generate snakeoil certificate]/creates: Checking that 'creates' path '/etc/pki/tls/private/localhost.key' exists
Debug: Exec[generate snakeoil certificate](provider=posix): Executing '/usr/libexec/httpd-ssl-gencerts'
Debug: Executing: '/usr/libexec/httpd-ssl-gencerts'
Notice: /Stage[main]/Main/Exec[generate snakeoil certificate]/returns: executed successfully
Debug: /Stage[main]/Main/Exec[generate snakeoil certificate]: The container Class[Main] will propagate my refresh event
Debug: Class[Main]: The container Stage[main] will propagate my refresh event
Debug: Finishing transaction 12600

Expected Behavior

I would expect the command to be executed if any of the files listed in creates are missing.

Environment

yakatz commented 7 months ago

Discussing on Slack, another option that preserves backwards compatibility would be to take a multi-dimensional array:

[
  [all of these files, 2, 3, 4],
  [or all of these files, 5, 6, 7],
  [or even all of these],
]
cthorn42 commented 7 months ago

@yakatz I think the best solution here is to take your #9167 PR and get that merged in there.

github-actions[bot] commented 7 months ago

Migrated issue to PUP-11989