From 6ecd0342d9ab0e2464e58e5e9d58fb6742c67670 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Pierson?= Date: Thu, 21 Aug 2025 11:35:02 +0200 Subject: [PATCH 1/4] Allow file content that looks like a checksum. Currently, if a file has a content attribute set to something that looks like a checksum, it will display a deprecation warning and then probably throw an error, as the checksum-link string won't match anything in filebuckets. This code is apparently intended to allow a checksum to be passed instead of actual content, with the effect of replacing file content if it doesn't match the checksum. It appears that this mecanism is replaced by "static catalogs" and was scheduled for removal in Puppet 7. As it is no longer documented (because deprecated), it is surprising to stumble upon the behavior by just having file content that looks like a checksum. I had to work around this in a real usecase involving some proprietary software that uses the same syntax for value to be encrypted at startup. This commit introduces a new setting "use_checksum_in_file_content" that default to true, preserving current behavior. If set to false, it will never look for checksums in file contents. This setting should probably be set to false by default in the next major release. --- lib/puppet/defaults.rb | 7 +++++ lib/puppet/type/file/content.rb | 54 +++++++++++++++++++++++---------- 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb index 87a8b10c50..636fc03d74 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.rb @@ -1114,6 +1114,13 @@ def self.initialize_default_settings!(settings) # Sure would be nice to set the Puppet::Util::Log destination here in an :on_initialize_and_write hook, # unfortunately we have a large number of tests that rely on the logging not resetting itself when the # settings are initialized as they test what gets logged during settings initialization. + }, + :use_checksum_in_file_content => { + :default => true, + :type => :boolean, + :desc => "Whether to allow specifying checksums in file content attributes; this is + deprecated, the checksum retrieval functionality is being replaced by the use of + static catalogs." } ) diff --git a/lib/puppet/type/file/content.rb b/lib/puppet/type/file/content.rb index 24bce1c7aa..ca88d45763 100644 --- a/lib/puppet/type/file/content.rb +++ b/lib/puppet/type/file/content.rb @@ -48,23 +48,43 @@ module Puppet if value == :absent value elsif value.is_a?(String) && checksum?(value) - # XXX This is potentially dangerous because it means users can't write a file whose - # entire contents are a plain checksum unless it is a Binary content. - Puppet.puppet_deprecation_warning([ - # TRANSLATORS "content" is an attribute and should not be translated - _('Using a checksum in a file\'s "content" property is deprecated.'), - # TRANSLATORS "filebucket" is a resource type and should not be translated. The quoted occurrence of "content" is an attribute and should not be translated. - _('The ability to use a checksum to retrieve content from the filebucket using the "content" property will be removed in a future release.'), - # TRANSLATORS "content" is an attribute and should not be translated. - _('The literal value of the "content" property will be written to the file.'), - # TRANSLATORS "static catalogs" should not be translated. - _('The checksum retrieval functionality is being replaced by the use of static catalogs.'), - _('See https://puppet.com/docs/puppet/latest/static_catalogs.html for more information.') - ].join(" "), - :file => @resource.file, - :line => @resource.line) if !@actual_content && !resource.parameter(:source) - value + # Our argument looks like a checksum. Is it the value of the content + # attribute in Puppet code, that happens to look like a checksum, or is + # it an actual checksum computed on the actual content? + if @actual_content || resource.parameter(:source) + # Actual content is already set, value contains it's checksum + value + else + # The value passed in the "content" attribute of this file looks like a checksum. + if Puppet[:use_checksum_in_file_content] + # Assume user wants the deprecated behavior; display a warning though. + # XXX This is potentially dangerous because it means users can't write a file whose + # entire contents are a plain checksum unless it is a Binary content. + Puppet.puppet_deprecation_warning([ + # TRANSLATORS "content" is an attribute and should not be translated + _('Using a checksum in a file\'s "content" property is deprecated.'), + # TRANSLATORS "filebucket" is a resource type and should not be translated. The quoted occurrence of "content" is an attribute and should not be translated. + _('The ability to use a checksum to retrieve content from the filebucket using the "content" property will be removed in a future release.'), + # TRANSLATORS "content" is an attribute and should not be translated. + _('The literal value of the "content" property will be written to the file.'), + # TRANSLATORS "static catalogs" should not be translated. + _('The checksum retrieval functionality is being replaced by the use of static catalogs.'), + _('See https://puppet.com/docs/puppet/latest/static_catalogs.html for more information.') + ].join(" "), + :file => @resource.file, + :line => @resource.line) if !@actual_content && !resource.parameter(:source) + # We return the value assuming it really is the checksum of the + # actual content we want. It should be fetched from filebucket + # later on. + value + else + # The content only happens to look like a checksum by chance. + @actual_content = value.is_a?(Puppet::Pops::Types::PBinaryType::Binary) ? value.binary_buffer : value + resource.parameter(:checksum).sum(@actual_content) + end + end else + # Our argument is definitely not a checksum: set actual_value and return calculated checksum. @actual_content = value.is_a?(Puppet::Pops::Types::PBinaryType::Binary) ? value.binary_buffer : value resource.parameter(:checksum).sum(@actual_content) end @@ -163,6 +183,8 @@ def each_chunk_from end def content_is_really_a_checksum? + return false unless Puppet[:use_checksum_in_file_content] + checksum?(should) end From 9ef8d3ec9d10d535a5c341464415dd55e44b5588 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Pierson?= Date: Thu, 21 Aug 2025 17:18:20 +0200 Subject: [PATCH 2/4] Correct Rubocop offense about nested ifs This is a bit less nice because the nested if allowed another comment, but hey, Rubocop. --- lib/puppet/type/file/content.rb | 51 ++++++++++++++++----------------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/lib/puppet/type/file/content.rb b/lib/puppet/type/file/content.rb index ca88d45763..d0d96a039b 100644 --- a/lib/puppet/type/file/content.rb +++ b/lib/puppet/type/file/content.rb @@ -54,34 +54,31 @@ module Puppet if @actual_content || resource.parameter(:source) # Actual content is already set, value contains it's checksum value + elsif Puppet[:use_checksum_in_file_content] + # The value passed in the "content" attribute of this file looks like + # a checksum, and this is intended by the user. + # Display a warning as this behavior is deprecated. + Puppet.puppet_deprecation_warning([ + # TRANSLATORS "content" is an attribute and should not be translated + _('Using a checksum in a file\'s "content" property is deprecated.'), + # TRANSLATORS "filebucket" is a resource type and should not be translated. The quoted occurrence of "content" is an attribute and should not be translated. + _('The ability to use a checksum to retrieve content from the filebucket using the "content" property will be removed in a future release.'), + # TRANSLATORS "content" is an attribute and should not be translated. + _('The literal value of the "content" property will be written to the file.'), + # TRANSLATORS "static catalogs" should not be translated. + _('The checksum retrieval functionality is being replaced by the use of static catalogs.'), + _('See https://puppet.com/docs/puppet/latest/static_catalogs.html for more information.') + ].join(" "), + :file => @resource.file, + :line => @resource.line) if !@actual_content && !resource.parameter(:source) + # We return the value assuming it really is the checksum of the + # actual content we want. It should be fetched from filebucket + # later on. + value else - # The value passed in the "content" attribute of this file looks like a checksum. - if Puppet[:use_checksum_in_file_content] - # Assume user wants the deprecated behavior; display a warning though. - # XXX This is potentially dangerous because it means users can't write a file whose - # entire contents are a plain checksum unless it is a Binary content. - Puppet.puppet_deprecation_warning([ - # TRANSLATORS "content" is an attribute and should not be translated - _('Using a checksum in a file\'s "content" property is deprecated.'), - # TRANSLATORS "filebucket" is a resource type and should not be translated. The quoted occurrence of "content" is an attribute and should not be translated. - _('The ability to use a checksum to retrieve content from the filebucket using the "content" property will be removed in a future release.'), - # TRANSLATORS "content" is an attribute and should not be translated. - _('The literal value of the "content" property will be written to the file.'), - # TRANSLATORS "static catalogs" should not be translated. - _('The checksum retrieval functionality is being replaced by the use of static catalogs.'), - _('See https://puppet.com/docs/puppet/latest/static_catalogs.html for more information.') - ].join(" "), - :file => @resource.file, - :line => @resource.line) if !@actual_content && !resource.parameter(:source) - # We return the value assuming it really is the checksum of the - # actual content we want. It should be fetched from filebucket - # later on. - value - else - # The content only happens to look like a checksum by chance. - @actual_content = value.is_a?(Puppet::Pops::Types::PBinaryType::Binary) ? value.binary_buffer : value - resource.parameter(:checksum).sum(@actual_content) - end + # The content only happens to look like a checksum by chance. + @actual_content = value.is_a?(Puppet::Pops::Types::PBinaryType::Binary) ? value.binary_buffer : value + resource.parameter(:checksum).sum(@actual_content) end else # Our argument is definitely not a checksum: set actual_value and return calculated checksum. From 2e20fa4b6476b8d435c3c307b14e114e83f10040 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Pierson?= Date: Thu, 21 Aug 2025 17:29:45 +0200 Subject: [PATCH 3/4] Update configuration reference for use_checksum_in_file_content --- references/configuration.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/references/configuration.md b/references/configuration.md index 6733ebca2b..9bdeb2c306 100644 --- a/references/configuration.md +++ b/references/configuration.md @@ -1,6 +1,6 @@ --- layout: default -built_from_commit: 812d7420ea5d7e19e8003b26486a7c8847afdb25 +built_from_commit: 9ef8d3ec9d10d535a5c341464415dd55e44b5588 title: Configuration Reference toc: columns canonical: "/puppet/latest/configuration.html" @@ -8,7 +8,7 @@ canonical: "/puppet/latest/configuration.html" # Configuration Reference -> **NOTE:** This page was generated from the Puppet source code on 2024-10-18 17:22:26 +0000 +> **NOTE:** This page was generated from the Puppet source code on 2025-08-21 17:25:10 +0200 @@ -2143,6 +2143,14 @@ the beginning of the Puppet run. - *Default*: `false` +### use_checksum_in_file_content + +Whether to allow specifying checksums in file content attributes; this is +deprecated, the checksum retrieval functionality is being replaced by the use of +static catalogs. + +- *Default*: `true` + ### use_last_environment Puppet saves both the initial and converged environment in the last_run_summary file. From f8bdf9ae1d6718efdbf3ed2f5ab2348fa15bbcb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Pierson?= Date: Mon, 25 Aug 2025 14:53:38 +0200 Subject: [PATCH 4/4] Add spec test for new setting use_checksum_in_file_content --- spec/integration/type/file_spec.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spec/integration/type/file_spec.rb b/spec/integration/type/file_spec.rb index 68849d99a0..7501564d29 100644 --- a/spec/integration/type/file_spec.rb +++ b/spec/integration/type/file_spec.rb @@ -636,6 +636,15 @@ def get_aces_for_path_by_sid(path, sid) catalog.apply end + it "should not give a deprecation warning when checksum are disabled in content" do + Puppet[:use_checksum_in_file_content] = false + expect(Puppet).not_to receive(:puppet_deprecation_warning) + file = described_class.new(:path => path, :content => '{ABCD}X') + catalog.add_resource file + catalog.apply + expect(File.read(file[:path])).to eq('{ABCD}X') + end + with_digest_algorithms do it_should_behave_like "files are backed up", {} do let(:filebucket_digest) { method(:digest) }