The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

Bug #3955

Specifying file content by checksum should not use 'content' property

Added by Markus Roberts about 4 years ago. Updated 8 months ago.

Status:AcceptedStart date:06/08/2010
Priority:HighDue date:
Assignee:-% Done:

0%

Category:-
Target version:-
Affected Puppet version:development Branch:
Keywords:

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This ticket is now tracked at: https://tickets.puppetlabs.com/browse/PUP-1043


Description

Rather than passing around the contents of a file (e.g. in logs) we use the checksum; to support this the users is also allowed to specify the contents as a checksum, but this leads to an ambiguity when the actual contents match the checksum pattern.

Instead of trying to do magic, we should have distinct syntax for referring to the contents by checksum (which should always expect a valid checksum) and for specifying the contents literally (which shouldn’t do anything different if the contents happen to resemble a checksum).


Related issues

Related to Puppet - Feature #6879: Add `capabilities` to the puppet agent / master communica... Accepted 03/29/2011
Related to Puppet - Bug #2072: File content always prints as checksums Accepted 03/12/2009

History

#1 Updated by Luke Kanies about 4 years ago

Note that it won’t be a separate syntax – I looked into that, and it didn’t make sense because it would only be a valid rvalue for files, which is silly.

Instead it’ll be a URL type for ‘source’.

#2 Updated by Luke Kanies almost 4 years ago

  • Subject changed from Should have a seperate syntax for specifying file content by checksum to Specifying file content by checksum should not use 'content' property
  • Assignee deleted (Luke Kanies)
  • Priority changed from Normal to High

#3 Updated by James Turnbull over 3 years ago

  • Target version deleted (52)

#4 Updated by Anonymous over 3 years ago

After investigation and UX discussion Luke and I have a proposed path forward on this ticket:

checksum is changed to mean “the desired checksum of the file content”, only. It can have the form “{TYPE}VALUE”, but if TYPE is omitted will have the value of the checksum_type attribute.

checksum_type is changed to mean “the type of checksum to use, if not otherwise specified”, and can be set from a sensible default, explicitly, or by giving a type in the checksum parameter.

content is changed to mean “the literal content of the file”, only.

Like source and content, checksum is an exclusive value where you can’t have more than one set; we enforce that the same way we do already. Possibly with the caveat that this makes it more desirable to fix the “foo => undef” is also not allowed bug.

Undetermined is what the correct behaviour is when checksum_type is explicitly set, but conflicts with the TYPE part of checksum, although leaning is to having that be an error.

A possible UI consideration is to add extra parameters for the checksums to support sha1 => "foo" and md5 => "foo" in addition / instead of the existing methods; this would be a little more exciting internally, but might be nicer for the user.

To implement this, we propose injecting into the compiler part of the code a hook point that we can add code to be run to “downgrade” a generated catalog based on the client version. So, when a 2.6 client connects, but doesn’t support this behaviour, we can transform the catalog to go to the less capable variant.

#5 Updated by donavan m over 3 years ago

Is the checksum => "{type}value" form one that really needs to be preserved? I don’t think I’ve ever seen it in the wild, and it’s a newish (under documented) feature IIRC. I’d rather explicitly use checksum => "VALUE", checksum_type => "md5" if we have the later parameter anyways. I don’t see much benefit to the TYPE => "VALUE" sugar. File is a param rich class already. There will be a sane default checksum_type (I assume). It’s simple to set a resource default in scope. Explicitly setting checksum_type only seems inferior in char count.

PS: Specifying file contents by checksum is awesome. I really hope this gets some more adoption in 2.7+.

#6 Updated by Ben Hughes over 2 years ago

  • Description updated (diff)
  • Status changed from Accepted to Unreviewed

#7 Updated by Luke Kanies over 2 years ago

  • Description updated (diff)
  • Status changed from Unreviewed to Accepted

#8 Updated by Erik Dalén about 2 years ago

A simple test case of this bug would be: puppet apply —execute “file { ‘/tmp/testfile’: content => ‘{foo}bar’}”

content can be anything that matches /{(\w{3,5})}\S+/

#9 Updated by Erik Dalén 8 months ago

Redmine Issue #3955 has been migrated to JIRA:

https://tickets.puppetlabs.com/browse/PUP-1043

Also available in: Atom PDF