Add capability to specify state args as a dict by bdrx312 · Pull Request #68368 · saltstack/salt

bdrx312 · 2025-10-04T02:50:26Z

Also add some more type hints and change some format calls to f strings. Fix state retry tests with invalid assumptions of duration and comment.

What does this PR do?

Adds the ability to specify arguments to a state as a dictionary instead of having to specify a list of individual key: value pairs

What issues does this PR fix or reference?

Fixes #68367

Previous Behavior

Could only specify arguments to a state like this

id1:
  cmd.run:
    - name: echo testing
    - umask: '0'

New Behavior

Can specify arguments to a state like this

id1:
  cmd.run:
    name: echo testing
    umask: '0'

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Docs
Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
Tests written/updated

Commits signed with GPG?

No

bdrx312 · 2025-10-31T18:28:16Z

I still think we should update the docs to show the new dict method in addition to the list method.

Alright I'll look at updating docs when I get some time.

lkubb

Thanks for creating this! Just some minor comments.

tests/pytests/functional/modules/state/test_state.py

salt/state.py

bdrx312 · 2025-11-11T22:01:53Z

I still think we should update the docs to show the new dict method in addition to the list method.

Alright I'll look at updating docs when I get some time.

@twangboy Alright I made a first pass at updating the documentation.

doc/ref/states/highstate.rst

bdrx312 · 2025-12-10T18:15:08Z

Build html is failing with the following:

Details

writing output... [  5%] faq .. ref/configuration/logging/index                                                                                                                              
writing output... [ 10%] ref/configuration/master .. ref/modules/all/salt.modules.bridge                                                                                                     
writing output... [ 15%] ref/modules/all/salt.modules.cassandra_cql .. ref/modules/all/salt.modules.linux_lvm                                                                                
writing output... [ 21%] ref/modules/all/salt.modules.linux_service .. ref/modules/all/salt.modules.pip                                                                                      
writing output... [ 26%] ref/modules/all/salt.modules.pkg .. ref/modules/all/salt.modules.supervisord                                                                                        
WARNING: unknown document: 'ref/states/requisites'

WARNING: unknown document: 'ref/states/extend'

writing output... [ 31%] ref/modules/all/salt.modules.sysctl .. ref/modules/all/salt.modules.x509                                                                                            
writing output... [ 36%] ref/modules/all/salt.modules.x509_v2 .. ref/returners/all/salt.returners.local                                                                                      
writing output... [ 42%] ref/returners/all/salt.returners.local_cache .. ref/states/all/salt.states.apache_conf                                                                              
writing output... [ 47%] ref/states/all/salt.states.apache_module .. ref/states/all/salt.states.postgres_group                                                                               
writing output... [ 52%] ref/states/all/salt.states.postgres_initdb .. ref/states/all/salt.states.x509                                                                                       
writing output... [ 57%] ref/states/all/salt.states.x509_v2 .. topics/cloud/aliyun                                                                                                           
writing output... [ 63%] topics/cloud/aws .. topics/development/conventions/documentation                                                                                                    
writing output... [ 68%] topics/development/conventions/formulas .. topics/pillar/index                                                                                                      
writing output... [ 73%] topics/projects/index .. topics/releases/2014.1.13                                                                                                                  
writing output... [ 78%] topics/releases/2014.1.2 .. topics/releases/2016.3.4                                                                                                                
writing output... [ 84%] topics/releases/2016.3.5 .. topics/releases/3003                                                                                                                    
writing output... [ 89%] topics/releases/3003.1 .. topics/salt_system_architecture                                                                                                           
writing output... [ 94%] topics/sdb/index .. topics/tutorials/rooted                                                                                                                         
writing output... [100%] topics/tutorials/salt_bootstrap .. topics/yaml/index                                                                                                                
generating indices... 
genindex 

http-routingtable 

py-modindex 

done

writing additional pages... 
404 

search 

done

copying images... [ 10%] _static/cloud-settings-inheritance.png                                                                                                                              
copying images... [ 20%] _static/external-job-cache.png                                                                                                                                      
copying images... [ 30%] _static/master-job-cache.png                                                                                                                                        
copying images... [ 40%] _static/napalm_logo.png                                                                                                                                             
copying images... [ 50%] _static/rest_status_screen.png                                                                                                                                      
copying images... [ 60%] _static/proxy_minions.png                                                                                                                                           
copying images... [ 70%] _static/salt-architecture.png                                                                                                                                       
copying images... [ 80%] _static/spm-overview.png                                                                                                                                            
copying images... [ 90%] _static/spm-package-contents.png                                                                                                                                    
copying images... [100%] _static/spm-package-extraction.png                                                                                                                                  
copying static files... 
done

