Bug #4633
Array flattening breaks idempotence
| Status: | Rejected | Start date: | 08/26/2010 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assignee: |  Paul Berry |
% Done: | 0% |
|
| Category: | - | |||
| Target version: | - | |||
| Affected Puppet version: | Branch: | |||
| Keywords: | ||||
| Votes: | 0 |
Description
Using this manifest:
ssh_authorized_key { key1:
user => pberry,
target => '/tmp/authorized_keys',
type => ssh-rsa,
key => 0123456789,
options => [a, [b, [c, d]]]
}
the first time I run “puppet apply init.pp” I get this output:
notice: /Stage[main]//Ssh_authorized_key[key1]/ensure: created
which is expected. If I look at the generated authorized_keys file I see that the options array has been “flattened out” from [a, [b, [c, d]]] to “a,b,c,d”:
# HEADER: This file was autogenerated at Thu Aug 26 09:08:14 -0700 2010 # HEADER: by puppet. While it can still be managed manually, it # HEADER: is definitely not recommended. a,b,c,d ssh-rsa 0123456789 key1
I’m not sure whether a naive user would have expected this, but it seems reasonable given that ssh doesn’t allow options to be arrays.
However, when I run the manifest again, I get this output:
notice: /Stage[main]//Ssh_authorized_key[key1]/options: options changed 'a,b,c,d' to 'a,b,c,d'
Because of the use of nested arrays, Puppet gets confused and thinks the options have changed when they haven’t, so it attempts to re-write the authorized_keys file every time it is run.
Related issues
History
Updated by Paul Berry over 1 year ago
On further investigation, I found that Puppet’s code for flattening arrays is inconsistent:
- The parser recursively flattens out any array whose first element is also an array. So, for example, this:
ssh_authorized_key { key1: options => [[[a, b], c], d]
parses exactly the same as:
ssh_authorized_key { key2: options => [a, b, c, d]
- The ASTArray#evaluate method flattens out any array that contains an array, but it doesn’t always flatten out more deeply-nested arrays. So this:
[a, [b, [c, d]]]
evaluates to this:
[a, b, [c, d]]
- further flattening is performed by the ssh_authorized_key provider, in the is_to_s() and should_to_s() methods. But these methods are called too late to avoid the idempotence problem.
This is a fairly benign bug, but I am currently touching this code, and the behavior is inconsistent enough that I would prefer to change it to the desired behavior rather than try to preserve the existing inconsistent semantics.
Based on a conversation with Luke this morning, I think our best option is probably to modify the ASTArray#evaluate method so that it completely flattens out arrays of arrays, and remove the flattening logic from the parser.
Updated by Paul Berry over 1 year ago
- Status changed from Accepted to In Topic Branch Pending Review
- % Done changed from 0 to 100
- Branch set to http://github.com/stereotype441/puppet/tree/ticket/next/4633
I have a fix for this issue that also cleans up some of the ambiguity involving arrays in the grammar.
Updated by Paul Berry over 1 year ago
- Status changed from In Topic Branch Pending Review to Rejected
- % Done changed from 100 to 0
- Branch deleted (
http://github.com/stereotype441/puppet/tree/ticket/next/4633)
On further discussion with Jesse, we realized this is not the right fix. There’s no reasonable use case where array auto-flattening would have been of use to users, and it limits our ability to expand the Puppet language in the future.
Updated by Matt Robinson over 1 year ago
Note, rejected in favor of #4638