Merge pull request #8840 from joshcooper/backport_user_ssh_11380

(PUP-11380) Backport user type changes from main

Author: ciprianbadescu

acceptance/tests/resource/user/should_correctly_ensure_depending_resources.rb

@@ -692,7 +692,7 @@ def delimiter
       defaultto false
     end
 
-    def eval_generate
+    def generate
       if !self[:purge_ssh_keys].empty?
         if Puppet::Type.type(:ssh_authorized_key).nil?
           warning _("Ssh_authorized_key type is not available. Cannot purge SSH keys.")
@@ -742,6 +742,45 @@ def eval_generate
         end
         raise ArgumentError, _("purge_ssh_keys must be true, false, or an array of file names, not %{value}") % { value: value.inspect }
       end
+
+      munge do |value|
+        # Resolve string, boolean and symbol forms of true and false to a
+        # single representation.
+        case value
+        when :false, false, "false"
+          []
+        when :true, true, "true"
+          home = homedir
+          home ? [ "#{home}/.ssh/authorized_keys" ] : []
+        else
+          # value can be a string or array - munge each value
+          [ value ].flatten.map do |entry|
+            authorized_keys_path(entry)
+          end.compact
+        end
+      end
+
+      private
+
+      def homedir
+        resource[:home] || Dir.home(resource[:name])
+      rescue ArgumentError
+        Puppet.debug("User '#{resource[:name]}' does not exist")
+        nil
+      end
+
+      def authorized_keys_path(entry)
+        return entry unless entry.match?(%r{^(?:~|%h)/})
+
+        # if user doesn't exist (yet), ignore nonexistent homedir
+        home = homedir
+        return nil unless home
+
+        # compiler freezes "value" so duplicate using a gsub, second mutating gsub! is then ok
+        entry = entry.gsub(%r{^~/}, "#{home}/")
+        entry.gsub!(%r{^%h/}, "#{home}/")
+        entry
+      end
     end
 
     newproperty(:loginclass, :required_features => :manages_loginclass) do
@@ -763,7 +802,7 @@ def eval_generate
     # @see generate
     # @api private
     def find_unmanaged_keys
-      munged_unmanaged_keys.
+      self[:purge_ssh_keys].
         select { |f| File.readable?(f) }.
         map { |f| unknown_keys_in_file(f) }.
         flatten.each do |res|
@@ -775,41 +814,6 @@ def find_unmanaged_keys
         end
     end
 
-    def munged_unmanaged_keys
-      value = self[:purge_ssh_keys]
-
-      # Resolve string, boolean and symbol forms of true and false to a
-      # single representation.
-      test_sym = value.to_s.intern
-      value = test_sym if [:true, :false].include? test_sym
-
-      return [] if value == :false
-
-      home = self[:home]
-      begin
-        home ||= provider.home
-      rescue
-        Puppet.debug("User '#{self[:name]}' does not exist")
-      end
-
-      if home.to_s.empty? || !Dir.exist?(home.to_s)
-        if value == :true || [ value ].flatten.any? { |v| v.start_with?('~/', '%h/') }
-          Puppet.debug("User '#{self[:name]}' has no home directory set to purge ssh keys from.")
-          return []
-        end
-      end
-
-      return [ "#{home}/.ssh/authorized_keys" ] if value == :true
-
-      # value is an array - munge each value
-      [ value ].flatten.map do |entry|
-        # make sure frozen value is duplicated by using a gsub, second mutating gsub! is then ok
-        entry = entry.gsub(/^~\//, "#{home}/")
-        entry.gsub!(/^%h\//, "#{home}/")
-        entry
-      end
-    end
-
     # Parse an ssh authorized keys file superficially, extract the comments
     # on the keys. These are considered names of possible ssh_authorized_keys
     # resources. Keys that are managed by the present catalog are ignored.

acceptance/tests/resource/user/should_manage_purge_ssh_keys_for_new_user.rb

@@ -479,6 +479,21 @@ class container {
                          "Notify[goodbye]"))
     end
 
+    it "sets resources_failed_to_generate to true if resource#generate raises an exception" do
+      catalog = compile_to_ral(<<-MANIFEST)
+        user { 'foo':
+          ensure => present,
+        }
+      MANIFEST
+
+      allow(catalog.resource("User[foo]")).to receive(:generate).and_raise(RuntimeError)
+      relationship_graph = relationship_graph_for(catalog)
+      generator = Puppet::Transaction::AdditionalResourceGenerator.new(catalog, relationship_graph, prioritizer)
+      generator.generate_additional_resources(catalog.resource("User[foo]"))
+
+      expect(generator.resources_failed_to_generate).to be_truthy
+    end
+
     def relationships_after_generating(manifest, resource_to_generate)
       catalog = compile_to_ral(manifest)
       generate_resources_in(catalog, nil, resource_to_generate)

lib/puppet/type/user.rb

@@ -174,51 +174,6 @@ def self.instances; []; end
     end
   end
 
-  describe "when managing the purge_ssh_keys property" do
-    context "with valid input" do
-      it "should support a :true value" do
-        expect { described_class.new(:name => 'foo', :purge_ssh_keys => :true) }.to_not raise_error
-      end
-
-      it "should support a :false value" do
-        expect { described_class.new(:name => 'foo', :purge_ssh_keys => :false) }.to_not raise_error
-      end
-
-      it "should support a String value" do
-        expect { described_class.new(:name => 'foo', :purge_ssh_keys => File.expand_path('home/foo/.ssh/authorized_keys')) }.to_not raise_error
-      end
-
-      it "should support an Array value" do
-        expect { described_class.new(:name => 'foo', :purge_ssh_keys => [File.expand_path('home/foo/.ssh/authorized_keys'),
-          File.expand_path('custom/authorized_keys')]) }.to_not raise_error
-      end
-    end
-
-    context "with faulty input" do
-      it "should raise error for relative path" do
-        expect { described_class.new(:name => 'foo', :purge_ssh_keys => 'home/foo/.ssh/authorized_keys') }.to raise_error(Puppet::ResourceError,
-          /Paths to keyfiles must be absolute/ )
-      end
-
-      it "should raise error for invalid type" do
-        expect { described_class.new(:name => 'foo', :purge_ssh_keys => :invalid) }.to raise_error(Puppet::ResourceError,
-          /purge_ssh_keys must be true, false, or an array of file names/ )
-      end
-
-      it "should raise error for array with relative path" do
-        expect { described_class.new(:name => 'foo', :purge_ssh_keys => ['home/foo/.ssh/authorized_keys',
-          File.expand_path('custom/authorized_keys')]) }.to raise_error(Puppet::ResourceError,
-          /Paths to keyfiles must be absolute/ )
-      end
-
-      it "should raise error for array with invalid type" do
-        expect { described_class.new(:name => 'foo', :purge_ssh_keys => [:invalid,
-          File.expand_path('custom/authorized_keys')]) }.to raise_error(Puppet::ResourceError,
-          /Each entry for purge_ssh_keys must be a string/ )
-      end
-    end
-  end
-
   describe "when managing the uid property" do
     it "should convert number-looking strings into actual numbers" do
       expect(described_class.new(:name => 'foo', :uid => '50')[:uid]).to eq(50)