copying extra files... done

dumping search index in English (code: en)... 
done
dumping object inventory... 
done

build finished with problems, 2 warnings.

[WARNING ] Unable to load range library

make: *** [Makefile:55: html] Error 1

[06:51:19] Command '('make', 'html', 'SPHINXOPTS=-W -j auto --keep-going --color')' returned non-zero exit status 2.                                                                          
Error: Process completed with exit code 2.

It appears it doesn't like my doc links

:doc:`ref/states/requisites`

https://github.com/saltstack/salt/pull/68368/files#diff-a0d563ea5979d3e412a93128f06d255fd9175f5ad9df06d193d6bcee9088af07R230 and

:doc:`ref/states/requisites`

https://github.com/saltstack/salt/pull/68368/files#diff-a0d563ea5979d3e412a93128f06d255fd9175f5ad9df06d193d6bcee9088af07R378

What is the correct way to link these documents?

bdrx312 · 2025-12-10T18:19:05Z

I still think we should update the docs to show the new dict method in addition to the list method.

@twangboy I updated the docs. Can you look over them and provide any feed back? I also have this question: #68368 (comment)

twangboy · 2025-12-11T16:07:32Z

What I meant by "add some docs" was to give some overall guidance on the different options for formatting the state, not going in to each state and changing the docs.
https://docs.saltproject.io/salt/user-guide/en/latest/topics/states.html

bdrx312 · 2025-12-11T19:56:07Z

What I meant by "add some docs" was to give some overall guidance on the different options for formatting the state, not going in to each state and changing the docs. https://docs.saltproject.io/salt/user-guide/en/latest/topics/states.html

I did update to give the different options https://github.com/saltstack/salt/pull/68368/files#diff-a0d563ea5979d3e412a93128f06d255fd9175f5ad9df06d193d6bcee9088af07R172-R201

I did not notice that page you linked; I will update that too. I also updated the examples because I think we want examples to show the recommended/ideal way to implement something. I find tools more difficult to use and frustrating when the tools improve over time and have better ways to configure and implement things but don't update the examples to use the newer methods.

