fix(macros): fix `format_kwargs` macro

[sticky-note]

PR progress checklist (to be filled in by reviewers)

• [ ] Changes to documentation are appropriate (or tick if not required)
• [ ] Changes to tests are appropriate (or tick if not required)
• [ ] Reviews completed

---

What type of PR is this?

Primary type

• [ ] [build] Changes related to the build system
• [ ] [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
• [ ] [ci] Changes to the continuous integration configuration
• [ ] [feat] A new feature
• [x] [fix] A bug fix
• [ ] [perf] A code change that improves performance
• [ ] [refactor] A code change that neither fixes a bug nor adds a feature
• [ ] [revert] A change used to revert a previous commit
• [ ] [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

• [ ] [docs] Documentation changes
• [ ] [test] Adding missing or correcting existing tests

Does this PR introduce a BREAKING CHANGE?

No.

Related issues and/or pull requests

Quote kwargs values on format_kwargs macro to resolve sls rendering failure on special characters :

    Data failed to compile:
----------
    Rendering SLS 'base:postgres.manage' failed: found character '%' that cannot start any token; line 17
---
[...]
  postgres_user.present:
    - createdb: False
    - createroles: False
    - inherit: True
    - name: username
    - password: %0q`]O/)/d|v(Sj3-JR$HK8;    <======================
    - replication: False
    - user: postgres
    - onchanges:
      - test: postgres-reload-modules

[...]
---
ERROR: Minions returned with non-zero exit code

Describe the changes you're proposing

Pillar / config required to test the proposed changes

Debug log showing how the proposed changes work

Documentation checklist

• [ ] Updated the README (e.g. Available states).
• [ ] Updated pillar.example.

Testing checklist

• [ ] Included in Kitchen (i.e. under state_top).
• [ ] Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
• [ ] Updated the relevant test pillar.

Additional context

[myii]

@sticky-note Thanks for the PR. Just tracking back through the use of format_kwargs:

https://github.com/saltstack-formulas/postgres-formula/blob/3df38b1be2aa6181d1ed6158c93cb8a006454209/postgres/upstream.sls#L22-L23

• This should be fine.

https://github.com/saltstack-formulas/postgres-formula/blob/fbd8f65561a9078b39e74e0ba40c82e93a6ee871/postgres/macros.jinja#L13-L32

• This needs further looking in to, which follows below.

https://github.com/saltstack-formulas/postgres-formula/blob/cd6eb0be1669f6a370d1314238fbe7f6bfa3e2d4/postgres/manage.sls#L26-L30

• This is the part you're trying to fix.
• However, there are potential issues based on the expected data here.

https://github.com/saltstack-formulas/postgres-formula/blob/35850da22cb4f61144a61098b9869603b6e0a682/pillar.example#L135-L150

• There are boolean values being supplied here; converting these to string may have consequences when consumed by the Salt state and modules -- almost definitely best to avoid quoting these booleans.

---

@vutny Can I get your opinion about this?

[sticky-note]

@Myii hmmmm, you're right. Non-empty strings are evaluated as true. Is there a way to test the type of a value ?

[sticky-note]

Tested a salt-call state.show_sls postgres on this pillar:

postgres:
  users:
    test_dba:
      ensure: present
      password: '%=Hec$K8;'
      createdb: false
      createroles: false
      inherit: true
      replication: false

It seems that boolean strings are correctly computed in Booleans :

(...)
    postgres_user-test_dba:
        ----------
        __env__:
            base
        __sls__:
            postgres.manage
        postgres_user:
            |_
              ----------
              createdb:
                  False
            |_
              ----------
              createroles:
                  False
            |_
              ----------
              inherit:
                  True
            |_
              ----------
              name:
                  test_dba
            |_
              ----------
              password:
                  %=Hec$K8;
            |_
              ----------
              replication:
                  False
            |_
              ----------
              user:
                  postgres
            |_
              ----------
              onchanges:
                  |_
                    ----------
                    test:
                        postgres-reload-modules
            - present
            |_
              ----------
              order:
                  10011
(...)

I think it's because format_kwargs is never called directly but not sure at all

[pull-assistant[bot]]

Best reviewed: commit by commit

---

Optimal code review plan

     fix(macros): fix `format_kwargs` macro

Powered by Pull Assistant. Last update 5e6511b ... 5e6511b. Read the comment docs.

[sticky-note]

@myii @vutny Is that better like that ?

[sticky-note]

What about this PR @vutny @myii ? :)

[myii]

@sticky-note Did you test this out first?

[sticky-note]

@myii | quote doesn't work with : in strings for example. | yaml_dquote has better escaping power and encapsulate into " like I wanted

[myii]

Merged, thanks @sticky-note -- apologies for the delay.

[sticky-note]

@myii No problem, I didn't have so much time too ;)

[saltstack-formulas-travis]

:tada: This PR is included in version 0.41.2 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: