(PUP-6114) Adds new HTTP headers for checksum validation.

[petems]

• This is primarily for the support of the artifactory HTTP headers for
  checksum validation. However, other headers could be configured in
  defaults for other providers

Resurrecting #4828, looking to implement the ideas from @MikaelSmith's previous comment:

These [artifactory headers] are non-standard, and could potentially be used for a different purpose on other websites. As such, I'm not comfortable hard-coding these headers as fallbacks.

However, this could be done as settings that can be configured in puppet.conf. See https://github.com/puppetlabs/puppet/blob/master/lib/puppet/defaults.rb#L825-L839 for an example of a setting that takes an array. My suggestion would be to introduce 3 settings - one each for custom md5, sha1, and sha256 headers - that can be an array of possible headers. Then #initialize would iterate through them to look for a valid checksum.

Might need some help on the defaults bit 😄

[puppetcla]

CLA signed by all contributors.

[adrienthebo]

Does artifactory require this specific header format? I'd be slightly more warm and fuzzy if we didn't have to insert a different header for every checksum type we support.

Also, I'm curious; what exactly are you doing with artifactory interfacing with the Puppet http file metadata endpoint?

[Iristyle]

I haven't seen the X-Checksum-MD5 header before - usually I've seen this done via ETag header, but I don't think the HTTP spec says that ETag must be MD5, hence the need for an X style extension here it looks like.

I think these are mutually exclusive though, correct? We can only serve one... which defaults to MD5 / is optionally configurable as SHA2 instead - see https://github.com/puppetlabs/puppet/blob/master/lib/puppet/defaults.rb#L867-L887

This code doesn't quite look right to me...

[MikaelSmith]

Why doesn't it look right? The file resource type can specify which checksum to use. However, this metadata endpoint has no knowledge of what's expected from the file type, so just appends everything.

[mafgh]

Regarding

However, this could be done as settings that can be configured in puppet.conf.

Something along those lines?

default.rb:

  define_settings(:agent,                                  
    :checksum_header_md5 => { 
        :default => [ 'X-Checksum-Md5' ],
        :type    => :array,    
        :desc    => 'fixme',
    },
    :checksum_header_sha256 => {
        :default => [ 'X-Checksum-Sha256' ],   
        :type    => :array,                                                        
        :desc    => 'fixme',                                                       
    },                                      

http_metadata.rb:

  Puppet.settings[:checksum_header_md5].each do |header|                         
     if checksum = http_response[header]
       Puppet.debug {"Using md5 header '#{header}'"} 
       @checksums[:md5] = "{md5}#{checksum}"
       break
     end
   end 
   
   Puppet.settings[:checksum_header_sha256].each do |header|
     if checksum = http_response[header]
       Puppet.debug {"Using sha256 header '#{header}'"}
       @checksums[:sha256] = "{sha256}#{checksum}"
       break
     end
   end 

[MikaelSmith]

Yes, that would work. I don't think it's a very good idea though, since it would apply to all HTTP requests (and different sites could use different headers). I guess an alternative would be to have a new parameter for the file type that lets you set that for a particular resource.

Update: On thinking about this more, my original settings description does seem to make some sense. And @mafgh's description looks about right.

[mafgh]

@petems do you still need help with the "defaults bit"?

[petems]

To be honest I don't have the bandwidth to complete this right now 😢

I'll close for now, if anyone wants to pick it up, possibly at #puppethack, then go for it! 😄

[tek0011]

Would love to see this implemented. Multiple vendors of our use artifactory for shares, chocolatey, builds, etc. We recently ran into a big headache where we realized files were not md5 checking, using artifactory and its X-CheckSum-xxx. As a side note, it also seems that mtime is also way off on numerous files vs. the artifactory (remote) mtime. Without this update, I will have to move all file resources that use artifactory, to a file uri. Which means having to move these to more of a file share type system.