Puppet users - Jan 2013 - generate function not working within conditional?

Hi,

could this be a bug?


I have this code:

$mypath = get_module_path(''ssh'')
if generate("${mypath}/scripts/generate_host_keys.sh",
"${keys_dir}/${::fqdn}/ssh") {

  notify {''something'':}
  include ssh::server::keys
}


generate_host_keys.sh script generates ssh host keys for ssh daemons,
and stores them in private section.

Those files are later defined as resources in class
''ssh::server::keys''



Now, if the files aren''t there, script in generate function creates
them, and then code under the "if" clause is executed. But if the
files
are already in the ${keys_dir}/${::fqdn}/ssh directory, script returns
0, but the code under the if statement isn''t run :-/

Even if I delete the resources on the agent (rm -f /etc/ssh/*), puppet
agent run doesn''t figure the changes...

I would have swore that this worked in earlier release... I''m on
3.0.2-1
CentOS 6.

-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

On 01/14/2013 04:57 PM, Jakov Sosic wrote:
> Hi,
> 
> could this be a bug?
> 
OK I''ve figured it out... If I look at puppet code, in the file:

/usr/lib/ruby/site_ruby/1.8/puppet/parser/functions/generate.rb

it seems that generate() doesn''t return anything :-/


So "if" in this case doesn''t make any sense...


But I''m still puzzled why it works in case generate function does
something, and doesn''t work if it doesn''t...

-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

On 01/14/2013 05:28 PM, Jakov Sosic wrote:
> On 01/14/2013 04:57 PM, Jakov Sosic wrote:
>> Hi,
>>
>> could this be a bug?
>>
> 
> OK I''ve figured it out... If I look at puppet code, in the file:
> 
> /usr/lib/ruby/site_ruby/1.8/puppet/parser/functions/generate.rb
> 
> it seems that generate() doesn''t return anything :-/
> 
> 
> So "if" in this case doesn''t make any sense...
> 
> 
> But I''m still puzzled why it works in case generate function does
> something, and doesn''t work if it doesn''t...
> 

OK I''ve figured it even further :D

Difference is that in the first case, my script outputs some text. And
in the second case, when files are already in place on the puppet
master, it just exits gracefully (with status 0).

So obviously, return value is the output of the command, and if there is
no output, condition of the "if" is false. If there is some output,
condition is not false...

So, I''ve changed my scripts to end not like this:

exit 0

But like this:

echo -n "Success" && exit 0



And in the puppet manifests, I''ve changed from:

if generate("${mypath}/scripts/generate_ricci_client_cert.sh",
$ricci_clientc_dir) { somecode }


to:

if ( generate("${mypath}/scripts/generate_ricci_client_cert.sh",
$ricci_clientc_dir) == ''Success'' ) { somecode }



And now it works...


I guess a fix in the documentation of generate() function, explaining
what function returns to puppet DSL would be a good addition...

-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

On Monday, January 14, 2013 10:38:33 AM UTC-6, Jakov Sosic
wrote:
>
>
> I guess a fix in the documentation of generate() function, explaining 
> what function returns to puppet DSL would be a good addition... 
>

That already seems pretty clear to me in the existing docs:

Calls an external command on the Puppet master and *returns the results of 
the command*. Any arguments are passed to the external command as 
arguments. * If the generator does not exit with return code of 0, the 
generator is considered to have failed and a parse error is thrown*. 
Generators can only have file separators, alphanumerics, dashes, and 
periods in them. This function will attempt to protect you from malicious 
generator calls (e.g., those with ‘..’ in them), but it can never be 
entirely safe. No subshell is used to execute generators, so all shell 
metacharacters are passed directly to the generator.

(Emphasis added.)  How would you suggest improving that?

I guess "results" could be changed to "output", but
it''s clear to me that
the function does not return the command''s exit code because any exit
code
other than zero constitutes a parse error.  Returning the output of the 
command is anyway the whole point of this function -- that is, to 
"generate" a value for use in your manifest.  Using it for the side
effects
of the command is not exactly improper, but it''s not the use case
toward
which the function is targeted.  Personally, I''d write a custom
function
for such a purpose, to try to ensure that the intent is clear.


John

-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To view this discussion on the web visit
https://groups.google.com/d/msg/puppet-users/-/QQ1sgGfDRQ4J.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

On 01/15/2013 04:58 PM, jcbollinger wrote:
> That already seems pretty clear to me in the existing docs:
> 
> Calls an external command on the Puppet master and *returns the results
> of the command*.
> (Emphasis added.)  How would you suggest improving that?
Hm, it''s maybe only me, but result of the command associates me with
exit codes, not with actual output.

Maybe the description should say:

Calls an external command on the Puppet master and returns the standard
output of the command.

> I guess "results" could be changed to "output", but
it''s clear to me
> that the function does not return the command''s exit code because
any
> exit code other than zero constitutes a parse error.  Returning the
> output of the command is anyway the whole point of this function -- that
> is, to "generate" a value for use in your manifest.  Using it for
the
> side effects of the command is not exactly improper, but it''s not
the
> use case toward which the function is targeted.  Personally, I''d
write a
> custom function for such a purpose, to try to ensure that the intent is
> clear.
Well, generate serves me pretty well for other purposes then generating
output value for puppet. For example I''m currently generating ssh host
keys on the master, ricc service (RedHat Cluster Suite remote cluster
management) certificates and client certificates to access ricci. No
doubt, with time I will find further use cases.

Benefit of using generate() is that I can reuse existing code. For
example, init script of RHEL/CentOS for ssh daemon has the code for
generation of ssh host keys. Code is separated in bash functions. So, I
can just include (parse) that script in my own shell script, change the
values of variables and generate keys. And if I want puppet to do it,
just put that wrapper script in generate() function and that''s it.
Otherwise, I would write my own code for generating host keys in ruby.
Not that I''m scared or lazy, but just don''t see the point of
reinventing
the wheel and introducing new bugs to the story. My vendor already
distributes that code, and as long as client ssh daemon can use
certificates generated on my master, I''m fine with it :)

If, for some reason, ssh daemons on some distribution become
incompatible with the keys generated this way, I will for sure thing
about doing things differently.

I hope u understand my point of view on this one.

-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

On Tuesday, January 15, 2013 11:30:10 AM UTC-6, Jakov Sosic
wrote:
>
>
> Hm, it''s maybe only me, but result of the command associates me
with
> exit codes, not with actual output. 
>

I can understand that interpretation, but again, if you take it that way 
then you have to assume that generate() will only ever return 0 (because 
any other exit code of the underlying command is documented to cause a 
parse error).

 
>
> Maybe the description should say: 
>
> Calls an external command on the Puppet master and returns the standard 
> output of the command. 
>
>
That would be less ambiguous.  If you filed an RFE against the docs for 
this then I think it would be very likely to be accepted and implemented 
quickly.
 
>
> Well, generate serves me pretty well for other purposes then generating 
> output value for puppet. For example I''m currently generating ssh
host
> keys on the master, ricc service (RedHat Cluster Suite remote cluster 
> management) certificates and client certificates to access ricci. No 
> doubt, with time I will find further use cases. 
>
>
YMMV, obviously, but I suggest that it is needlessly risky and confusing to 
conflate two different interpretations of "generate" and associate
both
with the function, or to altogether ignore the function''s intended
purpose
as expressed by its name and docs.  I appreciate that you misunderstood the 
function''s intended purpose (to generate a value for use in Puppet, as 
opposed to generating an external effect on the master), but now that it 
has been clarified, I advise you to give careful consideration to what you 
will do with that function moving forward.

As a separate issue, I''ll add that I am not enthusiastic about the idea
of
the master modifying itself as a side effect of catalog compilation.  
That''s not an absolute, however, inasmuch as I don''t object to
effects such
as caching compiled catalogs or logging.

 
> Benefit of using generate() is that I can reuse existing code. For 
> example, init script of RHEL/CentOS for ssh daemon has the code for 
> generation of ssh host keys. Code is separated in bash functions. So, I 
> can just include (parse) that script in my own shell script, change the 
> values of variables and generate keys. And if I want puppet to do it, 
> just put that wrapper script in generate() function and that''s it.
> Otherwise, I would write my own code for generating host keys in ruby.


You misunderstand me.  I don''t suggest that you should write a
pure-Ruby
function to (for example) generate host keys.  Rather, if you want to 
create artifacts such as host keys on the master, then I suggest you write 
a custom Puppet function that wraps the same bash script that you are 
currently invoking via generate().  In addition to that being clearer on 
its face, it may also be more flexible in that you can define multiple 
function arguments instead of requiring everything in one argument.  Also, 
you would be able to return a status code directly.
 
>
> I hope u understand my point of view on this one. 
>

Whether I understand or not is immaterial.  You need to decide how to 
proceed, and what I think about it doesn''t much matter.  I
can''t even be
certain that what I describe would indeed be better overall than what 
you''re now doing.  Even if this doesn''t change your mind or
help you,
however, I write with confidence that it will help *somebody*.


Cheers,

John

-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To view this discussion on the web visit
https://groups.google.com/d/msg/puppet-users/-/mY8Lm8SraiwJ.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

On 01/16/2013 04:30 PM, jcbollinger wrote:
> I can understand that interpretation, but again, if you take it that way
> then you have to assume that generate() will only ever return 0 (because
> any other exit code of the underlying command is documented to cause a
> parse error).
Yea, I know.
> That would be less ambiguous.  If you filed an RFE against the docs for
> this then I think it would be very likely to be accepted and implemented
> quickly.
Do I open this like a bug, or? Don''t know what RFE is :-/

> As a separate issue, I''ll add that I am not enthusiastic about the
idea
> of the master modifying itself as a side effect of catalog compilation. 
It''s only generating ssh keys and stuff like that. It helps much with
automation, otherwise I would have to manually fetch the keys and/or
ricci certificates after initial installation of every system, and store
them for future use.

> You misunderstand me.  I don''t suggest that you should write a
pure-Ruby
> function to (for example) generate host keys.  Rather, if you want to
> create artifacts such as host keys on the master, then I suggest you
> write a custom Puppet function that wraps the same bash script that you
> are currently invoking via generate().  In addition to that being
> clearer on its face, it may also be more flexible in that you can define
> multiple function arguments instead of requiring everything in one
> argument.  Also, you would be able to return a status code directly.
I will certainly have it on my mind, but currently I''m really not very
rich man concerning free time :)

-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.

On Wednesday, January 16, 2013 11:06:49 AM UTC-6, Jakov Sosic
wrote:
>
> On 01/16/2013 04:30 PM, jcbollinger wrote: 
> [...]
> > That would be less ambiguous.  If you filed an RFE against the docs
for
> > this then I think it would be very likely to be accepted and
implemented
> > quickly. 
>
> Do I open this like a bug, or? Don''t know what RFE is :-/ 
>
>
Sorry, RFE is "Request For Enhancement".  You file it in the issue
tracker,
very much like a bug report, but you will select "Feature" instead of
"Bug"
from the relevant drop-down list.  I believe you need to log in to be able 
to create new issues.


John

-- 
You received this message because you are subscribed to the Google Groups
"Puppet Users" group.
To view this discussion on the web visit
https://groups.google.com/d/msg/puppet-users/-/M4rPLUq0x-QJ.
To post to this group, send email to puppet-users@googlegroups.com.
To unsubscribe from this group, send email to
puppet-users+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-users?hl=en.