Fixes #19555 - Add a setting to bypass template locks

.rubocop.yml

@@ -16,7 +16,7 @@ Metrics/MethodLength:
   Max: 20
 
 Metrics/LineLength:
-  Enabled: false 
+  Enabled: false
 
 Style/Next:
   Enabled: false
@@ -98,6 +98,9 @@ Metrics/BlockLength:
 Metrics/PerceivedComplexity:
   Max: 20
 
+Metrics/ParameterLists:
+  Max: 6
+
 Lint/UnusedMethodArgument:
   Enabled: false
 

app/controllers/api/v2/template_controller.rb

@@ -9,6 +9,7 @@ class TemplateController < ::Api::V2::BaseController
       param :filter, String, :required => false, :desc => N_("Import names matching this regex (case-insensitive; snippets are not filtered).")
       param :negate, :bool, :required => false, :desc => N_("Negate the prefix (for purging).")
       param :associate, String, :required => false, :desc => N_("Associate to OS's, Locations & Organizations. Options are: always, new or never.")
+      param :force, :bool, :required => false, :desc => N_("Update templates that are locked")
       def import
         results = ForemanTemplates::TemplateImporter.new({
           verbose:   params['verbose'],
@@ -19,6 +20,7 @@ def import
           filter:    params['filter'],
           associate: params['associate'],
           negate:    params['negate'],
+          force:     params['force']
         }).import!
         render :json => { :message => results }
       end

app/models/concerns/foreman_templates/provisioning_template_import.rb

@@ -3,9 +3,9 @@ module ProvisioningTemplateImport
     extend ActiveSupport::Concern
 
     module ClassMethods
-      def import!(name, text, metadata)
+      def import!(name, text, metadata, force = false)
         # Check for snippet type
-        return import_snippet!(name, text) if metadata['snippet'] || metadata['kind'] == 'snippet'
+        return import_snippet!(name, text, force) if metadata['snippet'] || metadata['kind'] == 'snippet'
 
         # Get template type
         kind = TemplateKind.find_by(name: metadata['kind'])
@@ -23,39 +23,71 @@ def import!(name, text, metadata)
         organizations = map_metadata(metadata, 'organizations')
 
         # Printout helpers
-        c_or_u = template.new_record? ? 'Created' : 'Updated'
-        id_string = ('id' + template.id) rescue ''
+        c_or_u = template.new_record? ? 'Creating' : 'Updating'
+        id_string = template.new_record? ? '' : "id #{template.id}"
+        if template.locked? && !template.new_record? && !force
+          return { :diff => nil,
+                   :status => false,
+                   :result => "Skipping Template #{id_string}:#{name} - template is locked" }
+        end
+
+        associate_metadata data, template, metadata, oses, organizations, locations
+
+        diff = nil
+        status = nil
+        if template_changed?(data, template)
+          diff = create_diff(data, template)
+          template.ignore_locking do
+            status = template.update_attributes(data)
+          end
+          result = build_associations_result c_or_u, id_string, name, oses, organizations, locations
+        else
+          status  = true
+          result  = "  No change to Template #{id_string}:#{name}"
+        end
 
+        { :diff => diff, :status => status, :result => result, :errors => template.errors }
+      end
+
+      def associate_metadata(data, template, metadata, oses, organizations, locations)
         if (metadata['associate'] == 'new' && template.new_record?) || (metadata['associate'] == 'always')
           data[:operatingsystem_ids] = oses.map(&:id)
           data[:location_ids]        = locations.map(&:id)
           data[:organization_ids]    = organizations.map(&:id)
         end
+        data
+      end
 
