New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
MrFishFinger opened this Issue Sep 28, 2015 · 4 comments

Comments

Projects
None yet
4 participants
@MrFishFinger

MrFishFinger commented Sep 28, 2015

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
@babilen

This comment has been minimized.

Show comment
Hide comment
@babilen

babilen Sep 28, 2015

Contributor

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 )

Contributor

babilen commented Sep 28, 2015

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 jfindlay added this to the Approved milestone Sep 28, 2015

@jfindlay

This comment has been minimized.

Show comment
Hide comment
@jfindlay

jfindlay Sep 28, 2015

Contributor

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

Contributor

jfindlay commented Sep 28, 2015

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

@rallytime

This comment has been minimized.

Show comment
Hide comment
@rallytime

rallytime Oct 21, 2015

Contributor

@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?

Contributor

rallytime commented Oct 21, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@rallytime

rallytime Nov 6, 2015

Contributor

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.

Contributor

rallytime commented Nov 6, 2015

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.

@rallytime rallytime closed this Nov 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment