saltstack-formulas / openssh-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
90 stars 297 forks source link

Test config before restarting service #143

Open OrangeDog opened 5 years ago

OrangeDog commented 5 years ago

To avoid rendering a server inaccessible, should include something like this (for both .config and .config_ini)

/usr/sbin/sshd -t:
  cmd.run:
    - onchanges:
      - file: sshd_config
    - require_in:
      - service: openssh
daks commented 5 years ago

Testing ssh config before restarting the service looks like a good idea to me. Do you think you can provide a pull request with the code?

alxwr commented 5 years ago

@OrangeDog @daks This has already been implemented in .config via salt.states.file.managed's check_cmd. https://github.com/saltstack-formulas/openssh-formula/blob/master/openssh/config.sls#L17 https://docs.saltstack.com/en/latest/ref/states/all/salt.states.file.html#salt.states.file.managed

OrangeDog commented 5 years ago

@alxwr cool, just needs implementing for config_ini then.

alxwr commented 5 years ago

@OrangeDog check_cmd uses a temporary file which contains the new config. Based on your last comment I guess you agree with my view on salt.states.file.managed's check_cmd. Is that correct?

@OrangeDog @aboe76 In my view .config_ini is a special case which only changes the way in which sshd_config gets its values. It was implemented in #124. I propose to extend .config with .config_ini by overwriting ID sshd_config. This way we get all the additional features (i.e. key mgmt) within .config plus INI format.

aboe76 commented 5 years ago

@alxwr nice Idea, can you create a PR then we can test this.

alxwr commented 5 years ago

@aboe76 I assigned the issue to myself, but I can't promise any completion date. :-) So if anyone wants to create a PR before, feel free to do so. (Just give me an update in this thread when you start. I'll do the same.)