From 8ac4cb9526b35af60a1276868a54da6e909a2d0e Mon Sep 17 00:00:00 2001 From: uu59 Date: Thu, 2 Apr 2015 11:26:52 +0900 Subject: [PATCH 1/4] Fix configtest didn't catch syntax error --- .../fluentd/settings_controller.rb | 11 ++++- spec/features/setting_spec.rb | 44 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/app/controllers/fluentd/settings_controller.rb b/app/controllers/fluentd/settings_controller.rb index e89eeea..72a5f1c 100644 --- a/app/controllers/fluentd/settings_controller.rb +++ b/app/controllers/fluentd/settings_controller.rb @@ -37,7 +37,12 @@ class Fluentd::SettingsController < ApplicationController def handle_dryrun if dryrun(params[:config]) - flash.now[:success] = I18n.t('messages.dryrun_is_passed') + begin + parse_test(params[:config]) + flash.now[:success] = I18n.t('messages.dryrun_is_passed') + rescue Fluent::ConfigParseError => e + flash.now[:danger] = e.message + end else flash.now[:danger] = @fluentd.agent.log.last_error_message end @@ -61,6 +66,10 @@ class Fluentd::SettingsController < ApplicationController @fluentd.agent.dryrun(tmpfile.path) end + def parse_test(conf) + Fluent::Config::V1Parser.parse(params[:config], @fluentd.config_file) + end + def update_config(conf) Fluent::Config::V1Parser.parse(conf, @fluentd.config_file) @fluentd.agent.config_write conf diff --git a/spec/features/setting_spec.rb b/spec/features/setting_spec.rb index c3e134b..0f28e3f 100644 --- a/spec/features/setting_spec.rb +++ b/spec/features/setting_spec.rb @@ -47,4 +47,48 @@ describe 'setting', stub: :daemon do current_path.should == '/daemon/setting' page.should have_css('pre', text: 'YET ANOTHER CONFIG') end + + describe "config test" do + before do + daemon.agent.config_write conf + click_link I18n.t('terms.edit') + end + + context "plain config" do + let(:conf) { <<-'CONF' } + + type forward + + CONF + + it 'configtest' do + click_button I18n.t('terms.configtest') + page.should have_css('.alert-success') + end + + it "update & restart check" do + click_button I18n.t('terms.update') + daemon.agent.config.gsub("\r\n", "\n").should == conf # CodeMirror exchange \n -> \r\n + end + end + + context "embedded config" do + let(:conf) { <<-'CONF' } + + type forward + id "foo#{Time.now.to_s}" + + CONF + + it 'configtest' do + click_button I18n.t('terms.configtest') + page.should have_css('.alert-danger') + end + + it "update & restart check" do + click_button I18n.t('terms.update') + page.should have_css('.alert-danger') + end + end + end end From 818e5d96bfc88cb7b729509f8fa8ea36c7abbe5b Mon Sep 17 00:00:00 2001 From: uu59 Date: Thu, 2 Apr 2015 11:45:43 +0900 Subject: [PATCH 2/4] Refactor config parse processes --- app/controllers/fluentd/settings_controller.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/controllers/fluentd/settings_controller.rb b/app/controllers/fluentd/settings_controller.rb index 72a5f1c..aa8c3b2 100644 --- a/app/controllers/fluentd/settings_controller.rb +++ b/app/controllers/fluentd/settings_controller.rb @@ -38,7 +38,7 @@ class Fluentd::SettingsController < ApplicationController def handle_dryrun if dryrun(params[:config]) begin - parse_test(params[:config]) + parse_config(params[:config]) flash.now[:success] = I18n.t('messages.dryrun_is_passed') rescue Fluent::ConfigParseError => e flash.now[:danger] = e.message @@ -51,6 +51,7 @@ class Fluentd::SettingsController < ApplicationController end def handle_update + parse_config(params[:config]) update_config(params[:config]) redirect_to daemon_setting_path(@fluentd) rescue Fluent::ConfigParseError => e @@ -66,12 +67,12 @@ class Fluentd::SettingsController < ApplicationController @fluentd.agent.dryrun(tmpfile.path) end - def parse_test(conf) - Fluent::Config::V1Parser.parse(params[:config], @fluentd.config_file) + def parse_config(conf) + # V1Parser.parse could raise exception + Fluent::Config::V1Parser.parse(conf, @fluentd.config_file) end def update_config(conf) - Fluent::Config::V1Parser.parse(conf, @fluentd.config_file) @fluentd.agent.config_write conf @fluentd.agent.restart if @fluentd.agent.running? end From aaf4f179ab08b12da369d520996bfa7be52e07d4 Mon Sep 17 00:00:00 2001 From: uu59 Date: Thu, 2 Apr 2015 12:47:45 +0900 Subject: [PATCH 3/4] Fix to parse config with embedded code --- app/controllers/fluentd/settings_controller.rb | 2 +- spec/features/setting_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/fluentd/settings_controller.rb b/app/controllers/fluentd/settings_controller.rb index aa8c3b2..7f3247a 100644 --- a/app/controllers/fluentd/settings_controller.rb +++ b/app/controllers/fluentd/settings_controller.rb @@ -69,7 +69,7 @@ class Fluentd::SettingsController < ApplicationController def parse_config(conf) # V1Parser.parse could raise exception - Fluent::Config::V1Parser.parse(conf, @fluentd.config_file) + Fluent::Config::V1Parser.parse(conf, @fluentd.config_file, File.dirname(@fluentd.config_file), binding) end def update_config(conf) diff --git a/spec/features/setting_spec.rb b/spec/features/setting_spec.rb index 0f28e3f..9f24335 100644 --- a/spec/features/setting_spec.rb +++ b/spec/features/setting_spec.rb @@ -82,12 +82,12 @@ describe 'setting', stub: :daemon do it 'configtest' do click_button I18n.t('terms.configtest') - page.should have_css('.alert-danger') + page.should have_css('.alert-success') end it "update & restart check" do click_button I18n.t('terms.update') - page.should have_css('.alert-danger') + daemon.agent.config.gsub("\r\n", "\n").should == conf # CodeMirror exchange \n -> \r\n end end end From 92d026bdaa0a33ac734c619b059b90310b44d4fd Mon Sep 17 00:00:00 2001 From: uu59 Date: Thu, 2 Apr 2015 12:58:56 +0900 Subject: [PATCH 4/4] Use more proper description --- spec/features/setting_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/setting_spec.rb b/spec/features/setting_spec.rb index 9f24335..129a7cc 100644 --- a/spec/features/setting_spec.rb +++ b/spec/features/setting_spec.rb @@ -48,7 +48,7 @@ describe 'setting', stub: :daemon do page.should have_css('pre', text: 'YET ANOTHER CONFIG') end - describe "config test" do + describe "config" do before do daemon.agent.config_write conf click_link I18n.t('terms.edit')