Skip to content
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

Indenting does not render properly #145

Closed
johnnybubonic opened this issue Jan 22, 2019 · 2 comments
Closed

Indenting does not render properly #145

johnnybubonic opened this issue Jan 22, 2019 · 2 comments
Assignees

Comments

@johnnybubonic
Copy link

The rendering for ssh_config needs a bit of an overhaul since it renders with some options as unindented under Host sections and it doesn't even render the MACs, KeyAlgorithms, or Ciphers options as correctly (they cannot be specified cumulatively on multiple lines, they must be joined with commas to a single string).

Additionally, ConfigBanner was getting added as an actual config directive rather than used internally.

I'd recommend fiddling with the spacing as well, as there's a lot of empty lines at the end of the file.

Here's a patch:

--- a/openssh/files/ssh_config	2019-01-22 02:24:32.858871892 -0500
+++ b/openssh/files/ssh_config	2019-01-22 18:39:32.701567269 -0500
@@ -1,7 +1,7 @@
 {%- import_yaml "openssh/defaults.yaml" as default_settings -%}
 {%- set ssh_config = salt['pillar.get']('ssh_config', default=default_settings.ssh_config, merge=True) -%}
 {#- present in ssh_config and known in actual file options -#}
-{%- set processed_options = [] -%}
+{%- set processed_options = ['ConfigBanner'] -%}
 {%- set string_or_list_options = ['KexAlgorithms', 'Ciphers', 'MACs'] -%}
 
 {%- macro render_raw_option(keyword, value) -%}
@@ -18,6 +18,24 @@
   {%- endif -%}
 {%- endmacro -%}
 
+{%- macro render_host_option(keyword, value) -%}
+{%- if value is sameas true %}
+    {{ keyword }} yes
+  {%- elif value is sameas false %}
+    {{ keyword }} no
+  {%- elif value is string or value is number %}
+    {{ keyword }} {{ value }}
+  {%- else -%}
+    {%- if keyword in string_or_list_options %}
+    {{ keyword }} {{ value|join(',') }}
+    {%- else %}
+    {%- for single_value in value -%}
+    {{ keyword }} {{ single_value }}
+    {%- endfor -%}
+    {%- endif -%}
+{%- endif -%}
+{%- endmacro -%}
+
 {#- generic renderer used for ssh matches, known options, -#}
 {#- and unknown options -#}
 {%- macro render_option(keyword, default, config_dict=ssh_config) -%}
@@ -85,14 +103,14 @@
 {%- do processed_options.append('Hosts') %}
 {%  for host, conf in ssh_config['Hosts'].items() %}
 Host {{ host }}
-  {%- for key, val in conf.items() %}
-    {{ render_raw_option(key, val) }}
+  {%- for key, val in conf.items() -%}
+    {{ render_host_option(key, val) }}
   {%- endfor %}
 {%- endfor %}
 {%- endif %}
 
 {# Handling unknown in salt template options #}
-{%- for keyword in ssh_config.keys() %}
+{% for keyword in ssh_config.keys() -%}
   {#- Matches have to be at the bottom and should be handled differently -#}
   {%- if not keyword in processed_options and keyword != 'matches' -%}
     {%- if not keyword in string_or_list_options -%}
@@ -103,17 +121,16 @@
 {{ option_string_or_list(keyword, '', True) }}
     {%- endif -%}
   {%- endif -%}
-{%- endfor %}
+{%- endfor -%}
 
 {# Handle matches last as they need to go at the bottom #}
-{%- if 'matches' in ssh_config %}
+{% if 'matches' in ssh_config -%}
   {%- for match in ssh_config['matches'].values() %}
 Match {{ match['type'].keys()[0] }} {{ match['type'].values()[0] }}
     {%- for keyword in match['options'].keys() %}
     {{ render_option(keyword, '', config_dict=match['options']) }}
     {%- endfor %}
   {%- endfor %}
-{%- endif %}
+{%- endif -%}
 
 {#- vim: set ft=jinja : #}
-
@alxwr
Copy link
Member

alxwr commented Feb 12, 2019

@johnnybubonic I created PR #149. Would you please test it on your setup?

@aboe76 aboe76 closed this as completed in 3f9876f Feb 12, 2019
@johnnybubonic
Copy link
Author

i'll have to get back to you on this, unfortunately; hopefully sometime next week - looks like it was merged already though, so fingers crossed! i'll reply back if i notice anything borked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants