[BUG] Highstate still runs explicitly disabled state files?

[evenyougreg]

Description

I am trying to disable a specific state file from ever being ran with salt minion state.disable dont_run, but when I run a highstate against the same minion, the state file still runs without hesitation. It only doesn't run if I, again, call it out with salt minion state.apply dont_run, I'll get:

minion:
    Data failed to compile:
----------
    The state file "dont_run" is currently disabled, to re-enable, run state.enable dont_run.

This doesn't really make sense to me, and I can't imagine a scenario where this would be needed. If I explicitly mark a state file to be ignored it shouldn't be suddenly ok to run during a highstate.

Expected behavior

I would expect that during the highstate, the file I called out to be disabled would show up in the output as a successfully ignored state file, OR, the state file is omitted entirely in the output.

It should also not ever show up as an error, since there is no error. "Data failed to compile" is not something I would want to read when it is working as designed.

Versions Report

salt --versions-report

master

Salt Version:
           Salt: 3000.3

Dependency Versions:
           cffi: 1.14.0
       cherrypy: 5.6.0
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.11.1
        libgit2: 0.28.1
       M2Crypto: 0.35.2
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.6.2
   mysql-python: Not Installed
      pycparser: 2.20
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: 0.28.0
         Python: 3.6.8 (default, Sep 26 2019, 11:57:09)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 15.3.0
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.1.4

System Versions:
           dist: redhat 7.8 Maipo
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-1062.9.1.el7.x86_64
         system: Linux
        version: Red Hat Enterprise Linux Server 7.8 Maipo

minion

Salt Version:
           Salt: 2018.3.4

Dependency Versions:
           cffi: 1.10.0
       cherrypy: 10.2.1
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: 2.0.5
      gitpython: 2.1.3
          ioflo: Not Installed
         Jinja2: 2.9.6
        libgit2: Not Installed
        libnacl: 1.6.1
       M2Crypto: Not Installed
           Mako: 1.0.6
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: 2.17
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.5.3 (v3.5.3:1880cb95a742, Jan 16 2017, 16:02:32) [MSC v.1900 64 bit (AMD64)]
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.3
           RAET: Not Installed
          smmap: 2.0.5
        timelib: 0.2.4
        Tornado: 4.5.1
            ZMQ: 4.1.6

System Versions:
           dist:
         locale: cp1252
        machine: AMD64
        release: 2012ServerR2
         system: Windows
        version: 2012ServerR2 6.3.9600 SP0 Multiprocessor Free

[DmitryKuzmenko]

@evenyougreg thank you for report!
I'm confirming this bug. The state compiler doesn't match the disabled state by name. Only by module function. Meanwhile the state module matches disabled states by any name the state could be matched by state.apply: state file name, state name or state function name. This difference must be fixed.

[scornelissen85]

What should be the preferred option for this? I can imagine that all three options should be possible to use. It's still the case as @DmitryKuzmenko noted; with state.apply and the given state is listed, it won't apply the state (the specified SLS-file). However in a highstate the list of disabled states is matched against the state function name like file.managed so that it's able to disable to use this state function.

See this code for matching against state function name from within a highstate: https://github.com/saltstack/salt/blob/v3006.3/salt/state.py#L2666

And this code will check if the state filename is matched as disabled, but won't be hit when running a highstate: https://github.com/saltstack/salt/blob/v3006.3/salt/modules/state.py#L1361

In my case I would be able to disable a state by state file name from running. Also it should not return an error as now return is set to False which indicate an error.