From 12c0989ee4653741a76d14ac2caf65ca290a8793 Mon Sep 17 00:00:00 2001 From: adamlazik1 Date: Wed, 9 Oct 2024 15:29:11 +0000 Subject: [PATCH] Fixes #37900 - Allow syncing templates through HTTP proxy --- app/controllers/api/v2/template_controller.rb | 4 +- .../controller/parameters/template_params.rb | 2 +- .../ui_template_syncs_controller.rb | 22 ++++- app/services/foreman_templates/action.rb | 33 +++++++- .../foreman_templates/template_exporter.rb | 4 +- .../foreman_templates/template_importer.rb | 6 +- lib/foreman_templates.rb | 4 + lib/foreman_templates/engine.rb | 6 ++ test/unit/action_test.rb | 70 ++++++++++++++++ .../NewTemplateSyncFormHelpers.js | 18 +++- .../components/ProxySettingField.js | 44 ++++++++++ .../components/ProxySettingFields.js | 82 +++++++++++++++++++ .../components/SyncSettingField.js | 17 ++-- .../components/SyncSettingFields.js | 18 ++++ .../SyncSettingField.test.js.snap | 6 +- .../SyncSettingFields.test.js.snap | 14 ++++ 16 files changed, 323 insertions(+), 27 deletions(-) create mode 100644 webpack/components/NewTemplateSync/components/ProxySettingField.js create mode 100644 webpack/components/NewTemplateSync/components/ProxySettingFields.js diff --git a/app/controllers/api/v2/template_controller.rb b/app/controllers/api/v2/template_controller.rb index a29a731d..265fad45 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. If you choose 'selected', provide the `http_proxy_id` parameter.") + 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..6f5cf501 100644 --- a/app/controllers/ui_template_syncs_controller.rb +++ b/app/controllers/ui_template_syncs_controller.rb @@ -49,6 +49,26 @@ 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}") } + proxy_policy_setting = Foreman.settings.find('template_sync_http_proxy_policy').dup + proxy_id_setting = http_proxy_id_setting + # if default value is 'Custom HTTP proxy' but there is no proxy to select, value must be changed + proxy_policy_setting.value = proxy_policy_setting.default = 'none' if proxy_id_setting.value == '' && proxy_policy_setting.value == 'selected' + settings << proxy_policy_setting + settings << 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..76c26fdc 100644 --- a/lib/foreman_templates.rb +++ b/lib/foreman_templates.rb @@ -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..367ba4df 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? If you select Custom HTTP proxy, you will be prompted to select one.'), + default: 'global', + full_name: N_('HTTP proxy policy'), + collection: -> { ForemanTemplates.http_proxy_policy_types }) end end diff --git a/test/unit/action_test.rb b/test/unit/action_test.rb index b8dc1bdf..d96adfc8 100644 --- a/test/unit/action_test.rb +++ b/test/unit/action_test.rb @@ -85,5 +85,75 @@ class ActionTest < ActiveSupport::TestCase end end end + + context 'sync through http_proxy' do + before do + @template_sync_service = Action.new(:repo => 'https://github.com/theforeman/community-templates.git') + end + + test 'should sync through custom http proxy' do + proxy = FactoryBot.create(:http_proxy) + @template_sync_service.instance_variable_set(:@http_proxy_policy, 'selected') + @template_sync_service.instance_variable_set(:@http_proxy_id, proxy.id) + assert_equal proxy.full_url, show_repo_proxy_url + end + + test 'sync should fail if invalid http proxy id is provided' do + @template_sync_service.instance_variable_set(:@http_proxy_policy, 'selected') + @template_sync_service.instance_variable_set(:@http_proxy_id, 'invalid ID') + assert_raises(ActiveRecord::RecordNotFound) do + @template_sync_service.send(:init_git_repo) + end + end + + test 'should sync through https proxy using custom CA certificate' do + custom_cert = 'Custom proxy CA cert' + proxy = FactoryBot.create(:http_proxy, :cacert => custom_cert, :url => 'https://localhost:8888') + @template_sync_service.instance_variable_set(:@http_proxy_policy, 'selected') + @template_sync_service.instance_variable_set(:@http_proxy_id, proxy.id) + assert_equal custom_cert, show_repo_proxy_cert + end + + test 'should sync through global http proxy' do + Setting[:http_proxy] = 'https://localhost:8888' + @template_sync_service.instance_variable_set(:@http_proxy_policy, 'global') + assert_equal Setting[:http_proxy], show_repo_proxy_url + end + + test 'should sync without using http proxy if global proxy is not set' do + Setting[:http_proxy] = "" + @template_sync_service.instance_variable_set(:@http_proxy_policy, 'global') + assert_nil show_repo_proxy_url + end + + test 'should sync without using http proxy' do + @template_sync_service.instance_variable_set(:@http_proxy_policy, 'none') + assert_nil show_repo_proxy_url + end + + private + + def show_repo_proxy_url + dir = Dir.mktmpdir + @template_sync_service.instance_variable_set(:@dir, dir) + begin + repo = @template_sync_service.send(:init_git_repo) + repo.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 + @template_sync_service.instance_variable_set(:@dir, dir) + begin + repo = @template_sync_service.send(:init_git_repo) + File.read(repo.config('http.proxysslcainfo')) + ensure + FileUtils.remove_entry_secure(dir) if File.exist?(dir) + end + end + end end end diff --git a/webpack/components/NewTemplateSync/components/NewTemplateSyncForm/NewTemplateSyncFormHelpers.js b/webpack/components/NewTemplateSync/components/NewTemplateSyncForm/NewTemplateSyncFormHelpers.js index ac640bdb..d315ab9c 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' }); @@ -24,9 +26,9 @@ 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"), @@ -41,3 +43,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..60273c48 --- /dev/null +++ b/webpack/components/NewTemplateSync/components/ProxySettingFields.js @@ -0,0 +1,82 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { Field as FormikField } from 'formik'; + +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`; + + // removes the custom proxy option if no proxy is available + if (proxyIdSetting.value === '') { + proxyPolicySetting = proxyPolicySetting.set( + 'selection', + proxyPolicySetting.selection.slice(0, 2) + ); + } + + return ( + + ( + + )} + /> + { + if ( + proxyIdSetting.value !== '' && + // Changing name to camel case here would unnecessarily complicate the code + // eslint-disable-next-line camelcase + form.values[syncType]?.http_proxy_policy === 'selected' + ) { + 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" /> + `;