Merge pull request #168 from fluent/config_parse_also_configtest

Fix configtest/update behaviors are mismatched
fluent · Apr 2, 2015 · 0af3639 · 0af3639
2 parents b8bc6e0 + 92d026b
commit 0af3639
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 2 deletions.
diff --git a/app/controllers/fluentd/settings_controller.rb b/app/controllers/fluentd/settings_controller.rb
@@ -37,7 +37,12 @@ def set_config
 
   def handle_dryrun
     if dryrun(params[:config])
-      flash.now[:success] = I18n.t('messages.dryrun_is_passed')
+      begin
+        parse_config(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
@@ -46,6 +51,7 @@ def handle_dryrun
   end
 
   def handle_update
+    parse_config(params[:config])
     update_config(params[:config])
     redirect_to daemon_setting_path(@fluentd)
   rescue Fluent::ConfigParseError => e
@@ -61,8 +67,12 @@ def dryrun(conf)
     @fluentd.agent.dryrun(tmpfile.path)
   end
 
+  def parse_config(conf)
+    # V1Parser.parse could raise exception
+    Fluent::Config::V1Parser.parse(conf, @fluentd.config_file, File.dirname(@fluentd.config_file), binding)
+  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

diff --git a/spec/features/setting_spec.rb b/spec/features/setting_spec.rb
@@ -47,4 +47,48 @@
     current_path.should == '/daemon/setting'
     page.should have_css('pre', text: 'YET ANOTHER CONFIG')
   end
+
+  describe "config" do
+    before do
+      daemon.agent.config_write conf
+      click_link I18n.t('terms.edit')
+    end
+
+    context "plain config" do
+      let(:conf) { <<-'CONF' }
+      <source>
+        type forward
+      </source>
+      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' }
+      <source>
+        type forward
+        id "foo#{Time.now.to_s}"
+      </source>
+      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
+  end
 end