firewalld returns a dictionary rather than a string in the ret['comment']

[MrFishFinger]

When using firewalld state module in an SLS file, the highstate output returns a dictionary - see sample output below. Saltstack version is 2015.8.0 (Beryllium) on both master and minion.

According to a discussion on the #salt IRC channel, contributor @babilen indicated this is an issue because firewalld returns a dictionary rather than a string in the ret['comment'] which probably causes the whole datastructure to be rendered as a dictionary

[root@server salt]# salt '*' state.highstate
              ----------
              qvie-gitlab1.qvie.qitasc.com:
                  ----------
                  firewalld_|-firewallconfig_|-public_|-present:
                      ----------
                      __run_num__:
                          0
                      changes:
                          ----------
                          icmp_blocks:
                          port_fwd:
                          ports:
                          services:
                      comment:
                          ----------
                          icmp_blocks:
                          port_fwd:
                          ports:
                              - `80/tcp` already exists
                          public:
                              `public` zone already exists
                          services:
                      duration:
                          669.466
                      name:
                          public
                      result:
                          True
                      start_time:
                          12:14:44.083708

[wwentland]

This is almost certainly due to the fact that firewalld.present uses a dictionary as comment in the data it returns (cf. https://github.com/saltstack/salt/blob/develop/salt/states/firewalld.py#L72 )

[jfindlay]

@MrFishFinger, @babilen, thanks for the report. According to the documentation the comment should be a string.

[rallytime]

@MrFishFinger and @babilen - I've reworked the firewalld state in #28181 to be more stateful. This not only resolves the dict vs str issue in the comment output, but it incorporates the old/new dictionary paradigm found in most states.

Can you guys give it a try and let me know if you find any issues?

[rallytime]

I am going to close this issue since we didn't hear back and the original report is resolved. If you run into any problems with this change, please leave a comment and I can take another look. Or, feel free to open a new issue.