As a unrelated side note but related to documentation, I started trying to on-board some team members so we can try to use salt and they found that user guide you linked, started at the beginning, and clicked through next on each page. When they got to the fourth page (State system behind the scenes https://docs.saltproject.io/salt/user-guide/en/latest/topics/state-system.html) they got a little overwhelmed by the information on that page and also didn't understand why they were already reading about what seemed like low level or implementation details. I think it would be good if that State system behind the scenes came much later in the user guide once the basics were covered.

bdrx312 · 2025-12-12T02:30:04Z

@twangboy Alright I submitted a pull request for the user guide repo saltstack/salt-user-guide#8.

Also add some more type hints and change some format calls to f strings. Fix state retry tests with invalid assumptions of duration and comment.

…nstead of append

whytewolf · 2026-02-06T22:08:28Z

one major problem with this change. since you changed all the tests you no longer are testing backwards comparability. which was my MAJOR problem with this change to begin with.

This change cannot and should not go through without VERY comprehensive tests that test the old style.

This change without those tests could break every single install out there.

lkubb · 2026-02-06T23:22:14Z

Thanks for catching and fixing an issue with my wrapper module and sorry for introducing it!

since you changed all the tests you no longer are testing backwards comparability

I don't think that's correct – I could only spot a single existing test (test_state_apply_parallel_spawning_with_global_dunders via _state_requires_env) that was adjusted from the old style to the new style, not sure why though.

Edit:

I still have to investigate the underlying issue, but this PR exhibiting a TypeError for the faulty format (which wasn't there before) could be a breaking change nonetheless.

bdrx312 · 2026-02-07T01:07:58Z

I could only spot a single existing test (test_state_apply_parallel_spawning_with_global_dunders via _state_requires_env) that was adjusted from the old style to the new style, not sure why though.

I think that was the first place when I testing where I made a syntax change in a test to see if it was working correctly. That test change can be reverted if desired.

but this PR exhibiting a TypeError for the faulty format

We can make the code just emit a warning log message and then continue by ignoring the invalid values instead of raising the exception which seems to be the existing behavior. Continuing when invalid values (syntax error) are specified does not seem like a great idea though since the user/developer was probably trying to specify some argument that will not be getting applied and will possibly configure their system incorrectly.

twangboy · 2026-02-11T19:17:19Z

I would say revert the test and add a new to so that both scenarios are tested.

bdrx312 · 2026-02-11T22:33:58Z

I would say revert the test and add a new to so that both scenarios are tested.

There are other tests now in tests/pytests/functional/modules/state/test_state.py that are testing the new syntax. I don't think that one specifically needs to test both.

whytewolf · 2026-02-11T23:30:02Z

I would say revert the test and add a new to so that both scenarios are tested.

There are other tests now in tests/pytests/functional/modules/state/test_state.py that are testing the new syntax. I don't think that one specifically needs to test both.

then revert the test. don't remove a test that was working fine for the old format.

edit: sorry looks like it already was reverted.

whytewolf · 2026-02-11T23:57:26Z

Thanks for catching and fixing an issue with my wrapper module and sorry for introducing it!

since you changed all the tests you no longer are testing backwards comparability

I don't think that's correct – I could only spot a single existing test (test_state_apply_parallel_spawning_with_global_dunders via _state_requires_env) that was adjusted from the old style to the new style, not sure why though.

You are correct. I saw that single change and saw RED. and just assumed they had gone through and changed everything. my anger had made me make a mistake. personally I am not a fan of this change. I hate this format with a passion.

I will say one test i would like to see. is a single test that takes all three possible formats and checks the lowstate output comes out the same.

testname:
  test:
    - nop
    - name: testing

testname:
  test.nop:
    - name: testing

testname:
  test.nop:
    name: testing

should all come back as

fun: nop
name: testing
order: 10000
state: test

Edit:

I still have to investigate the underlying issue, but this PR exhibiting a TypeError for the faulty format (which wasn't there before) could be a breaking change nonetheless.

and agreed on the TypeError. if it causes a TypeError here. it is possible that it could break something more that isn't being tested. fundamentally a format change should make no behavior changes.

whytewolf · 2026-02-11T23:59:45Z

doc/ref/states/extend.rst

+A standard way to extend is via the extend declaration. The extend
+declaration is a top level declaration like ``include`` that allows
+overriding or augmenting state declarations from other SLS files.
+Use ``extend`` to override arguments, append requisites,
+or otherwise modify an existing ID without editing the original SLS.
+
+Overview
+--------
+
+- ``extend`` is a top-level mapping at the same syntactic level as ``include`` or a top-level ID.
+- The SLS module that defines the target ID must be included so the ID exists before the
+  extend merge is applied.
+- Requisite lists (for example ``watch`` and ``require``) are appended; most other keys
+  are replaced by the extend entry.
+- Only one top-level ``extend`` mapping may appear in a single SLS file; later mappings
+  will overwrite earlier ones.
+
+Example
+-------
+
+The following shows the original SLS entries (the files being extended) and an extending SLS
+that includes them and declares a single ``extend`` block.
+
+Original: ``salt://http/init.sls``
+
+.. code-block:: yaml
+
+    apache:
+      pkg.installed: []
+      file:
+        - name: /etc/httpd/conf/httpd.conf
+        - source: salt://http/httpd.conf
+      service.running:
+        - name: httpd
+        - watch:
+          - file: apache
+
+Original: ``salt://ssh/init.sls``
+
+.. code-block:: yaml
+
+    ssh-server:
+      pkg.installed: []
+      service.running:
+        - name: sshd
+
+Extending SLS: ``salt://profile/webserver_extend.sls``


why is this document change that has nothing to do with the format change included? this should be a separate PR

whytewolf · 2026-02-12T00:00:33Z

doc/ref/states/extend.rst

+Behavior for this example
+-------------------------
+
+- The ``apache:file`` mapping in the extending SLS overrides with the
+  ``name`` and ``source`` values from the original ``file`` mapping
+  in ``http/init.sls`` with the values supplied under ``extend``.
+- The ``ssh-server:service:watch`` list is appended with ``file: /etc/ssh/banner``; any
+  existing watch entries declared in ``ssh/init.sls`` are preserved.
+- The banner resource is declared locally (``/etc/ssh/banner``) so the appended watch has
+  a concrete state to observe; if the resource were absent from the compiled data the
+  relationship would be invalid.
+
+Minimal patterns
+----------------
+
+Replace a mapping (overwrite):
+
+.. code-block:: yaml
+
+    extend:
+      apache:
+        file:
+          - name: /etc/httpd/conf/httpd.conf
+          - source: salt://http/httpd2.conf
+
+Append to a requisite list (merge):
+
+.. code-block:: yaml
+
+    extend:
+      ssh-server:
+        service:
+          - watch:
+            - file: /etc/ssh/banner


same as above. these document changes are not a part of the format change.

doc/ref/states/highstate.rst

doc/ref/states/requisites.rst

salt/client/ssh/wrapper/x509_v2.py

bdrx312

I will say one test i would like to see. is a single test that takes all three possible formats and checks the lowstate output comes out the same.

Good idea. Added

doc/ref/states/highstate.rst

twangboy · 2026-02-12T15:49:02Z

got some pre-commit failures

…tation to use new dict arguments format in examples Revert change to an existing test back to using the old arguments format Relax raising exception on invalid argument type in the state arguments to instead just log a warning and ignore the item

bdrx312 requested a review from a team as a code owner October 4, 2025 02:50

bdrx312 temporarily deployed to ci October 4, 2025 02:50 — with GitHub Actions Inactive

bdrx312 temporarily deployed to ci October 4, 2025 03:18 — with GitHub Actions Inactive

bdrx312 force-pushed the support_state_args_dict branch from b2a27d9 to a8b7362 Compare October 4, 2025 18:09

bdrx312 temporarily deployed to ci October 4, 2025 18:10 — with GitHub Actions Inactive

bdrx312 temporarily deployed to ci October 4, 2025 18:31 — with GitHub Actions Inactive

barneysowood added the test:full Run the full test suite label Oct 5, 2025

barneysowood temporarily deployed to ci October 5, 2025 09:46 — with GitHub Actions Inactive

barneysowood temporarily deployed to ci October 5, 2025 09:52 — with GitHub Actions Inactive

barneysowood temporarily deployed to ci October 5, 2025 09:57 — with GitHub Actions Inactive

lkubb reviewed Nov 3, 2025

View reviewed changes

twangboy requested changes Nov 13, 2025

View reviewed changes

doc/ref/states/highstate.rst Outdated Show resolved Hide resolved

twangboy previously approved these changes Dec 12, 2025

View reviewed changes

bdrx312 added 4 commits February 1, 2026 15:50

Add capability to specify state args as a dict

d5183ff

Also add some more type hints and change some format calls to f strings. Fix state retry tests with invalid assumptions of duration and comment.

Replace Union type hint in state.py with |

d175dbd

Update documentation

6271504

Fixed call in x509_v2.py certificate_managed_wrapper to call extend i…

bd0da1c

…nstead of append

whytewolf reviewed Feb 11, 2026

View reviewed changes

whytewolf reviewed Feb 12, 2026

View reviewed changes

doc/ref/states/highstate.rst Show resolved Hide resolved

whytewolf reviewed Feb 12, 2026

View reviewed changes

doc/ref/states/requisites.rst Show resolved Hide resolved

whytewolf reviewed Feb 12, 2026

View reviewed changes

salt/client/ssh/wrapper/x509_v2.py Show resolved Hide resolved

bdrx312 commented Feb 12, 2026

View reviewed changes

doc/ref/states/highstate.rst Show resolved Hide resolved

twangboy approved these changes Feb 17, 2026

View reviewed changes

Conversation

bdrx312 commented Oct 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

What issues does this PR fix or reference?

Previous Behavior

New Behavior

Merge requirements satisfied?

Commits signed with GPG?

Uh oh!

bdrx312 commented Oct 31, 2025

Uh oh!

lkubb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bdrx312 commented Nov 11, 2025

Uh oh!

Uh oh!

bdrx312 commented Dec 10, 2025

Uh oh!

bdrx312 commented Dec 10, 2025

Uh oh!

twangboy commented Dec 11, 2025

Uh oh!

bdrx312 commented Dec 11, 2025

Uh oh!

bdrx312 commented Dec 12, 2025

Uh oh!

whytewolf commented Feb 6, 2026

Uh oh!

lkubb commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bdrx312 commented Feb 7, 2026

Uh oh!

twangboy commented Feb 11, 2026

Uh oh!

bdrx312 commented Feb 11, 2026

Uh oh!

whytewolf commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

whytewolf commented Feb 11, 2026

Uh oh!

whytewolf Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

whytewolf Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bdrx312 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

twangboy commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bdrx312 commented Oct 4, 2025 •

edited

Loading

lkubb commented Feb 6, 2026 •

edited

Loading

whytewolf commented Feb 11, 2026 •

edited

Loading