commit 671107663166f18c102eb61f3f865186234e0cf0
Author: Lukas Zapletal <lzap+git@redhat.com>
Date:   Mon Mar 5 14:16:23 2018 +0100

    Fixes #21007 - new unattended action 'failed'

diff --git a/app/controllers/hosts_controller.rb b/app/controllers/hosts_controller.rb
index 5c14f3136..14088a569 100644
--- a/app/controllers/hosts_controller.rb
+++ b/app/controllers/hosts_controller.rb
@@ -33,7 +33,7 @@ class HostsController < ApplicationController
   before_action :find_resource, :only => [:show, :clone, :edit, :update, :destroy, :puppetrun, :review_before_build,
                                           :setBuild, :cancelBuild, :power, :get_power_state, :overview, :bmc, :vm,
                                           :runtime, :resources, :nics, :ipmi_boot, :console,
-                                          :toggle_manage, :pxe_config, :disassociate]
+                                          :toggle_manage, :pxe_config, :disassociate, :build_errors]
 
   before_action :taxonomy_scope, :only => [:new, :edit] + AJAX_REQUESTS
   before_action :set_host_type, :only => [:update]
@@ -255,6 +255,10 @@ class HostsController < ApplicationController
     end
   end
 
+  def build_errors
+    render :plain => @host.build_errors
+  end
+
   def power
     return invalid_request unless PowerManager::REAL_ACTIONS.include?(params[:power_action])
     @host.power.send(params[:power_action].to_sym)
@@ -647,7 +651,7 @@ class HostsController < ApplicationController
 
   define_action_permission [
     'clone', 'externalNodes', 'overview', 'bmc', 'vm', 'runtime', 'resources', 'templates', 'nics',
-    'pxe_config', 'active', 'errors', 'out_of_sync', 'pending', 'disabled', 'get_power_state', 'preview_host_collection'
+    'pxe_config', 'active', 'errors', 'out_of_sync', 'pending', 'disabled', 'get_power_state', 'preview_host_collection', 'build_errors'
   ], :view
   define_action_permission [
     'setBuild', 'cancelBuild', 'multiple_build', 'submit_multiple_build', 'review_before_build',
diff --git a/app/controllers/unattended_controller.rb b/app/controllers/unattended_controller.rb
index a092a0fa3..7fae0c622 100644
--- a/app/controllers/unattended_controller.rb
+++ b/app/controllers/unattended_controller.rb
@@ -6,10 +6,13 @@ class UnattendedController < ApplicationController
   # We dont require any of these methods for provisioning
   skip_before_action :require_login, :session_expiry, :update_activity_time, :set_taxonomy, :authorize, :unless => Proc.new { preview? }
 
+  # Allow HTTP POST methods without CSRF
+  skip_before_action :verify_authenticity_token
+
   before_action :set_admin_user, :unless => Proc.new { preview? }
   # We want to find out our requesting host
-  before_action :get_host_details, :except => [:hostgroup_template, :built]
-  before_action :get_built_host_details, :only => :built
+  before_action :get_host_details, :except => [:hostgroup_template, :built, :failed]
+  before_action :get_built_host_details, :only => [:built, :failed]
   before_action :allowed_to_install?, :except => :hostgroup_template
   before_action :handle_ca, :if => Proc.new { params[:kind] == 'provision' }
   before_action :handle_realm, :if => Proc.new { params[:kind] == 'provision' }
@@ -18,13 +21,27 @@ class UnattendedController < ApplicationController
   # all of our requests should be returned in text/plain
   after_action :set_content_type
 
-  # this actions is called by each operatingsystem post/finish script - it notify us that the OS installation is done.
+  # maximum size of built/failed request body accepted to prevent DoS (in bytes)
+  MAX_BUILT_BODY = 65535
+
   def built
-    logger.info "#{controller_name}: #{@host.name} is Built!"
+    logger.info "#{controller_name}: #{@host.name} is built!"
+    # clear possible previous errors
+    @host.build_errors = nil
     update_ip if Setting[:update_ip_from_built_request]
     head(@host.built ? :created : :conflict)
   end
 
+  def failed
+    return if preview? || !@host.build
+    logger.warn "#{controller_name}: #{@host.name} build failed!"
+    @host.build_errors = request.body.read(MAX_BUILT_BODY)
+    body_length = @host.build_errors.try(:size) || 0
+    @host.build_errors += "\n\nOutput trimmed\n" if body_length >= MAX_BUILT_BODY
+    logger.warn { "Log lines from the OS installer:\n#{@host.build_errors}" }
+    head(@host.built ? :created : :conflict)
+  end
+
   def hostgroup_template
     return head(:not_found) unless (params.has_key?("id") && params.has_key?(:hostgroup))
 
diff --git a/app/helpers/hosts_helper.rb b/app/helpers/hosts_helper.rb
index b1d854cdb..5b1420a23 100644
--- a/app/helpers/hosts_helper.rb
+++ b/app/helpers/hosts_helper.rb
@@ -250,6 +250,15 @@ module HostsHelper
     (SETTINGS[:unattended] && host.managed?) ? host.shortname : host.name
   end
 
+  def build_duration(host)
+    return _("N/A") if host.initiated_at.nil? && host.installed_at.nil?
+    if host.installed_at.nil?
+      time_ago_in_words(host.initiated_at, include_seconds: true) + " (in progress)"
+    else
+      distance_of_time_in_words(host.initiated_at, host.installed_at, include_seconds: true)
+    end
+  end
+
   def overview_fields(host)
     global_status = host.build_global_status
     fields = [
@@ -260,6 +269,9 @@ module HostsHelper
       ]
     ]
     fields += host_detailed_status_list(host)
