diff --git a/app/controllers/api/v2/template_controller.rb b/app/controllers/api/v2/template_controller.rb index a29a731d..6cd36051 100644 --- a/app/controllers/api/v2/template_controller.rb +++ b/app/controllers/api/v2/template_controller.rb @@ -12,7 +12,9 @@ class TemplateController < ::Api::V2::BaseController param :repo, String, :required => false, :desc => N_("Override the default repo from settings.") param :filter, String, :required => false, :desc => N_("Export templates with names matching this regex (case-insensitive; snippets are not filtered).") param :negate, :bool, :required => false, :desc => N_("Negate the prefix (for purging).") - param :dirname, String, :required => false, :desc => N_("The directory within Git repo containing the templates") + param :dirname, String, :required => false, :desc => N_("Directory within Git repo containing the templates.") + param :http_proxy_policy, ForemanTemplates.http_proxy_policy_types.keys, :required => false, :desc => N_("HTTP proxy policy for template sync.") + param :http_proxy_id, :number, :required => false, :desc => N_("ID of an HTTP proxy to use for template sync. Use this parameter together with `'http_proxy_policy':'selected'`") end api :POST, "/templates/import/", N_("Initiate Import") diff --git a/app/controllers/concerns/foreman/controller/parameters/template_params.rb b/app/controllers/concerns/foreman/controller/parameters/template_params.rb index b225ac19..3cc01b77 100644 --- a/app/controllers/concerns/foreman/controller/parameters/template_params.rb +++ b/app/controllers/concerns/foreman/controller/parameters/template_params.rb @@ -7,7 +7,7 @@ module TemplateParams class_methods do def filter_params_list - %i(verbose repo branch dirname filter negate metadata_export_mode) + %i(verbose repo branch dirname filter negate metadata_export_mode http_proxy_policy http_proxy_id) end def extra_import_params diff --git a/app/controllers/ui_template_syncs_controller.rb b/app/controllers/ui_template_syncs_controller.rb index f88b9565..d3019c9b 100644 --- a/app/controllers/ui_template_syncs_controller.rb +++ b/app/controllers/ui_template_syncs_controller.rb @@ -49,6 +49,21 @@ def render_errors(messages, severity = 'danger') private def setting_definitions(short_names) - short_names.map { |name| Foreman.settings.find("template_sync_#{name}") } + settings = short_names.map { |name| Foreman.settings.find("template_sync_#{name}") } + settings << http_proxy_id_setting + settings + end + + def http_proxy_id_setting + proxy_list = HttpProxy.authorized(:view_http_proxies).with_taxonomy_scope.each_with_object({}) { |proxy, hash| hash[proxy.id] = proxy.name } + default_proxy_id = proxy_list.keys.first || "" + OpenStruct.new(id: 'template_sync_http_proxy_id', + name: 'template_sync_http_proxy_id', + description: N_('Select an HTTP proxy to use for template sync. You can add HTTP proxies on the Infrastructure > HTTP proxies page.'), + settings_type: :string, + value: default_proxy_id, + default: default_proxy_id, + full_name: N_('HTTP proxy'), + select_values: proxy_list) end end diff --git a/app/services/foreman_templates/action.rb b/app/services/foreman_templates/action.rb index 9899b657..4471e364 100644 --- a/app/services/foreman_templates/action.rb +++ b/app/services/foreman_templates/action.rb @@ -1,3 +1,5 @@ +require 'securerandom' + module ForemanTemplates class Action delegate :logger, :to => :Rails @@ -15,7 +17,7 @@ def self.repo_start_with end def self.setting_overrides - %i(verbose prefix dirname filter repo negate branch) + %i(verbose prefix dirname filter repo negate branch http_proxy_policy) end def method_missing(method, *args, &block) @@ -53,9 +55,38 @@ def verify_path!(path) private def assign_attributes(args = {}) + @http_proxy_id = args[:http_proxy_id] self.class.setting_overrides.each do |attribute| instance_variable_set("@#{attribute}", args[attribute.to_sym] || Setting["template_sync_#{attribute}".to_sym]) end end + + protected + + def init_git_repo + git_repo = Git.init(@dir) + + case @http_proxy_policy + when 'global' + http_proxy_url = Setting[:http_proxy] + when 'selected' + http_proxy = HttpProxy.authorized(:view_http_proxies).with_taxonomy_scope.find(@http_proxy_id) + http_proxy_url = http_proxy.full_url + + if URI(http_proxy_url).scheme == 'https' && http_proxy.cacert.present? + proxy_cert = "#{@dir}/.git/foreman_templates_proxy_cert_#{SecureRandom.hex(8)}.crt" + File.write(proxy_cert, http_proxy.cacert) + git_repo.config('http.proxySSLCAInfo', proxy_cert) + end + end + + if http_proxy_url.present? + git_repo.config('http.proxy', http_proxy_url) + end + + git_repo.add_remote('origin', @repo) + logger.debug "cloned '#{@repo}' to '#{@dir}'" + git_repo + end end end diff --git a/app/services/foreman_templates/template_exporter.rb b/app/services/foreman_templates/template_exporter.rb index 1ecf0061..418d1b77 100644 --- a/app/services/foreman_templates/template_exporter.rb +++ b/app/services/foreman_templates/template_exporter.rb @@ -31,8 +31,8 @@ def export_to_git @dir = Dir.mktmpdir return if branch_missing? - git_repo = Git.clone(@repo, @dir) - logger.debug "cloned '#{@repo}' to '#{@dir}'" + git_repo = init_git_repo + git_repo.fetch setup_git_branch git_repo dump_files! diff --git a/app/services/foreman_templates/template_importer.rb b/app/services/foreman_templates/template_importer.rb index 8a2477d5..691b46a1 100644 --- a/app/services/foreman_templates/template_importer.rb +++ b/app/services/foreman_templates/template_importer.rb @@ -32,9 +32,9 @@ def import_from_git @dir = Dir.mktmpdir begin - logger.debug "cloned '#{@repo}' to '#{@dir}'" - gitrepo = Git.clone(@repo, @dir) - if @branch + gitrepo = init_git_repo + gitrepo.fetch + if @branch.present? logger.debug "checking out branch '#{@branch}'" gitrepo.checkout(@branch) end diff --git a/lib/foreman_templates.rb b/lib/foreman_templates.rb index b36343a6..2354cc9d 100644 --- a/lib/foreman_templates.rb +++ b/lib/foreman_templates.rb @@ -1,7 +1,7 @@ require 'foreman_templates/engine' module ForemanTemplates - BASE_SETTING_NAMES = %w(repo branch dirname filter negate).freeze + BASE_SETTING_NAMES = %w(repo branch dirname filter negate http_proxy_policy).freeze IMPORT_SETTING_NAMES = (BASE_SETTING_NAMES | %w(prefix associate force lock)).freeze EXPORT_SETTING_NAMES = (BASE_SETTING_NAMES | %w(metadata_export_mode commit_msg)).freeze @@ -16,4 +16,8 @@ def self.lock_types def self.metadata_export_mode_types { 'refresh' => _('Refresh'), 'keep' => _('Keep'), 'remove' => _('Remove') } end + + def self.http_proxy_policy_types + { 'global' => _('Global default HTTP proxy'), 'none' => _('No HTTP proxy'), 'selected' => _('Custom HTTP proxy') } + end end diff --git a/lib/foreman_templates/engine.rb b/lib/foreman_templates/engine.rb index bdf0ab62..02c504ab 100644 --- a/lib/foreman_templates/engine.rb +++ b/lib/foreman_templates/engine.rb @@ -94,6 +94,12 @@ class Engine < ::Rails::Engine description: N_('Custom commit message for templates export'), default: 'Templates export made by a Foreman user', full_name: N_('Commit message')) + setting('template_sync_http_proxy_policy', + type: :string, + description: N_('Should an HTTP proxy be used for template sync?'), + default: 'global', + full_name: N_('HTTP proxy policy'), + collection: -> { ForemanTemplates.http_proxy_policy_types }) end end diff --git a/test/functional/api/v2/template_controller_test.rb b/test/functional/api/v2/template_controller_test.rb index 1045f206..6b26d40f 100644 --- a/test/functional/api/v2/template_controller_test.rb +++ b/test/functional/api/v2/template_controller_test.rb @@ -13,6 +13,18 @@ class TemplateControllerTest < ::ActionController::TestCase assert_response :success end + test "should import from git through proxy" do + proxy = FactoryBot.create(:http_proxy) + post :import, params: { 'repo' => "#{template_fixtures_path}/core", 'prefix' => '', 'http_proxy_policy' => 'selected', 'http_proxy_id' => proxy.id} + assert_response :success + end + + test "import should fail when provided with invalid proxy id" do + post :import, params: { 'repo' => "#{template_fixtures_path}/core", 'prefix' => '', 'http_proxy_policy' => 'selected', 'http_proxy_id' => 'invalid ID'} + assert_response :error + puts response.msg + end + test "should export to filesystem" do Dir.mktmpdir do |tmpdir| post :export, params: { 'repo' => tmpdir, 'metadata_export_mode' => 'keep' } diff --git a/test/unit/template_importer_test.rb b/test/unit/template_importer_test.rb index daf8780c..e9aef388 100644 --- a/test/unit/template_importer_test.rb +++ b/test/unit/template_importer_test.rb @@ -158,6 +158,65 @@ def audit_comment assert_equal succ, res.first end + test 'import should fail if invalid http proxy id is provided' do + setup_settings + imp = ForemanTemplates::TemplateImporter.new + imp.instance_variable_set(:@http_proxy_id, 'invalid ID') + imp.instance_variable_set(:@http_proxy_policy, 'selected') + assert_raises(ActiveRecord::RecordNotFound) do + res = imp.import! + end + end + + test 'should import through custom http proxy' do + setup_settings + @imp = ForemanTemplates::TemplateImporter.new + proxy = FactoryBot.create(:http_proxy) + @imp.instance_variable_set(:@http_proxy_policy, 'selected') + @imp.instance_variable_set(:@http_proxy_id, proxy.id) + @imp.stubs(:import_from_git).returns(show_repo_proxy_url) + repo_proxy_url = @imp.import! + assert_equal proxy.full_url, repo_proxy_url + end + + test 'should import through global http proxy' do + setup_settings + @imp = ForemanTemplates::TemplateImporter.new + @imp.stubs(:import_from_git).returns(show_repo_proxy_url) + repo_proxy_url = @imp.import! + assert_equal Setting[:http_proxy], repo_proxy_url + end + + test 'should import without using http proxy if global proxy is not set' do + setup_settings + Setting['http_proxy'] = "" + @imp = ForemanTemplates::TemplateImporter.new + @imp.stubs(:import_from_git).returns(show_repo_proxy_url) + repo_proxy_url = @imp.import! + assert_nil repo_proxy_url + end + + test 'should import without using http proxy' do + setup_settings + @imp = ForemanTemplates::TemplateImporter.new + @imp.instance_variable_set(:@http_proxy_policy, 'none') + @imp.stubs(:import_from_git).returns(show_repo_proxy_url) + repo_proxy_url = @imp.import! + assert_nil repo_proxy_url + end + + test 'should import through https proxy using custom CA certificate' do + setup_settings + @imp = ForemanTemplates::TemplateImporter.new + custom_cert = 'Custom proxy CA cert' + proxy = FactoryBot.create(:http_proxy, :cacert => custom_cert, :url => 'https://localhost:8888') + @imp.instance_variable_set(:@http_proxy_policy, 'selected') + @imp.instance_variable_set(:@http_proxy_id, proxy.id) + @imp.stubs(:import_from_git).returns(show_repo_proxy_cert) + proxy_cert = @imp.import! + assert_equal custom_cert, proxy_cert + end + test 'should import files from filesystem' do setup_settings :repo => @engine_root, :dirname => '/test/templates/core' imp = ForemanTemplates::TemplateImporter.new @@ -351,6 +410,8 @@ def setup_settings(opts = {}) Setting['template_sync_repo'] = default_repo Setting['template_sync_negate'] = false Setting['template_sync_branch'] = default_branch + Setting['template_sync_http_proxy_policy'] = 'global' + Setting['http_proxy'] = 'https://localhost:8888' end def assert_both_equal_nil(expected, actual) @@ -366,5 +427,27 @@ def assert_both_equal_nil(expected, actual) def find_result(results, template_name) results.find { |res| res.name == template_name } end + + def show_repo_proxy_url + dir = Dir.mktmpdir + @imp.instance_variable_set(:@dir, dir) + begin + gitrepo = @imp.send(:init_git_repo) + gitrepo.config.to_h['http.proxy'] + ensure + FileUtils.remove_entry_secure(dir) if File.exist?(dir) + end + end + + def show_repo_proxy_cert + dir = Dir.mktmpdir + @imp.instance_variable_set(:@dir, dir) + begin + gitrepo = @imp.send(:init_git_repo) + File.read(gitrepo.config('http.proxysslcainfo')) + ensure + FileUtils.remove_entry_secure(dir) if File.exist?(dir) + end + end end end diff --git a/webpack/components/NewTemplateSync/components/NewTemplateSyncForm/NewTemplateSyncFormHelpers.js b/webpack/components/NewTemplateSync/components/NewTemplateSyncForm/NewTemplateSyncFormHelpers.js index ac640bdb..698f0811 100644 --- a/webpack/components/NewTemplateSync/components/NewTemplateSyncForm/NewTemplateSyncFormHelpers.js +++ b/webpack/components/NewTemplateSync/components/NewTemplateSyncForm/NewTemplateSyncFormHelpers.js @@ -1,4 +1,6 @@ import * as Yup from 'yup'; +import React from 'react'; +import { translate as __ } from 'foremanReact/common/I18n'; export const redirectToResult = history => () => history.push({ pathname: '/template_syncs/result' }); @@ -15,6 +17,9 @@ const repoFormat = formatAry => value => { return value && valid; }; +const httpProxyAvailable = proxyId => value => + value !== 'selected' || proxyId.value !== ''; + export const syncFormSchema = (syncType, settingsObj, validationData) => { const schema = (settingsObj[syncType].asMutable() || []).reduce( (memo, setting) => { @@ -24,14 +29,30 @@ export const syncFormSchema = (syncType, settingsObj, validationData) => { repo: Yup.string() .test( 'repo-format', - `Invalid repo format, must start with one of: ${validationData.repo.join( - ', ' - )}`, + `${__( + 'Invalid repo format, must start with one of: ' + )}${validationData.repo.join(', ')}`, repoFormat(validationData.repo) ) .required("can't be blank"), }; } + if (setting.name === 'http_proxy_policy') { + return { + ...memo, + http_proxy_policy: Yup.mixed().test( + 'http-proxy-available', + __( + 'No HTTP proxies available. Please select a different HTTP proxy policy or switch to a different taxonomy context.' + ), + httpProxyAvailable( + settingsObj[syncType].find( + obj => obj.id === 'template_sync_http_proxy_id' + ) + ) + ), + }; + } return memo; }, {} @@ -41,3 +62,13 @@ export const syncFormSchema = (syncType, settingsObj, validationData) => { [syncType]: Yup.object().shape(schema), }); }; + +export const tooltipContent = setting => ( +
+); + +export const label = setting => `${__(setting.fullName)}`; diff --git a/webpack/components/NewTemplateSync/components/ProxySettingField.js b/webpack/components/NewTemplateSync/components/ProxySettingField.js new file mode 100644 index 00000000..41ec6444 --- /dev/null +++ b/webpack/components/NewTemplateSync/components/ProxySettingField.js @@ -0,0 +1,44 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { get } from 'lodash'; + +import { FieldLevelHelp } from 'patternfly-react'; +import RenderField from './TextButtonField/RenderField'; +import ButtonTooltip from './ButtonTooltip'; + +import { + tooltipContent, + label, +} from './NewTemplateSyncForm/NewTemplateSyncFormHelpers'; + +const ProxySettingField = ({ setting, resetField, field, form, fieldName }) => ( + 'select'} + tooltipHelp={} + buttonAttrs={{ + buttonText: , + buttonAction: () => + resetField(fieldName, setting.value)(form.setFieldValue), + }} + blank={{}} + item={setting} + disabled={false} + fieldRequired + meta={{ + touched: get(form.touched, fieldName), + error: get(form.errors, fieldName), + }} + input={field} + /> +); + +ProxySettingField.propTypes = { + setting: PropTypes.object.isRequired, + resetField: PropTypes.func.isRequired, + field: PropTypes.object.isRequired, + form: PropTypes.object.isRequired, + fieldName: PropTypes.string.isRequired, +}; + +export default ProxySettingField; diff --git a/webpack/components/NewTemplateSync/components/ProxySettingFields.js b/webpack/components/NewTemplateSync/components/ProxySettingFields.js new file mode 100644 index 00000000..cfa2788e --- /dev/null +++ b/webpack/components/NewTemplateSync/components/ProxySettingFields.js @@ -0,0 +1,108 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { Field as FormikField } from 'formik'; +import { translate as __ } from 'foremanReact/common/I18n'; +import CommonForm from 'foremanReact/components/common/forms/CommonForm'; +import { InputGroup } from 'patternfly-react'; +import { FieldLevelHelp } from 'patternfly-react/dist/js/components/FieldLevelHelp'; + +import { + tooltipContent, + label, +} from './NewTemplateSyncForm/NewTemplateSyncFormHelpers'; +import ProxySettingField from './ProxySettingField'; + +const ProxySettingsFields = ({ + proxyPolicySetting, + proxyIdSetting, + syncType, + resetField, + formProps: { isSubmitting }, +}) => { + if (Object.keys(proxyPolicySetting).length === 0) { + return <>; + } + const proxyPolicyFieldName = `${syncType}.http_proxy_policy`; + const proxyIdFieldName = `${syncType}.http_proxy_id`; + + proxyPolicySetting = proxyPolicySetting.set( + 'description', + `${proxyPolicySetting.description}
If you select Custom HTTP proxy, you will be prompted to select one.` + ); + proxyIdSetting = proxyIdSetting.set( + 'description', + proxyIdSetting.description + .replace(/\. /, '.
') + .replace( + 'Infrastructure > HTTP proxies', + 'Infrastructure > HTTP proxies' + ) + ); + + return ( + + ( + + )} + /> + { + // Changing name to camel case here would unnecessarily complicate the code + // eslint-disable-next-line camelcase + if (form.values[syncType]?.http_proxy_policy === 'selected') { + if (proxyIdSetting.value === '') { + return ( + + } + > + +
{__('No HTTP proxies available')}
+ + + ); + } + return ( + + ); + } + return <>; + }} + /> + + ); +}; + +ProxySettingsFields.propTypes = { + proxyPolicySetting: PropTypes.object, + proxyIdSetting: PropTypes.object, + syncType: PropTypes.string.isRequired, + resetField: PropTypes.func.isRequired, + formProps: PropTypes.object, +}; + +ProxySettingsFields.defaultProps = { + formProps: {}, + proxyPolicySetting: {}, + proxyIdSetting: {}, +}; + +export default ProxySettingsFields; diff --git a/webpack/components/NewTemplateSync/components/SyncSettingField.js b/webpack/components/NewTemplateSync/components/SyncSettingField.js index aed848ea..32822dba 100644 --- a/webpack/components/NewTemplateSync/components/SyncSettingField.js +++ b/webpack/components/NewTemplateSync/components/SyncSettingField.js @@ -1,14 +1,15 @@ import React from 'react'; import PropTypes from 'prop-types'; import { FieldLevelHelp } from 'patternfly-react'; -import { translate as __ } from 'foremanReact/common/I18n'; +import { + tooltipContent, + label, +} from './NewTemplateSyncForm/NewTemplateSyncFormHelpers'; import TextButtonField from './TextButtonField'; import ButtonTooltip from './ButtonTooltip'; const SyncSettingField = ({ setting, resetField, disabled, syncType }) => { - const label = settingObj => `${__(settingObj.fullName)} `; - const fieldSelector = settingObj => { if (settingObj.settingsType === 'boolean') { return 'checkbox'; @@ -21,14 +22,6 @@ const SyncSettingField = ({ setting, resetField, disabled, syncType }) => { return 'text'; }; - const tooltipContent = ( -
- ); - return ( { fieldSelector={fieldSelector} disabled={disabled} fieldRequired={setting.required} - tooltipHelp={} + tooltipHelp={} > {setting.value} diff --git a/webpack/components/NewTemplateSync/components/SyncSettingFields.js b/webpack/components/NewTemplateSync/components/SyncSettingFields.js index 0563e0a5..d77815e1 100644 --- a/webpack/components/NewTemplateSync/components/SyncSettingFields.js +++ b/webpack/components/NewTemplateSync/components/SyncSettingFields.js @@ -3,6 +3,7 @@ import { upperFirst } from 'lodash'; import PropTypes from 'prop-types'; import SyncSettingField from './SyncSettingField'; +import ProxySettingsFields from './ProxySettingFields'; const SyncSettingsFields = ({ importSettings, @@ -38,10 +39,21 @@ const SyncSettingsFields = ({ ); const settingsAry = syncType === 'import' ? importSettings : exportSettings; + const proxyPolicySetting = settingsAry.find( + setting => setting.id === 'template_sync_http_proxy_policy' + ); + const proxyIdSetting = settingsAry.find( + setting => setting.id === 'template_sync_http_proxy_id' + ); return ( {settingsAry + .filter( + setting => + setting.id !== 'template_sync_http_proxy_policy' && + setting.id !== 'template_sync_http_proxy_id' + ) .map(addRequiredToSetting) .map(setting => modifyDescription(setting, syncType)) .map(setting => specializeDescription(setting, syncType)) @@ -54,6 +66,12 @@ const SyncSettingsFields = ({ resetField={resetField} /> ))} + ); }; diff --git a/webpack/components/NewTemplateSync/components/__tests__/__snapshots__/SyncSettingField.test.js.snap b/webpack/components/NewTemplateSync/components/__tests__/__snapshots__/SyncSettingField.test.js.snap index 8ce4f6d9..1d9046e3 100644 --- a/webpack/components/NewTemplateSync/components/__tests__/__snapshots__/SyncSettingField.test.js.snap +++ b/webpack/components/NewTemplateSync/components/__tests__/__snapshots__/SyncSettingField.test.js.snap @@ -23,7 +23,7 @@ exports[`SyncSettingField should render boolean setting as checkbox 1`] = ` "value": false, } } - label="undefined " + label="undefined" name="import.force" tooltipHelp={ + `; @@ -97,5 +104,12 @@ exports[`SyncSettingFields should show import settings 1`] = ` } syncType="import" /> + `;