[BUG] file.managed: source_hash ignored when contents_pillar or source=salt://... is used

[gluf-technik]

Description
When defining a file.managed state with both contents_pillar and source_hash set the later one gets ignored.

Building test states for this bugreport I found the same behavior with a salt://-source for the file.

Setup
Master and minions are (amd64) KVM guests, with the master and one of the minions running Debian 11. Other minions
are running CentOS 7 and Ubuntu 22.04 jammy. All of them are using Salt version 3005.1 from https://repo.saltproject.io/ which makes the Debian-based ones onedir-installations and the CentOS ones, well, not.
My current (test) setup uses ext_pillar with Vault (for TLS keys and such) and cmd_yaml (for assigning roles) but my examples below "work" without those.

• on-prem machine
• VM (Virtualbox, KVM, etc. please specify)
• VM running on a cloud service, please be explicit and add details
• container (Kubernetes, Docker, containerd, etc. please specify)
• or a combination, please be explicit
• jails if it is FreeBSD
• classic packaging
• onedir packaging
• used bootstrap to install

No much changed since #63785 ; )

Steps to Reproduce the behavior

• Create a file with some example text and generate its checksum:

  kvm-master:/srv/salt/states.git$ vim bugs/source_hash_ignored.sls
  kvm-master:/srv/salt/states.git$ sha256sum bugs/lorem_ipsum.txt | tee bugs/lorem_ipsum.sha256 
  56293a80e0394d252e995f2debccea8223e4b5b2b150bee212729b3b39ac4d46  bugs/lorem_ipsum.txt
  

• Add the same text to the minions' pillar:

  kvm-master:/srv/salt/pillar.git$ head -n 4 top.sls 
  {{ saltenv }}:
    '*':
      - lorem_ipsum
      - defaults.*
  kvm-master:/srv/salt/pillar.git$ cat lorem_ipsum.sls 
  lorem:
      ipsum: Lorem ipsum dolor sit amet, consectetur adipiscing elit, [...]
  

• Create states for files with one using the source provided by the master's fileserver and the other one using pillar as the source for the file's content:

  kvm-master:/srv/salt/states.git$ cat bugs/source_hash_ignored.sls 
  /tmp/lorem_ipsum_checked:
      file.managed:
        - source: salt://bugs/lorem_ipsum.txt
        - source_hash: 56293a80e0394d252e995f2debccea8223e4b5b2b150bee212729b3b39ac4d46
  
  /tmp/lorem_ipsum_unchecked:
      file.managed:
        - contents_pillar: lorem:ipsum
        - source_hash: 56293a80e0394d252e995f2debccea8223e4b5b2b150bee212729b3b39ac4d46 
  

• Run the states to make sure everything works fine

  kvm-master:~# export MINION="*minion*"                                                                 
  kvm-master:~# salt -C "$MINION" saltutil.refresh_pillar; salt -C "$MINION" state.sls bugs.source_hash_ignored
  u2204-kvm-minion:
    True          
  deb11-kvm-minion:                                                               
    [...]
  u2204-kvm-minion:
  ----------        
          ID: /tmp/lorem_ipsum_checked                                                                   
    Function: file.managed   
      Result: True    
     Comment: File /tmp/lorem_ipsum_checked is in the correct state
     Started: 11:56:54.883455
    Duration: 534.394 ms                                                                                 
     Changes:   
  ----------  
          ID: /tmp/lorem_ipsum_unchecked
    Function: file.managed
      Result: True     
     Comment: File /tmp/lorem_ipsum_unchecked is in the correct state
     Started: 11:56:55.417993             
    Duration: 2.668 ms
     Changes:                         
                                                    
  Summary for u2204-kvm-minion
  ------------                                                                                             
  Succeeded: 2
  [...]
  

• Mess up the checksum and run the states again:

  kvm-master:/srv/salt/states.git$ grep hash bugs/source_hash_ignored.sls
        - source_hash: 56111111122222222222233333333344444444455555555556766666666c4d46
        - source_hash: 56111111122222222222233333333344444444455555555556766666666c4d46
  

  And realized both ignore the source_hash so I've added this part:

  /tmp/lorem_ipsum_local_src:
    file.managed:
        - source_hash: 56111111122222222222233333333344444444455555555556766666666c4d46
  {% if 'salt-master' in pillar.get('roles') %}
        - source: file:///srv/salt/states.git/bugs/lorem_ipsum.txt
  {% else %}
        - source: file:///tmp/lorem_ipsum_checked
        - require:
            - file: /tmp/lorem_ipsum_checked
  {% endif %}
  

