ConfigBanner option seems to add data at end that causes the sshd_config file not to parse

[brianholland99]

I played with some options here and ConfigBanner seems to cause an issue when I use it.

Pillar data:

sshd_config:
  ConfigBanner: |
    # This is a test of the
    # ConfigBanner pillar setting.

This results in the following:

Beginning of file (as expected):

# This is a test of the
# ConfigBanner pillar setting.

End of file (not expected and causes the parse issue):

ConfigBanner # This is a test of the
# ConfigBanner pillar setting.

This appears to be due to the code in the sshd_config Jinja file that attempts to write out all options not handled by the time it reaches the end of the file. I was able to fix this by adding ConfigBanner to the processed_options array. This could be done blindly at the beginning or when handling ConfgBanner if it exists. I tried the second approach and tested that. I did not bother making a pull request as it was not clear which would be preferred and if a comment would be desired (no comments elsewhere describing what processed_options is used for.

If it would help, I will make a pull request using either approach if I know which method is preferred and what comments might be desired.

Here is the one-line addition that worked for me:

diff --git a/openssh/files/sshd_config b/openssh/files/sshd_config
index 8038e4a..b9ea342 100644
--- a/openssh/files/sshd_config
+++ b/openssh/files/sshd_config
@@ -62,6 +62,7 @@
 {%- endmacro -%}

 {%- if sshd_config.get('ConfigBanner', False) -%}
+  {%- do processed_options.append('ConfigBanner') -%}
 {{ sshd_config['ConfigBanner'] }}
 {%- else -%}
 # This file is managed by salt. Manual changes risk being overwritten.

Both the initial issue and patch tests were done on a ubuntu/xenial VM from salt master on ubuntu/xenial.

Thanks, Brian

[aboe76]

Hi Brian,

Please make the pull request with your online addition,

[brianholland99]

Sorry for the delay. I finally got a chance to look at it and had long since messed up my environment for testing. I reinstalled my main system, pulled the latest code, tested that the option still failed, made the change, and verified it worked properly.

Pull request is #113 .

Indenting in the file was not totally consistent. I indented both clauses for the 'if' and the resulting file had the comments left justified as is normal for that file. Hopefully, that was acceptable.

Thanks, Brian

[alxwr]

Fixed in #113