+    fields += [[_("Build duration"), build_duration(host)]]
+    fields += [[_("Build errors"), link_to("Logs from OS installer", build_errors_host_path(:id => host.id))]] if host.build_errors.present?
+    fields += [[_("Token"), host.token || _("N/A")]] if User.current.admin?
     fields += [[_("Domain"), link_to(host.domain, hosts_path(:search => %{domain = "#{host.domain}"}))]] if host.domain.present?
     fields += [[_("Realm"), link_to(host.realm, hosts_path(:search => %{realm = "#{host.realm}"}))]] if host.realm.present?
     fields += [[_("IP Address"), host.ip]] if host.ip.present?
diff --git a/app/models/host/managed.rb b/app/models/host/managed.rb
index 6d20c71a5..a60a945f4 100644
--- a/app/models/host/managed.rb
+++ b/app/models/host/managed.rb
@@ -281,6 +281,7 @@ class Host::Managed < Host::Base
     return unless respond_to?(:old) && old && build? && !old.build?
     clear_facts
     clear_reports
+    self.build_errors = nil
   end
 
   # Called from the host build post install process to indicate that the base build has completed
@@ -424,7 +425,8 @@ class Host::Managed < Host::Base
   # Any facts are discarded
   def setBuild
     self.build = true
-    self.save
+    self.initiated_at = Time.now.utc
+    logger.warn("Set build failed: #{errors.inspect}") unless self.save
     errors.empty?
   end
 
diff --git a/app/models/host_status/build_status.rb b/app/models/host_status/build_status.rb
index ba8dc2cdc..d4cb2fa3d 100644
--- a/app/models/host_status/build_status.rb
+++ b/app/models/host_status/build_status.rb
@@ -2,6 +2,7 @@ module HostStatus
   class BuildStatus < Status
     PENDING = 1
     TOKEN_EXPIRED = 2
+    BUILD_FAILED = 3
     BUILT = 0
 
     def self.status_name
@@ -16,6 +17,8 @@ module HostStatus
           N_("Token expired")
         when BUILT
           N_("Installed")
+        when BUILD_FAILED
+          N_("Installation error")
         else
           N_("Unknown build status")
       end
@@ -23,7 +26,7 @@ module HostStatus
 
     def to_global(options = {})
       case to_status
-        when TOKEN_EXPIRED
+        when TOKEN_EXPIRED, BUILD_FAILED
           HostStatus::Global::ERROR
         else
           HostStatus::Global::OK
@@ -38,7 +41,11 @@ module HostStatus
           PENDING
         end
       else
-        BUILT
+        if build_errors?
+          BUILD_FAILED
+        else
+          BUILT
+        end
       end
     end
 
@@ -53,6 +60,10 @@ module HostStatus
     def token_expired?
       host&.token_expired?
     end
+
+    def build_errors?
+      host && host.build_errors.present?
+    end
   end
 end
 