• So here the full output from the (minion running on the host with the) master:

  kvm-master:~# salt -C "*master*" state.sls bugs.source_hash_ignored
  kvm-master:
  ----------
            ID: /tmp/lorem_ipsum_checked
      Function: file.managed
        Result: True
       Comment: File /tmp/lorem_ipsum_checked is in the correct state
       Started: 14:01:00.388121
      Duration: 40.475 ms
       Changes:   
  ----------
            ID: /tmp/lorem_ipsum_unchecked
      Function: file.managed
        Result: True
       Comment: File /tmp/lorem_ipsum_unchecked is in the correct state
       Started: 14:01:00.428808
      Duration: 2.639 ms
       Changes:   
  ----------
            ID: /tmp/lorem_ipsum_local_src
      Function: file.managed
        Result: False
       Comment: Specified sha256 checksum for /tmp/lorem_ipsum_local_src (56111111122222222222233333333344444444455555555556766666666c4d46) does not match actual checksum (56293a80e0394d252e995f2debccea8223e4b5b2b150bee212729b3b39ac4d46)
       Started: 14:01:00.431629
      Duration: 1.533 ms
       Changes:   
  
  Summary for kvm-master
  ------------
  Succeeded: 2
  Failed:    1
  ------------
  Total states run:     3
  Total run time:  44.647 ms
  ERROR: Minions returned with non-zero exit code
  
  kvm-master:~# grep 56293a80e0394d252e995f2debccea8223e4b5b2b150bee212729b3b39ac4d46 /srv/salt/states.git/bugs/lorem_ipsum.sha256 
  56293a80e0394d252e995f2debccea8223e4b5b2b150bee212729b3b39ac4d46  bugs/lorem_ipsum.txt
  

Expected behavior
Files with contents taken from the minion's pillar and ones with content from a salt:// URL should have their checksum verified when source_hash is set.

Screenshots
If applicable, add screenshots to help explain your problem.

Versions Report

salt --versions-report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) A CentOS minion, an Ubuntu one and the one on the host running the master: ```yaml centos7-kvm-minion: Salt Version: Salt: 3005.1

Dependency Versions:         
          cffi: 1.9.1             
      cherrypy: Not Installed
      dateutil: Not Installed
     docker-py: Not Installed  
         gitdb: Not Installed
     gitpython: Not Installed               
        Jinja2: 2.11.1       
       libgit2: Not Installed
      M2Crypto: 0.35.2
          Mako: Not Installed
       msgpack: 0.6.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: 2.14
      pycrypto: Not Installed
  pycryptodome: Not Installed
        pygit2: Not Installed
        Python: 3.6.8 (default, Nov 16 2020, 16:55:22)
  python-gnupg: Not Installed
        PyYAML: 3.13
         PyZMQ: 18.0.1
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.1.4
 
System Versions:
          dist: centos 7 Core
        locale: UTF-8
       machine: x86_64
       release: 3.10.0-1160.80.1.el7.x86_64
        system: Linux
       version: CentOS Linux 7 Core

kvm-master:
Salt Version:
Salt: 3005.1

Dependency Versions:         
          cffi: 1.14.6
      cherrypy: 18.6.1
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed     
        Jinja2: 3.1.0
       libgit2: 1.5.0 
      M2Crypto: Not Installed  
          Mako: Not Installed
       msgpack: 1.0.2                       
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.9.8
        pygit2: 1.11.1
        Python: 3.9.16 (main, Jan  6 2023, 22:49:58)
  python-gnupg: 0.4.8
        PyYAML: 5.4.1
         PyZMQ: 23.2.0
         smmap: Not Installed
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4
 
System Versions:
          dist: debian 11 bullseye
        locale: utf-8
       machine: x86_64
       release: 5.10.0-21-amd64
        system: Linux
       version: Debian GNU/Linux 11 bullseye

u2204-kvm-minion:
Salt Version:
Salt: 3005.1

Dependency Versions:         
          cffi: 1.14.6
      cherrypy: 18.6.1
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed     
        Jinja2: 3.1.0
       libgit2: 1.5.0 
      M2Crypto: Not Installed  
          Mako: Not Installed
       msgpack: 1.0.2                       
  msgpack-pure: Not Installed
  mysql-python: Not Installed         
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.9.8
        pygit2: 1.11.1
        Python: 3.9.16 (main, Jan  6 2023, 22:50:07)
  python-gnupg: 0.4.8 
        PyYAML: 5.4.1
         PyZMQ: 23.2.0       
         smmap: Not Installed
       timelib: 0.2.4        
       Tornado: 4.5.3
           ZMQ: 4.3.4        
                             
System Versions:             
          dist: ubuntu 22.04 jammy
        locale: utf-8        
       machine: x86_64       
       release: 5.15.0-60-generic
        system: Linux        
       version: Ubuntu 22.04 jammy

</details>

**Additional context**
(Sorry if I've missed some existing issue related to this problem)

[gluf-technik]

A colleague just pointed out the option source_hash isn't required for anything but HTTP and FTP but even if the behavior I've demonstrated here is "works as intended" at least the documentation needs to be improved.

[OrangeDog]

The main purpose of it is to verify untrusted sources. salt:// and pillar come from Salt itself, so are already trusted.

The secondary purpose of change detection is also covered by Salt's refresh mechanisms.

[gluf-technik]

[...]
The secondary purpose of change detection is also covered by Salt's refresh mechanisms.

Sorry for taking so long... If not by checksum how does Salt detect differences in what might be a binary file?
I would have thought this to be the easiest way if not having something like rsync at hand but I got to admit I couldn't figure out the code in states/file.py / modules/file.py.

[OrangeDog]

For files served from Salt, it does the checksums itself.

[Ch3LL]

I'm going to close this as expected behavior.

It is also currently documented "If the file is hosted on an HTTP or FTP server, the source_hash argument is also required."

here: https://docs.saltproject.io/en/latest/ref/states/all/salt.states.file.html#salt.states.file.managed

[OrangeDog]

It doesn't say that it is ignored if not required.

[Ch3LL]

Good point. I'll re-open to make that more clear. Or the other approach could be to fail if http/ftp is not defined and source_hash is used.

[anilsil]

Closing as per #64227