-        if data[:template] != template.template
-          diff = Diffy::Diff.new(
+      def build_associations_result(c_or_u, id_string, name, oses, organizations, locations)
+        res  = "  #{c_or_u} Template #{id_string}:#{name}"
+        res += "\n    Operatingsystem Associations:\n    - #{oses.map(&:fullname).join("\n    - ")}" unless oses.empty?
+        res += "\n    Organizations Associations:\n    - #{organizations.map(&:name).join("\n    - ")}" unless organizations.empty?
+        res += "\n    Location Associations:\n    - #{locations.map(&:name).join("\n    - ")}" unless locations.empty?
+        res
+      end
+
+      def create_diff(data, template)
+        if template_content_changed?(template.template, data[:template])
+          Diffy::Diff.new(
             template.template,
             data[:template],
             :include_diff_info => true
           ).to_s(:color)
-          status  = template.update_attributes(data)
-          result  = "  #{c_or_u} Template #{id_string}:#{name}"
-          result += "\n    Operatingsystem Associations:\n    - #{oses.map(&:fullname).join("\n    - ")}" unless oses.empty?
-          result += "\n    Organizations Associations:\n    - #{organizations.map(&:name).join("\n    - ")}" unless organizations.empty?
-          result += "\n    Location Associations:\n    - #{locations.map(&:name).join("\n    - ")}" unless locations.empty?
-        elsif data[:operatingsystem_ids] || data[:location_ids] || data[:organization_ids]
-          diff    = nil
-          status  = template.update_attributes(data)
-          result  = "  #{c_or_u} Template Associations #{id_string}:#{name}"
-          result += "\n    Operatingsystem Associations:\n    - #{oses.map(&:fullname).join("\n    - ")}" unless oses.empty?
-          result += "\n    Organizations Associations:\n    - #{organizations.map(&:name).join("\n    - ")}" unless organizations.empty?
-          result += "\n    Location Associations:\n    - #{locations.map(&:name).join("\n    - ")}" unless locations.empty?
         else
-          diff    = nil
-          status  = true
-          result  = "  No change to Template #{id_string}:#{name}"
+          nil
         end
-        { :diff => diff, :status => status, :result => result }
+      end
+
+      def template_content_changed?(template_template, data_template)
+        template_template != data_template
+      end
+
+      def associations_changed?(data)
+        !(data[:operatingsystem_ids] || data[:location_ids] || data[:organization_ids]).nil?
+      end
+
+      def template_changed?(data, template)
+        template_content_changed?(template.template, data[:template]) || associations_changed?(data)
       end
     end
   end

app/models/concerns/foreman_templates/ptable_import.rb

@@ -3,9 +3,9 @@ module PtableImport
     extend ActiveSupport::Concern
 
     module ClassMethods
-      def import!(name, text, metadata)
+      def import!(name, text, metadata, force = false)
         # Check for snippet type
-        return import_snippet!(name, text) if metadata['snippet']
+        return import_snippet!(name, text, force) if metadata['snippet']
 
         # Data
         ptable = Ptable.where(:name => name).first_or_initialize
@@ -17,42 +17,72 @@ def import!(name, text, metadata)
         organizations = map_metadata(metadata, 'organizations')
 
         # Printout helpers
-        c_or_u = ptable.new_record? ? 'Created' : 'Updated'
-        id_string = ('id' + ptable.id) rescue ''
+        c_or_u = ptable.new_record? ? 'Creating' : 'Updating'
+        id_string = ptable.new_record? ? '' : "id #{ptable.id}"
+        if ptable.locked? && !ptable.new_record? && !force
+          return { :diff => nil,
+                   :status => false,
+                   :result => "Skipping Partition Table #{id_string}:#{name} - partition table is locked" }
+        end
+
+        associate_metadata data, ptable, metadata, oses, organizations, locations
+
+        diff = nil
+        status = nil
+        if ptable_changed?(data, ptable)
+          diff = create_diff(data, ptable)
+          ptable.ignore_locking do
+            status = ptable.update_attributes(data)
+          end
+          result  = build_associations_result c_or_u, id_string, name, oses, organizations, locations
+        else
+          status  = true
+          result  = "  No change to Ptable #{id_string}:#{name}"
+        end
+        { :diff => diff, :status => status, :result => result, :errors => ptable.errors }
+      end
 
+      def associate_metadata(data, ptable, metadata, oses, organizations, locations)
         if (metadata['associate'] == 'new' && ptable.new_record?) || (metadata['associate'] == 'always')
           data[:operatingsystem_ids] = oses.map(&:id)
           data[:os_family]           = oses.map(&:family).uniq.first
           data[:location_ids]        = locations.map(&:id)
           data[:organization_ids]    = organizations.map(&:id)
         end
+        data
+      end
 
-        if data[:layout] != ptable.layout
-          diff = Diffy::Diff.new(
+      def ptable_content_changed?(data_layout, ptable_layout)
+        data_layout != ptable_layout
+      end
+
+      def associations_changed?(data)
+        !(data[:os_family] || data[:location_ids] || data[:organization_ids]).nil?
+      end
+
+      def create_diff(data, ptable)
+        if ptable_content_changed?(data[:layout], ptable.layout)
+          Diffy::Diff.new(
             ptable.layout,
             data[:layout],
             :include_diff_info => true
           ).to_s(:color)
-          status  = ptable.update_attributes(data)
-          result  = "  #{c_or_u} Ptable #{id_string}:#{name}"
-          result += "\n    Operatingsystem Family:\n    - #{oses.map(&:family).uniq.first}" unless oses.empty?
-          result += "\n    Operatingsystem Associations:\n    - #{oses.map(&:fullname).join("\n    - ")}" unless oses.empty?
-          result += "\n    Organizations Associations:\n    - #{organizations.map(&:name).join("\n    - ")}" unless organizations.empty?
-          result += "\n    Location Associations:\n    - #{locations.map(&:name).join("\n    - ")}" unless locations.empty?
-        elsif data[:os_family] || data[:location_ids] || data[:organization_ids]
-          diff    = nil
-          status  = ptable.update_attributes(data)
-          result  = "  #{c_or_u} Ptable Associations #{id_string}:#{name}"
-          result += "\n    Operatingsystem Family:\n    - #{oses.map(&:family).uniq.first}" unless oses.empty?
-          result += "\n    Operatingsystem Associations:\n    - #{oses.map(&:fullname).join("\n    - ")}" unless oses.empty?
-          result += "\n    Organizations Associations:\n    - #{organizations.map(&:name).join("\n    - ")}" unless organizations.empty?
-          result += "\n    Location Associations:\n    - #{locations.map(&:name).join("\n    - ")}" unless locations.empty?
         else
-          diff    = nil
-          status  = true
-          result  = "  No change to Ptable #{id_string}:#{name}"
+          nil
         end
-        { :diff => diff, :status => status, :result => result }
+      end
+
+      def build_associations_result(c_or_u, id_string, name, oses, organizations, locations)
+        res  = "  #{c_or_u} Ptable #{id_string}:#{name}"
+        res += "\n    Operatingsystem Family:\n    - #{oses.map(&:family).uniq.first}" unless oses.empty?
+        res += "\n    Operatingsystem Associations:\n    - #{oses.map(&:fullname).join("\n    - ")}" unless oses.empty?
+        res += "\n    Organizations Associations:\n    - #{organizations.map(&:name).join("\n    - ")}" unless organizations.empty?
+        res += "\n    Location Associations:\n    - #{locations.map(&:name).join("\n    - ")}" unless locations.empty?
+        res
+      end
+
+      def ptable_changed?(data, ptable)
+        ptable_content_changed?(data[:layout], ptable.layout) || associations_changed?(data)
       end
     end
   end

app/models/concerns/foreman_templates/template_import.rb

@@ -3,7 +3,7 @@ module TemplateImport
     extend ActiveSupport::Concern
 
     module ClassMethods
-      def import_snippet!(name, text)
+      def import_snippet!(name, text, force = false)
         # Data
         snippet = self.where(:name => name).first_or_initialize
         data = {
@@ -12,23 +12,32 @@ def import_snippet!(name, text)
         }
 
         # Printout helpers
-        c_or_u = snippet.new_record? ? 'Created' : 'Updated'
-        id_string = ('id' + snippet.id) rescue ''
+        c_or_u = snippet.new_record? ? 'Creating' : 'Updating'
+        id_string = snippet.new_record? ? '' : "id #{snippet.id}"
 
+        if snippet.locked? && !snippet.new_record? && !force
+          return { :diff => nil,
+                   :status => false,
+                   :result => "Skipping snippet #{id_string}:#{name} - template is locked" }
+        end
+
+        status = nil
         if data[:template] != snippet.template
           diff = Diffy::Diff.new(
             snippet.template,
             data[:template],
             :include_diff_info => true
           ).to_s(:color)
-          status  = snippet.update_attributes(data)
+          snippet.ignore_locking do
+            status = snippet.update_attributes(data)
+          end
           result  = "  #{c_or_u} Snippet #{id_string}:#{name}"
         else
           diff    = nil
           status  = true
           result  = "  No change to Snippet #{id_string}:#{name}"
         end
-        { :diff => diff, :status => status, :result => result }
+        { :diff => diff, :status => status, :result => result, :errors => snippet.errors }
       end
 
       def map_metadata(metadata, param)
@@ -40,11 +49,11 @@ def map_metadata(metadata, param)
             end.flatten.compact
           when 'locations'
             metadata[param].map do |loc|
-              Location.all.map { |db| db.name =~ /^#{loc}/ ? db : nil }
+              User.current.my_locations.map { |db| db.name =~ /^#{loc}/ ? db : nil }
             end.flatten.compact
           when 'organizations'
             metadata[param].map do |org|
-              Organization.all.map { |db| db.name =~ /^#{org}/ ? db : nil }
+              User.current.my_organizations.map { |db| db.name =~ /^#{org}/ ? db : nil }
             end.flatten.compact
           end
         else

app/models/setting/template_sync.rb

@@ -31,7 +31,8 @@ def self.load_defaults
           self.set('template_sync_repo', N_('Default Git repo to sync from'), 'https://github.com/theforeman/community-templates.git', N_('Repo')),
           self.set('template_sync_negate', N_('Negate the prefix (for purging) / filter (for importing/exporting)'), false, N_('Negate')),
           self.set('template_sync_branch', N_('Default branch in Git repo'), nil, N_('Branch')),
-          self.set('template_sync_metadata_export_mode', N_('Default metadata export mode, refresh re-renders metadata, keep will keep existing metadata, remove exports template withou metadata'), 'refresh', N_('Metadata export mode'), nil, { :collection => Proc.new { self.metadata_export_mode_types } })
+          self.set('template_sync_metadata_export_mode', N_('Default metadata export mode, refresh re-renders metadata, keep will keep existing metadata, remove exports template withou metadata'), 'refresh', N_('Metadata export mode'), nil, { :collection => Proc.new { self.metadata_export_mode_types } }),
+          self.set('template_sync_force', N_('Should importing overwrite locked templates?'), false, N_('Force import')),
         ].compact.each { |s| self.create! s.update(:category => "Setting::TemplateSync") }
       end
 

app/services/foreman_templates/template_importer.rb

@@ -6,15 +6,13 @@ class TemplateImporter < Action
     attr_accessor :metadata, :name, :text
 
     def self.setting_overrides
-      super + %i(associate)
+      super + %i(associate force)
     end
 
     def initialize(args = {})
       super
-      # Rake hands off strings, not booleans, and "false" is true...
-      if @verbose.is_a?(String)
-        @verbose = @verbose != 'false'
-      end
+      @verbose = parse_bool(@verbose)
+      @force = parse_bool(@force)
     end
 
     def import!
@@ -69,32 +67,32 @@ def parse_files!
         end
 
         begin
-          # Expects a return of { :diff, :status, :result }
+          # Expects a return of { :diff, :status, :result, :errors }
           data = if metadata['model'].present?
-                   metadata['model'].constantize.import!(name, text, metadata)
+                   metadata['model'].constantize.import!(name, text, metadata, @force)
                  else
                    # For backwards-compat before "model" metadata was added
                    case metadata['kind']
                    when 'ptable'
-                     Ptable.import!(name, text, metadata)
+                     Ptable.import!(name, text, metadata, @force)
                    when 'job_template'
                      # TODO: update REX templates to have `model` and delete this
                      update_job_template(name, text)
                    else
-                     ProvisioningTemplate.import!(name, text, metadata)
+                     ProvisioningTemplate.import!(name, text, metadata, @force)
                    end
                  end
 
           if data[:diff].nil? && data[:old].present? && data[:new].present?
             data[:diff] = calculate_diff(data[:old], data[:new])
           end
 
-          if data[:status] == true && @verbose
+          if @verbose
             result_lines << data[:result]
             result_lines << data[:diff] unless data[:diff].nil?
-          elsif data[:status] == false
-            result_lines << "Template \"#{name}\": #{data[:result]}"
           end
+          result_lines << status_to_text(data[:status], name)
+          result_lines << data[:errors] unless data[:errors].empty?
         rescue MissingKindError
           result_lines << "  Skipping: '#{name}' - No template kind or model detected"
           next
@@ -165,5 +163,21 @@ def purge!
         template.destroy
       end
     end # :purge
+
+    private
+
+    def parse_bool(bool_name)
+      bool_name.is_a?(String) ? bool_name != 'false' : bool_name
+    end
+
+    def status_to_text(status, name)
+      msg = "#{name} - import "
+      msg << if status
+               "success"
+             else
+               'failure'
+             end
+      msg
+    end
   end
 end