diff --git a/app/registries/foreman/access_permissions.rb b/app/registries/foreman/access_permissions.rb
index 8aa88ffe2..2ae0d10d4 100644
--- a/app/registries/foreman/access_permissions.rb
+++ b/app/registries/foreman/access_permissions.rb
@@ -304,7 +304,7 @@ Foreman::AccessControl.map do |permission_set|
     tasks_ajax_actions = [:show]
 
     map.permission :view_hosts,    {:hosts => [:index, :show, :errors, :active, :out_of_sync, :disabled, :pending, :vm,
-                                               :externalNodes, :pxe_config, :auto_complete_search, :bmc,
+                                               :externalNodes, :pxe_config, :auto_complete_search, :bmc, :build_errors,
                                                :runtime, :resources, :templates, :overview, :nics, :get_power_state, :preview_host_collection],
                                     :dashboard => [:OutOfSync, :errors, :active],
                                     :unattended => [:host_template, :hostgroup_template],
diff --git a/config/routes.rb b/config/routes.rb
index deedb396e..263857403 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -23,6 +23,7 @@ Foreman::Application.routes.draw do
         get 'review_before_build'
         put 'setBuild'
         get 'cancelBuild'
+        get 'build_errors'
         get 'puppetrun'
         get 'pxe_config'
         put 'toggle_manage'
@@ -463,8 +464,11 @@ Foreman::Application.routes.draw do
   # get only for alterator unattended scripts
   get 'unattended/provision/:metadata', :controller => 'unattended', :action => 'host_template', :format => 'text',
     :constraints => { :metadata => /(autoinstall\.scm|vm-profile\.scm|pkg-groups\.tar)/ }
-  # get for end of build action
+  # built call can be done both via GET (for backward compatibility) and POST
   get 'unattended/built/(:id(:format))', :controller => 'unattended', :action => 'built', :format => 'text'
+  post 'unattended/built/(:id(:format))', :controller => 'unattended', :action => 'built', :format => 'text'
+  # failed call only via POST
+  post 'unattended/failed/(:id(:format))', :controller => 'unattended', :action => 'failed', :format => 'text'
   # get for all unattended scripts
   get 'unattended/(:kind/(:id(:format)))', :controller => 'unattended', :action => 'host_template', :format => 'text'
 
diff --git a/db/migrate/20180305111232_add_build_errors_to_hosts.rb b/db/migrate/20180305111232_add_build_errors_to_hosts.rb
new file mode 100644
index 000000000..841ab5cd3
--- /dev/null
+++ b/db/migrate/20180305111232_add_build_errors_to_hosts.rb
@@ -0,0 +1,6 @@
+class AddBuildErrorsToHosts < ActiveRecord::Migration[5.1]
+  def change
+    add_column :hosts, :initiated_at, :datetime
+    add_column :hosts, :build_errors, :text
+  end
+end
diff --git a/test/controllers/unattended_controller_test.rb b/test/controllers/unattended_controller_test.rb
index fa65a32ef..9ca80e7e6 100644
--- a/test/controllers/unattended_controller_test.rb
+++ b/test/controllers/unattended_controller_test.rb
@@ -288,12 +288,37 @@ class UnattendedControllerTest < ActionController::TestCase
       assert_response :success
     end
 
-    test "should accept built notifications" do
+    test "should accept built notifications via legacy GET method" do
       @request.env["REMOTE_ADDR"] = @ub_host.ip
       get :built
       assert_response :created
-      host = Nic::Base.primary.find_by_ip(@ub_host.ip)
-      assert_equal host.build, false
+      nic = Nic::Base.primary.find_by_ip(@ub_host.ip)
+      refute nic.build
+    end
+
+    test "should accept built notifications" do
+      @request.env["REMOTE_ADDR"] = @ub_host.ip
+      post :built
+      assert_response :created
+      nic = Nic::Base.primary.find_by_ip(@ub_host.ip)
+      refute nic.build
+    end
+
+    test "should accept failed notifications" do
+      @request.env["REMOTE_ADDR"] = @ub_host.ip
+      post :failed
+      assert_response :created
+      nic = Nic::Base.primary.find_by_ip(@ub_host.ip)
+      refute nic.build
+    end
+
+    test "should accept failed notifications with large body" do
+      @request.env["REMOTE_ADDR"] = @ub_host.ip
+      post :failed, body: (' ' * 65537)
+      assert_response :created
+      host = Nic::Base.primary.find_by_ip(@ub_host.ip).host
+      refute host.build
+      assert_match(/Output trimmed/, host.build_errors)
     end
 
     test "should accept built notifications_with_expired_token" do
diff --git a/test/unit/foreman/access_permissions_test.rb b/test/unit/foreman/access_permissions_test.rb
index 6c1f55ff3..2b6b6150f 100644
--- a/test/unit/foreman/access_permissions_test.rb
+++ b/test/unit/foreman/access_permissions_test.rb
@@ -15,8 +15,8 @@ class AccessPermissionsTest < ActiveSupport::TestCase
   MAY_SKIP_REQUIRE_LOGIN = [
     "users/login", "users/logout", "users/extlogin", "users/extlogout", "home/status", "notices/destroy",
 
-    # unattended built action is not for interactive use
-    "unattended/built",
+    # unattended built and failed action is not for interactive use
+    "unattended/built", "unattended/failed",
 
     # puppetmaster interfaces
     "fact_values/create", "reports/create",
diff --git a/test/unit/shared/access_permissions_test_base.rb b/test/unit/shared/access_permissions_test_base.rb
index dba4820ae..8bcde66d8 100644
--- a/test/unit/shared/access_permissions_test_base.rb
+++ b/test/unit/shared/access_permissions_test_base.rb
@@ -26,7 +26,7 @@ module AccessPermissionsTestBase
           # Pass if the controller deliberately skips login requirement
           next if controller < ApplicationController && filters.select { |f| f.filter == :require_login }.empty?
 
-          assert_not_empty Foreman::AccessControl.permissions.select { |p| p.actions.include? path }
+          assert_not_empty Foreman::AccessControl.permissions.select { |p| p.actions.include? path }, "permission for #{path} not found, check access_permissions.rb"
         end
       end
     end
