Fix unnecessary file updates when HTTP source lacks checksum headers

[jorhett]

Short description

This is I believe a more minimal fix for #581 than what is proposed in #582.

Fixes #581
Replaces #582

Root Cause

The HttpMetadata class in http_metadata.rb was setting a default mtime checksum using Time.now when HTTP servers didn't provide checksum headers (Content-MD5, X-Checksum-*, ETag, or Last-Modified). This default now timestamp checksum changed on every run, triggering unnecessary updates.

Solution

1. Removed the problematic Time.now default in HttpMetadata#initialize
2. When the headers give no usable checksum, the HTTP file_metadata terminus now fetches the content and checksums it (HttpMetadata#collect_from_body). It uses the requested checksum type when that is a content digest, and Puppet[:digest_algorithm] otherwise (for example with checksum => mtime).
3. A HEAD that returns 404 or 500 still returns nil without fetching content, and the partial GET fallback for 403/405 is unchanged.
4. Updated the unit specs for the new behaviour and added an integration spec showing that a file whose content matches the source is no longer replaced.

Impact

• HTTP-sourced files without checksum headers now have stable checksums based on actual content
• Files are only updated when their content actually changes
• Performance: for servers without checksum headers, the content is fetched to compute the checksum, and fetched again only when it differs from the local file. Previously the content was fetched and the file rewritten on every run.

Checklist

I have:

• read the CONTRIBUTING.md document
• read and accepted the Developer Certificate of Origin document and added a Signed-off-by annotation to each of my commits
• read and accepted the AI Policy document and added Generated-by or Assisted-by annotations to each of my commits created with the help of an AI agent
• tested this code
• included documentation (including possible behaviour changes)
• documented the code
• added or modified regression test(s)
• added or modified unit test(s)

Testing

Tested with agent 8.29.0 against a file resource sourced from https://github.com/<user>.keys, which sends only a weak ETag. The stock agent rewrote the file on every run; with this change it was written once and left unchanged on the following runs. A local edit was still restored, and a 404 URL failed the same way as before.

Key considerations

I believe this to be a bug/regression and that this fix should be considered for a patch level update. I can find no documentation that says that files should be treated as updated NOW when a timestamp is unavailable, nor that files should always be replaced when timestamp or checksum metadata are unavailable.

I also believe that this would align with user's expectations, such that fixing it in a patch update would be acceptable.

Assisted-by: Cascade:SWE-1.6 (Devin Desktop)
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com

[bastelfreak]

Isn't the .freeze redundant because it's a constant?

[Sharpie]

No. The constant produces a warning if re-assigned to a different object, but the array object its self is mutable unless explicitly frozen:

https://docs.ruby-lang.org/en/master/language/frozen_objects_md.html#frozen-objects-in-ruby

[jorhett]

FWIW I'd personally be fine either way. I used freeze because it seemed to be used often in more recent changes.

[jorhett]

FWIW @Sharpie @bastelfreak I wrote the original code with just tab completion. I asked Claude to improve/create new tests. I reviewed and tested everything extensively. This is working on 10k+ nodes right now, which have wide usage of http through file, archive, and even some direct calls in private module Facts.

[Sharpie]

If I'm reading this correctly, it looks like we would now download the full content and compute the checksum.

So, still one downloaded copy when content has not changed on the file server, but two downloaded copies when the file has changed?

[jorhett]

If I'm reading this correctly, it looks like we would now download the full content and compute the checksum.

So, still one downloaded copy when content has not changed on the file server, but two downloaded copies when the file has changed?

Yes, the second GET comes from the File type's write path which always downloads the content itself. I fought myself on this one and leaned on "smaller patch, limited impact" but we can go the other way if you prefer: the patch would be larger because we have to add conditions to the file resource chunk_file_from_source() gets content to make use of the copy we've already downloaded.

EtA: perhaps we do the 2nd change to the file resource as a followup PR? 🤔

For some context, we never see these problems on large files. We see them on

• GitHub replies to queries about keys (files get checksums, seems to be only things they'd pull from a DB)
• Artifactory and other package repos when providing the signing key

So the cost of downloading twice (in every case that I've witnessed) is minimal.