Add support for comments in all types and providers

* ssh_config * sshd_config * sshd_config_match * sshd_config_subsystem Co-authored-by: Jason Grammenos <jason.grammenos@agilitypr.com>
voxpupuli · Jul 15, 2020 · 41bf271 · 41bf271
1 parent 1052668
commit 41bf271
Show file tree

Hide file tree

Showing 16 changed files with 406 additions and 46 deletions.
diff --git a/lib/puppet/provider/ssh_config/augeas.rb b/lib/puppet/provider/ssh_config/augeas.rb
@@ -104,6 +104,7 @@ def self.set_value(aug, base, path, label, value)
         end
         lastsp = aug.match("#{path}[last()]")[0]
       end
+      aug.defvar('resource', path)
     end
   end
 
@@ -114,6 +115,7 @@ def create
       # create base_path
       aug.set(base_path, resource[:host])
       self.class.set_value(aug, base_path, "#{base_path}/#{key}", key, resource[:value])
+      self.class.set_comment(aug, base_path, resource[:name], resource[:comment]) if resource[:comment]
     end
   end
 
@@ -129,4 +131,33 @@ def value=(value)
       self.class.set_value(aug, self.class.base_path(resource), resource_path, key, value)
     end
   end
+
+  def comment
+    base_path = self.class.base_path(resource)
+    augopen do |aug|
+      comment = aug.get("#{base_path}/#comment[following-sibling::*[1][label() =~ regexp('#{resource[:name]}', 'i')]][. =~ regexp('#{resource[:name]}:.*', 'i')]")
+      comment.sub!(/^#{resource[:name]}:\s*/i, "") if comment
+      comment || ""
+    end
+  end
+
+  def comment=(value)
+    base_path = self.class.base_path(resource)
+    augopen! do |aug|
+      self.class.set_comment(aug, base_path, resource[:name], value)
+    end
+  end
+
+  def self.set_comment(aug, base, name, value)
+    cmtnode = "#{base}/#comment[following-sibling::*[1][label() =~ regexp('#{name}', 'i')]][. =~ regexp('#{name}:.*', 'i')]"
+    if value.empty?
+      aug.rm(cmtnode)
+    else
+      if aug.match(cmtnode).empty?
+        aug.insert('$resource', "#comment", true)
+      end
+      aug.set("#{base}/#comment[following-sibling::*[1][label() =~ regexp('#{name}', 'i')]]",
+              "#{name}: #{value}")
+    end
+  end
 end
diff --git a/lib/puppet/provider/sshd_config/augeas.rb b/lib/puppet/provider/sshd_config/augeas.rb
@@ -176,15 +176,16 @@ def create
         aug.insert("#{base_path}/ListenAddress[1]", key, true)
       end
 
-     if key.downcase == 'listenaddress' && !aug.match("#{base_path}/AddressFamily").empty?
-       aug.insert("#{base_path}/AddressFamily", key, false)
-     end
+      if key.downcase == 'listenaddress' && !aug.match("#{base_path}/AddressFamily").empty?
+        aug.insert("#{base_path}/AddressFamily", key, false)
+      end
 
-     if key.downcase == 'addressfamily' && !aug.match("#{base_path}/ListenAddress").empty?
-       aug.insert("#{base_path}/ListenAddress", key, true)
-     end
+      if key.downcase == 'addressfamily' && !aug.match("#{base_path}/ListenAddress").empty?
+        aug.insert("#{base_path}/ListenAddress", key, true)
+      end
 
       self.class.set_value(aug, base_path, "#{base_path}/#{key}", key, resource[:value])
+      self.class.set_comment(aug, base_path, key, resource[:comment]) if resource[:comment]
     end
   end
 
@@ -207,4 +208,35 @@ def value=(value)
       self.class.set_value(aug, self.class.base_path(resource), resource_path, key, value)
     end
   end
+
+  def comment
+    base_path = self.class.base_path(resource)
+    key = resource[:key] ? resource[:key] : resource[:name]
+    augopen do |aug|
+      comment = aug.get("#{base_path}/#comment[following-sibling::*[1][label() =~ regexp('#{key}', 'i')]][. =~ regexp('#{key}:.*', 'i')]")
+      comment.sub!(/^#{key}:\s*/i, "") if comment
+      comment || ""
+    end
+  end
+
+  def comment=(value)
+    base_path = self.class.base_path(resource)
+    key = resource[:key] ? resource[:key] : resource[:name]
+    augopen! do |aug|
+      self.class.set_comment(aug, base_path, key, value)
+    end
+  end
+
+  def self.set_comment(aug, base, name, value)
+    cmtnode = "#{base}/#comment[following-sibling::*[1][label() =~ regexp('#{name}', 'i')]][. =~ regexp('#{name}:.*', 'i')]"
+    if value.empty?
+      aug.rm(cmtnode)
+    else
+      if aug.match(cmtnode).empty?
+        aug.insert("#{base}/#{name}", "#comment", true)
+      end
+      aug.set("#{base}/#comment[following-sibling::*[1][label() =~ regexp('#{name}', 'i')]]",
+              "#{name}: #{value}")
+    end
+  end
 end
diff --git a/lib/puppet/provider/sshd_config_match/augeas.rb b/lib/puppet/provider/sshd_config_match/augeas.rb
@@ -23,7 +23,8 @@ def self.static_path(resource)
   end
 
   def self.path(resource)
-    path = "$target/*[label()=~regexp('match', 'i') and *[label()=~regexp('condition', 'i') and count(*)=#{resource[:condition].keys.size}]"
+    path = "$target/*"
+    path += "[label()=~regexp('match', 'i') and *[label()=~regexp('condition', 'i') and count(*)=#{resource[:condition].keys.size}]"
     resource[:condition].each do |c, v|
       path += "[*[label()=~regexp('#{c}', 'i')]='#{v}']"
     end
@@ -104,6 +105,7 @@ def self.position!(aug, resource)
   def position!
     augopen! do |aug|
       self.class.position!(aug, resource)
+      self.comment = resource[:comment]
     end
   end
 
@@ -115,12 +117,35 @@ def create
         aug.set("$resource/Condition/#{c}", v)
       end
       aug.clear("$resource/Settings")
-      # At least one entry is mandatory (in the lens at least)
-      aug.set("$resource/Settings/#comment", 'Created by Puppet')
 
       self.class.position!(aug, resource) \
         if !self.class.in_position?(aug, resource) and resource[:position]
+
+          # At least one entry is mandatory (in the lens at least)
+          self.comment = resource[:comment]
+      end
+    end
+
+    def comment
+      augopen do |aug|
+        comment = aug.get("$resource/Settings/#comment[1]")
+        comment.sub!(/^#{resource[:name]}:\s*/i, "") if comment
+        comment || ""
+      end
     end
-  end
-end
 
+    def comment=(value)
+      augopen! do |aug|
+        cmtnode = "$resource/Settings/#comment[1]"
+
+        if aug.match(cmtnode).empty?
+          if aug.match("$resource/Settings/*").any?
+            # Insert before first entry
+            aug.insert("$resource/Settings/*[1]", "#comment", true)
+          end
+        end
+
+        aug.set(cmtnode, "#{resource[:name]}: #{resource[:comment]}")
+      end
+    end
+  end
diff --git a/lib/puppet/provider/sshd_config_subsystem/augeas.rb b/lib/puppet/provider/sshd_config_subsystem/augeas.rb
@@ -31,13 +31,16 @@ def self.instances
     end
   end
 
-  define_aug_method!(:create) do |aug, resource|
+  def create
     key = resource[:name]
-    unless aug.match("$target/Match").empty?
-      aug.insert("$target/Match[1]", "Subsystem", true)
-      aug.clear("$target/Subsystem[last()]/#{key}")
+    augopen! do |aug|
+      unless aug.match("$target/Match").empty?
+        aug.insert("$target/Match[1]", "Subsystem", true)
+        aug.clear("$target/Subsystem[last()]/#{key}")
+      end
+      aug.set("$target/Subsystem/#{resource[:name]}", resource[:command])
+      self.comment = resource[:comment] if resource[:comment]
     end
-    aug.set("$target/Subsystem/#{resource[:name]}", resource[:command])
   end
 
   define_aug_method!(:destroy) do |aug, resource|
@@ -46,4 +49,28 @@ def self.instances
   end
 
   attr_aug_accessor(:command, :label => :resource)
+
+
+  def comment
+    augopen do |aug|
+      comment = aug.get("$target/#comment[following-sibling::*[1][label() =~ regexp('Subsystem', 'i') and #{resource[:name]}]]")
+      comment.sub!(/^#{resource[:name]}:\s*/i, "") if comment
+      comment || ""
+    end
+  end
+
+  def comment=(value)
+    augopen! do |aug|
+      cmtnode = "$target/#comment[following-sibling::*[1][label() =~ regexp('Subsystem', 'i') and #{resource[:name]}]]"
+
+      if value.empty?
+        aug.rm(cmtnode)
+      else
+        if aug.match(cmtnode).empty?
+          aug.insert("$target/*[label()=~regexp('Subsystem', 'i') and #{resource[:name]}]", "#comment", true)
+        end
+        aug.set(cmtnode, "#{resource[:name]}: #{resource[:comment]}")
+      end
+    end
+  end
 end
diff --git a/lib/puppet/type/ssh_config.rb b/lib/puppet/type/ssh_config.rb
@@ -9,7 +9,7 @@
 The resource name is used for the setting name, but if the `host` is
 given, then the name can be something else and the `key` given as the name
 of the setting.
-"
+  "
 
   ensurable
 
@@ -50,6 +50,10 @@
     defaultto { '*' }
   end
 
+  newproperty(:comment) do
+    desc "Text to be stored in a comment immediately above the entry.  It will be automatically prepended with the name of the variable in order for the provider to know whether it controls the comment or not."
+  end
+
   autorequire(:file) do
     self[:target]
   end

diff --git a/lib/puppet/type/sshd_config.rb b/lib/puppet/type/sshd_config.rb
@@ -71,13 +71,13 @@ def sync
 
     def should_to_s(new_value)
       if provider.resource[:array_append]
-          # Merge the two arrays
-          is = @resource.property(:value).retrieve
-          is_arr = Array(is)
+        # Merge the two arrays
+        is = @resource.property(:value).retrieve
+        is_arr = Array(is)
 
-          super(is_arr | Array(new_value))
+        super(is_arr | Array(new_value))
       else
-          super(new_value)
+        super(new_value)
       end
     end
   end
@@ -117,7 +117,7 @@ def should_to_s(new_value)
 whitespace.  This is used if the `Match` block has multiple criteria.
 
     condition => 'Host example.net User root'
-      "
+    "
 
     munge do |value|
       if value.is_a? Hash
@@ -130,6 +130,10 @@ def should_to_s(new_value)
     end
   end
 
+  newproperty(:comment) do
+    desc "Text to be stored in a comment immediately above the entry.  It will be automatically prepended with the name of the variable in order for the provider to know whether it controls the comment or not."
+  end
+
   autorequire(:file) do
     self[:target]
   end

diff --git a/lib/puppet/type/sshd_config_match.rb b/lib/puppet/type/sshd_config_match.rb
@@ -84,6 +84,11 @@ def self.title_patterns
     end
   end
 
+  newproperty(:comment) do
+    desc "Text to be stored in a comment immediately above the entry.  It will be automatically prepended with the name of the variable in order for the provider to know whether it controls the comment or not."
+    defaultto { "created by Puppet" }
+  end
+
   autorequire(:file) do
     self[:target]
   end

diff --git a/lib/puppet/type/sshd_config_subsystem.rb b/lib/puppet/type/sshd_config_subsystem.rb
@@ -22,6 +22,10 @@
       `/etc/ssh/sshd_config`."
   end
 
+  newproperty(:comment) do
+    desc "Text to be stored in a comment immediately above the entry.  It will be automatically prepended with the name of the variable in order for the provider to know whether it controls the comment or not."
+  end
+
   autorequire(:file) do
     self[:target]
   end

diff --git a/spec/fixtures/unit/puppet/provider/ssh_config/augeas/full b/spec/fixtures/unit/puppet/provider/ssh_config/augeas/full
@@ -45,11 +45,13 @@ Host *
 #   Tunnel no
 #   TunnelDevice any:any
 #   PermitLocalCommand no
+#   This is a comment
 #   VisualHostKey no
 #   ProxyCommand ssh -q -W %h:%p gateway.example.com
 #   RekeyLimit 1G 1h
     SendEnv LANG LC_*
     SendEnv QUX
+    # HashKnownHosts: more secure
     HashKnownHosts yes
     GSSAPIAuthentication yes
     GSSAPIDelegateCredentials no
diff --git a/spec/fixtures/unit/puppet/provider/sshd_config/augeas/full b/spec/fixtures/unit/puppet/provider/sshd_config/augeas/full
@@ -29,7 +29,7 @@ ListenAddress ::
 
 # Logging
 # obsoletes QuietMode and FascistLogging
-#SyslogFacility AUTH
+#SyslogFacility: AUTHPRIV
 SyslogFacility AUTHPRIV
 #LogLevel INFO
 
@@ -83,8 +83,8 @@ GSSAPICleanupCredentials yes
 #GSSAPIStrictAcceptorCheck yes
 #GSSAPIKeyExchange no
 
-# Set this to 'yes' to enable PAM authentication, account processing, 
-# and session processing. If this is enabled, PAM authentication will 
+# Set this to 'yes' to enable PAM authentication, account processing,
+# and session processing. If this is enabled, PAM authentication will
 # be allowed through the ChallengeResponseAuthentication and
 # PasswordAuthentication.  Depending on your PAM configuration,
 # PAM authentication via ChallengeResponseAuthentication may bypass
@@ -101,6 +101,8 @@ AcceptEnv LC_PAPER LC_NAME LC_ADDRESS LC_TELEPHONE LC_MEASUREMENT
 AcceptEnv LC_IDENTIFICATION LC_ALL LANGUAGE
 AcceptEnv XMODIFIERS
 
+#   This is a comment
+#   VisualHostKey no
 #AllowAgentForwarding yes
 #AllowTcpForwarding yes
 #GatewayPorts no

diff --git a/spec/fixtures/unit/puppet/provider/sshd_config_match/augeas/full b/spec/fixtures/unit/puppet/provider/sshd_config_match/augeas/full
@@ -83,8 +83,8 @@ GSSAPICleanupCredentials yes
 #GSSAPIStrictAcceptorCheck yes
 #GSSAPIKeyExchange no
 
-# Set this to 'yes' to enable PAM authentication, account processing, 
-# and session processing. If this is enabled, PAM authentication will 
+# Set this to 'yes' to enable PAM authentication, account processing,
+# and session processing. If this is enabled, PAM authentication will
 # be allowed through the ChallengeResponseAuthentication and
 # PasswordAuthentication.  Depending on your PAM configuration,
 # PAM authentication via ChallengeResponseAuthentication may bypass
@@ -101,6 +101,8 @@ AcceptEnv LC_PAPER LC_NAME LC_ADDRESS LC_TELEPHONE LC_MEASUREMENT
 AcceptEnv LC_IDENTIFICATION LC_ALL LANGUAGE
 AcceptEnv XMODIFIERS
 
+#   This is a comment
+#   VisualHostKey no
 #AllowAgentForwarding yes
 #AllowTcpForwarding yes
 #GatewayPorts no
@@ -130,7 +132,7 @@ X11Forwarding yes
 # override default of no subsystems
 Subsystem	sftp	/usr/libexec/openssh/sftp-server
 
-# Example of overriding settings on a per-user basis
+# User anoncvs: Example of overriding settings on a per-user basis
 Match User anoncvs
 	X11Forwarding no
 	AllowTcpForwarding no

diff --git a/spec/fixtures/unit/puppet/provider/sshd_config_subsystem/augeas/full b/spec/fixtures/unit/puppet/provider/sshd_config_subsystem/augeas/full
@@ -102,6 +102,8 @@ AcceptEnv LC_PAPER LC_NAME LC_ADDRESS LC_TELEPHONE LC_MEASUREMENT
 AcceptEnv LC_IDENTIFICATION LC_ALL LANGUAGE
 AcceptEnv XMODIFIERS
 
+#   This is a comment
+#   VisualHostKey no
 #AllowAgentForwarding yes
 #AllowTcpForwarding yes
 #GatewayPorts no