From 7aeead4c9a74b3e600143baeafab4159e63a620a Mon Sep 17 00:00:00 2001
From: Trevor Vaughan <tvaughan@onyxpoint.com>
Date: Mon, 29 Jul 2019 18:43:35 -0400
Subject: [PATCH] Fix String value issues in grub_config

* Ensure that Boolean values are converted to Strings
* Ensure the String values have quotes around them if not already
  present

Fixes #44, #45
---
 lib/puppet/provider/grub_config/grub2.rb |  6 +++
 lib/puppet/type/grub_config.rb           | 12 +++++
 spec/acceptance/10_grub_config_spec.rb   | 56 +++++++++++++++++++++---
 3 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/lib/puppet/provider/grub_config/grub2.rb b/lib/puppet/provider/grub_config/grub2.rb
index a34bd4b..bafe45a 100644
--- a/lib/puppet/provider/grub_config/grub2.rb
+++ b/lib/puppet/provider/grub_config/grub2.rb
@@ -66,6 +66,12 @@ def value
   end
 
   def value=(newval)
+    if newval.is_a?(String)
+      unless %w[' "].include?(newval[0].chr)
+        newval = %Q("#{newval}")
+      end
+    end
+
     augopen! do |aug|
       aug.set("$target/#{resource[:name]}", newval)
     end
diff --git a/lib/puppet/type/grub_config.rb b/lib/puppet/type/grub_config.rb
index 4d20d53..c2571c8 100644
--- a/lib/puppet/type/grub_config.rb
+++ b/lib/puppet/type/grub_config.rb
@@ -45,6 +45,18 @@
     desc <<-EOM
       Value of the GRUB parameter.
     EOM
+
+    munge do |value|
+      value.to_s unless [Hash, Array].include?(value.class)
+    end
+
+    def insync?(is)
+      if is.is_a?(String) && should.is_a?(String)
+        is.gsub(/\A("|')|("|')\Z/,'') == should.gsub(/\A("|')|("|')\Z/,'')
+      else
+        is == should
+      end
+    end
   end
 
   autorequire(:file) do
diff --git a/spec/acceptance/10_grub_config_spec.rb b/spec/acceptance/10_grub_config_spec.rb
index aaf4c2f..94059cc 100644
--- a/spec/acceptance/10_grub_config_spec.rb
+++ b/spec/acceptance/10_grub_config_spec.rb
@@ -7,7 +7,7 @@
     context 'set timeout in grub' do
       let(:manifest) { %(
         grub_config { 'timeout':
-          value    => '1'
+          value => 1
         }
       )}
 
@@ -40,7 +40,7 @@
     context 'set fallback in grub' do
       let(:manifest) { %(
         grub_config { 'fallback':
-          value    => '0'
+          value => 0
         }
       )}
 
@@ -62,7 +62,7 @@
     context 'set timeout in grub2' do
       let(:manifest) { %(
         grub_config { 'GRUB_TIMEOUT':
-          value    => '1'
+          value => 1
         }
       )}
 
@@ -76,15 +76,15 @@
       end
 
       it 'should have a timeout of 1' do
-        on(host, %(grep "GRUB_TIMEOUT=1" /etc/default/grub))
-        on(host, %(grep "timeout=1" /boot/grub2/grub.cfg))
+        on(host, %(grep 'GRUB_TIMEOUT="1"' /etc/default/grub))
+        on(host, %(grep 'timeout=1' /boot/grub2/grub.cfg))
       end
     end
 
     context 'set arbitrary value in grub2' do
       let(:manifest) { %(
         grub_config { 'GRUB_FOOBAR':
-          value    => 'BAZ'
+          value => 'BAZ'
         }
       )}
 
@@ -98,7 +98,7 @@
       end
 
       it 'should have a GRUB_FOOBAR of BAZ' do
-        on(host, %(grep "GRUB_FOOBAR=BAZ" /etc/default/grub))
+        on(host, %(grep 'GRUB_FOOBAR="BAZ"' /etc/default/grub))
       end
     end
 
@@ -122,5 +122,47 @@
         on(host, %(grep "GRUB_FOOBAR" /etc/default/grub), :acceptable_exit_codes => [1])
       end
     end
+
+    context 'set Boolean value in grub2' do
+      let(:manifest) { %(
+        grub_config { 'GRUB_BOOLEAN_TEST':
+          value => true
+        }
+      )}
+
+      # Using puppet_apply as a helper
+      it 'should work with no errors' do
+        apply_manifest_on(host, manifest, :catch_failures => true)
+      end
+
+      it 'should be idempotent' do
+        apply_manifest_on(host, manifest, {:catch_changes => true})
+      end
+
+      it 'should have a GRUB_BOOLEAN_TEST that is true' do
+        on(host, %(grep 'GRUB_BOOLEAN_TEST="true"' /etc/default/grub))
+      end
+    end
+
+    context 'set a value with spaces in grub2' do
+      let(:manifest) { %(
+        grub_config { 'GRUB_SPACES_TEST':
+          value => 'this thing -has --spaces'
+        }
+      )}
+
+      # Using puppet_apply as a helper
+      it 'should work with no errors' do
+        apply_manifest_on(host, manifest, :catch_failures => true)
+      end
+
+      it 'should be idempotent' do
+        apply_manifest_on(host, manifest, {:catch_changes => true})
+      end
+
+      it 'should have a valid GRUB_SPACES_TEST entry' do
+        on(host, %(grep 'GRUB_SPACES_TEST="this thing -has --spaces"' /etc/default/grub))
+      end
+    end
   end
 end